Subject: [PATCH v3 0/6] replay: die descriptively when invalid commit-ish

From: [email-masked]
Date: Mon, 5 Jan 2026 20:53:16 +0100

From: Kristoffer Haugsbakk <[email-masked]>

You get this error when you for example mistype the argument to
`--onto`:

    fatal: Replaying down to root commit is not supported yet!

Consider that you might not know yourself that you have mistyped
something; then this looks even more puzzling. You might have given a
range like `main..topic` but the command says that it would need to
replay down to the root commit.

The only thing that’s happened though is that `NULL` has been
interpreted in the wrong way.

Let’s instead die immediately when the real error happens, in other
words when we can’t find the commit for the given commit-ish.

Also:

• Add more regression tests
• Remove dead code
• Slightly more robust object parsing (parse_object_or_die)

§ Dropped section (see v1)

Somewhat unrelated to this change ...

§ Changes in v3

Apply review feedback from Elijah. See patches for details.

• Patch 1: More terse function name
• Patch 2: Improve commit message
• Patch 3: Improve commit message: fix outdated function name mention
• Patch 4: [new] Apply code comment/error message tweaks

§ Link to v2

https://lore.kernel.org/git/[email-masked]/

Kristoffer Haugsbakk (6):
  replay: remove dead code and rearrange
  replay: find *onto only after testing for ref name
  replay: die descriptively when invalid commit-ish is given
  replay: improve code comment and die message
  replay: die if we cannot parse object
  t3650: add more regression tests for failure conditions

 builtin/replay.c         | 87 ++++++++++++----------------------------
 t/t3650-replay-basics.sh | 54 +++++++++++++++++++++++++
 2 files changed, 79 insertions(+), 62 deletions(-)

Interdiff against v2:
diff --git a/builtin/replay.c b/builtin/replay.c
index ca5a14de4c7..dc46c921667 100644
--- a/builtin/replay.c
+++ b/builtin/replay.c
@@ -156,16 +156,16 @@ static void get_ref_information(struct repository *repo,
 
 		free(fullname);
 	}
 }
 
-static void populate_for_onto_or_advance_mode(struct repository *repo,
-					      struct rev_cmdline_info *cmd_info,
-					      const char *onto_name,
-					      char **advance_name,
-					      struct commit **onto,
-					      struct strset **update_refs)
+static void set_up_replay_mode(struct repository *repo,
+			       struct rev_cmdline_info *cmd_info,
+			       const char *onto_name,
+			       char **advance_name,
+			       struct commit **onto,
+			       struct strset **update_refs)
 {
 	struct ref_info rinfo;
 
 	get_ref_information(repo, cmd_info, &rinfo);
 	if (!rinfo.positive_refexprs)
@@ -347,13 +347,15 @@ int cmd_replay(int argc,
 			  "'%s' bit in 'struct rev_info' will be forced"),
 			"simplify_history");
 		revs.simplify_history = 0;
 	}
 
-	populate_for_onto_or_advance_mode(repo, &revs.cmdline,
-					  onto_name, &advance_name,
-					  &onto, &update_refs);
+	set_up_replay_mode(repo, &revs.cmdline,
+			   onto_name, &advance_name,
+			   &onto, &update_refs);
+
+	/* FIXME: Should allow replaying commits with the first as a root commit */
 
 	if (prepare_revision_walk(&revs) < 0) {
 		ret = error(_("error preparing revisions"));
 		goto cleanup;
 	}
