[PATCH 2/2] sequencer: tighten label lookups
- From
- Phillip Wood via GitGitGadget <gitgitgadget@gmail.com>
- Date
- Nov 10, 2022, 16:43 UTC
- Message-ID
- <d3a7ed20b9282348a9ebc2490ca70c9e89cf324b.1668098622.git.gitgitgadget@gmail.com>
- In-Reply-To
- <pull.1414.git.1668098622.gitgitgadget@gmail.com>
From: Phillip Wood <phillip.wood@dunelm.org.uk>
The `label` command creates a ref refs/rewritten/<label> that the `reset` and `merge` commands resolve by calling lookup_label(). That uses lookup_commit_reference_by_name() to look up the label ref. As lookup_commit_reference_by_name() uses the dwim rules when looking up the label it will look for a branch named refs/heads/refs/rewritten/<label> and return that instead of an error if the branch exists and the label does not. Fix this by using read_ref() followed by lookup_commit_object() when looking up labels.
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> --- sequencer.c | 14 ++++++++------ t/t3430-rebase-merges.sh | 9 +++++++++ 2 files changed, 17 insertions(+), 6 deletions(-)
diff --git a/sequencer.c b/sequencer.c index 21f5032df0d..927da04e709 100644 --- a/sequencer.c +++ b/sequencer.c @@ -3696,15 +3696,17 @@ static const char *reflog_message(struct replay_opts *opts, return buf.buf; } -static struct commit *lookup_label(const char *label, int len, - struct strbuf *buf) +static struct commit *lookup_label(struct repository *r, const char *label, + int len, struct strbuf *buf) { struct commit *commit; + struct object_id oid; strbuf_reset(buf); strbuf_addf(buf, "refs/rewritten/%.*s", len, label); - commit = lookup_commit_reference_by_name(buf->buf); - if (!commit) { + if (!read_ref(buf->buf, &oid)) { + commit = lookup_commit_object(r, &oid); + } else { /* fall back to non-rewritten ref or commit */ strbuf_splice(buf, 0, strlen("refs/rewritten/"), "", 0); commit = lookup_commit_reference_by_name(buf->buf); @@ -3755,7 +3757,7 @@ static int do_reset(struct repository *r, break; len = i; - commit = lookup_label(name, len, &ref_name); + commit = lookup_label(r, name, len, &ref_name); if (!commit) { ret = -1; goto cleanup; @@ -3854,7 +3856,7 @@ static int do_merge(struct repository *r, k = strcspn(p, " \t\n"); if (!k) continue; - merge_commit = lookup_label(p, k, &ref_name); + merge_commit = lookup_label(r, p, k, &ref_name); if (!merge_commit) { ret = error(_("unable to parse '%.*s'"), k, p); goto leave_merge; diff --git a/t/t3430-rebase-merges.sh b/t/t3430-rebase-merges.sh index fbbc4439bfe..fa2a06c19f0 100755 --- a/t/t3430-rebase-merges.sh +++ b/t/t3430-rebase-merges.sh @@ -146,6 +146,15 @@ test_expect_success '`reset` rejects trees' ' test_must_be_empty out ' +test_expect_success '`reset` only looks for labels under refs/rewritten/' ' + test_when_finished "test_might_fail git rebase --abort" && + git branch refs/rewritten/my-label A && + test_must_fail env GIT_SEQUENCE_EDITOR="echo reset my-label >" \ + git rebase -i B C >out 2>err && + grep "could not resolve ${SQ}my-label${SQ}" err && + test_must_be_empty out +' + test_expect_success 'failed `merge -C` writes patch (may be rescheduled, too)' ' test_when_finished "test_might_fail git rebase --abort" && git checkout -b conflicting-merge A &&
-- gitgitgadget