On Tue, Jul 25, 2023 at 11:41 PM Junio C Hamano <gitster@xxxxxxxxx> wrote:
>
> Christian Couder <christian.couder@xxxxxxxxx> writes:
>
> > 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."
> > +--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.
> > @@ -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"
>
> > +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."
