sequencer: avoid dropping fixup commit that targets self via commit-ish

Commit 68d5d03bc4 (rebase: teach --autosquash to match on sha1 in addition to message, 2010-11-04) taught autosquash to recognize subjects like "fixup! 7a235b" where 7a235b is an OID-prefix. It actually did more than advertised: 7a235b can be an arbitrary commit-ish (as long as it's not trailed by spaces). Accidental(?) use of this secret feature revealed a bug where we would silently drop a fixup commit. The bug can also be triggered when using an OID-prefix but that's unlikely in practice. Let the commit with subject "fixup! main" be the tip of the "main" branch. When computing the fixup target for this commit, we find the commit itself. This is wrong because, by definition, a fixup target must be an earlier commit in the todo list. We wrongly find the current commit because we added it to the todo list prematurely. Avoid these fixup-cycles by only adding the current commit to the todo list after we have finished looking for the fixup target. Reported-by: Erik Cervin Edin <erik@cervined.in> Signed-off-by: Johannes Altmanninger <aclopte@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Johannes Altmanninger committed Sep 24, 2022 at 17:29 UTC 3e367a5f2f53f19130b90a97c3e2668eca3461ae
2 files changed +15 -2
sequencer.c
+2 -2
@@ -5774,8 +5774,6 @@ int todo_list_rearrange_squash(struct todo_list *todo_list)
5774 return error(_("the script was already rearranged."));
5775 }
5776
5777 - *commit_todo_item_at(&commit_todo, item->commit) = item;
5778 -
5777 parse_commit(item->commit);
5778 commit_buffer = logmsg_reencode(item->commit, NULL, "UTF-8");
5779 find_commit_subject(commit_buffer, &subject);
@@ -5842,6 +5840,8 @@ int todo_list_rearrange_squash(struct todo_list *todo_list)
5840 strhash(entry->subject));
5841 hashmap_put(&subject2item, &entry->entry);
5842 }
5843 +
5844 + *commit_todo_item_at(&commit_todo, item->commit) = item;
5845 }
5846
5847 if (rearranged) {
t/t3415-rebase-autosquash.sh
+13
@@ -232,6 +232,19 @@ test_expect_success 'auto squash that matches longer sha1' '
232 test_line_count = 1 actual
233 '
234
235 +test_expect_success 'auto squash of fixup commit that matches branch name which points back to fixup commit' '
236 + git reset --hard base &&
237 + git commit --allow-empty -m "fixup! self-cycle" &&
238 + git branch self-cycle &&
239 + GIT_SEQUENCE_EDITOR="cat >tmp" git rebase --autosquash -i HEAD^^ &&
240 + sed -ne "/^[^#]/{s/[0-9a-f]\{7,\}/HASH/g;p;}" tmp >actual &&
241 + cat <<-EOF >expect &&
242 + pick HASH second commit
243 + pick HASH fixup! self-cycle # empty
244 + EOF
245 + test_cmp expect actual
246 +'
247 +
248 test_auto_commit_flags () {
249 git reset --hard base &&
250 echo 1 >file1 &&