[PATCH v2 2/5] replay: find *onto only after testing for ref name

From: kristofferhaugsbakk@fastmail.com <kristofferhaugsbakk@fastmail.com>
Date: Dec 30, 2025, 15:01 UTC
Message-ID: <V2_~axonto_after_ref_test.17d@msgid.xyz>
In-Reply-To: <V2_CV_replay_die_descr.17b@msgid.xyz>

From: Kristoffer Haugsbakk <code@khaugsbakk.name>

We are about to make `peel_committish` die when it cannot find a commit-ish instead of returning `NULL`. But that would make e.g. `git replay --advance=refs/non-existent` die with a less descriptive error message; the highest-level error message is that the name does not exist as a ref, not that we cannot find a commit-ish based on the name.

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 modifications.

Suggested-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
---

Notes (series):
    v2: [new]
    
    Fallout of v1. Needs to be moved so that the new error message does not
    “shadow” this one.
    
    See: https://lore.kernel.org/git/xmqqpl85pb7k.fsf@gitster.g/

 builtin/replay.c         | 2 +-
 t/t3650-replay-basics.sh | 7 +++++++
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/builtin/replay.c b/builtin/replay.c
index 54849f65c87..35813140e99 100644
--- a/builtin/replay.c
+++ b/builtin/replay.c
@@ -184,18 +184,18 @@ static void populate_for_onto_or_advance_mode(struct repository *repo,
 		char *fullname = NULL;
 
 		if (!*advance_name)
 			BUG("expected either onto_name or *advance_name in this function");
 
-		*onto = peel_committish(repo, *advance_name);
 		if (repo_dwim_ref(repo, *advance_name, strlen(*advance_name),
 			     &oid, &fullname, 0) == 1) {
 			free(*advance_name);
 			*advance_name = fullname;
 		} else {
 			die(_("argument to --advance must be a reference"));
 		}
+		*onto = peel_committish(repo, *advance_name);
 		if (rinfo.positive_refexprs > 1)
 			die(_("cannot advance target with multiple sources because ordering would be ill-defined"));
 	}
 	strset_clear(&rinfo.negative_refs);
 	strset_clear(&rinfo.positive_refs);
diff --git a/t/t3650-replay-basics.sh b/t/t3650-replay-basics.sh
index 58b37599357..7dea62f064f 100755
--- a/t/t3650-replay-basics.sh
+++ b/t/t3650-replay-basics.sh
@@ -49,10 +49,17 @@ test_expect_success 'setup' '
 
 test_expect_success 'setup bare' '
 	git clone --bare . bare
 '
 
+test_expect_success 'argument to --advance must be a reference' '
+	echo "fatal: argument to --advance must be a reference" >expect &&
+	oid=$(git rev-parse main) &&
+	test_must_fail git replay --advance=$oid topic1..topic2 2>actual &&
+	test_cmp expect actual
+'
+
 test_expect_success 'using replay to rebase two branches, one on top of other' '
 	git replay --onto main topic1..topic2 >result &&
 
 	test_line_count = 1 result &&

-- 
2.52.0.10.g08704017180