FR: Gerrit support

[ilyagr]

I'm creating a placeholder bug to reference.

See https://martinvonz.github.io/jj/latest/FAQ/#how-do-i-integrate-jujutsu-with-gerrit for current status.

This would likely be a collection of jj gerrit commands.

[steveklabnik]

See also https://github.com/martinvonz/jj/pull/2845

[PhilipMetzger]

And also related #485.

[dbarnett]

What's the scope of Gerrit-specific functionality we're planning to integrate? From the links above it looks like gerrit send and/or something for rewriting commit messages on push, and then something for pushing stacked reviews.

I just filed a corresponding GitHub issue #4555 so I'm wondering what functionality would have Gerrit equivalents (and also whether jj [forgename] foo commands for those things are really necessary or should be modeled at another level).

[avamsi]

Mirroring https://github.com/martinvonz/jj/pull/2845#issuecomment-2543447845:

In case it helps anyone, I set Change-Id using templates.draft_commit_description and just git push:

[template-aliases]
	'gerrit_change_id(change_id)' = '"Id0000000" ++ change_id.normal_hex()'

[templates]
	draft_commit_description = '''
		separate("\n",
			description.remove_suffix("\n"),
			if(!description.contains(change_id.normal_hex()),
				"\nChange-Id: " ++ gerrit_change_id(change_id)
			),
			"\n",
			surround("JJ: Changes:\n", "", indent("JJ: \t", diff.summary()))
		)
	'''

