replay: find *onto only after testing for ref 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>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/replay.c b/builtin/replay.c
index 524bf96ffd6c9d59f2545114503bec511ce89538..9265ebcd05d569024e381a0f6726a5e08d010ae0 100644 (file)
--- a/builtin/replay.c
+++ b/builtin/replay.c
@@ -192,7 +192,6 @@ static void set_up_replay_mode(struct repository *repo,
                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);
@@ -200,6 +199,7 @@ static void set_up_replay_mode(struct repository *repo,
                } 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"));
        }

diff --git a/t/t3650-replay-basics.sh b/t/t3650-replay-basics.sh
index cf3aacf3551f8ee16803ae2a1ddbb7d9d6935980..8ef0b1984d73246f1eb875952d7259d51709f3cb 100755 (executable)
--- a/t/t3650-replay-basics.sh
+++ b/t/t3650-replay-basics.sh
@@ -51,6 +51,13 @@ 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 --ref-action=print --onto main topic1..topic2 >result &&