Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bluesky: copy tracing API #3510

Closed
wants to merge 1 commit into from
Closed

bluesky: copy tracing API #3510

wants to merge 1 commit into from

Conversation

torquestomp
Copy link
Contributor

Checklist

If applicable:

  • I have updated CHANGELOG.md
  • I have updated the documentation (README.md, docs/, demos/)
  • I have updated the config schema (cli/src/config-schema.json)
  • I have added tests to cover my changes

///
/// This can be set on write to explicitly record copies in the backend. The
/// backend may discard these silently if it does not support explicit copy
/// tracking (e.g. git). Backends which do support explicit copy tracking may

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the only place you use "copy tracking". Everywhere else is "copy trace" or "copy tracing".
You could make the purpose clearer by using "file rename", if that's what it's for.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the distinction here is somewhat intentional. I rephrased it a bit to make it clearer.

In my mind, 'copy tracing' is answering the question "where was this file copied from?" which git does support in a limited capacity by inferring copies of similar content between two tree states. Copy tracking is the explicit record of "Hey vcs, remember that I copied foo.cc -> bar.cc here" which git does not support.

@torquestomp torquestomp force-pushed the dploch/copy-bluesky branch from 4acd905 to f8a40a3 Compare April 16, 2024 20:07
@torquestomp torquestomp force-pushed the dploch/copy-bluesky branch from f8a40a3 to ed4a515 Compare April 17, 2024 20:42
@khionu
Copy link
Contributor

khionu commented Apr 18, 2024

Curious, what is the "bluesky" for? Seems unrelated to the rest of the PR?

@martinvonz
Copy link
Member

Curious, what is the "bluesky" for? Seems unrelated to the rest of the PR?

I think it's the "visionary" meaning (https://www.merriam-webster.com/dictionary/blue-sky), probably not related to https://bsky.app/ :)

@martinvonz
Copy link
Member

I think copy tracing is going to be complicated enough that a design doc would be useful to have. It would help figure out what design we need. It would also be good to have a document to be able to point back to when people ask about the reasoning later (or when we forget). You may have considered all of the below and incorporated it in the design already but I would still prefer to have a record of the thought process to point back to.

I think we'll need to read and write copy information in at least these cases:

  • Read copy information when the user does jj diff -r. This needs to work on merge commits too. It should show copies whether they're tracked or detected.
  • Read copy information when the user does jj diff --from X --to Y. It should show copies whether they're tracked or detected.
  • Read copy information when doing jj blame/annotate (which doesn't yet exist)
  • Write copy information when the user explicitly runs jj mv/cp. We would then first read the copy information for the current commit and then add the new copy information. The git backend would discard the information it was given.
  • Write copy information when rebasing a commit. We would read the copy information in the old commit, then trace copies in affected files between the old parent and the new parent. We then calculate an updated set of copies and write them.

There might be other cases I'm forgetting.

Can we handle the case where you squash a rename into some old commit and descendant commits that modify the old file get update to update the new file? Mercurial doesn't handle this case.

How do we handle renamed directories? Do we support recording a directory rename (probably not)? We can probably check if all files in directory were renamed and treat that as a directory rename. When merging, that would mean that any new files in the old directory get moved to the new directory. I think that's what Mercurial does.

Do we want to have flags to disable copy tracing?

@joyously
Copy link

Do the other VCSs have docs for how they do it?

@torquestomp
Copy link
Contributor Author

I'm closing this PR in favor of #3574 instead

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants