sequencer: tighten label lookups
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> Signed-off-by: Taylor Blau <me@ttaylorr.com>
Phillip Wood committed
Nov 10, 2022 at 16:43 UTC
688d82f254f8fc5da019fc4962e6e8646767aba0
2 files changed
+17
-6
sequencer.c
+8
-6
@@ -3694,15 +3694,17 @@ static const char *reflog_message(struct replay_opts *opts,
3694
return buf.buf;
3695
}
3696
3697
-static struct commit *lookup_label(const char *label, int len,
3698
- struct strbuf *buf)
3697
+static struct commit *lookup_label(struct repository *r, const char *label,
3698
+ int len, struct strbuf *buf)
3699
{
3700
struct commit *commit;
3701
+ struct object_id oid;
3702
3703
strbuf_reset(buf);
3704
strbuf_addf(buf, "refs/rewritten/%.*s", len, label);
3704
- commit = lookup_commit_reference_by_name(buf->buf);
3705
- if (!commit) {
3705
+ if (!read_ref(buf->buf, &oid)) {
3706
+ commit = lookup_commit_object(r, &oid);
3707
+ } else {
3708
/* fall back to non-rewritten ref or commit */
3709
strbuf_splice(buf, 0, strlen("refs/rewritten/"), "", 0);
3710
commit = lookup_commit_reference_by_name(buf->buf);
@@ -3753,7 +3755,7 @@ static int do_reset(struct repository *r,
3755
break;
3756
len = i;
3757
3756
- commit = lookup_label(name, len, &ref_name);
3758
+ commit = lookup_label(r, name, len, &ref_name);
3759
if (!commit) {
3760
ret = -1;
3761
goto cleanup;
@@ -3852,7 +3854,7 @@ static int do_merge(struct repository *r,
3854
k = strcspn(p, " \t\n");
3855
if (!k)
3856
continue;
3855
- merge_commit = lookup_label(p, k, &ref_name);
3857
+ merge_commit = lookup_label(r, p, k, &ref_name);
3858
if (!merge_commit) {
3859
ret = error(_("unable to parse '%.*s'"), k, p);
3860
goto leave_merge;
t/t3430-rebase-merges.sh
+9
@@ -146,6 +146,15 @@ test_expect_success '`reset` rejects trees' '
146
test_must_be_empty out
147
'
148
149
+test_expect_success '`reset` only looks for labels under refs/rewritten/' '
150
+ test_when_finished "test_might_fail git rebase --abort" &&
151
+ git branch refs/rewritten/my-label A &&
152
+ test_must_fail env GIT_SEQUENCE_EDITOR="echo reset my-label >" \
153
+ git rebase -i B C >out 2>err &&
154
+ grep "could not resolve ${SQ}my-label${SQ}" err &&
155
+ test_must_be_empty out
156
+'
157
+
158
test_expect_success 'failed `merge -C` writes patch (may be rescheduled, too)' '
159
test_when_finished "test_might_fail git rebase --abort" &&
160
git checkout -b conflicting-merge A &&