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

FR: make fetch automatic rebase behavior optional #2504

Closed
BatmanAoD opened this issue Nov 2, 2023 · 25 comments
Closed

FR: make fetch automatic rebase behavior optional #2504

BatmanAoD opened this issue Nov 2, 2023 · 25 comments

Comments

@BatmanAoD
Copy link
Collaborator

Is your feature request related to a problem? Please describe.
Users accustomed to git do not expect fetch to make any changes to the working state or to perform any rebasing work. Even with --prune, fetch will leave some commits unreachable, but it will never cause anything like the automatic-abandoning behavior in jj.

However, jj fetch automatically abandons commits if the only branch pointing to those commits is deleted in the remote. This is true even for commits that are still reachable from the working copy. Subsequent commits are rebased to account for the deletion, so the final state of the working copy is akin to applying git revert <deleted branch>.

Describe the solution you'd like
I think that ideally this behavior wouldn't even be part of fetch; it should be part of a command that implies modification to the working copy, such as pull.

Describe alternatives you've considered
This behavior should at least be avoidable. Options:

  • Provide a config option to turn this behavior off entirely
  • Change the default fetch behavior, and put this behavior behind a command-line flag
  • Keep the default fetch behavior, and put a non-destructive update behind a command-line flag

Additional context
Original discussion here: #2481 (reply in thread)

@martinvonz
Copy link
Owner

martinvonz commented Nov 2, 2023

There's #1039 about adding a jj git sync command. We could move the functionality there and let jj git fetch be more like git fetch.

Fun fact: hg pull is like git fetch and hg fetch is like git pull. So "a command that implies modification to the working copy" depends on your background, not surprisingly. And obviously there are about 100 times as many git users as hg users, so we need to care more about that.

@PhilipMetzger
Copy link
Collaborator

I'd rather see #1039 completed and the command being promoted into jj fetch with a pull alias, for all of those who need it.

Making it configurable seems to me a promise of stability, which I rather would avoid.

@BatmanAoD BatmanAoD changed the title FR: make fetch automatic rebase behavior configurable FR: make fetch automatic rebase behavior optional Nov 2, 2023
@BatmanAoD
Copy link
Collaborator Author

Fun fact: hg pull is like git fetch and hg fetch is like git pull.

That's interesting, but the very first thing I discovered about hg fetch when I googled it is that it's an "unloved feature": https://wiki.mercurial-scm.org/FetchExtension

@PhilipMetzger when you say you'd rather see "the command being promoted into jj fetch with a pull alias", I'm not really sure what you mean. Are you saying jj fetch should be an alias for jj sync, and would still cause automatic updating/rebasing?

I've changed the title from "configurable" to "optional"; a configuration option is only one of the possible solutions I mentioned.

@PhilipMetzger
Copy link
Collaborator

