| Message ID | 20230907092521.733746-12-christian.couder@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Introduce new `git replay` command | expand |
Hi Christian, It is a bit surprising to see the manual page added in _this_ patch, in the middle of the series... I can live with it, though. On Thu, 7 Sep 2023, Christian Couder wrote: > diff --git a/Documentation/git-replay.txt b/Documentation/git-replay.txt > new file mode 100644 > index 0000000000..9a2087b01a > --- /dev/null > +++ b/Documentation/git-replay.txt > @@ -0,0 +1,90 @@ > +git-replay(1) > +============= > + > +NAME > +---- > +git-replay - Replay commits on a different base, without touching working tree > + > + > +SYNOPSIS > +-------- > +[verse] > +'git replay' --onto <newbase> <revision-range>... We need to make it clear here, already in the SYNOPSIS, that this is experimental. Let's add an `(EXPERIMENTAL!)` prefix here. > [...] > diff --git a/t/t3650-replay-basics.sh b/t/t3650-replay-basics.sh > new file mode 100755 Just like the manual page, I would have expected this test to be introduced earlier, and not piggy-backed onto one of the handful "let's turn fast-rebase into replay" patches. Ciao, Johannes
Hi Christian, I am only reviewing the docs. To assume the mindset of a Git user unfamiliar with this command, I purposely did not read the cover letter until after this review was done. Christian Couder <christian.couder@gmail.com> writes: > [...] > > diff --git a/Documentation/git-replay.txt b/Documentation/git-replay.txt > new file mode 100644 > index 0000000000..9a2087b01a > --- /dev/null > +++ b/Documentation/git-replay.txt > @@ -0,0 +1,90 @@ > +git-replay(1) > +============= > + > +NAME > +---- > +git-replay - Replay commits on a different base, without touching working tree How about using the same language ("new location") as in the DESCRIPTION heading? Also, the "without touching working tree" part is incomplete because as explained later on, the index and refs are also left alone. How about just "safely"? git-replay - Replay commits onto a new location, safely > +SYNOPSIS > +-------- > +[verse] > +'git replay' --onto <newbase> <revision-range>... > + > +DESCRIPTION > +----------- > + > +Takes a range of commits, and replays them onto a new location. OK. > Does > +not touch the working tree or index, and does not update any > +references. How about this version? The new commits are created without modifying the working tree, index, or any references. Also, by "references" we mean the refs in ".git/refs/*". In the gitrevisions man page we use the term "refnames" to refer to these bits, so maybe "refnames" is better than "references"? The simpler "branches" is another option. > However, the output of this command is meant to be used > +as input to `git update-ref --stdin`, which would update the relevant > +branches. Before we get to this sentence, it would be great to explain why this command is useful (what problem does it solve)? Also, it would help to add "(see OUTPUT section below)" as a navigational aid in case some readers are wondering what the output looks like (and have not yet gotten to that section). I've noticed that you're using the phrase "starting point" in the OPTIONS section. I think this is better than "location" or "base" (FWIW we started using "starting point" in 0a02ca2383 (SubmittingPatches: simplify guidance for choosing a starting point, 2023-07-14)). The following is a version of this section that attempts to address my comments above (I've included the other bits already reviewed earlier to make it easier to read): Takes a range of commits, and replays them onto a new starting point. The new commits are created without modifying the working tree, index, or any branches. If there are branches that point to the commits in <revision-range>, list them in the output with both the original commit hash and the corresponding (replayed) commit hash (see OUTPUT section below) . This command is like linkgit:git-rebase[1], but notably does not require a working tree or index. This means you can run this command in a bare repo (useful for server-side environments). And because nothing is modified (only new commits are created), it's like a "dry run" rebase. By combining this command with `git update-ref --stdin`, the relevant branches can be updated. That is, the branches that were pointing originally to the commits given in the <revision-range> will be updated to point to the replayed commits. This is similar to the way how `git rebase --update-refs` updates multiple branches in the affected range. > +THIS COMMAND IS EXPERIMENTAL. THE BEHAVIOR MAY CHANGE. > + > +OPTIONS > +------- > + > +--onto <newbase>:: How about 'starting-point' instead of 'newbase'? > + Starting point at which to create the new commits. May be any > + valid commit, and not just an existing branch name. Add "See linkgit:gitrevisions[7]." at the end? > +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. Ah, good example. I've moved this to my larger example above, so I don't think this paragraph is needed here any more (it probably didn't belong in OPTIONS anyway). > +<revision-range>:: > + Range of commits to replay; see "Specifying Ranges" in > + linkgit:git-rev-parse. OK. > +OUTPUT > +------ > + > +When there are no conflicts, the output of this command is usable as > +input to `git update-ref --stdin`. What happens if there are conflicts? Probably good to mention in the DISCUSSION section. Some questions you may want to answer for the reader: (1) Is git-replay an all-or-nothing operation? That is, if there are any conflicts, is the output always empty (or do we still output those branches that *can* be updated without conflicts)? (2) What is meant by "conflict" for git-replay? Is it the same meaning as the case for git-rebase? For (1), in your cover letter under "# Important limitations" you say "No resumability" but I am not sure if this means git-replay will output *something* before exiting with an error, or simply nothing at all. Speaking of the limitations section, perhaps it's worth pointing those out under DISCUSSION as well? > It is basically of the form: Why "basically"? Are there cases where the output can be different than the example given below? If not, then perhaps drop the word "basically"? > + update refs/heads/branch1 ${NEW_branch1_HASH} ${OLD_branch1_HASH} > + 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 depends on the arguments passed and > +the shape of the history being replayed. Let's use "number of branches" instead of "number of refs" here to be consistent with the language elsewhere. > +EXIT STATUS > +----------- > + > +For a successful, non-conflicted replay, the exit status is 0. When > +the replay has conflicts, the exit status is 1. OK. > If the replay is not > +able to complete (or start) due to some kind of error, the exit status > +is something other than 0 or 1. Not sure how useful "due to some kind of error" is here --- presumably the inability to replay is always due to some kind of error. Would it be worth including a brief explanation about why git-replay might be unable to complete or start (is this behavior in practice a common-enough thing to document here)? > +EXAMPLES > +-------- > + > +To simply rebase mybranch onto target: Looking at the CLI arguments, I think this phrase should be: Replay the commits in `origin/main..mybranch` onto `target`: > +------------ > +$ git replay --onto target origin/main..mybranch > +update refs/heads/mybranch ${NEW_mybranch_HASH} ${OLD_mybranch_HASH} > +------------ > + > +When calling `git replay`, one does not need to specify a range of > +commits to replay using the syntax `A..B`; any range expression will > +do: > + > +------------ > +$ git replay --onto origin/main ^base branch1 branch2 branch3 Instead of `base`, how about `olderbranch`? > +update refs/heads/branch1 ${NEW_branch1_HASH} ${OLD_branch1_HASH} > +update refs/heads/branch2 ${NEW_branch2_HASH} ${OLD_branch2_HASH} > +update refs/heads/branch3 ${NEW_branch3_HASH} ${OLD_branch3_HASH} > +------------ > + > +This will simultaneously rebase branch1, branch2, and branch3 -- all > +commits they have since base, playing them on top of origin/main. How about This will rebase the commits in `branch1`, `branch2`, and `branch3` (excluding those in `base`), preplaying them on top of `origin/main`. > +These three branches may have commits on top of base that they have in > +common, but that does not need to be the case. s/base/`base` > +GIT > +--- > +Part of the linkgit:git[1] suite One question I do have is what happens if you run git-replay twice (successfully)? Does the second invocation create another set of (new) successfully replayed commits? I ask because I'm interested in informing the readers of our docs about any potential pitfalls from abusing this command by mistake.
Linus Arver <linusa@google.com> writes: >> +update refs/heads/branch1 ${NEW_branch1_HASH} ${OLD_branch1_HASH} >> +update refs/heads/branch2 ${NEW_branch2_HASH} ${OLD_branch2_HASH} >> +update refs/heads/branch3 ${NEW_branch3_HASH} ${OLD_branch3_HASH} >> +------------ >> + >> +This will simultaneously rebase branch1, branch2, and branch3 -- all >> +commits they have since base, playing them on top of origin/main. > > How about > > This will rebase the commits in `branch1`, `branch2`, and `branch3` > (excluding those in `base`), preplaying them on top of `origin/main`. Oops, I meant "replaying" not "preplaying". But also, perhaps the following is simpler? This will replay the commits in `branch1`, `branch2`, and `branch3` (excluding those in `base`), on top of `origin/main`.
On Sat, Sep 9, 2023 at 12:55 AM Linus Arver <linusa@google.com> wrote: > > Hi Christian, > > I am only reviewing the docs. To assume the mindset of a Git user > unfamiliar with this command, I purposely did not read the cover letter > until after this review was done. Ok, thanks! > Christian Couder <christian.couder@gmail.com> writes: > > > [...] > > > > diff --git a/Documentation/git-replay.txt b/Documentation/git-replay.txt > > new file mode 100644 > > index 0000000000..9a2087b01a > > --- /dev/null > > +++ b/Documentation/git-replay.txt > > @@ -0,0 +1,90 @@ > > +git-replay(1) > > +============= > > + > > +NAME > > +---- > > +git-replay - Replay commits on a different base, without touching working tree > > How about using the same language ("new location") as in the DESCRIPTION > heading? I actually think that "base" is better than "location", also perhaps saying things a bit differently than in the description can help better understand the command. I am Ok with replacing "different" with "new", mainly because it is shorter, though. > Also, the "without touching working tree" part is incomplete > because as explained later on, the index and refs are also left alone. I agree that it is incomplete. > How about just "safely"? I think that we want to say that the command can work on bare repos, and I don't think "safely" conveys that meaning well. So now it is: "git-replay - Replay commits on a new base, works on bare repos too" > > +SYNOPSIS > > +-------- > > +[verse] > > +'git replay' --onto <newbase> <revision-range>... > > + > > +DESCRIPTION > > +----------- > > + > > +Takes a range of commits, and replays them onto a new location. > > OK. > > > Does > > +not touch the working tree or index, and does not update any > > +references. > > How about this version? > > The new commits are created without modifying the working tree, > index, or any references. I agree it's nicer, but I prefer Dragan Simic's version. > Also, by "references" we mean the refs in ".git/refs/*". In the > gitrevisions man page we use the term "refnames" to refer to these bits, > so maybe "refnames" is better than "references"? The simpler "branches" > is another option. "references" seems to be used much more often in the docs than "refnames" (204 vs 18 occurrences). And deciding between "refnames" and "references" in the docs is a global issue clearly outside the scope of this series. So for now I will keep "references". I don't like "branches" as I think tags and other refs are concerned too. > > However, the output of this command is meant to be used > > +as input to `git update-ref --stdin`, which would update the relevant > > +branches. > > Before we get to this sentence, it would be great to explain why this > command is useful (what problem does it solve)? > > Also, it would help to add "(see OUTPUT section below)" as a > navigational aid in case some readers are wondering what the output > looks like (and have not yet gotten to that section). > > I've noticed that you're using the phrase "starting point" in the > OPTIONS section. I think this is better than "location" or "base" (FWIW > we started using "starting point" in 0a02ca2383 (SubmittingPatches: > simplify guidance for choosing a starting point, 2023-07-14)). > > The following is a version of this section that attempts to address my > comments above (I've included the other bits already reviewed earlier to > make it easier to read): > > Takes a range of commits, and replays them onto a new starting > point. The new commits are created without modifying the working > tree, index, or any branches. If there are branches that point to > the commits in <revision-range>, list them in the output with both > the original commit hash and the corresponding (replayed) commit > hash (see OUTPUT section below) . I like the "(see OUTPUT section below)" part, but I don't like the fact that passing the output as input to `git update-ref --stdin` isn't mentioned anymore. So I have just added "(see the OUTPUT section below)". > This command is like linkgit:git-rebase[1], but notably does not > require a working tree or index. I don't quite like this as the command will later also be like cherry-pick (with the --advance mode). > This means you can run this command > in a bare repo (useful for server-side environments). And because > nothing is modified (only new commits are created), it's like a "dry > run" rebase. Bare repos are now mentioned in the title of the page, so I am not sure it's worth mentioning. > By combining this command with `git update-ref --stdin`, the > relevant branches can be updated. That is, the branches that were > pointing originally to the commits given in the <revision-range> > will be updated to point to the replayed commits. This is similar to > the way how `git rebase --update-refs` updates multiple branches in > the affected range. I am not sure this is very useful, also I'm afraid that with all the changes you propose the description section would be a bit too long. Maybe you should propose some of the changes above in a follow up patch series after this patch series has been merged. > > +THIS COMMAND IS EXPERIMENTAL. THE BEHAVIOR MAY CHANGE. > > + > > +OPTIONS > > +------- > > + > > +--onto <newbase>:: > > How about 'starting-point' instead of 'newbase'? I think "newbase" is very short and to the point. With --onto we clearly want something that can do a rebase, so I think using something with "base" in it is a good choice here. > > + Starting point at which to create the new commits. May be any > > + valid commit, and not just an existing branch name. > > Add "See linkgit:gitrevisions[7]." at the end? I don't think it's worth it to mention "linkgit:gitrevisions[7]" everywhere we can pass a revision. I think it makes the doc heavier than it should be, especially here where the command is a plumbing one for now and users should be quite experienced with Git already. > > +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. > > Ah, good example. I've moved this to my larger example above, so I don't > think this paragraph is needed here any more (it probably didn't belong > in OPTIONS anyway). I think it's important here to help users understand that with this option they have something very similar to "rebase". > > +<revision-range>:: > > + Range of commits to replay; see "Specifying Ranges" in > > + linkgit:git-rev-parse. > > OK. > > > +OUTPUT > > +------ > > + > > +When there are no conflicts, the output of this command is usable as > > +input to `git update-ref --stdin`. > > What happens if there are conflicts? Probably good to mention in the > DISCUSSION section. Some questions you may want to answer for the > reader: > > (1) Is git-replay an all-or-nothing operation? That is, if there are any > conflicts, is the output always empty (or do we still output those > branches that *can* be updated without conflicts)? > > (2) What is meant by "conflict" for git-replay? Is it the same meaning > as the case for git-rebase? > > For (1), in your cover letter under "# Important limitations" you say > "No resumability" but I am not sure if this means git-replay will output > *something* before exiting with an error, or simply nothing at all. In a follow up series we might add options to generate some output in case of conflicts. When we do that we will discuss this. So I think for now it should be Ok to not talk more about the output in case of conflicts. > Speaking of the limitations section, perhaps it's worth pointing those > out under DISCUSSION as well? I don't think it's worth officially discussing the limitations in the docs when some of them might be lifted soon after this series graduates. The command is experimental, so I think users can understand it if everything is not spelled out. Also if we spell out things too much, users might rely on what we say when it could actually change soon. > > It is basically of the form: > > Why "basically"? Are there cases where the output can be different than > the example given below? If not, then perhaps drop the word "basically"? Ok, I have dropped "basically". > > + update refs/heads/branch1 ${NEW_branch1_HASH} ${OLD_branch1_HASH} > > + 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 depends on the arguments passed and > > +the shape of the history being replayed. > > Let's use "number of branches" instead of "number of refs" here to be > consistent with the language elsewhere. I prefer "refs" or "references" here. > > +EXIT STATUS > > +----------- > > + > > +For a successful, non-conflicted replay, the exit status is 0. When > > +the replay has conflicts, the exit status is 1. > > OK. > > > If the replay is not > > +able to complete (or start) due to some kind of error, the exit status > > +is something other than 0 or 1. > > Not sure how useful "due to some kind of error" is here --- presumably > the inability to replay is always due to some kind of error. I think here "due to some kind of error" means something else than a conflict which could also prevent the reply from fully "working". > Would it be worth including a brief explanation about why git-replay > might be unable to complete or start (is this behavior in practice a > common-enough thing to document here)? I don't think it's worth it at this step for this new command. > > +EXAMPLES > > +-------- > > + > > +To simply rebase mybranch onto target: > > Looking at the CLI arguments, I think this phrase should be: > > Replay the commits in `origin/main..mybranch` onto `target`: We suppose that users can understand that "rebase mybranch onto target" means the same thing as what you suggest. Also I think it's useful to associate "rebase" with "--onto" by deliberately using "rebase" here. However to make it a bit clearer, I agree that quoting `mybranch` and `target` is better, so I have changed it to: "To simply rebase `mybranch` onto `target`:" > > +------------ > > +$ git replay --onto target origin/main..mybranch > > +update refs/heads/mybranch ${NEW_mybranch_HASH} ${OLD_mybranch_HASH} > > +------------ > > + > > +When calling `git replay`, one does not need to specify a range of > > +commits to replay using the syntax `A..B`; any range expression will > > +do: > > + > > +------------ > > +$ git replay --onto origin/main ^base branch1 branch2 branch3 > > Instead of `base`, how about `olderbranch`? Sorry but like above, I like "base" here. > > +update refs/heads/branch1 ${NEW_branch1_HASH} ${OLD_branch1_HASH} > > +update refs/heads/branch2 ${NEW_branch2_HASH} ${OLD_branch2_HASH} > > +update refs/heads/branch3 ${NEW_branch3_HASH} ${OLD_branch3_HASH} > > +------------ > > + > > +This will simultaneously rebase branch1, branch2, and branch3 -- all > > +commits they have since base, playing them on top of origin/main. > > How about > > This will rebase the commits in `branch1`, `branch2`, and `branch3` > (excluding those in `base`), preplaying them on top of `origin/main`. > > > +These three branches may have commits on top of base that they have in > > +common, but that does not need to be the case. > > s/base/`base` I am Ok with quoting `branch1`, `branch2`, `branch3`, `base` and `origin/main`, but otherwise I prefer to keep the original wording. > > +GIT > > +--- > > +Part of the linkgit:git[1] suite > > One question I do have is what happens if you run git-replay twice > (successfully)? Does the second invocation create another set of (new) > successfully replayed commits? Yes, I think it does that. > I ask because I'm interested in informing > the readers of our docs about any potential pitfalls from abusing this > command by mistake. I appreciate your desire to give high quality docs to our users, but I don't think it's a big pitfall and I think that this command is still very much "in the works" and is also designed for experienced users for now, so I am not sure it's the right time to spend too much time on this.
On Sun, Sep 10, 2023 at 5:20 AM Linus Arver <linusa@google.com> wrote: > > Linus Arver <linusa@google.com> writes: > > >> +update refs/heads/branch1 ${NEW_branch1_HASH} ${OLD_branch1_HASH} > >> +update refs/heads/branch2 ${NEW_branch2_HASH} ${OLD_branch2_HASH} > >> +update refs/heads/branch3 ${NEW_branch3_HASH} ${OLD_branch3_HASH} > >> +------------ > >> + > >> +This will simultaneously rebase branch1, branch2, and branch3 -- all > >> +commits they have since base, playing them on top of origin/main. > > > > How about > > > > This will rebase the commits in `branch1`, `branch2`, and `branch3` > > (excluding those in `base`), preplaying them on top of `origin/main`. > > Oops, I meant "replaying" not "preplaying". But also, perhaps the > following is simpler? > > This will replay the commits in `branch1`, `branch2`, and `branch3` > (excluding those in `base`), on top of `origin/main`. Here also I prefer to keep "rebase".
On Thu, Sep 7, 2023 at 1:02 PM Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote: > > Hi Christian, > > It is a bit surprising to see the manual page added in _this_ patch, in > the middle of the series... I can live with it, though. It's explained in the cover letter and a bit in some commit messages. For example in the "Possibly controversial issues" section of the cover letter, there is: " * when and where to add tests and docs: although t6429 has tests that are changed to use the new command instead of the fast-rebase test-tool command as soon as the former is introduced, there is no specific test script and no doc for the new command until commit 11/15 when standard revision ranges are used. This is done to avoid churn in tests and docs while the final form of the command hasn't crystalized enough. Adding tests and doc at this point makes this commit quite big and possibly more difficult to review than if they were in separate commits though. On the other hand when tests and docs are added in specific commits, some reviewers say it would be better to introduce them when the related changes are made. " > On Thu, 7 Sep 2023, Christian Couder wrote: > > +SYNOPSIS > > +-------- > > +[verse] > > +'git replay' --onto <newbase> <revision-range>... > > We need to make it clear here, already in the SYNOPSIS, that this is > experimental. Let's add an `(EXPERIMENTAL!)` prefix here. I haven't done that as other commands marked as EXPERIMENTAL don't have that in their SYNOPSIS. > > [...] > > diff --git a/t/t3650-replay-basics.sh b/t/t3650-replay-basics.sh > > new file mode 100755 > > Just like the manual page, I would have expected this test to be > introduced earlier, and not piggy-backed onto one of the handful "let's > turn fast-rebase into replay" patches. This is discussed above.
Christian Couder <christian.couder@gmail.com> writes: > On Sat, Sep 9, 2023 at 12:55 AM Linus Arver <linusa@google.com> wrote: >> >> Hi Christian, >> >> I am only reviewing the docs. To assume the mindset of a Git user >> unfamiliar with this command, I purposely did not read the cover letter >> until after this review was done. > > Ok, thanks! > > [...] >> > + Starting point at which to create the new commits. May be any >> > + valid commit, and not just an existing branch name. >> >> Add "See linkgit:gitrevisions[7]." at the end? > > I don't think it's worth it to mention "linkgit:gitrevisions[7]" > everywhere we can pass a revision. I think it makes the doc heavier > than it should be, especially here where the command is a plumbing one > for now and users should be quite experienced with Git already. I agree that plumbing commands assume that users are more experienced with Git already, so SGTM. >> I ask because I'm interested in informing >> the readers of our docs about any potential pitfalls from abusing this >> command by mistake. > > I appreciate your desire to give high quality docs to our users, but I > don't think it's a big pitfall and I think that this command is still > very much "in the works" and is also designed for experienced users > for now, so I am not sure it's the right time to spend too much time > on this. Also sounds reasonable to me. Thank you for considering my suggestions, much appreciated!
diff --git a/Documentation/git-replay.txt b/Documentation/git-replay.txt new file mode 100644 index 0000000000..9a2087b01a --- /dev/null +++ b/Documentation/git-replay.txt @@ -0,0 +1,90 @@ +git-replay(1) +============= + +NAME +---- +git-replay - Replay commits on a different base, without touching working tree + + +SYNOPSIS +-------- +[verse] +'git replay' --onto <newbase> <revision-range>... + +DESCRIPTION +----------- + +Takes a range of commits, and replays them onto a new location. Does +not touch the working tree or index, and does not update any +references. However, the output of this command is meant to be used +as input to `git update-ref --stdin`, which would update the relevant +branches. + +THIS COMMAND IS EXPERIMENTAL. THE BEHAVIOR MAY CHANGE. + +OPTIONS +------- + +--onto <newbase>:: + Starting point at which to create the new commits. May be any + valid commit, and not just an existing branch name. ++ +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. + +<revision-range>:: + Range of commits to replay; see "Specifying Ranges" in + linkgit:git-rev-parse. + +OUTPUT +------ + +When there are no conflicts, the output of this command is usable as +input to `git update-ref --stdin`. It is basically of the form: + + update refs/heads/branch1 ${NEW_branch1_HASH} ${OLD_branch1_HASH} + 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 depends on the arguments passed and +the shape of the history being replayed. + +EXIT STATUS +----------- + +For a successful, non-conflicted replay, the exit status is 0. When +the replay has conflicts, the exit status is 1. If the replay is not +able to complete (or start) due to some kind of error, the exit status +is something other than 0 or 1. + +EXAMPLES +-------- + +To simply rebase mybranch onto target: + +------------ +$ git replay --onto target origin/main..mybranch +update refs/heads/mybranch ${NEW_mybranch_HASH} ${OLD_mybranch_HASH} +------------ + +When calling `git replay`, one does not need to specify a range of +commits to replay using the syntax `A..B`; any range expression will +do: + +------------ +$ git replay --onto origin/main ^base branch1 branch2 branch3 +update refs/heads/branch1 ${NEW_branch1_HASH} ${OLD_branch1_HASH} +update refs/heads/branch2 ${NEW_branch2_HASH} ${OLD_branch2_HASH} +update refs/heads/branch3 ${NEW_branch3_HASH} ${OLD_branch3_HASH} +------------ + +This will simultaneously rebase branch1, branch2, and branch3 -- all +commits they have since base, playing them on top of origin/main. +These three branches may have commits on top of base that they have in +common, but that does not need to be the case. + +GIT +--- +Part of the linkgit:git[1] suite diff --git a/builtin/replay.c b/builtin/replay.c index e45cd59da1..de2ddeae3e 100644 --- a/builtin/replay.c +++ b/builtin/replay.c @@ -14,7 +14,6 @@ #include "parse-options.h" #include "refs.h" #include "revision.h" -#include "strvec.h" #include <oidset.h> #include <tree.h> @@ -118,16 +117,14 @@ int cmd_replay(int argc, const char **argv, const char *prefix) struct commit *onto; const char *onto_name = NULL; struct commit *last_commit = NULL; - struct strvec rev_walk_args = STRVEC_INIT; struct rev_info revs; struct commit *commit; struct merge_options merge_opt; struct merge_result result; - struct strbuf branch_name = STRBUF_INIT; int ret = 0; const char * const replay_usage[] = { - N_("git replay --onto <newbase> <oldbase> <branch>"), + N_("git replay --onto <newbase> <revision-range>..."), NULL }; struct option replay_options[] = { @@ -145,20 +142,13 @@ int cmd_replay(int argc, const char **argv, const char *prefix) usage_with_options(replay_usage, replay_options); } - if (argc != 3) { - error(_("bad number of arguments")); - usage_with_options(replay_usage, replay_options); - } - onto = peel_committish(onto_name); - strbuf_addf(&branch_name, "refs/heads/%s", argv[2]); repo_init_revisions(the_repository, &revs, prefix); - strvec_pushl(&rev_walk_args, "", argv[2], "--not", argv[1], NULL); - - if (setup_revisions(rev_walk_args.nr, rev_walk_args.v, &revs, NULL) > 1) { - ret = error(_("unhandled options")); + argc = setup_revisions(argc, argv, &revs, NULL); + if (argc > 1) { + ret = error(_("unrecognized argument: %s"), argv[1]); goto cleanup; } @@ -168,8 +158,6 @@ int cmd_replay(int argc, const char **argv, const char *prefix) revs.topo_order = 1; revs.simplify_history = 0; - strvec_clear(&rev_walk_args); - if (prepare_revision_walk(&revs) < 0) { ret = error(_("error preparing revisions")); goto cleanup; @@ -211,7 +199,6 @@ int cmd_replay(int argc, const char **argv, const char *prefix) ret = result.clean; cleanup: - strbuf_release(&branch_name); release_revisions(&revs); /* Return */ diff --git a/t/t3650-replay-basics.sh b/t/t3650-replay-basics.sh new file mode 100755 index 0000000000..a1da4f9ef9 --- /dev/null +++ b/t/t3650-replay-basics.sh @@ -0,0 +1,83 @@ +#!/bin/sh + +test_description='basic git replay tests' + +GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main +export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME + +. ./test-lib.sh + +GIT_AUTHOR_NAME=author@name +GIT_AUTHOR_EMAIL=bogus@email@address +export GIT_AUTHOR_NAME GIT_AUTHOR_EMAIL + +test_expect_success 'setup' ' + test_commit A && + test_commit B && + + git switch -c topic1 && + test_commit C && + git switch -c topic2 && + test_commit D && + test_commit E && + git switch topic1 && + test_commit F && + git switch -c topic3 && + test_commit G && + test_commit H && + git switch -c topic4 main && + test_commit I && + test_commit J && + + git switch -c next main && + test_commit K && + git merge -m "Merge topic1" topic1 && + git merge -m "Merge topic2" topic2 && + git merge -m "Merge topic3" topic3 && + >evil && + git add evil && + git commit --amend && + git merge -m "Merge topic4" topic4 && + + git switch main && + test_commit L && + test_commit M && + + git switch -c conflict B && + test_commit C.conflict C.t conflict +' + +test_expect_success 'setup bare' ' + git clone --bare . bare +' + +test_expect_success 'using replay to rebase two branches, one on top of other' ' + git replay --onto main topic1..topic2 >result && + + test_line_count = 1 result && + + git log --format=%s $(cut -f 3 -d " " result) >actual && + test_write_lines E D M L B A >expect && + test_cmp expect actual && + + printf "update refs/heads/topic2 " >expect && + printf "%s " $(cut -f 3 -d " " result) >>expect && + git rev-parse topic2 >>expect && + + test_cmp expect result +' + +test_expect_success 'using replay on bare repo to rebase two branches, one on top of other' ' + git -C bare replay --onto main topic1..topic2 >result-bare && + test_cmp expect result-bare +' + +test_expect_success 'using replay to rebase with a conflict' ' + test_expect_code 1 git replay --onto topic1 B..conflict +' + +test_expect_success 'using replay on bare repo to rebase with a conflict' ' + test_expect_code 1 git -C bare replay --onto topic1 B..conflict +' + +test_done diff --git a/t/t6429-merge-sequence-rename-caching.sh b/t/t6429-merge-sequence-rename-caching.sh index 099aefeffc..0f39ed0d08 100755 --- a/t/t6429-merge-sequence-rename-caching.sh +++ b/t/t6429-merge-sequence-rename-caching.sh @@ -71,7 +71,7 @@ test_expect_success 'caching renames does not preclude finding new ones' ' git switch upstream && - git replay --onto HEAD upstream~1 topic >out && + git replay --onto HEAD upstream~1..topic >out && git update-ref --stdin <out && git checkout topic && @@ -141,7 +141,7 @@ test_expect_success 'cherry-pick both a commit and its immediate revert' ' GIT_TRACE2_PERF="$(pwd)/trace.output" && export GIT_TRACE2_PERF && - git replay --onto HEAD upstream~1 topic >out && + git replay --onto HEAD upstream~1..topic >out && git update-ref --stdin <out && git checkout topic && @@ -201,7 +201,7 @@ test_expect_success 'rename same file identically, then reintroduce it' ' GIT_TRACE2_PERF="$(pwd)/trace.output" && export GIT_TRACE2_PERF && - git replay --onto HEAD upstream~1 topic >out && + git replay --onto HEAD upstream~1..topic >out && git update-ref --stdin <out && git checkout topic && @@ -279,7 +279,7 @@ test_expect_success 'rename same file identically, then add file to old dir' ' GIT_TRACE2_PERF="$(pwd)/trace.output" && export GIT_TRACE2_PERF && - git replay --onto HEAD upstream~1 topic >out && + git replay --onto HEAD upstream~1..topic >out && git update-ref --stdin <out && git checkout topic && @@ -357,7 +357,7 @@ test_expect_success 'cached dir rename does not prevent noticing later conflict' GIT_TRACE2_PERF="$(pwd)/trace.output" && export GIT_TRACE2_PERF && - test_must_fail git replay --onto HEAD upstream~1 topic >output && + test_must_fail git replay --onto HEAD upstream~1..topic >output && grep region_enter.*diffcore_rename trace.output >calls && test_line_count = 2 calls @@ -456,7 +456,7 @@ test_expect_success 'dir rename unneeded, then add new file to old dir' ' GIT_TRACE2_PERF="$(pwd)/trace.output" && export GIT_TRACE2_PERF && - git replay --onto HEAD upstream~1 topic >out && + git replay --onto HEAD upstream~1..topic >out && git update-ref --stdin <out && git checkout topic && @@ -523,7 +523,7 @@ test_expect_success 'dir rename unneeded, then rename existing file into old dir GIT_TRACE2_PERF="$(pwd)/trace.output" && export GIT_TRACE2_PERF && - git replay --onto HEAD upstream~1 topic >out && + git replay --onto HEAD upstream~1..topic >out && git update-ref --stdin <out && git checkout topic && @@ -626,7 +626,7 @@ test_expect_success 'caching renames only on upstream side, part 1' ' GIT_TRACE2_PERF="$(pwd)/trace.output" && export GIT_TRACE2_PERF && - git replay --onto HEAD upstream~1 topic >out && + git replay --onto HEAD upstream~1..topic >out && git update-ref --stdin <out && git checkout topic && @@ -685,7 +685,7 @@ test_expect_success 'caching renames only on upstream side, part 2' ' GIT_TRACE2_PERF="$(pwd)/trace.output" && export GIT_TRACE2_PERF && - git replay --onto HEAD upstream~1 topic >out && + git replay --onto HEAD upstream~1..topic >out && git update-ref --stdin <out && git checkout topic &&