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: --advance-branches flag for jj commit #2338

Open
emilykfox opened this issue Oct 5, 2023 · 52 comments · Fixed by #3129
Open

FR: --advance-branches flag for jj commit #2338

emilykfox opened this issue Oct 5, 2023 · 52 comments · Fixed by #3129
Labels
good first issue Good for newcomers

Comments

@emilykfox
Copy link
Collaborator

emilykfox commented Oct 5, 2023

Is your feature request related to a problem? Please describe.
I often work directly on main for personal projects, but to successfully push my changes to GitHub, I have to run something like jj commit -m [message] && jj branch set main -r @- or jj branch set main && jj commit -m [message]. The former is error prone as it's easy to forget the -r flag. The latter feels slightly unnatural as there's a brief moment where the main branch is pointing to something without a description. They're both pretty verbose.

Describe the solution you'd like
In Discord, @martinvonz suggested adding a --advance-branches flag to jj commit which could move any branches pointing to the old @- to point to the new @-.

Describe alternatives you've considered
None

Additional context
See the brief conversation starting at https://discord.com/channels/968932220549103686/969829516539228222/1159510245849186335.

Probably more important is that this feature would also improve workflows that involve adding several commits in sequence to a single feature branch.

@ilyagr
Copy link
Collaborator

ilyagr commented Oct 5, 2023

I'd like this too.

Some miscellaneous thoughts:

  1. I think jj new could also have such a flag, for moving branches from whichever commit jj new makes the parent of the new commit (usually @).
  2. This is minor, but I think jj commit --advance-branches should also move branches from old @ to new @ (if any).
  3. (Not for the first version) I wish there was some way to specify the branches that define immutable commits (e.g. main), so that jj commit --advance-branches would know not to move these branches. This would make defining the immutable commits revset harder, though, since it's actually defined by main@origin and main@upstream.

@necauqua
Copy link
Collaborator

necauqua commented Oct 8, 2023

I think I've advocated in the past that jj commit should just do that - makes sense as commit is "an alias to be familiar for git users, as well as just a shortcut for new+describe"

Maybe there should be an option for jj new, and then we describe commit as an alias for new with such flag+describe - again, makes total sense as it is just for more git-like workflow

@martinvonz
Copy link
Owner

martinvonz commented Oct 10, 2023

EDIT: The below was in response to @necauqua's comment above.

I'm concerned that not all users who do jj new main; <edit files>; jj commit want to move main forward. Some prefer to work with anonymous branches (aka detached heads) but still like to use jj commit as a describe+new shortcut.

@necauqua
Copy link
Collaborator

necauqua commented Oct 10, 2023

Well, I now realised that if we just have the flag (probably on both commands), then I can alias it anyway