@BatmanAoD Sorry for being unclear, quoting myself from the past: (#1039 (comment))

I think jj sync should only pull, like its [Perforce predecessor]
(https://www.perforce.com/manuals/cmdref/Content/CmdRef/p4_sync.html). If that isn't clear enough, we can alias the command for Git users to jj pull

If we promote jj git fetch to jj fetch, I'd expect jj pull to be aliased to the nearest equivalent for git pull. Then jj fetch can keep it's behavior for your backend™.

the command being promoted into jj fetch with a pull alias, for all of those who need it

This sentence implies that we'd alias jj git fetch/fetch to jj pull if we went the other way from your FR.

I've changed the title from "configurable" to "optional"; a configuration option is only one of the possible solutions I mentioned.

👍

I hope this explains my thoughts a bit better.

@martinvonz
Copy link
Owner

By the way, jj git import and the automatic import that happens in colocated repos does the same thing w.r.t. propagating deletions (from the colocated git repo in that case). I don't know if the config option should apply to that as well or if we would want two separate ones. We don't need to decide that now, but I thought I'd mention it here so we don't forget about that when we start working on this.

@yuja
Copy link
Collaborator

yuja commented Nov 3, 2023

It might be the option to not merge remote branches in to locals.
If jj git fetch doesn't abandon commits at all, any previous remote-only branches would persist as anonymous branches. I think that's quite different from git fetch where anonymous branches are practically dead.

@BatmanAoD
Copy link
Collaborator Author

@martinvonz Just discovered that myself. This means, as far as I can tell, that it is impossible to perform any kind of sync without getting this automatic-rebase behavior. I used git fetch, then the next jj status performed this automatic rebase operation, creating conflicts on three of my branches. That's...not ideal.

...and, as far as I can tell, jj undo doesn't even work to get out of this situation. And after using jj op restore <latest-before-import>, it seems that jj git import no longer performs this automatic delete-and-rebase?

@martinvonz
Copy link
Owner

Just a quick note for now:

creating conflicts on three of my branches. That's...not ideal.

Just to be sure, you might be able to get rid of the conflicts by simply rebasing the commits onto main (or wherever the replacements for the deleted commits are).

@BatmanAoD
Copy link
Collaborator Author

That makes sense; thanks.

Though I do occasionally have multiple branches with intermingled history, and then I delete one of them; e.g. I'll do this even just to ensure that when I open an MR, the branch name is relevant. Will fetch in this case delete the commits on the remaining branch that were reachable from the deleted branch?

@BatmanAoD
Copy link
Collaborator Author

@PhilipMetzger Sorry, I'm actually still not clear: are you suggesting that any of those commands would skip the automatic-commit-deletion that I'm asking about, or not?

@martinvonz
Copy link
Owner

Though I do occasionally have multiple branches with intermingled history, and then I delete one of them; e.g. I'll do this even just to ensure that when I open an MR, the branch name is relevant. Will fetch in this case delete the commits on the remaining branch that were reachable from the deleted branch?

No (IIUC your question), any commits that were shared between two branches are still reachable when one branch gets deleted, so they will not be considered abandoned by the remote.

@PhilipMetzger
Copy link
Collaborator

@BatmanAoD Yes, that's correct.

@BatmanAoD
Copy link
Collaborator Author

Just to be sure, you might be able to get rid of the conflicts by simply rebasing the commits onto main (or wherever the replacements for the deleted commits are).

This worked for all but one branch, and, eventually, I figured out that that's because that branch had commits from not one but two branches I had squash-merged into master and deleted.

I squash-merge nearly all my MRs, so I'm concerned that this makes jj pretty much unusable for me until there's some way to sync with git without forcibly pruning commits in this way. In this particular case I ended up just deleting the .jj repository entirely.

@BatmanAoD
Copy link
Collaborator Author

For what it's worth the branch that got screwed up still had a remote tracking branch. So I don't know if the fact that a rebase was attempted is a bug, or if this is the desired behavior and I still just don't grok what's happening or why.

...but to be honest, getting a pageful of conflicted commits because I ran jj status has...uh... soured me a lot on even attempting to use jj.

@martinvonz
Copy link
Owner

This worked for all but one branch, and, eventually, I figured out that that's because that branch had commits from not one but two branches I had squash-merged into master and deleted.

Which of these cases do you mean that you had before branch 1 and branch 2 were merged?

1:

o branch3
|
o branch2
|
o
|
o branch1 
|
o
|
o main

2:

o branch3
|\
o | branch2
| |
o |
| |
| o branch1
| |
| o
|/
o main

3:

o branch3
|
| o branch2
| |
| | o branch1
| |/
|/
o
|
o main

I squash-merge nearly all my MRs, so I'm concerned that this makes jj pretty much unusable for me until there's some way to sync with git without forcibly pruning commits in this way. In this particular case I ended up just deleting the .jj repository entirely.

I'm still not convinced that leaving the commits that were deleted on the remote around is going to help you. Sometimes it might just look like a mess with commits that have conflicts, but if you just rebase them to the right place, the conflicts often go away. One of the reasons I'd like to get a jj git sync implemented is exactly so we don't expose the scary intermediate state with conflicted commits as often.

Let's look at a simple case where your repo looks like this before fetching:

@ zzy
|
o zzx
|
o yyy branch1
|
o yyx
|
o xxx main

If you now jj git fetch, and branch1 on the remote had been squash-merged into main, you might now be in this state:

o wwx main
|
| @ zzy conflict
| |
| o zzx conflict
|/
o xxx

If you then jj rebase -d main, your conflicts would go away, because your commits are based on the same state again.

...but to be honest, getting a pageful of conflicted commits because I ran jj status has...uh... soured me a lot on even attempting to use jj.

Is your project such that you are able to share the jj log output? That might make it much easier to help explain.

@martinvonz
Copy link
Owner

Let's look at a simple case where your repo looks like this before fetching:

@ zzy
|
o zzx
|
o yyy branch1
|
o yyx
|
o xxx main

If you now jj git fetch, and branch1 on the remote had been squash-merged into main, you might now be in this state:

o wwx main
|
| @ zzy conflict
| |
| o zzx conflict
|/
o xxx

Using this simple example is probably also a good way to illustrate what this FR is asking for. You're asking to instead be left in this state after jj git fetch:

o wwx main
|
| @ zzy
| |
| o zzx
| |
| o yyy
| |
| o yyx
|/
o xxx

Here, there is no conflict in zzx and zzy. After fetching, you will do jj rebase -s zzx -d main and then jj abandon yyx:: to get into the state you want to be.

Let me know if I misunderstood.

@BatmanAoD
Copy link
Collaborator Author

BatmanAoD commented Nov 5, 2023

Is your project such that you are able to share the jj log output?

I'd rather not share it directly, but I'm not even sure a redacted version would help much; the log I captured is 359 lines long. For now let's try looking at a toy repo.

Here's a scenario where I expected some commits (68350472 and 847ab580) to be deleted:

RM-LOSX-0F2WP14 in jj-demo (187ab4a) [!]
❯ : jj log
◉  ztuylvok kstrand@rigetti.com 2023-11-04 19:04:06.000 -06:00 main 525b9333
│  "branch-1 change"
│ @    lrmqluzv kstrand@rigetti.com 2023-11-04 19:03:27.000 -06:00 git-branch-2 98b7574c
│ ├─╮  Merge branch 'git-branch-1' into git-branch-2
│ │ ◉  rqxnvlvo kstrand@rigetti.com 2023-11-04 18:55:44.000 -06:00 git-branch-1 68350472
│ │ │  "branch-1 edit"
│ │ ◉  kktoynvy kstrand@rigetti.com 2023-11-04 18:48:01.000 -06:00 847ab580
├───╯  "branch-1 change"
│ ◉  rsyxlkys kstrand@rigetti.com 2023-11-04 18:48:31.000 -06:00 HEAD@git 187ab4a1
├─╯  "branch-2 change"
◉  tskvnyqv kstrand@rigetti.com 2023-11-04 18:46:58.000 -06:00 98f3fa9c
│  "initial Git commit"
◉  zzzzzzzz root() 00000000

RM-LOSX-0F2WP14 in jj-demo (187ab4a) [!]
❯ : git branch -D git-branch-1
Deleted branch git-branch-1 (was 6835047).

...but they actually weren't:

RM-LOSX-0F2WP14 in jj-demo (187ab4a) [!]
❯ : jj log
Done importing changes from the underlying Git repo.
◉  ztuylvok kstrand@rigetti.com 2023-11-04 19:04:06.000 -06:00 main 525b9333
│  "branch-1 change"
│ @    lrmqluzv kstrand@rigetti.com 2023-11-04 19:03:27.000 -06:00 git-branch-2 98b7574c
│ ├─╮  Merge branch 'git-branch-1' into git-branch-2
│ │ ◉  rqxnvlvo kstrand@rigetti.com 2023-11-04 18:55:44.000 -06:00 68350472
│ │ │  "branch-1 edit"
│ │ ◉  kktoynvy kstrand@rigetti.com 2023-11-04 18:48:01.000 -06:00 847ab580
├───╯  "branch-1 change"
│ ◉  rsyxlkys kstrand@rigetti.com 2023-11-04 18:48:31.000 -06:00 HEAD@git 187ab4a1
├─╯  "branch-2 change"
◉  tskvnyqv kstrand@rigetti.com 2023-11-04 18:46:58.000 -06:00 98f3fa9c
│  "initial Git commit"
◉  zzzzzzzz root() 00000000

Why is that?

@BatmanAoD
Copy link
Collaborator Author

In any case, yes, that last code-block you showed is indeed the state I'd prefer the repo to be left in:

o wwx main
|
| @ zzy
| |
| o zzx
| |
| o yyy
| |
| o yyx
|/
o xxx

@martinvonz
Copy link
Owner

Here's a scenario where I expected some commits (68350472 and 847ab580) to be deleted:

That's working as intended. JJ supports "anonymous heads", just like mercurial does. That means we keep track of visibility separately from keeping track of branches. As you probably know, you don't need a branch to keep a commit visible. In your example, you just deleted a branch. We could have an option for deleting a branch and commits reachable from only that branch, if we think that's a common thing to want (I basically never use branches myself, so I don't want it). Or we could make it an option to jj abandon to also delete branches pointing to the abandoned commits.

In any case, yes, that last code-block you showed is
indeed the state I'd prefer the repo to be left in:

Okay, that should be easy to support, but I haven't attempted it yet. Seems like we would just need a config option and have the passes into function that imports refs from the git repo.

@BatmanAoD
Copy link
Collaborator Author

Sorry, what I'm trying to get at is that I still don't actually understand the rules of the jj behavior I'm complaining about. I'm confused about what actually triggers the automatic-rebase in jj git import that I'm hoping becomes optional. I thought that "deleting a branch and commits reachable from only that branch" was exactly what you were saying jj git import always does when it sees that a branch has been deleted from Git. (I wasn't using jj git fetch because you said that jj git import "does the same thing," and I observed this behavior with git fetch -p; jj status myself.)

