Implement FR #2338 for current branches / automatically advancing branches

[emesterhazy]

This feature is still a work in progress. To use advance-branches, you need to adjust your config.toml. The feature can be enabled globally with overrides, or disabled globally (the default) with overrides. Any branches appearing in the override list will be treated as if the value of advance-branches.enabled is negated.

For example:

[advance-branches]
enabled = true
overrides = ["main"]

For this config all branches except main will be automatically advanced by jj commit (more on what that means later).

So far, this PR implements two features:

1. When jj commit is run it will look at the parent of @ for a branch that is eligible to be advanced based on the user's config. If more than one branch is found, an error is returned and the user is asked to resolve the ambiguity. If a qualified branch is found, the branch will automatically be moved to @, which becomes the parent of the new working commit once jj commit finishes.
2. When the working commit is changed in a colocated repo and there is an advance-branches eligible branch pointing to its first parent, the Git HEAD is set to the branch ref instead of being detached at the parent commit id. This applies any time the working commit is changed, not just when jj commit is run.

To recap, curently only jj commit will advance a branch, and any action that changes the working commit in a colocate repo may set Git HEAD to an advance-branches eligible branching pointing to the parent of the working copy instead of detaching the Git HEAD.

I'd like to collect feedback before implementing branch advancement for jj new.

[emesterhazy]

@martinvonz @yuja Would you mind taking a look at this and sharing your thoughts when you get a chance?

We've been discussing variations of what I think is the same idea in https://github.com/martinvonz/jj/issues/2338. I think this implementation will work well for Git users, and still makes sense for jj users where the working commit (i.e. what's created by jj commit)is typically WIP analogous to an uncommitted working copy in Git.

[martinvonz]

I think this implementation will work well for Git users, and still makes sense for jj users where the working commit (i.e. what's created by jj commit)is typically WIP analogous to an uncommitted working copy in Git.

Thanks for mentioning that. I think it's important that we design something that works well in repos that are not colocated with Git. Once we have come up with something we like, we should try to make our code for colocated repos adapt to that model. So I think we should try to ignore how Git behaves when we design this feature, unless we say that this is a feature designed only for Git interop (so it would apply only when using the Git backend?).

@martinvonz @yuja Would you mind taking a look at this and sharing your thoughts when you get a chance?

I think I'm back to preferring the model where we consider branches pointing to @ as active, even if that means more work when exporting to Git.

[emesterhazy]

I think I'm back to preferring the model where we consider branches pointing to @ as active, even if that means more work when exporting to Git.

I think this means that when jj commit is run, only the branch pointing at @ is eligible to advance, and it will be moved to the new commit created by jj commit is that right? The current PR does this for @- instead of @.

Can you elaborate on why you think it's better to use @ instead of @-? I actually think that using @- as the target makes more sense because the new commit created by jj commit or jj new is usually either left empty (if you're done and pushing your work to Github or some other forge) or is an undescribed work-in-progress commit. I actually don't think I'd want the branch to move to the new @ commit until I'm ready done with it and ready to jj new on top of it.

[martinvonz]

I think this means that when jj commit is run, only the branch pointing at @ is eligible to advance, and it will be moved to the new commit created by jj commit is that right? The current PR does this for @- instead of @.

Correct.

Can you elaborate on why you think it's better to use @ instead of @-? I actually think that using @- as the target makes more sense because the new commit created by jj commit or jj new is usually either left empty (if you're done and pushing your work to Github or some other forge) or is an undescribed work-in-progress commit. I actually don't think I'd want the branch to move to the new @ commit until I'm ready done with it and ready to jj new on top of it.

You might be right. I never use branches so I'm not sure what behavior people want. We generally don't treat @- differently. Maybe that's all that bothers me about this proposal. But again, I'm not the target audience for this.

[yuja]

Just dropping random comments. I'm also not the target audience as I always start git with checkout --detach.

(In the commit message)

We need to support jj new, and we need to add a --branch argument to jj new

It would be weird if jj new --branch created a branch on the parent of the newly created commit. And it's not possible if new created a merge. I think this is the same kind of problem that jj git push -b would have to push the parent if the current branch were advanced to @.

FWIW, when learning Git, I find it's confusing that git checkout -b is most likely the right command to create new branch, and git branch is NOT the command to start working on the created branch.

https://github.com/martinvonz/jj/pull/3129#issuecomment-1962774589

We generally don't treat @- differently. Maybe that's all that bothers me about this proposal.

For some local commands like jj log -r..<current-branch>, it would be more useful if the current branch is pointing to @. For git push/export (and native push?), it's not. I have no idea how to model that in jj.

[emesterhazy]

I think whatever we do, if anything, the model needs to be simple and intuitive. I don't think that we should, for example, have the branch set to @ in jj but push @- to github, or have the branch set to @ in jj and set the git ref for the branch to @-. I think that type of inconsistency will be cumbersome and confuse users.

I think there are probably three user "stories" for this feature:

1. I don't care about automatically advancing branches and will leave this feature disabled.
2. I am using jj in a non-colocated repo and am pushing multi-commit branches to a Git forge. When I am working on a branch, I want it to point to my last finished commit. When I am done working, I will have @ set to an empty commit as the result of jj commit or jj new. I don't want to push this empty commit to my forge.
3. I am using jj in a git colocated repo and am pushing multi-commit branches to a Git forge. When I am working on a branch, I want it to point to my last finished commit. If I use the git CLI, I want HEAD to be set to the branch instead of being detached so that git behaves normally. When I am done working, I will have @ set to an empty commit as the result of jj commit or jj new. I don't want to push this empty commit to my forge.

If you look at the original FR https://github.com/martinvonz/jj/issues/2338#issue-1928609981 I think it's asking for an easy way to set the "working" branch to @- automatically. I think that user probably falls into category 2.

Do you think there's a user story that's better served by advancing the branch to @ instead of @-? What do you think it looks like?

[martinvonz]

If there are unfinished commits on top of branches, I probably would want to see those changes when I run jj log main..my-branch, for example. That would require the branch to point to the latest commit, whether or not that commit is finished.

It sounds like this feature is only for Git interop. Your two no-op user stories were both about Git interop. So do you think we should simply put this in a jj git commit command? User who use that workflow a lot can create an alias for that command. That seems like a simple solution that doesn't involve trying to figure out a jj-native workflow for advancing branches.

[BatmanAoD]

So do you think we should simply put this in a jj git commit command? User who use that workflow a lot can create an alias for that command. That seems like a simple solution that doesn't involve trying to figure out a jj-native workflow for advancing branches.

I'm not sure how much sense that makes logically, but as one data point, I do pretty often mistype jj commit as jj git commit.

But isn't this really about how branches are used, rather than how git is used? How about jj branch commit?

[BatmanAoD]

I already said this in the issue thread, but I don't think this feature would make much sense if it used @ instead of @-. I'll quote what I wrote there:

• Possibly the biggest benefit of jj is being able to have a record of every workflow state the tool has seen stored as a proper commit, like an auto-updating stash, without "committing" changes to a branch until you're actually ready to do so.
• @ is usually empty once you're ready to push to a remote (hence new being preferred to edit, and commit being equivalent to describe+new rather than just describe). But it doesn't make sense to commit empty commits to a remote, and commits without descriptions can't be pushed.

[emesterhazy]

I think the question about whether the branch should move to @ or @- is fundamentally about what a branch represents and which changes should be included. Right now it's completely up to the user because they need to manually move the branch. This is where my opinions come in, but I think that one of the most common uses of branches is to point to a line of "completed" work, and the more I think about that use case the more I'm convinced that advancing the branch to @- is the only way to make this feature ergonomic.

If we move the branch to @:

• In your jj log main..my_branch example you would see the empty or unfinished commit.
• If you run jj new my_branch it will create a second empty commit on top of the one my_branch points to.
• If you don't make any changes to the empty commit the branch points to and run jj new on a different revision, the empty commit will not be abandoned. Without a branch set to the empty commit, it would be abandoned.
• If you want to build on top of the last "completed" commit in my_branch you'll need to use jj edit my_branch instead of jj new
• If you are pushing a branch to a forge (git or otherwise), you'll need to manually move it behind the empty commit.

If we move the branch to @-:

• You can run jj log (main..my_branch):: to see any unfinished commits on top of your branch.
• You can run jj new my_branch to get a new empty commit on top of the branch. If you don't make any changes and run jj new on a different revision, the empty commit will be abandoned (this is the current default behavior).
• If you want to build on top of the last "completed" commit in my_branch, you can just run jj new my_branch. You don't need to switch to the edit command to avoid creating an extra empty commit.
• If you are pushing a branch to a forge, you don't need to move the branch before pushing to avoid including an empty commit.

jj doesn't distinguish between committed and uncommitted changes, but that doesn't mean that users don't treat the "working commit" at @ differently than @-. Even jj itself treats the empty @ commits created by new and commit differently since it will abandon them if you make no changes and run jj new on a different revision.

I think "branches point to finished work" for some definition of "finished work" that doesn't include empty commits created by jj new and commit is probably the most common use of branches. As long as we support a common and reasonable workflow I think we're in good shape. Users can always opt out of automatically advancing branches and have full control over the branch pointers.

Let's try to sort out the behavior and supported use cases first and then decide if this should be controlled by a config setting or flag on jj new and jj commit.

[yuja]

I think that one of the most common uses of branches is to point to a line of "completed" work, and the more I think about that use case the more I'm convinced that advancing the branch to @- is the only way to make this feature ergonomic.

Regarding "completed" work, I think @- is often unfinished (wip) commit under topic branch worflow, and for me, @- is not so different from @ in that they are all draft changes. I would expect jj log -r [email protected]_branch shows the stack of draft changes including @ if my_branch auto-advances.

If we move the branch to @: [...]

• If you don't make any changes to the empty commit the branch points to and run jj new on a different revision, the empty commit will not be abandoned. Without a branch set to the empty commit, it would be abandoned.