Especially since in personal repos I do anonymous heads and at work I have to maintain feature branches (and I do use the commit shortcut as much as it "un-coreness" bothers me, it's still a great shortcut) :)

Such an alias (don't remember off the top of my head if you can alias used words like "ci/commit") could then be somewhere at the "useful aliases" page

@ilyagr
Copy link
Collaborator

ilyagr commented Oct 13, 2023

3. (Not for the first version) I wish there was some way to specify the branches that define immutable commits (e.g. main), so that jj commit --advance-branches would know not to move these branches. This would make defining the immutable commits revset harder, though, since it's actually defined by main@origin and main@upstream.

A simpler version of this idea: we could have a config that lists some names of branches that are not advanced by --advance-branches, defaulting to ["main", "master"] (and perhaps some more, I'm not sure).

We could then think about whether we can define immutable-heads using such a list of branches but, again, this could be a bit tricky or controversial.

@martinvonz martinvonz added the good first issue Good for newcomers label Oct 25, 2023
@BatmanAoD
Copy link
Collaborator

I'm a bit confused by the above conversation about main and master; does it predate the existence of the trunk() alias? Doesn't the trunk() alias pretty much resolve the question of how to exclude main and master from automatic advancement?

@BatmanAoD
Copy link
Collaborator

Separately: I think in general, if I have to manually --advance-branches, I'd want the ability to go back more than one commit. I.e. if I use jj new or jj commit several times and then realize I left my tracking branch behind, I want an easy way to do the equivalent of jj branch set <name> -r @- (or -r @, depending on whether the working copy is empty).

...speaking of which, the fact that the "obvious" commit to choose is either -r @ or -r @- conditionally on the state of the working copy is, I think, a really good reason to make this functionality more automatic.

@ilyagr
Copy link
Collaborator

ilyagr commented Oct 31, 2023

Doesn't the trunk() alias pretty much resolve the question of how to exclude main and master from automatic advancement?

trunk() would indeed often work, but it is currently supposed to refer to exactly one branch. I'm worried about the situation where you'd want to preserve multiple branches. For example, this would occur if immutable_heads() contained more than one branch (e.g. main and develop). You wouldn't want jj new --advance-branches to change which revisions are considered immutable.

@ilyagr
Copy link
Collaborator

ilyagr commented Feb 2, 2024

Probably in addition to --advance-branches, here's an idea that could maybe help with the fancier use-cases. It feels a bit incomplete/underbaked at the moment, but might be worth considering and evolving.

We could support a syntax along the lines of jj branch s topo:ancestor, which would find the closest ancestor commit with a branch and move that branch to the current commit.

If more than one branch fits the description, it's unclear to me what's best (exit with an error or move them all).

ancestor could also be a function, so jj branch s topo:ancestor -r qq is an alias for jj branch s 'topo:ancestor(::qq)' -r qq.

@yuja
Copy link
Collaborator

yuja commented Feb 2, 2024

What if we could support a syntax along the lines of jj branch s topo:ancestor, which would find the closest ancestor commit with a branch and move that branch to the current commit.

It could be modeled as a "branch set" expression, but I would add a separate command jj branch advance [--from REV] --to REV (or jj branch move --advance --to REV) for that purpose.

@BatmanAoD
Copy link
Collaborator

I have been thinking about this, and if we can agree on the design, would like to help implement it.

I have a few observations and questions, plus a possible design.

I'll start by explicitly stating the intent of this issue as I understand it:

Introduce a more convenient way to advance to advance branches to newer commits than branch set

Here are some characteristics I'd like the solution to have:

  • Rather than a new command, this should be a flag on new, commit, and/or describe (for new and commit, the operation would modify the "old" revision, i.e. the one to which the commit message is attached)
  • the user shouldn't need to manually specify branch names unless there's unavoidable ambiguity
  • Possibly this should be the default behavior for commit (since it's a combination of describe and new that is intended to imitate Git behavior)
  • The operation should not implicitly/automatically advance certain special branches:
    • any protected branch (in the GitHub sense; see "questions")
    • the default (i.e. remote HEAD) branch (see "questions")
    • trunk()
  • can commit & sync with the remote in one command (hopefully this could be a user-defined alias; see "questions")
  • the user should be prompted to provide descriptions for any commits without commit-messages

Questions:

  • Does jj have, or could it have, any native concept of "protected" branches? ("protected" in the GitHub/GitLab sense, not immutable_heads)
  • Similarly, does it, or could it, be aware of "default" branches? (This at least corresponds to a native git concept, the HEAD of each remote)
  • Can a user-defined alias define a combination of commands, the way commit combines describe and new?
    • Can the arguments to the original commands still be used?

Anyway, given all of that, my proposal is that this new behavior should be added as a flag to describe rather than new, because the purpose is to attach semantic information to a particular existing workspace-snapshot. This lines up well with the fact that jj git push requires both a description and a branch for each new commit being shared with the remote; so it makes sense that jj describe --advance-branches would enable providing both of these in a single command.

Proposal in more detail:

  • New flag on describe: --advance-branches
  • "forward" this flag to commit, or, better yet, make it the default behavior for commit
  • Default behavior of branch selection for advancing:
    • advance the "nearest" branch head, searching breadth-first through ancestor commits
    • Exclusions:
      • when a remote is configured, protected & default branches are excluded
      • trunk() is always excluded
      • all ancestors of excluded branches (i.e. when encountering a commit that's the head of an excluded branch, prune the rest of that ancestry path)
    • If a single candidate commit is the head of multiple branches, the command fails
    • If multiple candidate branch-head commits are at the same depth, they are all advanced
  • Configuration options:
    • additional excluded branches may be configured
    • users may opt-out of any of these exclusions (e.g. opt-in to advancing main)
    • users may choose from these non-default behaviors for multiple branches on a single commit:
      • interactively select which branch(es) to advance
      • always advance all branches
  • If it's not possible for a jj alias to combine multiple commands, then I would like for there to be a built-in "combination alias" (like commit) that would enable synchronizing work in one step (a la svn commit):
    • advance branches
    • add commit messages
    • push

@BatmanAoD
Copy link
Collaborator

@IlyaGar I'm not sure what you mean by this:

We could support a syntax along the lines of jj branch s topo:ancestor, which would find the closest ancestor commit with a branch and move that branch to the current commit.

I thought this was basically what --advance-branches was asking for? I'm also not sure what topo means in this example.

@joyously
Copy link

Does this issue arise from the difference in how jj and Git define branches? Is it only needed when pushing to Git or is it ingrained in the mental model?
Breezy commands are centered around branches, and the default is for a branch to be a separate folder, but it can also be configured to colocate branches like Git does. And it interoperates with Git either way, I suppose because they both use the same concept of the branch pointer being the tip.

The model in jj seems like a tag instead of a branch. That would work also, if the interface to the different system takes that into account. I don't think the --advance-branch flag is the right way. I think it should be a revset for push.

@BatmanAoD
Copy link
Collaborator

@joyously In git and jj, branches are indeed essentially tags. But they can still be useful locally without pushing them, so this should not be part of push.

@joyously
Copy link

@joyously In git and jj, branches are indeed essentially tags. But they can still be useful locally without pushing them, so this should not be part of push.

Perhaps it shouldn't be called a branch if it isn't one? Or define what a branch is in jj as the existing implementation, and use a different concept to equate to a branch in Git.

@BatmanAoD
Copy link
Collaborator

@joyously This is a bit of a tangent, I think, since the branch model is fairly well established and unlikely to change as part of this particular feature request. I'm not sure why or how "jj branches" and "git branches" could be distinguished, because they're the same concept; and when using git as the backing store for jj (which is currently recommended), I believe jj branches are stored as git branches.

This is a good, brief explanation of how to think about branches in git, which applies to jj as well: https://wizardzines.com/comics/meet-the-branch/

@joyously
Copy link

I'm not sure why or how "jj branches" and "git branches" could be distinguished, because they're the same concept

Well, obviously they don't quite match or this new flag would not be needed. What I've heard is that jj is its own thing and just happens to use Git as a backend right now, but could also use other VCS libraries instead. As Darcs and Breezy use a slightly different concept for branches, I don't think it's right to clutter the CLI with flags for making it work like Git.

If the jj "branch" is really a "tag", maybe call it a tag and make a "real" branch that moves with latest commit.

@BatmanAoD
Copy link
Collaborator

@joyously I'm sorry, but you're mistaken: this request is not due jj and git having different branch models, but because jj always operates in what git calls the "detached HEAD" state. It's pretty rare for git users to operate in this mode, but some people use it a lot. This post has some details about using Git in headless mode.

tags are also an existing concept, and while they are very similar to branches, they're not exactly identical, so branches cannot simply be renamed "tags".

@PhilipMetzger
Copy link
Collaborator

PhilipMetzger commented Feb 12, 2024

Sorry to barge into your conversation, we recently talked about renaming branches to bookmarks like Mercurial, which definitely fits better to the current jj branch implementation.

We then should implement also something like topics, which are the canonical set of "named commits" which everybody knows. (See #2425 (comment))

@BatmanAoD
Copy link
Collaborator

That sounds good. But would that make --advance-branches unnecessary?

For that, I think we'd need two conditions:

  • either new or describe (or both) automatically applies the current "topic" to @ (the "old" commit in the case of new)
  • topics can be pushed as branches to git remotes

@PhilipMetzger
Copy link
Collaborator

PhilipMetzger commented Feb 12, 2024

And since I'm here anyway.

Here are some characteristics I'd like the solution to have:

  • Rather than a new command, this should be a flag on new, commit, and/or describe (for new and commit, the operation would modify the "old" revision, i.e. the one to which the commit message is attached)
  • the user shouldn't need to manually specify branch names unless there's unavoidable ambiguity

This is pretty accurately describing the feature request.

  • Possibly this should be the default behavior for commit (since it's a combination of describe and new that is intended to imitate Git behavior)

Not really, only because it is a convience built-in, does not mean it is a git commit equivalent. It could be gone in a few years. A configurable option, maybe?

  • can commit & sync with the remote in one command (hopefully this could be a user-defined alias; see "questions")

This functionality does not belong here, see #1039. And even then whats the reason for it?

  • the user should be prompted to provide descriptions for any commits without commit-messages

This is also does not belong here, see #2977.

The operation should not implicitly/automatically advance certain special branches:

  • any protected branch (in the GitHub sense; see "questions")
  • the default (i.e. remote HEAD) branch (see "questions")
  • trunk()
  • Does jj have, or could it have, any native concept of "protected" branches? ("protected" in the GitHub/GitLab sense, not immutable_heads)

Answering both, no there are no special branches for jj as it isn't required and won't be required. This is purely related to Git and its implementation of branches.

  • Similarly, does it, or could it, be aware of "default" branches? (This at least corresponds to a native git concept, the HEAD of each remote)

We fetch the default Git branch since, 0.14. But this is once again a pure Git concept which doesn't make sense in a Jujutsu world.

  • Can a user-defined alias define a combination of commands, the way commit combines describe and new?

    • Can the arguments to the original commands still be used?

No, jj commit is as before a special built-in.

Anyway, given all of that, my proposal is that this new behavior should be added as a flag to describe rather than new, because the purpose is to attach semantic information to a particular existing workspace-snapshot. This lines up well with the fact that jj git push requires both a description and a branch for each new commit being shared with the remote; so it makes sense that jj describe --advance-branches would enable providing both of these in a single command.

Since this is mostly a crutch for Git users, implementing it on describe and commit makes sense.

  • New flag on describe: --advance-branches
  • "forward" this flag to commit, or, better yet, make it the default behavior for commit

👍

Default behavior of branch selection for advancing:

  • advance the "nearest" branch head, searching breadth-first through ancestor commits

This is something @ilyagr implied with topo:<branches>, where topo is a revset for the topological nearest branch/commit.

Exclusions:

  • when a remote is configured, protected & default branches are excluded
  • trunk() is always excluded
  • all ancestors of excluded branches (i.e. when encountering a commit that's the head of an excluded branch, prune the rest of that ancestry path)

The trunk() part may only make sense for your projects, but not for everybody else. So make it configurable? I get your idea but we're not Git, so I'd keep exclusions and other ideas far away from this.

  • If a single candidate commit is the head of multiple branches, the command fails

This does not make sense with jj's current branches, I'd just drop it or make it a warning.

  • If multiple candidate branch-head commits are at the same depth, they are all advanced

👍

Configuration options:

  • additional excluded branches may be configured
  • users may opt-out of any of these exclusions (e.g. opt-in to advancing main)

These kinda make sense, but should not be in the first cut.

users may choose from these non-default behaviors for multiple branches on a single commit:

  • interactively select which branch(es) to advance
  • always advance all branches

I'd provide the interactive option only for ambigous branches, the other option makes sense.

If it's not possible for a jj alias to combine multiple commands, then I would like for there to be a built-in "combination alias" (like commit) that would enable synchronizing work in one step (a la svn commit):

  • advance branches
  • add commit messages
  • push

This is a straight "No" from me. Maybe it has a place in your downstream if you find the time.

@PhilipMetzger
Copy link
Collaborator

Sorry, the above took a bit longer than expected.

that sounds good. But would that make --advance-branches unnecessary?

Yes, it would make --advance-branches unnecessary, but until something like it exists, branch-oriented workflows need crutches, which we'll need to provide.

For that, I think we'd need two conditions:

  • either new or describe (or both) automatically applies the current "topic" to @ (the "old" commit in the case of new)
  • topics can be pushed as branches to git remotes

Both provided options make more sense than providing a janky Git-like "branch". I'd assume jj new -t <topic> would advance the branch automatically matching your expectation, then it would be entirely unecessary on describe.

@BatmanAoD
Copy link
Collaborator

BatmanAoD commented Feb 13, 2024

@PhilipMetzger I think we have pretty different workflows and/or philosophies, but I do want to understand each of your points and where you're coming from.

Not really, only because it is a convience built-in, does not mean it is a git commit equivalent. It could be gone in a few years

Sorry, what is "it" here? Are you saying that --advance-branches "could be gone in a few years" or that jj commit itself could be gone?

So make it configurable? I get your idea but we're not Git, so I'd keep exclusions and other ideas far away from this.

These kinda make sense, but should not be in the first cut.

These seem at odds. I was suggestion a default-but-configurable behavior, but the "not in the first cut" comment is in response to a description of how the behavior would be configurable. The need for exclusions already came up in the original few comments in the thread, since main and master, at least, should not be automatically advanced.

This is purely related to Git and its implementation of branches.

I'm assuming that by "this" here you mean "protected branches"? I'm not familiar with many version control systems, but I would be surprised if this is truly unique to Git forges. By "protected branch" I just mean "somewhere that you won't be allowed to push commits to." This is a feature of the remote (i.e. the forge, github/gitlab/etc), not of the version control system. But I acknowledge that, in general, this isn't something that's knowable from the local repository state, so it isn't something the tool should attempt to infer automatically.

This does not make sense with jj's current branches, I'd just drop it or make it a warning.

I'm not sure what part doesn't make sense with jj's model. jj already gives a warning when branch set-ing multiple branches at once explicitly, so doing so automatically feels like it probably shouldn't happen. But I agree that a warning would be sufficient.

But [the default branch] is once again a pure Git concept which doesn't make sense in a Jujutsu world.

"Default branch" in the sense I'm using it here is another property of the forge rather than of the version control system. It simply means "the basis against which pull requests are compared, by default." This seems very non-git-specific to me. Any forge that supports any kind of pull-request-like feature needs some kind of basis for comparison. Even in a classic patch-based mailing-list system (i.e. without a forge), submitting patches still requires some way of indicating or assuming what version of the code the patch was generated against.

...where topo is a revset for the topological nearest branch/commit.

Thanks for clarifying. So the idea is that topo would be a new "keyword" in the revset syntax? (I'm not sure I understand why this would require a new syntax; surely topo(ancestor()) would be okay? I also don't love the name topo but won't start bikeshedding that here.)

Since this is mostly a crutch for Git users, implementing it on describe and commit makes sense.

I'm glad you agree that describe and commit make sense as the base commands for the new flag. I'm not sure why you're describing --advance-branches as a "crutch for Git users," though. Currently, as far as I can tell from the documentation, jj git push is the only way to publish local changes, and that requires either pushing all anonymous branches (which is not generally recommended) or assigning a named branch to the commits. So until the native backend or some other backend supports pushing changes, every user who wants to make multiple separate commits in a sequence (rather than simply squashing every change preemptively) must advance branches before pushing, either explicitly with branch set or implicitly with --advance-branches. Is that not the case? Is there another way to push new commits that I'm missing?

This is a straight "No" from me. Maybe it has a place in your downstream if you find the time.

I sort of assumed this would be the least-well-received part of my proposal, but I'd actually understand what's objectionable about it. (I'd also like to note that I think having a way to alias a sequence of commands would be a very useful feature that would make it completely unnecessary to have commands like commit built into jj itself; but I realize this would be a very complex feature to add and that it wouldn't necessarily be preferable to just writing a short shell script.)

In svn and older version control systems, commit was sufficient to share your local work with others. In git, this became a longer sequence, something like git add ... ; git commit ; git push. With jj, we no longer need add, but we have a different extra step, which is the branch-updating that's required in order to push with git. So we have something like jj [describe|commit]; jj branch set ... ; jj [backend] push.

I understand that there's value in separating out each of these operations both in one's mental model and in the design of the tool. But the most common thing I do in version control is still a combination of all of these in quick succession: jj commit -m "what these changes are for"; jj branch set -r @- <name>; jj git push. What would be wrong with just...doing all of these together, without so much typing? As soon as I've assigned a named branch to my work, I'm pretty sure I want to send that to the remote. (Again, not as the default or only behavior as in svn, but as an option supported by the tool's CLI.)

@yuja
Copy link
Collaborator

yuja commented Feb 13, 2024

...where topo is a revset for the topological nearest branch/commit.

So the idea is that topo would be a new "keyword" in the revset syntax?

I think it's a kind of a "branch set expression" to select a branch name (just like a revset expression to select revisions.)
For example, topo:ancestor in jj branch set topo:ancestor -r q will be resolved to the branch name of the closest ancestor revisions.


  • Rather than a new command, this should be a flag on new, commit, and/or describe (for new and commit, the operation would modify the "old" revision, i.e. the one to which the commit message is attached)

I think it would be a bit odd if describe had --advance-branch. commit --advance-branch makes sense because it creates new commit, and in Git-like workflow, the current branch will have to be moved to the new commit. OTOH, describe is the command to mutate commit metadata. It could be considered a command to finish up the commit, but the command name describe doesn't suggest that.

btw, I usually go without named branches, and occasionally need to pull up the main branch locally. In that situation, a separate branch advance/move command will be useful.

  • The operation should not implicitly/automatically advance certain special branches:

    • any protected branch (in the GitHub sense; see "questions")
    • the default (i.e. remote HEAD) branch (see "questions")
    • trunk()

Perhaps, this configuration will become a revset?
I wouldn't want to move someone else's branch while reviewing code, so I might want to specify ... & mine().

@PhilipMetzger
Copy link
Collaborator

PhilipMetzger commented Feb 14, 2024

I think we have pretty different workflows and/or philosophies, but I do want to understand each of your points and where you're coming from.

I generally don't think we differ in workflows but mostly in philosophies, I just mentally moved on from Git and its branches quite a while ago.

Sorry, what is "it" here? Are you saying that --advance-branches "could be gone in a few years" or that jj commit itself could be gone?

"It" means jj commit here, as I'm vastly in favor of teaching people the jj new + jj describe workflow and I think that jj commit should be deprecated like checkout in a decade or two.

These seem at odds. I was suggestion a default-but-configurable behavior, but the "not in the first cut" comment is in response to a description of how the behavior would be configurable. The need for exclusions already came up in the original few comments in the thread, since main and master, at least, should not be automatically advanced.

What I meant here, is to say that --advance-branches should probably pick the nearest branch in the initial implementation and a follow-up then implements exclusions, when we already have a idea on how jj commit --advance-branches is used. Ideally we just take the immutable_heads() revset initially to mark some branches as un-moveable.

I'm assuming that by "this" here you mean "protected branches"?

Yes.

I'm not familiar with many version control systems, but I would be surprised if this is truly unique to Git forges. By "protected branch" I just mean "somewhere that you won't be allowed to push commits to." This is a feature of the remote (i.e. the forge, github/gitlab/etc), not of the version control system.

Yes, something similar exists in different version control systems but my issue with it is, that you only consider the Git/GitHub/Gitlab workflow in your solution. If you had to implement it for something Perforce-like or Mercurial-like, how would you proceed?
The goal is to have a somewhat portable abstraction, not something built purely for Git.

But I acknowledge that, in general, this isn't something that's knowable from the local repository state, so it isn't something the tool should attempt to infer automatically.

It atleast could consider trunk().

So the idea is that topo would be a new "keyword" in the revset syntax? (I'm not sure I understand why this would require a new syntax; surely topo(ancestor()) would be okay? I also don't love the name topo but won't start bikeshedding that here.)

Ilya just made a suggestion which wasn't final at all. But agreed let's not open a bikeshed here.

Since this is mostly a crutch for Git users, implementing it on describe and commit makes sense.

I'm glad you agree that describe and commit make sense as the base commands for the new flag. I'm not sure why you're describing --advance-branches as a "crutch for Git users," though.

I'm describing it as a "crutch for Git users", as other workflows manage fine with anonymous heads. See https://social.jvns.ca/@b0rk/111709458396281239 and this (requires a Discord account).

Currently, as far as I can tell from the documentation, jj git push is the only way to publish local changes, and that requires either pushing all anonymous branches (which is not generally recommended) or assigning a named branch to the commits.

That totally is correct. I do think that you should try a workflow, where you don't immediately set a branch after a new commit as it vastly simplifies the usage of jj.

This is my answer for today, it took a good chunk of my evening, I'll try to explain my thoughts on the last part tomorrow or during the weekend. @BatmanAoD

@BatmanAoD
Copy link
Collaborator

BatmanAoD commented Feb 15, 2024

@PhilipMetzger Thanks for the detailed reply; but I'm sorry, I'm still extremely confused about whether branches are necessary or not.

Currently, as far as I can tell from the documentation, jj git push is the only way to publish local changes, and that requires either pushing all anonymous branches...or assigning a named branch to the commits.

That totally is correct. I do think that you should try a workflow, where you don't immediately set a branch after a new commit as it vastly simplifies the usage of jj.

If I don't set a branch, then I need to use jj git push -a, right? So are you saying that I should try using jj git push -a for all of my pushing?

@martinvonz
Copy link
Owner

Sorry it has taken me so long to get to this thread, and I still haven't read all of it.

  • Can a user-defined alias define a combination of commands, the way commit combines describe and new?

No, that's not supported yet. I think we will want support for that eventually.

  • If a single candidate commit is the head of multiple branches, the command fails
  • If multiple candidate branch-head commits are at the same depth, they are all advanced

These seem inconsistent. I'd rather make both of the advance all branches. A configurable revset sounds as @yuja said might be good. Or maybe we even use immutable_heads(). I think that's what @ilyagr also said. So advance all branches pointing to commits in heads(immutable_heads()..@ & branches())? And advance the branches to 'heads(immutable_heads()..@ ~ empty())', perhaps? You can even try that like this:

jj log --no-graph -r 'heads(immutable_heads()..@ & branches())' -T 'local_branches ++ " "' | \
  xargs jj branch set -r 'heads(immutable_heads()..@ ~ empty())'

Consider putting that in a jj-advance-branches.sh or something and see how you like. Sure, you'll have to run it manually, but it's an easy way to see if the behavior seems to be what you want.

If I don't set a branch, then I need to use jj git push -a, right? So are you saying that I should try using jj git push -a for all of my pushing?

I think @PhilipMetzger suggested that you create commits without moving branches and then move the branches just when you're about to push. (I very rarely move branches myself because GitHub takes care of moving the main branch forward in the martinvonz/jj repo, and GitHub also takes care of deleting branches for merged PRs.)

@PhilipMetzger
Copy link
Collaborator

Here's the last part:

This is a straight "No" from me. Maybe it has a place in your downstream if you find the time.

I sort of assumed this would be the least-well-received part of my proposal, but I'd actually understand what's objectionable about it. [...]

In svn and older version control systems, commit was sufficient to share your local work with others. In git, this became a longer sequence, something like git add ... ; git commit ; git push. With jj, we no longer need add, but we have a different extra step, which is the branch-updating that's required in order to push with git. So we have something like jj [describe|commit]; jj branch set ... ; jj [backend] push.

I understand that there's value in separating out each of these operations both in one's mental model and in the design of the tool.

Basically my philosophy for new CLIs is that they should be composable and shouldn't special-case behaviors, as it is very confusing for new users which we're all at some point. Thus having a single command which does everything, as in sync, commit and push is in heavy conflict with this philosophy.

Another point to consider with jj at least is; there are different backends and not every backend would opt-in or benefit from such a command.

What would be wrong with just...doing all of these together, without so much typing?

There is absolutely nothing wrong with that, saying that as a big fan of automating everything and everywhere, it just doesn't fit in here. 🙁

As soon as I've assigned a named branch to my work, I'm pretty sure I want to send that to the remote. (Again, not as the default or only behavior as in svn, but as an option supported by the tool's CLI.)

This is in my opinion once again a workflow issue, which is only needed because Git is the dominant system. If we had a native repo + server where it didn't matter, it wouldn't cross your mind to use branches here.

Currently, as far as I can tell from the documentation, jj git push is the only way to publish local changes, and that requires either pushing all anonymous branches...or assigning a named branch to the commits.

That totally is correct. I do think that you should try a workflow, where you don't immediately set a branch after a new commit as it vastly simplifies the usage of jj.

If I don't set a branch, then I need to use jj git push -a, right? So are you saying that I should try using jj git push -a for all of my pushing?

No, what I meant is what Martin and Yuya in the previous comments said.

Short description of the workflow below:

You first build a stack commits of <main branch>, which are your actual "feature" and then at a later point only create the branch to push it to Github/Gitlab. jj git push will push all the commits in <main branch>@origin..<cool-feature>, which can be your complete feature.

@emesterhazy
Copy link
Collaborator

emesterhazy commented Feb 17, 2024

One of the potential problems of this model is that, in colocated repo, git will see the current branch pointing to the editing commit.

Can you elaborate on what the issue is? I just tested against the WIP commit that I pushed and git is still seeing a detached head after the branch pointer is updated. And, even without this feature, the user can set the branch to the current editing commit, so that must be ok?

➜  ~/o/g/e/test2 ((3ec6c558)) jjx
@  spwmxroq emesterhazy@ 2024-02-17 00:25:35.000 -05:00 foobar c7c76efb
│  (no description set)
◉  ovowmqmn emesterhazy@ 2024-02-17 00:25:05.000 -05:00 HEAD@git 3ec6c558
│  file1
◉  zzzzzzzz root() 00000000
➜  ~/o/g/e/test2 ((3ec6c558)) jjx config set --repo branches.advance-branches true
➜  ~/o/g/e/test2 ((3ec6c558)) jjx commit -m "file2"
Working copy now at: nytolypo 9fed31fe foobar | (empty) (no description set)
Parent commit      : spwmxroq bb262bfb file2
➜  ~/o/g/e/test2 ((bb262bfb)) jjx
@  nytolypo emesterhazy@ 2024-02-17 00:25:57.000 -05:00 foobar 9fed31fe
│  (empty) (no description set)
◉  spwmxroq emesterhazy@ 2024-02-17 00:25:57.000 -05:00 HEAD@git bb262bfb
│  file2
◉  ovowmqmn emesterhazy@ 2024-02-17 00:25:05.000 -05:00 3ec6c558
│  file1
◉  zzzzzzzz root() 00000000
➜  ~/o/g/e/test2 ((bb262bfb)) git log --oneline
bb262bf (HEAD) file2
3ec6c55 file1
➜  ~/o/g/e/test2 ((bb262bfb)) git log foobar --oneline
9fed31f (foobar)
bb262bf (HEAD) file2
3ec6c55 file1

You can get into exactly the same state by calling jj branch set -r @ on the editing commit, right?

@yuja
Copy link
Collaborator

yuja commented Feb 17, 2024

Can you elaborate on what the issue is? I just tested against the WIP commit that I pushed and git is still seeing a detached head after the branch pointer is updated. And, even without this feature, the user can set the branch to the current editing commit, so that must be ok?

I don't have real examples, but it's unusual in Git land that the current branch is ahead of the HEAD, so some frontend tools (like IDE integration) might behave badly. For example, the user could checkout the branch by using IDE Git integration without noticing that he should do jj edit instead.

Suppose this is the feature primarily designed for Git users, I think Git interop matters the most.

@emesterhazy
Copy link
Collaborator

It's also unusual in Git land to constantly have a detached head. I think that's probably the real peculiarity, and my IDE is already complaining about it:

image

The documentation mentions this as a potential issue for colocated repos:

It is allowed to mix `jj` and `git` commands in such a repo in any order.
However, it may be easier to keep track of what is going on if you mostly use
read-only `git` commands and use `jj` to make changes to the repo. One reason
for this (see below for more) is that `jj` commands will usually put the git
repo in a "detached HEAD" state, since in `jj` there is not concept of a
"currently tracked branch". Before doing mutating Git commands, you may need to
tell Git what the current branch should be with a `git switch` command.

So maybe what we really need to do is fix the detached head state if there's a single local branch pointing to the editing commit and advance branches is enabled. If anything, that should improve the UX for colocated repos. The commit I'm working on for this feature refuses to automatically advance the branch if more than two branches point to the same commit, so we can ask the user to fix any ambiguity manually. What do you think?

@yuja
Copy link
Collaborator

yuja commented Feb 17, 2024

So maybe what we really need to do is fix the detached head state if there's a single local branch pointing to the editing commit and advance branches is enabled.

I honestly don't know. I'm not sure how much complexity would be added to translate the "current branch pointing to @" model to Git.

  • We need at least one special case for jj git push as Martin mentioned.
    • What if the editing branch is non-empty and has commit message, should it be skipped?
  • In order to map the "current" editing branch to the Git model, the current branch will have to be exported to the parent commit, and the HEAD will be set to that branch.
    • What should we do if the editing branch is a merge? Pick the first parent, maybe?
    • On leaving from the editing commit, should the exported branch be moved forward to the commit? Maybe yes, because there's no permanent information that the commit was previously editing.

@emesterhazy
Copy link
Collaborator

From your second bullet point it sounds like you're describing a model where the branch sits at @- instead of @. Is that correct? I'm not sure that's what Martin was describing:

We would presumably have something like jj new --activate-branch foo, which would create a new commit on top of commit foo and move any branches pointing to foo to the new commit.

But I think you're right that keeping the branch at the @- instead of @ could yield better git compatibility. I guess I don't have a full view of the complexity involved either.

emesterhazy added a commit that referenced this issue Feb 17, 2024
## TODO
There is still a lot of work to do for this feature. We need to support `jj
new`, and we need to add a `--branch` argument to `jj new` (and possibly `jj
commit`) to resolve instances where more than one branch would be moved. The
--branch option for `jj new` will allow users to create a new topic branch on
top of an existing branch such as `main` without moving the `main` branch or
needing to set an override for it in the config.toml.

## Feature Description

If enabled in the user or repository settings, the local branch pointing to the
revision targeted by `jj commit` will be advanced to the newly created commit.

This behavior can be enabled by default for all branches by setting
the following in the config.toml:

```
[branches]
advance-branches = true
```

The default value of `advance-branches` is `false`. You can override the
behavior for specific branches by setting the value of
`advance-branches-overrides` in the config.toml:

```
[branches]
advance-branches-overrides = ["main"]
```

Branches that have been overridden behave as if the value of `advance-branches`
was negated.

This implements FR #2338.
@martinvonz
Copy link
Owner

From your second bullet point it sounds like you're describing a model where the branch sits at @- instead of @. Is that correct? I'm not sure that's what Martin was describing:

I think we're talking about different things. My proposal was to have the branch point to @ in jj. Yuya pointed out that we would have to point it to @- when we export to Git in colocated repos. Note that jj has its own record of where branches point, so it is possible to have them point to different commits.

But I think you're right that keeping the branch at the @- instead of @ could yield better git compatibility. I guess I don't have a full view of the complexity involved either.

Yes, it does seem complicated. Thanks for pointing that out, Yuya.

@BatmanAoD
Copy link
Collaborator

I absolutely agree that having a branch automatically pointing at @- rather than @ makes more sense. There are two reasons:

  • 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
Copy link
Collaborator

I updated my prototype to move the branch to @-. @yuja would you mind linking me to where in the code we update the git HEAD? I'd like to make it check out the branch that's being advanced instead of the commit hash for @-. This should fix the "detached head" state when advanceable branches is being used.

@yuja
Copy link
Collaborator

yuja commented Feb 21, 2024

where in the code we update the git HEAD? I'd like to make it check out the branch that's being advanced instead of the commit hash for @-.

It's handled by git::reset_head(). Since this function doesn't know if the @- branch should be set to the "current branch" or not, we'll need some config knob. People like me prefer detached HEAD state.

Some related previous attempt: #2210

@emesterhazy
Copy link
Collaborator

Thanks, I'll take a look at that PR. From #2338 (comment) the plan is to make this behavior strictly opt-in. You will be able to enable it globally, or only for certain branches. I'm optimistic that we can do this in a way that's useful for the people that want this feature, and a no-op for everyone else.

@emesterhazy
Copy link
Collaborator

I have a draft pull request up with a basic implementation. This is still WIP, but it's useable and supports automatically advancing branches with jj commit and setting the Git HEAD to the branch instead of detaching it.

I'd appreciate if anyone interested in this feature takes a look at the explanation in the PR and tries it in a test repo to provide feedback.

emesterhazy added a commit that referenced this issue Mar 29, 2024
## Feature Description

If enabled in the user or repository settings, the local branches pointing to the
parents of the revision targeted by `jj commit` will be advanced to the newly
created commit. Support for `jj new` will be added in a future change.

This behavior can be enabled by default for all branches by setting
the following in the config.toml:

```
[experimental-advance-branches]
enabled-branches = ["glob:*"]
```

Specific branches can also be disabled:
```
[experimental-advance-branches]
enabled-branches = ["glob:*"]
disabled-branches = ["main"]
```

Branches that match a disabled pattern will not be advanced, even if they also
match an enabled pattern.

This implements feature request #2338.

WIP: use patterns instead of overrides list to control advance-branches
emesterhazy added a commit that referenced this issue Mar 29, 2024
## Feature Description

If enabled in the user or repository settings, the local branches pointing to the
parents of the revision targeted by `jj commit` will be advanced to the newly
created commit. Support for `jj new` will be added in a future change.

This behavior can be enabled by default for all branches by setting
the following in the config.toml:

```
[experimental-advance-branches]
enabled-branches = ["glob:*"]
```

Specific branches can also be disabled:
```
[experimental-advance-branches]
enabled-branches = ["glob:*"]
disabled-branches = ["main"]
```

Branches that match a disabled pattern will not be advanced, even if they also
match an enabled pattern.

This implements feature request #2338.

WIP: use patterns instead of overrides list to control advance-branches
emesterhazy added a commit that referenced this issue Mar 30, 2024
## Feature Description

If enabled in the user or repository settings, the local branches pointing to the
parents of the revision targeted by `jj commit` will be advanced to the newly
created commit. Support for `jj new` will be added in a future change.

This behavior can be enabled by default for all branches by setting
the following in the config.toml:

```
[experimental-advance-branches]
enabled-branches = ["glob:*"]
```

Specific branches can also be disabled:
```
[experimental-advance-branches]
enabled-branches = ["glob:*"]
disabled-branches = ["main"]
```

Branches that match a disabled pattern will not be advanced, even if they also
match an enabled pattern.

This implements feature request #2338.

WIP: use patterns instead of overrides list to control advance-branches
emesterhazy added a commit that referenced this issue Mar 31, 2024
## Feature Description

If enabled in the user or repository settings, the local branches pointing to the
parents of the revision targeted by `jj commit` will be advanced to the newly
created commit. Support for `jj new` will be added in a future change.

This behavior can be enabled by default for all branches by setting
the following in the config.toml:

```
[experimental-advance-branches]
enabled-branches = ["glob:*"]
```

Specific branches can also be disabled:
```
[experimental-advance-branches]
enabled-branches = ["glob:*"]
disabled-branches = ["main"]
```

Branches that match a disabled pattern will not be advanced, even if they also
match an enabled pattern.

This implements feature request #2338.
emesterhazy added a commit that referenced this issue Mar 31, 2024
## Feature Description

If enabled in the user or repository settings, the local branches pointing to the
parents of the revision targeted by `jj commit` will be advanced to the newly
created commit. Support for `jj new` will be added in a future change.

This behavior can be enabled by default for all branches by setting
the following in the config.toml:

```
[experimental-advance-branches]
enabled-branches = ["glob:*"]
```

Specific branches can also be disabled:
```
[experimental-advance-branches]
enabled-branches = ["glob:*"]
disabled-branches = ["main"]
```

Branches that match a disabled pattern will not be advanced, even if they also
match an enabled pattern.

This implements feature request #2338.
emesterhazy added a commit that referenced this issue Apr 9, 2024
## Feature Description

If enabled in the user or repository settings, the local branches pointing to the
parents of the revision targeted by `jj commit` will be advanced to the newly
created commit. Support for `jj new` will be added in a future change.

This behavior can be enabled by default for all branches by setting
the following in the config.toml:

```
[experimental-advance-branches]
enabled-branches = ["glob:*"]
```

Specific branches can also be disabled:
```
[experimental-advance-branches]
enabled-branches = ["glob:*"]
disabled-branches = ["main"]
```

Branches that match a disabled pattern will not be advanced, even if they also
match an enabled pattern.

This implements feature request #2338.
emesterhazy added a commit that referenced this issue Apr 20, 2024
## Feature Description

If enabled in the user or repository settings, the local branches pointing to the
parents of the revision targeted by `jj commit` will be advanced to the newly
created commit. Support for `jj new` will be added in a future change.

This behavior can be enabled by default for all branches by setting
the following in the config.toml:

```
[experimental-advance-branches]
enabled-branches = ["glob:*"]
```

Specific branches can also be disabled:
```
[experimental-advance-branches]
enabled-branches = ["glob:*"]
disabled-branches = ["main"]
```

Branches that match a disabled pattern will not be advanced, even if they also
match an enabled pattern.

This implements feature request #2338.
emesterhazy added a commit that referenced this issue Apr 20, 2024
## Feature Description

If enabled in the user or repository settings, the local branches pointing to the
parents of the revision targeted by `jj commit` will be advanced to the newly
created commit. Support for `jj new` will be added in a future change.

This behavior can be enabled by default for all branches by setting
the following in the config.toml:

```
[experimental-advance-branches]
enabled-branches = ["glob:*"]
```

Specific branches can also be disabled:
```
[experimental-advance-branches]
enabled-branches = ["glob:*"]
disabled-branches = ["main"]
```

Branches that match a disabled pattern will not be advanced, even if they also
match an enabled pattern.

This implements feature request #2338.
@emesterhazy
Copy link
Collaborator

Oops, I accidentally closed this with:

I don't think this is fully resolved, since there are still a few open questions from that PR, and it might not be exactly what people want. But, if you just want your branches to move forward when you run jj commit and jj new this may be a solution for you. The instructions for setting this up are in the first comment in the PR.

This is an experimental feature that could be removed in the future. It should be available in the 0.17 release, or now if you build from source at head. Please leave any feedback here:

@emesterhazy emesterhazy reopened this Apr 20, 2024
@Cretezy
Copy link
Collaborator

Cretezy commented Apr 29, 2024

Based on @emesterhazy, could revsets or other types of patterns be implemented, such as thunk()? Having disabled-branches = ["thunk()"] seems like a useful way to declare this.

Possibly this should be the default to prevent footguns? Or maybe at least the recommended default in the docs for enabling this.

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

Successfully merging a pull request may close this issue.