replay: die descriptively when invalid commit-ish is given

Giving an invalid commit-ish to `--onto` makes git-replay(1) fail with: fatal: Replaying down to root commit is not supported yet! Going backwards from this point: 1. `onto` is `NULL` from `set_up_replay_mode`; 2. that function in turn calls `peel_committish`; and 3. here we return `NULL` if `repo_get_oid` fails. Let’s die immediately with a descriptive error message instead. Doing this also provides us with a descriptive error if we “forget” to provide an argument to `--onto` (but we really do unintentionally):[1] $ git replay --onto ^main topic1 fatal: '^main' is not a valid commit-ish Note that the `--advance` case won’t be triggered in practice because of the “argument to --advance must be a reference” check (see the previous test, and commit). † 1: The argument to `--onto` is mandatory and the option parser accepts both `--onto=<name>` (stuck form) and `--onto name`. The latter form makes it easy to unintentionally pass something to the option when you really meant to pass a positional argument. Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Kristoffer Haugsbakk committed Jan 5, 2026 at 20:53 UTC 3074d08cfa1bfb75f96fa4a240c575fad4cb8060
2 files changed +14 -6
builtin/replay.c
+7 -6
@@ -33,13 +33,15 @@ static const char *short_commit_name(struct repository *repo,
33 DEFAULT_ABBREV);
34 }
35
36 -static struct commit *peel_committish(struct repository *repo, const char *name)
36 +static struct commit *peel_committish(struct repository *repo,
37 + const char *name,
38 + const char *mode)
39 {
40 struct object *obj;
41 struct object_id oid;
42
43 if (repo_get_oid(repo, name, &oid))
42 - return NULL;
44 + die(_("'%s' is not a valid commit-ish for %s"), name, mode);
45 obj = parse_object(repo, &oid);
46 return (struct commit *)repo_peel_to_type(repo, name, 0, obj,
47 OBJ_COMMIT);
@@ -178,7 +180,7 @@ static void set_up_replay_mode(struct repository *repo,
180 die_for_incompatible_opt2(!!onto_name, "--onto",
181 !!*advance_name, "--advance");
182 if (onto_name) {
181 - *onto = peel_committish(repo, onto_name);
183 + *onto = peel_committish(repo, onto_name, "--onto");
184 if (rinfo.positive_refexprs <
185 strset_get_size(&rinfo.positive_refs))
186 die(_("all positive revisions given must be references"));
@@ -199,7 +201,7 @@ static void set_up_replay_mode(struct repository *repo,
201 } else {
202 die(_("argument to --advance must be a reference"));
203 }
202 - *onto = peel_committish(repo, *advance_name);
204 + *onto = peel_committish(repo, *advance_name, "--advance");
205 if (rinfo.positive_refexprs > 1)
206 die(_("cannot advance target with multiple sources because ordering would be ill-defined"));
207 }
@@ -416,8 +418,7 @@ int cmd_replay(int argc,
418 onto_name, &advance_name,
419 &onto, &update_refs);
420
419 - if (!onto) /* FIXME: Should handle replaying down to root commit */
420 - die("Replaying down to root commit is not supported yet!");
421 + /* FIXME: Should handle replaying down to root commit */
422
423 /* Build reflog message */
424 if (advance_name_opt)
t/t3650-replay-basics.sh
+7
@@ -58,6 +58,13 @@ test_expect_success 'argument to --advance must be a reference' '
58 test_cmp expect actual
59 '
60
61 +test_expect_success '--onto with invalid commit-ish' '
62 + printf "fatal: ${SQ}refs/not-valid${SQ} is not " >expect &&
63 + printf "a valid commit-ish for --onto\n" >>expect &&
64 + test_must_fail git replay --onto=refs/not-valid topic1..topic2 2>actual &&
65 + test_cmp expect actual
66 +'
67 +
68 test_expect_success 'using replay to rebase two branches, one on top of other' '
69 git replay --ref-action=print --onto main topic1..topic2 >result &&
70