(https://github.com/avamsi/dotfiles/blob/28a0e40439de56fe179a1b0e705727c85e075464/.jjconfig.toml#L146-L156)

[Benjamin-Davies]

Perhaps there should be a nice way to download the latest commit for a given change (referenced by either the Gerrit "Change-Id", the Gerrit numeric id, or a revset).

Edit (just read https://github.com/jj-vcs/jj/pull/2845#issuecomment-2543359105): The tool could also (by default) update a change that has the same Gerrit "Change-Id" to use the newly pulled commit.

[prattmic]

I've been using jj with Gerrit for over a year now and I think various improvements over the last year have made the Gerrit experience much smoother, even without an explicit Gerrit subcommand (#2845).

Notably:

• With the switch to using a git subprocess for fetch, authentication pain is gone. As long as git authenticates, jj does as well. I used to have two remotes: one unauthenticated I used for jj git fetch, another authenticated I used for git push.
• With the new commit_trailers and format_gerrit_change_id_trailer templates (#6088), it's now very easy to get Change-Id support (even more so than https://github.com/jj-vcs/jj/issues/4387#issuecomment-2543448225 which was honestly fine too).

For completeness, these are the things that I find are still pain points:

1. The obvious one is the inability to push to an arbitrary ref makes it impossible to actually push changes to Gerrit. But I hesitate to actually call this a "pain point" because I use a colocated repo and git push origin HEAD:refs/for/master and it works perfectly fine. I honestly don't have any issues with the way this works. It's primarily just "ugly" to need to use git directly when I do everything else through jj. Plus there is a minor baseline fear with a colocated repo that I will accidentally mutate git state in some way that breaks jj,

2. With regards to Change-Ids, I'm coming around to @thoughtpolice's position in https://github.com/jj-vcs/jj/pull/2845#issuecomment-2543359105 that having a Change-Id based directly on the jj change ID (as format_gerrit_change_id_trailer does) is a bad idea.

In addition to the abandoned and replaced case @thoughtpolice mentions, the place I run into this is splitting changes. Suppose I upload a change to Gerrit but later realize I want to split out a minor typo fix into a different change. I jj split -i the change, selecting all the main (non-typo-fix) changes for the first half of the split. I leave it's commit message and Change-Id alone. The second half of the split is the typo fix. I give it a completely new commit message and delete the old Change-Id. Finally I need one extra jj describe to re-run the commit_trailers template and replace the Change-Id I deleted.

This is all fine and works perfectly.

Now suppose I did this in the reverse order. In jj split -i I select the typo change first, replace the commit message and delete the Change-Id line. When jj describe reruns the template, it is going to add back the exact same Change-Id! That's because the first half of a split keeps its original jj change ID.

I think it would be better for the Change-Id to be random (I think based on the commit id would work fine) so that deleting and regenerating a Change-Id is a consistent way to get a different one. Of course, I can do this myself today by simply defining my own template instead of using format_gerrit_change_id_trailer.

(I acknowledge that there is an alternate viewpoint here that jj change IDs and Gerrit Change-Ids are conceptually very similar: a stable identifier for commits that change over time. If I want a set of changes conceptually associated with a specific Gerrit Change-Id then I should also want them conceptually associated with with the specific jj change ID. Thus I should simply not do the split in the "wrong" order. That's totally fair, but honestly just not something I'm thinking about closely most of the time. As a user, for my user experience it rarely matters that the change IDs are stable because I tend to look them up in the log right when I need them. The stability is of course important for internal operations, but for UX I wouldn't notice too much if they changed. Gerrit Change-Ids on the other hand are associated with other out-of-band data like code review comments and email subject lines that never update, so it is very noticeable if I were to move the main part of a change to a new Change-Id.)

3. Downloading changes is probably my one true pain point. I don't do this often, but it is painful enough that I simply don't do it in a jj repo and use a separate git clone instead. (I also haven't tried this in a while, so perhaps it has already improved)

The first line problem is that jj git fetch can't fetch from arbitrary refs, but like push I can use git fetch directly and that mostly works fine.

The real trouble comes when there are complicated dependencies.

Suppose I've uploaded patch set 1 of change A to Gerrit. My coworker downloads my change, adds another commit on top of it, which they upload to Gerrit as change B.

In the meantime, I make some adjustments to change A, which I upload to Gerrit as patch set 2.

Later, I decide I want to try out my coworker's change B, so I fetch it into my jj repo.

Now we have a problem. Change B depends on an old version of change A. This makes jj really upset because this isn't normally possible in jj's model and makes references to change A ambiguous. To jj's credit, it never does anything wrong as far as I can tell, but any time change A is mentioned, jj asks which version of change A I meant. If I'm actually working on change A that gets really old and really annoying fast.

Admittedly, I'm not sure what jj should do here, the situation really is ambiguous. My initial thought is that references to change A should always refer to the latest version of A, and if old versions are reachable they should be given some modified change ID (e.g., A~1). But then that is adding a whole new layer of syntax for a situation that is normally impossible.

[martinvonz]

There has been a lot of discussion about many of these things lately.

One recent change is that we will start writing the jj change id to a change-id Git commit header (which means it's not part of the description like footers are). That's now opt-in (git.write-change-id-header = true) but it will be enabled by default from the next release. Gerrit will probably learn to read the change id from that header eventually. They have a design doc here.

Regarding which commit keeps the change id, there's open PR #6458 for adding -A/-B/-d flags to jj split. When given any of those flags, the selected changes will go into the new commit that is created in the specified location. The commit with the remaining changes will keep the change id. Once that's been merged, we can all try those flags and see how we like it. In particular, jj split -B @ will behave similar to the current jj split but it will put the change id on the child commit.

Your last scenario (fetching an old version of commit from the remote) is interesting. One possible solution is to rebase your coworker's change B on top of your updated change A. I don't know if we would want to try to do that automatically. It could possibly be done by some new command designed to try to resolve divergence.

Btw, what does Gerrit do if your coworker rewrites change A and pushes change B based on that? Will Gerrit reject it since you own change A?

[prattmic]

There has been a lot of discussion about many of these things lately.

One recent change is that we will start writing the jj change id to a change-id Git commit header (which means it's not part of the description like footers are). That's now opt-in (git.write-change-id-header = true) but it will be enabled by default from the next release. Gerrit will probably learn to read the change id from that header eventually. They have a design doc here.

I did see this, it is exciting! In my mind it does raise similar concerns as my split example because a change ID header makes it even harder to manually fixup the Change-Ids. I do appreciate that the design says that a Change-Id trailer will override the header, so that provides a workaround. And maybe this is a sign to lean into my prior parenthetical about making sure I actually get the jj change IDs right as well.

Given the current git compatibility issues, my team would probably not enable this Gerrit feature. Cherry picks would be my primary concern. Our team cherry picks fixes from master to release branches as backports for minor releases. Generally we do this entirly in the Gerrit UI, which would work fine. But if there are conflicts, whoever does the cherry pick will fall back to using git locally to resolve the conflicts. I don't want to be the odd one on the team who's changes are extra painful to cherry pick. Hopefully git will start preserving these headers.

Regarding which commit keeps the change id, there's open PR #6458 for adding -A/-B/-d flags to jj split. When given any of those flags, the selected changes will go into the new commit that is created in the specified location. The commit with the remaining changes will keep the change id. Once that's been merged, we can all try those flags and see how we like it. In particular, jj split -B @ will behave similar to the current jj split but it will put the change id on the child commit.

Nice, that sounds like a great improvement! I do think it would be nice to have some way to fix the change IDs post-facto. I'm thinking some sort of swap command that just swaps the change ID of two changes (I have not thought through the implications if one of these changes has other descendants, sounds a bit fraught...). I can of course undo the split and do it over in the correct order, but if I just did an interactive split where I meticulously selected two dozen hunks the idea of doing that again isn't very appealing.

Your last scenario (fetching an old version of commit from the remote) is interesting. One possible solution is to rebase your coworker's change B on top of your updated change A. I don't know if we would want to try to do that automatically. It could possibly be done by some new command designed to try to resolve divergence.

I'm not sure either. Sometimes that would be nice, sometimes I want to reproduce some problem/results the coworker reporter, so I really want to use exactly the same version they used.

Btw, what does Gerrit do if your coworker rewrites change A and pushes change B based on that? Will Gerrit reject it since you own change A?

This is a configurable Gerrit server option. Our team configures Gerrit to reject pushes that modify a change you do not own. Catching the exact case you mention is IMO one of the biggest benefits of this option, because this is a very easy mistake to make.

Also, to be clear, making changes on top of someone else's unsubmitted change honestly doesn't work that well in Gerrit, even as a git user. It's not really Gerrit's fault, it is just fairly tedious to rebase change B on a new version of change A. You need to manually fetch the latest version of A and then do a rebase on the downloaded ref. Most users do this quite rarely. Otherwise they are normally only doing simple rebases on main or rebases within their own stack of changes, so it's easy to forget (or not know in the first place) the right way to handle this.

On my team, making changes on top of someone else's unsubmitted change is fairly rare. Usually it only happens when person A has some experimental change that won't be submitted any time soon. Person B wants to try it out, and then has some additional experimental changes to pile on top. It quickly gets to the point that maybe they should just make an experiment branch so the changes can actually be submitted.