# [PATCH 0/2] rebase: stricter label lookups 6 messages from 2022-11-10 to 2022-11-21. Participants: Phillip Wood via GitGitGadget, Taylor Blau, Johannes Schindelin, Phillip Wood. Thread: https://gitlist.dev/t/58784 ## Phillip Wood via GitGitGadget, 2022-11-10 16:43 Subject: [PATCH 0/2] rebase: stricter label lookups Message-ID: URL: https://gitlist.dev/e/pull.1414.git.1668098622.gitgitgadget%40gmail.com ``` If a rebase label does not exist and a branch or tag named refs/rewritten/ does exist it will use that instead of returning an error. Phillip Wood (2): sequencer: unify label lookup sequencer: tighten label lookups sequencer.c | 53 +++++++++++++++++++++------------------- t/t3430-rebase-merges.sh | 17 +++++++++++++ 2 files changed, 45 insertions(+), 25 deletions(-) base-commit: 3b08839926fcc7cc48cf4c759737c1a71af430c1 Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1414%2Fphillipwood%2Fsequencer-label-read-refs-v1 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1414/phillipwood/sequencer-label-read-refs-v1 Pull-Request: https://github.com/gitgitgadget/git/pull/1414 -- gitgitgadget ``` ## Phillip Wood via GitGitGadget, 2022-11-10 16:43 Subject: [PATCH 1/2] sequencer: unify label lookup Message-ID: URL: https://gitlist.dev/e/ecb81b873a8d72c57fd4812ec86d103927437cf1.1668098622.git.gitgitgadget%40gmail.com In-Reply-To: ``` From: Phillip Wood The arguments to the `reset` and `merge` commands may be a label created with a `label` command or an arbitrary commit name. The `merge` command uses the lookup_label() function to lookup its arguments but `reset` has a slightly different version of that function in do_reset(). Reduce this code duplication by calling lookup_label() from do_reset() as well. This change improves the behavior of `reset` when the argument is a tree. Previously `reset` would accept a tree only for the rebase to fail with update_ref failed for ref 'HEAD': cannot update ref 'HEAD': trying to write non-commit object da5497437fd67ca928333aab79c4b4b55036ea66 to branch 'HEAD' Using lookup_label() means do_reset() will now error out straight away if its argument is not a commit. Signed-off-by: Phillip Wood --- sequencer.c | 49 ++++++++++++++++++++-------------------- t/t3430-rebase-merges.sh | 8 +++++++ 2 files changed, 33 insertions(+), 24 deletions(-) diff --git a/sequencer.c b/sequencer.c index e658df7e8ff..21f5032df0d 100644 --- a/sequencer.c +++ b/sequencer.c @@ -3696,6 +3696,26 @@ 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) +{ + struct commit *commit; + + strbuf_reset(buf); + strbuf_addf(buf, "refs/rewritten/%.*s", len, label); + commit = lookup_commit_reference_by_name(buf->buf); + if (!commit) { + /* fall back to non-rewritten ref or commit */ + strbuf_splice(buf, 0, strlen("refs/rewritten/"), "", 0); + commit = lookup_commit_reference_by_name(buf->buf); + } + + if (!commit) + error(_("could not resolve '%s'"), buf->buf); + + return commit; +} + static int do_reset(struct repository *r, const char *name, int len, struct replay_opts *opts) @@ -3727,6 +3747,7 @@ static int do_reset(struct repository *r, oidcpy(&oid, &opts->squash_onto); } else { int i; + struct commit *commit; /* Determine the length of the label */ for (i = 0; i < len; i++) @@ -3734,12 +3755,12 @@ static int do_reset(struct repository *r, break; len = i; - strbuf_addf(&ref_name, "refs/rewritten/%.*s", len, name); - if (get_oid(ref_name.buf, &oid) && - get_oid(ref_name.buf + strlen("refs/rewritten/"), &oid)) { - ret = error(_("could not read '%s'"), ref_name.buf); + commit = lookup_label(name, len, &ref_name); + if (!commit) { + ret = -1; goto cleanup; } + oid = commit->object.oid; } setup_unpack_trees_porcelain(&unpack_tree_opts, "reset"); @@ -3786,26 +3807,6 @@ cleanup: return ret; } -static struct commit *lookup_label(const char *label, int len, - struct strbuf *buf) -{ - struct commit *commit; - - strbuf_reset(buf); - strbuf_addf(buf, "refs/rewritten/%.*s", len, label); - commit = lookup_commit_reference_by_name(buf->buf); - if (!commit) { - /* fall back to non-rewritten ref or commit */ - strbuf_splice(buf, 0, strlen("refs/rewritten/"), "", 0); - commit = lookup_commit_reference_by_name(buf->buf); - } - - if (!commit) - error(_("could not resolve '%s'"), buf->buf); - - return commit; -} - static int do_merge(struct repository *r, struct commit *commit, const char *arg, int arg_len, diff --git a/t/t3430-rebase-merges.sh b/t/t3430-rebase-merges.sh index f351701fec2..fbbc4439bfe 100755 --- a/t/t3430-rebase-merges.sh +++ b/t/t3430-rebase-merges.sh @@ -138,6 +138,14 @@ test_expect_success '`reset` refuses to overwrite untracked files' ' git rebase --abort ' +test_expect_success '`reset` rejects trees' ' + test_when_finished "test_might_fail git rebase --abort" && + test_must_fail env GIT_SEQUENCE_EDITOR="echo reset A^{tree} >" \ + git rebase -i B C >out 2>err && + grep "object .* is a tree" 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 ``` ## Phillip Wood via GitGitGadget, 2022-11-10 16:43 Subject: [PATCH 2/2] sequencer: tighten label lookups Message-ID: URL: https://gitlist.dev/e/d3a7ed20b9282348a9ebc2490ca70c9e89cf324b.1668098622.git.gitgitgadget%40gmail.com In-Reply-To: ``` From: Phillip Wood The `label` command creates a ref refs/rewritten/