So why are 68350472 and 847ab580 considered "visible" in that example, and how would I change the example to demonstrate the auto-rebase behavior?

@martinvonz
Copy link
Owner

Sorry, what I'm trying to get at is that I still don't actually understand the rules of the jj behavior I'm complaining about.

Oh, that was my fault for misreading your example. I didn't realize before that you deleted the branch using git. Sorry about adding to the confusion :(

I'm confused about what actually triggers the automatic-rebase in jj git import that I'm hoping becomes optional. I thought that "deleting a branch and commits reachable from only that branch" was exactly what you were saying jj git import always does when it sees that a branch has been deleted from Git. (I wasn't using jj git fetch because you said that jj git import "does the same thing," and I observed this behavior with git fetch -p; jj status myself.)

The rule is that if a commit used to be reachable by git from branches, tags, or HEAD and no longer is, then we consider it abandoned, and we auto-rebased descendants off of those commits.

So why are 68350472 and 847ab580 considered "visible" in that example, and how would I change the example to demonstrate the auto-rebase behavior?

It looks like they're both still reachable from git-branch-2 pointing to 98b7574c.

@BatmanAoD
Copy link
Collaborator Author

In that case, I'm still not sure why a rebase would ever occur, except in the special case of the working-copy commit not being pushed back to the remote in any way.

When I did the git fetch -p; jj status that caused multiple branches to have conflicts, those branches all had corresponding remote tracking branches. So all commits on those branches should have been reachable; thus none should have been abandoned; right?

@BatmanAoD
Copy link
Collaborator Author

Or, to look at your example:

@ zzy
|
o zzx
|
o yyy branch1
|
o yyx
|
o xxx main

Then squash-merge branch1 and re-sync with git:

o wwx main
|
| @ zzy conflict
| |
| o zzx conflict
|/
o xxx

Why are zzx and zzy preserved, but yyy and yyx deleted? Is it that, as far as jj knows, git never had a branch or tag with those commits?

@martinvonz
Copy link
Owner

Why are zzx and zzy preserved, but yyy and yyx deleted? Is it that, as far as jj knows, git never had a branch or tag with those commits?

Exactly.

In the case where there are several stacked branches in git or on the remote and a branch gets deleted from a non-leaf/head commit in git, then we don't consider that as abandoning the commits (because they're still reachable). For me (who is of course used to the propagation of deleted commits and the associated auto-rebasing), that's actually annoying sometimes. It's a bit of the reverse problem that you're having. A few days ago, I had two stacked PRs and the bottom one got merged. I wish those commits had been abandoned then so I could just jj rebase -d main as usual after fetching. But because jj couldn't assume that the commits on the bottom branch were meant to be abandoned, I had to manually abandon them instead.