@@ -366,12 +368,12 @@ int cmd_replay(int argc,
 	while ((commit = get_revision(&revs))) {
 		const struct name_decoration *decoration;
 		khint_t pos;
 		int hr;
 
-		if (!commit->parents) /* FIXME: Should handle replaying down to root commit */
-			die(_("replaying down to root commit is not supported yet!"));
+		if (!commit->parents)
+			die(_("replaying down from root commit is not supported yet!"));
 		if (commit->parents->next)
 			die(_("replaying merge commits is not supported yet!"));
 
 		last_commit = pick_regular_commit(repo, commit, replayed_commits,
 						  onto, &merge_opt, &result);
diff --git a/t/t3650-replay-basics.sh b/t/t3650-replay-basics.sh
index c0c59ae6938..d10c01506f1 100755
--- a/t/t3650-replay-basics.sh
+++ b/t/t3650-replay-basics.sh
@@ -78,11 +78,11 @@ test_expect_success 'option --onto or --advance is mandatory' '
 	test_must_fail git replay topic1..topic2 2>actual &&
 	test_cmp expect actual
 '
 
 test_expect_success 'no base or negative ref gives no-replaying down to root error' '
-	echo "fatal: replaying down to root commit is not supported yet!" >expect &&
+	echo "fatal: replaying down from root commit is not supported yet!" >expect &&
 	test_must_fail git replay --onto=topic1 topic2 2>actual &&
 	test_cmp expect actual
 '
 
 test_expect_success 'options --advance and --contained cannot be used together' '
Range-diff against v2:
1:  314ba49dd2f ! 1:  6d785c6b45f replay: remove dead code and rearrange
    @@ Commit message
     
             *advance_name = xstrdup_or_null(last_key);
     
    -    Let’s also rename the function since we do not determine
    -    the replay mode here. We simply populate data structures.
    +    Let’s also rename the function since we do not determine the
    +    replay mode here. We just set up `*onto` and refs to update.
     
         Note that there might be more dead code caused by this *guess mode*.
         We only concern ourselves with this function for now.
    @@ Commit message

      ## Notes (series) ##
    +    v3:
    +
    +    Use more terse function rename.[1] Also tweak commit message based on
    +    this change.
    +
    +    🔗 1: https://lore.kernel.org/git/[email-masked]/
    +
         v2: [new]
     
         See the link in the commit message.
    @@ builtin/replay.c: static void get_ref_information(struct repository *repo,
     -				  char **advance_name,
     -				  struct commit **onto,
     -				  struct strset **update_refs)
    -+static void populate_for_onto_or_advance_mode(struct repository *repo,
    -+					      struct rev_cmdline_info *cmd_info,
    -+					      const char *onto_name,
    -+					      char **advance_name,
    -+					      struct commit **onto,
    -+					      struct strset **update_refs)
    ++static void set_up_replay_mode(struct repository *repo,
    ++			       struct rev_cmdline_info *cmd_info,
    ++			       const char *onto_name,
    ++			       char **advance_name,
    ++			       struct commit **onto,
    ++			       struct strset **update_refs)
      {
      	struct ref_info rinfo;
      
    @@ builtin/replay.c: int cmd_replay(int argc,
      
     -	determine_replay_mode(repo, &revs.cmdline, onto_name, &advance_name,
     -			      &onto, &update_refs);
    -+	populate_for_onto_or_advance_mode(repo, &revs.cmdline,
    -+					  onto_name, &advance_name,
    -+					  &onto, &update_refs);
    ++	set_up_replay_mode(repo, &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!");
2:  976d336adef ! 2:  7f9aac28792 replay: find *onto only after testing for ref name
    @@ Commit message
         Let’s try to find the ref and only after that try to peel to
         as a commit-ish.
     
    -    Also add a regression test to protect this error-order from future
    +    Also add a regression test to protect this error order from future
         modifications.
     
         Suggested-by: Junio C Hamano <[email-masked]>
    @@ Commit message

      ## Notes (series) ##
    +    v3:
    +
    +    Don’t use a hyphen in “error-order” since that can be confusing.[1]
    +
    +    🔗 1: https://lore.kernel.org/git/[email-masked]/
    +
         v2: [new]
     
         Fallout of v1. Needs to be moved so that the new error message does not
    @@ Notes (series)
         See: https://lore.kernel.org/git/[email-masked]/
     
      ## builtin/replay.c ##
    -@@ builtin/replay.c: static void populate_for_onto_or_advance_mode(struct repository *repo,
    +@@ builtin/replay.c: static void set_up_replay_mode(struct repository *repo,
      		if (!*advance_name)
      			BUG("expected either onto_name or *advance_name in this function");
      
    @@ builtin/replay.c: static void populate_for_onto_or_advance_mode(struct repositor
      		if (repo_dwim_ref(repo, *advance_name, strlen(*advance_name),
      			     &oid, &fullname, 0) == 1) {
      			free(*advance_name);
    -@@ builtin/replay.c: static void populate_for_onto_or_advance_mode(struct repository *repo,
    +@@ builtin/replay.c: static void set_up_replay_mode(struct repository *repo,
      		} else {
      			die(_("argument to --advance must be a reference"));
      		}
3:  a7a7e1e720e ! 3:  88544dcad3e replay: die descriptively when invalid commit-ish is given
    @@ Commit message
     
         Going backwards from this point:
     
    -    1. `onto` is `NULL` from `determine_replay_mode`;
    +    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.
     
    @@ Commit message

      ## Notes (series) ##
    +    v3:
    +
    +    Update commit message since it uses the outdated function name.[1]
    +
    +    🔗 1: https://lore.kernel.org/git/CABPp-BH1b3rHi96qXLQwQRX6g7POmqYLKyAc=[email-masked]/#t
    +
         v2:
     
         Let’s use a slightly longer subject line in the commit message so that it
    @@ builtin/replay.c: static const char *short_commit_name(struct repository *repo,
      	obj = parse_object(repo, &oid);
      	return (struct commit *)repo_peel_to_type(repo, name, 0, obj,
      						  OBJ_COMMIT);
    -@@ builtin/replay.c: static void populate_for_onto_or_advance_mode(struct repository *repo,
    +@@ builtin/replay.c: static void set_up_replay_mode(struct repository *repo,
      	die_for_incompatible_opt2(!!onto_name, "--onto",
      				  !!*advance_name, "--advance");
      	if (onto_name) {
    @@ builtin/replay.c: static void populate_for_onto_or_advance_mode(struct repositor
      		if (rinfo.positive_refexprs <
      		    strset_get_size(&rinfo.positive_refs))
      			die(_("all positive revisions given must be references"));
    -@@ builtin/replay.c: static void populate_for_onto_or_advance_mode(struct repository *repo,
    +@@ builtin/replay.c: static void set_up_replay_mode(struct repository *repo,
      		} else {
      			die(_("argument to --advance must be a reference"));
      		}
    @@ builtin/replay.c: static void populate_for_onto_or_advance_mode(struct repositor
      			die(_("cannot advance target with multiple sources because ordering would be ill-defined"));
      	}
     @@ builtin/replay.c: int cmd_replay(int argc,
    - 					  onto_name, &advance_name,
    - 					  &onto, &update_refs);
    + 			   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!");
    --
    ++	/* FIXME: Should handle replaying down to root commit */
    + 
      	if (prepare_revision_walk(&revs) < 0) {
      		ret = error(_("error preparing revisions"));
    - 		goto cleanup;
    -@@ builtin/replay.c: int cmd_replay(int argc,
    - 		khint_t pos;
    - 		int hr;
    - 
    --		if (!commit->parents)
    -+		if (!commit->parents) /* FIXME: Should handle replaying down to root commit */
    - 			die(_("replaying down to root commit is not supported yet!"));
    - 		if (commit->parents->next)
    - 			die(_("replaying merge commits is not supported yet!"));
     
      ## t/t3650-replay-basics.sh ##
     @@ t/t3650-replay-basics.sh: test_expect_success 'argument to --advance must be a reference' '
-:  ----------- > 4:  2e149c41634 replay: improve code comment and die message
4:  9700630d95e = 5:  35cce78b469 replay: die if we cannot parse object
5:  5e1b9205df5 ! 6:  cd87a11f96b t3650: add more regression tests for failure conditions
    @@ t/t3650-replay-basics.sh: test_expect_success '--onto with invalid commit-ish' '
     +'
     +
     +test_expect_success 'no base or negative ref gives no-replaying down to root error' '
    -+	echo "fatal: replaying down to root commit is not supported yet!" >expect &&
    ++	echo "fatal: replaying down from root commit is not supported yet!" >expect &&
     +	test_must_fail git replay --onto=topic1 topic2 2>actual &&
     +	test_cmp expect actual
     +'

base-commit: 9a2fb147f2c61d0cab52c883e7e26f5b7948e3ed
-- 
2.52.0.383.gb1c58d6b301