I made this change to support a workflow where named branch is set at the root of the stack #1854, not at the head (which would require auto-advancing.) If we decide to set the active branch to @, it might be better to revert the change (or at least make it configurable.)

• If you are pushing a branch to a forge (git or otherwise), you'll need to manually move it behind the empty commit.

Yes, this is the problem. It's unclear whether the @ commit should be pushed if it's not empty.

[martinvonz]

If we decide to set the active branch to @, it might be better to revert the change (or at least make it configurable.)

+1

• If you want to build on top of the last "completed" commit in my_branch, you can just run jj new my_branch. You don't need to switch to the edit command to avoid creating an extra empty commit.

OTOH, you will then start a new commit when you might have wanted to continue working on the unfinished commit you already had on top of my_branch.

• If you are pushing a branch to a forge (git or otherwise), you'll need to manually move it behind the empty commit.

Yes, this is the problem. It's unclear whether the @ commit should be pushed if it's not empty.

I think jj git push should not push it. That would better match Git's behavior.

I don't know what I would expect non-Git forges to do. FWIW, when exporting to our internal forge at Google, we do export the @ commit too, but it's too early to say yet if users will like that behavior.

I'm still not sure which is the right choice. I don't want to block progress on this. As you long as you don't change the default behavior too much, we can just get something in so people can experiment with it.

[emesterhazy]

I'm starting to think there's no perfect solution here since there are many possible workflows. I still think that @- is the "lesser of two evils", but that's as much as it is since you can conceive of a useable workflow where branches are advanced to @ as well.

So, I would just like to acknowledge that there are lots of different opinions here, and we may not be able to come up with a solution that's simple and useful for everyone.

@martinvonz @yuja if you're interested in submitting something experimental so that users can try it without needing to build jj from this branch, here's what I'd suggest:

1. I will finish the code for jj new and add more tests.
2. Can we name the config option experimental-advance-branches to make it clear that this feature may change or be removed?
3. Whether or not to advance to @ or @- doesn't actually make much difference in the code. I can add another config option to control that behavior.
4. If we add the @ config option, I can look into disabling the behavior from #1854. I don't think we should add a separate off switch, so if we think it should be disabled if branches are advanced to @, then we should just do it.

What do you think?

[martinvonz]

SGTM. I think we can ignore items 3 and 4 until people have experimented with the feature for a while. I don't want you to waste time implementing item 3 if it seems like everyone is happy with what you already have in this PR.

Thanks for all the time you've spent thinking and coding on this.

[yuja]

1. I will finish the code for jj new and add more tests.
2. Can we name the config option experimental-advance-branches to make it clear that this feature may change or be removed?
3. Whether or not to advance to @ or @- doesn't actually make much difference in the code. I can add another config option to control that behavior.

If we choose the @ model, we won't need the advance-branches config. Having a branch on @ means the branch is current/active, and it will advance. In other words, HEAD = <branch> (not detached) in git is encoded as @ = <branch>, @- = <branch>@git in jj. Anyway, I agree that trying 3 isn't needed. IMHO, it would be quite different from the @- design.

I'm skeptical about jj new --branch because it wouldn't make sense if experimental-advance-branches is off.

[BatmanAoD]

Tried this out very briefly just now, and it looks like it does exactly what I was expecting. I'll be using it this week and see if I have further feedback. Thanks very much for implementing!

[BatmanAoD]

This is excellent. It's exactly what I want.

[emesterhazy]

Thanks for the feedback. I'll add support for jj new as well. Just to be clear, this means that if jj new is run on revision xyz and one of the parents of xyz has an advanceable branch pointing to it, then the branch will be advanced to xyz. It is analogous to how jj commit works, except that you can specify a revision.

I'm skeptical about jj new --branch because it wouldn't make sense if experimental-advance-branches is off.

I'm not planning to add any flags to jj new or other commands. I think in an earlier version I proposed adding a flag to disambiguate in scenarios where multiple branches might advance, but I think we should just make the user manually move the branch with jj branch set in those cases. So now my plan is to have this controlled exclusively by the config.

[emesterhazy]

@martinvonz

Hi everyone, sorry for the delay. This is ready for review now. A couple of notes:

1. The config setting is called experimental-advance-branches. If we decide to stabilize this we can remove the experimental prefix.
2. I added support for jj new. It will only advance branches if a single revision is targeted and neither --after or --before are used. I think this is a reasonable limitation to keep the behavior simple. In the common case where commits are usually appended, this limitation doesn't matter.
3. This PR does not include the change to not detach the Git head for colocated repos. I will mail a separate PR after this one is submitted. That PR will also include updates to the documentation and changelog.
4. As I mentioned in a comment earlier, I changed the semantics so that multiple branches can advance at the same time. This ends up being simpler for the user since they don't need to worry about resolving ambiguity.
5. I updated the PR description based on the new updated semantics.

[emesterhazy]

Thanks Yuya for all of the helpful feedback. I'll go through your comments and address one by one.

[emesterhazy]

I've made changes and resolved most of the conversations based on your comments. Please take another look. Thanks!