Re: [PATCH v3 13/15] replay: add --advance or 'cherry-pick' mode
From: Christian Couder <hidden>
Date: 2023-09-07 15:26:02
On Tue, Jul 25, 2023 at 11:41 PM Junio C Hamano [off-list ref] wrote:
Christian Couder [off-list ref] writes:quoted
There is already a 'rebase' mode with `--onto`. Let's add an 'advance' or 'cherry-pick' mode with `--advance`. This new mode will make the target branch advance as we replay commits onto it.If I say $ git replay --(advance|onto) xyzzy frotz..nitfol yomin where the topology of the cherry-picked range look like this x---x---Y yomin / E---F---x----x----N nitfol / frotz / X xyzzy after transplanting the commits, we would get something like x---x---Y yomin / E---F---x----x----N nitfol / frotz / X---x----x----N' \ x---x---Y' Now, if this was done with --onto, nitfol and yomin would point at N' and Y', but with --advance, where would xyzzy go? Yes, my point is that without --advance, there always is a reasonable set of branch tips that will be moved, but with "--advance", you cannot guarantee that you have any reasonable answer to that question. The answer could be "when there is no single 'tip of the new history', the command with '--advance' errors out", but whatever behaviour we choose, it should be documented.
Ok, I have improved the commit message by adding the following:
"The replayed commits should have a single tip, so that it's clear where
the target branch should be advanced. If they have more than one tip,
this new mode will error out."
I have also updated the doc for <revision-range> like this:
"<revision-range>::
Range of commits to replay. More than one <revision-range> can
be passed, but in `--advance <branch>` mode, they should have
a single tip, so that it's clear where <branch> should point
to. See "Specifying Ranges" in linkgit:git-rev-parse."
quoted
+--advance <branch>:: + Starting point at which to create the new commits; must be a + branch name. ++ +When `--advance` is specified, the update-ref command(s) in the output +will update the branch passed as an argument to `--advance` to point at +the new commits (in other words, this mimics a cherry-pick operation).This part is not giving much useful information to determine the answer (which might be fine, as long as the answer can easily be found in some other parts of this document, but I would have expected everything necessary would be found here or one item before this one about --onto).
The doc about <revision-range> is just after the above, so I think the above change in the <revision-range> doc is Ok and enough here.
quoted
@@ -47,7 +55,10 @@ input to `git update-ref --stdin`. It is basically of the form: update refs/heads/branch2 ${NEW_branch2_HASH} ${OLD_branch2_HASH} update refs/heads/branch3 ${NEW_branch3_HASH} ${OLD_branch3_HASH} -where the number of refs updated depend on the arguments passed. +where the number of refs updated depend on the arguments passed. When +using `--advance`, the number of refs updated is always one, but for +`--onto`, it can be one or more (rebasing multiple branches +simultaneously is supported)."dependS on the arguments passed" is not incorrect per-se, in the sense that if you "replay --onto X E..N" (in the above picture), I think you'll move F and N (two), while "F..N" will only move N (one). But the important thing to convey is how many branches had their tips in the replayed range, no? "depends on the shape of the history being replayed" would be a more correct thing to say for the "--onto" mode. For "--advance", presumably you would require to have a single positive endpoint [*], so "depends on the arguments" is still not wrong per-se, because "when --advance is part of the arguments, the number becomes one".
Yeah, I agree.
Side note: even then, I suspect that
git replay --advance X E..F N
should be allowed, simply because there is only one sensible
interpretation. You'll end up with a single strand of
pearls F--x--x--N transplanted on top of X, and the range
happens to contain F and N but it is obvious the end result
should advance xyzzy to N because F fast-forwards to N.
I'd say "where the number of ..." and everything after these sample
"update" lines should be removed,I am not sure it's a good thing to remove that, as I think repeating how things work in the context of an example output can help people understand. I have updated these sentences to the following: "where the number of refs updated depends on the arguments passed and the shape of the history being replayed. When using `--advance`, the number of refs updated is always one, but for `--onto`, it can be one or more (rebasing multiple branches simultaneously is supported)."
and instead we should add a few words to the main description of the options, e.g. for "--onto"quoted
+When `--onto` is specified, the update-ref command(s) in the output will +update the branch(es) in the revision range to point at the new +commits (in other words, this mimics a rebase operation).we could update the above to something like ... will update the branches in the revision range to point at the new commits, similar to the way how "rebase --update-refs" updates multiple branches in the affected range.
Yeah, I agree. In the version 4 I will send soon, have updated the above to: "When `--onto` is specified, the update-ref command(s) in the output will update the branch(es) in the revision range to point at the new commits, similar to the way how `git rebase --update-refs` updates multiple branches in the affected range."