Thread (2 messages) 2 messages, 2 authors, 2023-09-07

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."
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help