I'm working on the config option, btw. Looks promisingly simple so far.

@yuja
Copy link
Collaborator

yuja commented Nov 5, 2023

When I did the git fetch -p; jj status that caused multiple branches to have conflicts, those branches all had corresponding remote tracking branches. So all commits on those branches should have been reachable; thus none should have been abandoned; right?

Existing remote branches aren't preserved because there may be multiple remotes having the same branch, and deletion in one remote should be propagated to the local commits. FWIW, if you've set git.auto-local-branch = false, and are using old jj, you might see scary rebase behavior. It's broken at 520f692 (in jj 0.10.0) and fixed by 57167ce (in jj 0.11.0).

martinvonz added a commit that referenced this issue Nov 5, 2023
This motivation for this is so we can easily skip calling the function
if the user has opted out of the propagation of abandoned commits we
usually do (#2504). However, it seems like a good piece of code to
extract regardless of that feature.
martinvonz added a commit that referenced this issue Nov 5, 2023
Some users prefer to have commits not get abandoned when importing
refs. This adds a config option for that.

Closes #2504.
martinvonz added a commit that referenced this issue Nov 5, 2023
This motivation for this is so we can easily skip calling the function
if the user has opted out of the propagation of abandoned commits we
usually do (#2504). However, it seems like a good piece of code to
extract regardless of that feature.
martinvonz added a commit that referenced this issue Nov 5, 2023
Some users prefer to have commits not get abandoned when importing
refs. This adds a config option for that.

Closes #2504.
martinvonz added a commit that referenced this issue Nov 5, 2023
This motivation for this is so we can easily skip calling the function
if the user has opted out of the propagation of abandoned commits we
usually do (#2504). However, it seems like a good piece of code to
extract regardless of that feature.
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this issue Dec 11, 2023
## [0.12.0] - 2023-12-05

### Breaking changes

* The `remote_branches()` revset no longer includes branches exported to the Git
  repository (so called Git-tracking branches.)

* `jj branch set` no longer creates a new branch. Use `jj branch create`
  instead.

* `jj init --git` in an existing Git repository now errors and exits rather than
  creating a second Git store.

### New features

* `jj workspace add` can now take _multiple_ `--revision` arguments, which will
  create a new workspace with its working-copy commit on top of all the parents,
  as if you had run `jj new r1 r2 r3 ...`.

* You can now set `git.abandon-unreachable-commits = false` to disable the
  usual behavior where commits that became unreachable in the Git repo are
  abandoned ([#2504](martinvonz/jj#2504)).

* `jj new` gained a `--no-edit` option to prevent editing the newly created
  commit. For example, `jj new a b --no-edit -m Merge` creates a merge commit
  without affecting the working copy.

* `jj rebase` now takes the flag `--skip-empty`, which doesn't copy over commits
  that would become empty after a rebase.

* There is a new `jj util gc` command for cleaning up the repository storage.
  For now, it simply runs `git gc` on the backing Git repo (when using the Git
  backend).

### Fixed bugs

* Fixed another file conflict resolution issue where `jj status` would disagree
  with the actual file content.
  [#2654](martinvonz/jj#2654)
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

No branches or pull requests

4 participants