| Message ID | 20230602102533.876905-14-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: > + /* > + * When the user specifies e.g. > + * git replay origin/main..mybranch > + * git replay ^origin/next mybranch1 mybranch2 When I'm trying these, I'm getting the error: error: option --onto or --advance is mandatory In what situation can I omit both --onto and --advance? > +static void determine_replay_mode(struct rev_cmdline_info *cmd_info, > + const char *onto_name, > + const char **advance_name, > + struct commit **onto, Would it make sense to call this target? > + struct strset **update_refs) > +{ > + struct ref_info rinfo; > + > + get_ref_information(cmd_info, &rinfo); > + if (!rinfo.positive_refexprs) > + die(_("need some commits to replay")); > + if (onto_name && *advance_name) > + die(_("--onto and --advance are incompatible")); Do we actually need to disallow this? I mean from git-replay's point of view, there's no technical limitation why can cannot support both modes at once. The update-ref commands in the output will update both target and source branches, but it's not up to us whether that's desired. > + else if (onto_name) { No need to 'else' here IMHO. > + *onto = peel_committish(onto_name); > + if (rinfo.positive_refexprs < > + strset_get_size(&rinfo.positive_refs)) > + die(_("all positive revisions given must be references")); I tested this locally with the following command: $ git replay --onto main OID..OID This command didn't give any errors, neither did it return any update-ref lines. I would have expected to hit this die(). > + } else if (*advance_name) { > + struct object_id oid; > + char *fullname = NULL; > + > + *onto = peel_committish(*advance_name); > + if (repo_dwim_ref(the_repository, *advance_name, strlen(*advance_name), > + &oid, &fullname, 0) == 1) { > + *advance_name = fullname; > + } else { > + die(_("argument to --advance must be a reference")); > + } > + if (rinfo.positive_refexprs > 1) > + die(_("cannot advance target with multiple source branches because ordering would be ill-defined")); The sources aren't always branches, so I suggest something like: + die(_("cannot advance target with multiple sources because ordering would be ill-defined")); > + determine_replay_mode(&revs.cmdline, onto_name, &advance_name, > + &onto, &update_refs); > + > + if (!onto) /* FIXME: Should handle replaying down to root commit */ > + die("Replaying down to root commit is not supported yet!"); When I was testing locally I tried the following: $ git replay --onto main feature I was expecting this command to find the common ancestor automatically, but instead I got this error. I'm fine if for now the command does not determine the common ancestor yet, but I think we should provide a better error for this scenario. > +test_expect_success 'using replay on bare repo to perform basic cherry-pick' ' > + git -C bare replay --advance main topic1..topic2 >result-bare && > + test_cmp expect result-bare > +' > + > test_done Shall we add a test case when providing both --onto and --advance? And one that omits both?
Christian Couder <christian.couder@gmail.com> 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. > @@ -29,9 +29,17 @@ OPTIONS > Starting point at which to create the new commits. May be any > valid commit, and not just an existing branch name. > + > -The update-ref commands 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). > +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). > + > +--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). > @@ -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". 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, 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.
On Thu, Jun 22, 2023 at 12:09 PM Toon Claes <toon@iotcl.com> wrote: > > > Christian Couder <christian.couder@gmail.com> writes: > > > + /* > > + * When the user specifies e.g. > > + * git replay origin/main..mybranch > > + * git replay ^origin/next mybranch1 mybranch2 > > When I'm trying these, I'm getting the error: > error: option --onto or --advance is mandatory > > In what situation can I omit both --onto and --advance? It was possible with version 1 of this series as one of the patches allowed the command to guess the base: https://lore.kernel.org/git/20230407072415.1360068-13-christian.couder@gmail.com/ so --onto wasn't needed to specify it. Comments on that patch said that it might be better to focus on a plumbing command first and for that the patch wasn't needed, so I removed it in version 2. > > +static void determine_replay_mode(struct rev_cmdline_info *cmd_info, > > + const char *onto_name, > > + const char **advance_name, > > + struct commit **onto, > > Would it make sense to call this target? This is more the new base we are rebasing target branches onto. So if we want to change the name, `--base` or `--newbase` would make more sense. `git rebase` already has `--onto` though, so, if we want to be consistent with it, we should keep `--onto` for this. > > + struct strset **update_refs) > > +{ > > + struct ref_info rinfo; > > + > > + get_ref_information(cmd_info, &rinfo); > > + if (!rinfo.positive_refexprs) > > + die(_("need some commits to replay")); > > + if (onto_name && *advance_name) > > + die(_("--onto and --advance are incompatible")); > > Do we actually need to disallow this? I mean from git-replay's point of > view, there's no technical limitation why can cannot support both modes > at once. The update-ref commands in the output will update both target > and source branches, but it's not up to us whether that's desired. I am not sure what you call "target" and "source" branches. Anyway here is in simple terms the way the command works: 1) it takes either `--onto <newbase>` or `--advance <branch>` and then one or more <revision-range>, 2) it replays all the commits in the <revision-range> onto either <newbase> or <branch>, 3) in case of `--advance`, it outputs a single command for `git update-ref --stdin` to advance <branch> to the last commit that was just replayed, 4) in case of `--onto`, it outputs a number of commands for `git update-ref --stdin` to update the branches in <revision-range> to where the tip commits of these branches have been replayed. So `--advance` is like a cherry-pick, and `--onto` is like a rebase. It would be possible to do both a rebase onto a branch and a cherry-pick of the rebased commits onto that branch, but this is not very common and you can achieve the same result by just rebasing and then using `git reset` or `git update-ref` to make the branch point to the result of the rebase. So I don't see the point of complicating the command at this point. > > + else if (onto_name) { > > No need to 'else' here IMHO. > > > + *onto = peel_committish(onto_name); > > + if (rinfo.positive_refexprs < > > + strset_get_size(&rinfo.positive_refs)) > > + die(_("all positive revisions given must be references")); > > I tested this locally with the following command: > > $ git replay --onto main OID..OID > > This command didn't give any errors, neither did it return any > update-ref lines. I would have expected to hit this die(). Yeah, this might be unexpected. I tested it too and 'rinfo.positive_refexprs' is 1 while 'strset_get_size(&rinfo.positive_refs)' is 0 with such a command. The result does not look wrong though. Above that code there is: if (!rinfo.positive_refexprs) die(_("need some commits to replay")); so it looks like there is at least a check that the revision range passed to the command contains positive commits. It might be possible that users prefer a command that outputs nothing when there is nothing to replay instead of erroring out. > > + } else if (*advance_name) { > > + struct object_id oid; > > + char *fullname = NULL; > > + > > + *onto = peel_committish(*advance_name); > > + if (repo_dwim_ref(the_repository, *advance_name, strlen(*advance_name), > > + &oid, &fullname, 0) == 1) { > > + *advance_name = fullname; > > + } else { > > + die(_("argument to --advance must be a reference")); > > + } > > + if (rinfo.positive_refexprs > 1) > > + die(_("cannot advance target with multiple source branches because ordering would be ill-defined")); > > The sources aren't always branches, so I suggest something like: > > + die(_("cannot advance target with multiple sources because ordering would be ill-defined")); Yeah, that looks reasonable. I have made this change in version 4 I will send very soon. > > + determine_replay_mode(&revs.cmdline, onto_name, &advance_name, > > + &onto, &update_refs); > > + > > + if (!onto) /* FIXME: Should handle replaying down to root commit */ > > + die("Replaying down to root commit is not supported yet!"); > > When I was testing locally I tried the following: > > $ git replay --onto main feature > > I was expecting this command to find the common ancestor automatically, > but instead I got this error. I'm fine if for now the command does not > determine the common ancestor yet, but I think we should provide a > better error for this scenario. I agree that it isn't very user friendly. We could indeed try to find if there is a common ancestor, and, if that's the case, suggest another way to call the command. This is a plumbing command in its early stage though for now. So I guess it's Ok to postpone working on nicer error messages. > > +test_expect_success 'using replay on bare repo to perform basic cherry-pick' ' > > + git -C bare replay --advance main topic1..topic2 >result-bare && > > + test_cmp expect result-bare > > +' > > + > > test_done > > Shall we add a test case when providing both --onto and --advance? And > one that omits both? Ok, I have made this change in version 4.
On Tue, Jul 25, 2023 at 11:41 PM Junio C Hamano <gitster@pobox.com> wrote: > > Christian Couder <christian.couder@gmail.com> 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."
diff --git a/Documentation/git-replay.txt b/Documentation/git-replay.txt index 394d7b0050..439b2f92e7 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> <revision-range>... +'git replay' (--onto <newbase> | --advance <branch>) <revision-range>... DESCRIPTION ----------- @@ -29,9 +29,17 @@ OPTIONS Starting point at which to create the new commits. May be any valid commit, and not just an existing branch name. + -The update-ref commands 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). +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). + +--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). <revision-range>:: Range of commits to replay; see "Specifying Ranges" in @@ -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). EXIT STATUS ----------- @@ -67,6 +78,18 @@ $ git replay --onto target origin/main..mybranch update refs/heads/mybranch ${NEW_mybranch_HASH} ${OLD_mybranch_HASH} ------------ +To cherry-pick the commits from mybranch onto target: + +------------ +$ git replay --advance target origin/main..mybranch +update refs/heads/target ${NEW_target_HASH} ${OLD_target_HASH} +------------ + +Note that the first two examples replay the exact same commits and on +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. + 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 cffbf34290..4b64d15159 100644 --- a/builtin/replay.c +++ b/builtin/replay.c @@ -14,6 +14,7 @@ #include "parse-options.h" #include "refs.h" #include "revision.h" +#include "strmap.h" #include <oidset.h> #include <tree.h> @@ -82,6 +83,146 @@ static struct commit *create_commit(struct tree *tree, return (struct commit *)obj; } +struct ref_info { + struct commit *onto; + struct strset positive_refs; + struct strset negative_refs; + int positive_refexprs; + int negative_refexprs; +}; + +static void get_ref_information(struct rev_cmdline_info *cmd_info, + struct ref_info *ref_info) +{ + int i; + + ref_info->onto = NULL; + strset_init(&ref_info->positive_refs); + strset_init(&ref_info->negative_refs); + ref_info->positive_refexprs = 0; + ref_info->negative_refexprs = 0; + + /* + * When the user specifies e.g. + * git replay origin/main..mybranch + * git replay ^origin/next mybranch1 mybranch2 + * we want to be able to determine where to replay the commits. In + * these examples, the branches are probably based on an old version + * of either origin/main or origin/next, so we want to replay on the + * newest version of that branch. In contrast we would want to error + * out if they ran + * git replay ^origin/master ^origin/next mybranch + * git replay mybranch~2..mybranch + * the first of those because there's no unique base to choose, and + * the second because they'd likely just be replaying commits on top + * of the same commit and not making any difference. + */ + for (i = 0; i < cmd_info->nr; i++) { + struct rev_cmdline_entry *e = cmd_info->rev + i; + struct object_id oid; + const char *refexpr = e->name; + char *fullname = NULL; + int can_uniquely_dwim = 1; + + if (*refexpr == '^') + refexpr++; + if (repo_dwim_ref(the_repository, refexpr, strlen(refexpr), &oid, &fullname, 0) != 1) + can_uniquely_dwim = 0; + + if (e->flags & BOTTOM) { + if (can_uniquely_dwim) + strset_add(&ref_info->negative_refs, fullname); + if (!ref_info->negative_refexprs) + ref_info->onto = lookup_commit_reference_gently(the_repository, + &e->item->oid, 1); + ref_info->negative_refexprs++; + } else { + if (can_uniquely_dwim) + strset_add(&ref_info->positive_refs, fullname); + ref_info->positive_refexprs++; + } + + free(fullname); + } +} + +static void determine_replay_mode(struct rev_cmdline_info *cmd_info, + const char *onto_name, + const char **advance_name, + struct commit **onto, + struct strset **update_refs) +{ + struct ref_info rinfo; + + get_ref_information(cmd_info, &rinfo); + if (!rinfo.positive_refexprs) + die(_("need some commits to replay")); + if (onto_name && *advance_name) + die(_("--onto and --advance are incompatible")); + else if (onto_name) { + *onto = peel_committish(onto_name); + if (rinfo.positive_refexprs < + strset_get_size(&rinfo.positive_refs)) + die(_("all positive revisions given must be references")); + } else if (*advance_name) { + struct object_id oid; + char *fullname = NULL; + + *onto = peel_committish(*advance_name); + if (repo_dwim_ref(the_repository, *advance_name, strlen(*advance_name), + &oid, &fullname, 0) == 1) { + *advance_name = fullname; + } else { + die(_("argument to --advance must be a reference")); + } + if (rinfo.positive_refexprs > 1) + die(_("cannot advance target with multiple source branches because ordering would be ill-defined")); + } else { + int positive_refs_complete = ( + rinfo.positive_refexprs == + strset_get_size(&rinfo.positive_refs)); + int negative_refs_complete = ( + rinfo.negative_refexprs == + strset_get_size(&rinfo.negative_refs)); + /* + * We need either positive_refs_complete or + * negative_refs_complete, but not both. + */ + if (rinfo.negative_refexprs > 0 && + positive_refs_complete == negative_refs_complete) + die(_("cannot implicitly determine whether this is an --advance or --onto operation")); + if (negative_refs_complete) { + struct hashmap_iter iter; + struct strmap_entry *entry; + + if (rinfo.negative_refexprs == 0) + die(_("all positive revisions given must be references")); + else if (rinfo.negative_refexprs > 1) + die(_("cannot implicitly determine whether this is an --advance or --onto operation")); + else if (rinfo.positive_refexprs > 1) + die(_("cannot advance target with multiple source branches because ordering would be ill-defined")); + + /* Only one entry, but we have to loop to get it */ + strset_for_each_entry(&rinfo.negative_refs, + &iter, entry) { + *advance_name = entry->key; + } + } else { /* positive_refs_complete */ + if (rinfo.negative_refexprs > 1) + die(_("cannot implicitly determine correct base for --onto")); + if (rinfo.negative_refexprs == 1) + *onto = rinfo.onto; + } + } + if (!*advance_name) { + *update_refs = xcalloc(1, sizeof(**update_refs)); + **update_refs = rinfo.positive_refs; + memset(&rinfo.positive_refs, 0, sizeof(**update_refs)); + } + strset_clear(&rinfo.negative_refs); + strset_clear(&rinfo.positive_refs); +} + static struct commit *pick_regular_commit(struct commit *pickme, struct commit *last_commit, struct merge_options *merge_opt, @@ -114,20 +255,26 @@ static struct commit *pick_regular_commit(struct commit *pickme, int cmd_replay(int argc, const char **argv, const char *prefix) { - struct commit *onto; + const char *advance_name = NULL; + struct commit *onto = NULL; const char *onto_name = NULL; - struct commit *last_commit = NULL; + struct rev_info revs; + struct commit *last_commit = NULL; struct commit *commit; struct merge_options merge_opt; struct merge_result result; + struct strset *update_refs = NULL; int ret = 0, i; const char * const replay_usage[] = { - N_("git replay --onto <newbase> <revision-range>..."), + N_("git replay (--onto <newbase> | --advance <branch>) <revision-range>..."), NULL }; struct option replay_options[] = { + OPT_STRING(0, "advance", &advance_name, + N_("branch"), + N_("make replay advance given branch")), OPT_STRING(0, "onto", &onto_name, N_("revision"), N_("replay onto given commit")), @@ -151,13 +298,11 @@ int cmd_replay(int argc, const char **argv, const char *prefix) usage_with_options(replay_usage, replay_options); } - if (!onto_name) { - error(_("option --onto is mandatory")); + if (!onto_name && !advance_name) { + error(_("option --onto or --advance is mandatory")); usage_with_options(replay_usage, replay_options); } - onto = peel_committish(onto_name); - repo_init_revisions(the_repository, &revs, prefix); argc = setup_revisions(argc, argv, &revs, NULL); @@ -182,6 +327,12 @@ int cmd_replay(int argc, const char **argv, const char *prefix) revs.topo_order = 1; revs.simplify_history = 0; + determine_replay_mode(&revs.cmdline, onto_name, &advance_name, + &onto, &update_refs); + + if (!onto) /* FIXME: Should handle replaying down to root commit */ + die("Replaying down to root commit is not supported yet!"); + if (prepare_revision_walk(&revs) < 0) { ret = error(_("error preparing revisions")); goto cleanup; @@ -190,6 +341,7 @@ int cmd_replay(int argc, const char **argv, const char *prefix) init_merge_options(&merge_opt, the_repository); memset(&result, 0, sizeof(result)); merge_opt.show_rename_progress = 0; + result.tree = repo_get_commit_tree(the_repository, onto); last_commit = onto; while ((commit = get_revision(&revs))) { @@ -204,12 +356,15 @@ int cmd_replay(int argc, const char **argv, const char *prefix) if (!last_commit) break; + /* Update any necessary branches */ + if (advance_name) + continue; decoration = get_name_decoration(&commit->object); if (!decoration) continue; - while (decoration) { - if (decoration->type == DECORATION_REF_LOCAL) { + if (decoration->type == DECORATION_REF_LOCAL && + strset_contains(update_refs, decoration->name)) { printf("update %s %s %s\n", decoration->name, oid_to_hex(&last_commit->object.oid), @@ -219,11 +374,23 @@ int cmd_replay(int argc, const char **argv, const char *prefix) } } + /* In --advance mode, advance the target ref */ + if (result.clean == 1 && advance_name) { + printf("update %s %s %s\n", + advance_name, + oid_to_hex(&last_commit->object.oid), + oid_to_hex(&onto->object.oid)); + } + /* Cleanup */ merge_finalize(&merge_opt, &result); ret = result.clean; cleanup: + if (update_refs) { + strset_clear(update_refs); + free(update_refs); + } release_revisions(&revs); /* Return */ diff --git a/t/t3650-replay-basics.sh b/t/t3650-replay-basics.sh index de6e40950e..bca405c431 100755 --- a/t/t3650-replay-basics.sh +++ b/t/t3650-replay-basics.sh @@ -96,4 +96,30 @@ test_expect_success 'using replay on bare repo with disallowed pathspec' ' test_must_fail git -C bare replay --onto main topic1..topic2 -- A.t ' +test_expect_success 'using replay to perform basic cherry-pick' ' + # The differences between this test and previous ones are: + # --advance vs --onto + # 2nd field of result is refs/heads/main vs. refs/heads/topic2 + # 4th field of result is hash for main instead of hash for topic2 + + git replay --advance 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/main " >expect && + printf "%s " $(cut -f 3 -d " " result) >>expect && + git rev-parse main >>expect && + + test_cmp expect result +' + +test_expect_success 'using replay on bare repo to perform basic cherry-pick' ' + git -C bare replay --advance main topic1..topic2 >result-bare && + test_cmp expect result-bare +' + test_done