Christian Couder <christian.couder@xxxxxxxxx> 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?
