| Message ID | 20230602102533.876905-15-christian.couder@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Introduce new `git replay` command | expand |
Christian Couder <christian.couder@gmail.com> writes: > diff --git a/Documentation/git-replay.txt b/Documentation/git-replay.txt > index 439b2f92e7..6fcaa44ef2 100644 > --- a/Documentation/git-replay.txt > +++ b/Documentation/git-replay.txt > @@ -9,7 +9,7 @@ git-replay - Replay commits on a different base, without touching working tree > SYNOPSIS > -------- > [verse] > -'git replay' (--onto <newbase> | --advance <branch>) <revision-range>... > +'git replay' ([--contained] --onto <newbase> | --advance <branch>) > <revision-range>... I'm not sure we need this, or at least not right now. I've been testing with a repo having: * a13d9c4 (another-feature) yet another commit * c7afc2e (HEAD -> feature) third commit * e95cecc second commit * f141e77 first commit | * 7bb62ac (main) later commit | * 506cb0a another commit |/ * e7acac6 initial commit I tried both commands: $ git replay --onto main main..feature main..another-feature $ git replay --onto main --contained main..another-feature and they both give the same result (especially with the commit following up this one). What is the upside of having this --contained option? Maybe it's better to defer this patch to a separate series. And another question, in git-rebase(1) a similar option is called --update-refs. Would you consider reusing that name here is a good idea for consistency? > diff --git a/builtin/replay.c b/builtin/replay.c > index 4b64d15159..9b182eaa90 100644 > --- a/builtin/replay.c > +++ b/builtin/replay.c > @@ -303,6 +306,10 @@ int cmd_replay(int argc, const char **argv, const char *prefix) > usage_with_options(replay_usage, replay_options); > } > > + if (advance_name && contained) > + die(_("options '%s' and '%s' cannot be used together"), > + "--advance", "--contained"); Is there a technical issue why we don't support this? I don't really see why we couldn't. -- Toon
On Thu, Jun 22, 2023 at 12:13 PM Toon Claes <toon@iotcl.com> wrote: > > > Christian Couder <christian.couder@gmail.com> writes: > > > diff --git a/Documentation/git-replay.txt b/Documentation/git-replay.txt > > index 439b2f92e7..6fcaa44ef2 100644 > > --- a/Documentation/git-replay.txt > > +++ b/Documentation/git-replay.txt > > @@ -9,7 +9,7 @@ git-replay - Replay commits on a different base, without touching working tree > > SYNOPSIS > > -------- > > [verse] > > -'git replay' (--onto <newbase> | --advance <branch>) <revision-range>... > > +'git replay' ([--contained] --onto <newbase> | --advance <branch>) > > <revision-range>... > > I'm not sure we need this, or at least not right now. > I've been testing with a repo having: > > * a13d9c4 (another-feature) yet another commit > * c7afc2e (HEAD -> feature) third commit > * e95cecc second commit > * f141e77 first commit > | * 7bb62ac (main) later commit > | * 506cb0a another commit > |/ > * e7acac6 initial commit > > I tried both commands: > > $ git replay --onto main main..feature main..another-feature > $ git replay --onto main --contained main..another-feature > > and they both give the same result (especially with the commit following > up this one). What is the upside of having this --contained option? This is expected. The thing is that: $ git replay --onto main main..another-feature will only output something to update 'another-feature' while: $ git replay --onto main --contained main..another-feature will output something to update 'another-feature' and also something to update 'feature'. So when you use --contained you don't need to first find the other branches like 'feature' that point to commits between 'main' and 'another-feature', as --contained will find them for you. > Maybe it's better to defer this patch to a separate series. I am not sure why you are proposing this. It's true that there are other means to achieve the same thing as --contained, but that doesn't mean that it cannot be useful already. If there were things that needed to be more polished in this feature, then maybe leaving it for a separate series later might allow this series to graduate while --contained is polished, but I don't think we are in this case. > And another question, in git-rebase(1) a similar option is called > --update-refs. Would you consider reusing that name here is a good idea > for consistency? `git replay` outputs commands that can be passed to `git update-ref --stdin`, but for now it doesn't run that command itself. So no refs are actually updated. If we ever add an option for git replay to also update the refs, it would have a name probably quite similar to --update-refs so it would be unfortunate if --update-refs is already used for something else. --contained also tells about the fact that the branches affected by the option are "contained" in the revision range that is passed, which is nice. In short I think it's just unfortunate that git rebase already uses --update-refs for the "contained" branches, as it would be likely to confuse people if we would use it here for that.
diff --git a/Documentation/git-replay.txt b/Documentation/git-replay.txt index 439b2f92e7..6fcaa44ef2 100644 --- a/Documentation/git-replay.txt +++ b/Documentation/git-replay.txt @@ -9,7 +9,7 @@ git-replay - Replay commits on a different base, without touching working tree SYNOPSIS -------- [verse] -'git replay' (--onto <newbase> | --advance <branch>) <revision-range>... +'git replay' ([--contained] --onto <newbase> | --advance <branch>) <revision-range>... DESCRIPTION ----------- @@ -90,6 +90,16 @@ top of the exact same new base, they only differ in that the first provides instructions to make mybranch point at the new commits and the second provides instructions to make target point at them. +What if you have a stack of branches, one depending upon another, and +you'd really like to rebase the whole set? + +------------ +$ git replay --contained --onto origin/main origin/main..tipbranch +update refs/heads/branch1 ${NEW_branch1_HASH} ${OLD_branch1_HASH} +update refs/heads/branch2 ${NEW_branch2_HASH} ${OLD_branch2_HASH} +update refs/heads/tipbranch ${NEW_tipbranch_HASH} ${OLD_tipbranch_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: diff --git a/builtin/replay.c b/builtin/replay.c index 4b64d15159..9b182eaa90 100644 --- a/builtin/replay.c +++ b/builtin/replay.c @@ -258,6 +258,7 @@ int cmd_replay(int argc, const char **argv, const char *prefix) const char *advance_name = NULL; struct commit *onto = NULL; const char *onto_name = NULL; + int contained = 0; struct rev_info revs; struct commit *last_commit = NULL; @@ -268,7 +269,7 @@ int cmd_replay(int argc, const char **argv, const char *prefix) int ret = 0, i; const char * const replay_usage[] = { - N_("git replay (--onto <newbase> | --advance <branch>) <revision-range>..."), + N_("git replay ([--contained] --onto <newbase> | --advance <branch>) <revision-range>..."), NULL }; struct option replay_options[] = { @@ -278,6 +279,8 @@ int cmd_replay(int argc, const char **argv, const char *prefix) OPT_STRING(0, "onto", &onto_name, N_("revision"), N_("replay onto given commit")), + OPT_BOOL(0, "contained", &contained, + N_("advance all branches contained in revision-range")), OPT_END() }; @@ -303,6 +306,10 @@ int cmd_replay(int argc, const char **argv, const char *prefix) usage_with_options(replay_usage, replay_options); } + if (advance_name && contained) + die(_("options '%s' and '%s' cannot be used together"), + "--advance", "--contained"); + repo_init_revisions(the_repository, &revs, prefix); argc = setup_revisions(argc, argv, &revs, NULL); @@ -364,7 +371,8 @@ int cmd_replay(int argc, const char **argv, const char *prefix) continue; while (decoration) { if (decoration->type == DECORATION_REF_LOCAL && - strset_contains(update_refs, decoration->name)) { + (contained || strset_contains(update_refs, + decoration->name))) { printf("update %s %s %s\n", decoration->name, oid_to_hex(&last_commit->object.oid), diff --git a/t/t3650-replay-basics.sh b/t/t3650-replay-basics.sh index bca405c431..3fb4167e69 100755 --- a/t/t3650-replay-basics.sh +++ b/t/t3650-replay-basics.sh @@ -122,4 +122,33 @@ test_expect_success 'using replay on bare repo to perform basic cherry-pick' ' test_cmp expect result-bare ' +test_expect_success 'using replay to also rebase a contained branch' ' + git replay --contained --onto main main..topic3 >result && + + test_line_count = 2 result && + cut -f 3 -d " " result >new-branch-tips && + + git log --format=%s $(head -n 1 new-branch-tips) >actual && + test_write_lines F C M L B A >expect && + test_cmp expect actual && + + git log --format=%s $(tail -n 1 new-branch-tips) >actual && + test_write_lines H G F C M L B A >expect && + test_cmp expect actual && + + printf "update refs/heads/topic1 " >expect && + printf "%s " $(head -n 1 new-branch-tips) >>expect && + git rev-parse topic1 >>expect && + printf "update refs/heads/topic3 " >>expect && + printf "%s " $(tail -n 1 new-branch-tips) >>expect && + git rev-parse topic3 >>expect && + + test_cmp expect result +' + +test_expect_success 'using replay on bare repo to also rebase a contained branch' ' + git -C bare replay --contained --onto main main..topic3 >result-bare && + test_cmp expect result-bare +' + test_done