@matheusbsbs / matheus-bagliano / commits / 42554b78fd

sequencer: do not record dropped commits as rewritten

If a commit gets dropped because its changes are already upstream then we should not record it as rewritten. As well as confusing any post-rewrite hooks, it means we end up copying the notes from the dropped commit to the commit that was picked immediately before the one that was dropped. While we do not want to record the dropped commit as rewritten, if it is the final commit in a chain of fixups then we need to flush the list of rewritten commits. The behavior of an "edit" command where the commit is dropped is changed so that "rebase --continue" will not amend the previous pick. However, as the code comment notes it will still be erroneously recorded as rewritten when the rebase continues. That will need to be addressed separately along with not recording skipped commits as rewritten. The initialization of "drop_commit" is moved to ensure it is initialized when rewording a fast-forwarded commit. Reported-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com> Tested-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Phillip Wood committed Jul 15, 2026 at 16:22 UTC 42554b78fd2c3ce252647c9c5afbf04d2f2885f5

3 files changed +54 -5

sequencer.c

+19 -5

@@ -2264,6 +2264,7 @@ enum pick_result {

2264 PICK_RESULT_ERROR = -1,

2265 PICK_RESULT_OK,

2266 PICK_RESULT_CONFLICTS,

2267 + PICK_RESULT_DROPPED,

2268 };

2269

2270 static enum pick_result do_pick_commit(struct repository *r,

@@ -2279,7 +2280,7 @@ static enum pick_result do_pick_commit(struct repository *r,

2280 const char *base_label, *next_label, *reflog_action;

2281 char *author = NULL;

2282 struct commit_message msg = { NULL, NULL, NULL, NULL };

2282 - int res, unborn = 0, reword = 0, allow, drop_commit;

2283 + int res, unborn = 0, reword = 0, allow, drop_commit = 0;

2284 enum todo_command command = item->command;

2285 struct commit *commit = item->commit;

2286

@@ -2509,7 +2510,6 @@ static enum pick_result do_pick_commit(struct repository *r,

2510 goto leave;

2511 }

2512

2512 - drop_commit = 0;

2513 allow = allow_empty(r, opts, commit);

2514 if (allow < 0) {

2515 res = allow;

@@ -2574,6 +2574,8 @@ leave:

2574 return PICK_RESULT_ERROR;

2575 else if (res > 0)

2576 return PICK_RESULT_CONFLICTS;

2577 + else if (drop_commit)

2578 + return PICK_RESULT_DROPPED;

2579 else

2580 return PICK_RESULT_OK;

2581 }

@@ -4994,19 +4996,31 @@ static int pick_one_commit(struct repository *r,

4996 } else if (item->command == TODO_EDIT) {

4997 struct commit *commit = item->commit;

4998 int res = pick_res == PICK_RESULT_CONFLICTS;

4999 + int to_amend = pick_res != PICK_RESULT_CONFLICTS &&

5000 + pick_res != PICK_RESULT_DROPPED;

5001

4998 - if (pick_res == PICK_RESULT_OK) {

5002 + /*

5003 + * NEEDSWORK: Do not record the commit as rewritten when

5004 + * continuing if it was dropped. Does it even make sense

5005 + * to stop if the commit was dropped?

5006 + */

5007 + if (pick_res == PICK_RESULT_OK ||

5008 + pick_res == PICK_RESULT_DROPPED) {

5009 if (!opts->verbose)

5010 term_clear_line();

5011 fprintf(stderr, _("Stopped at %s... %.*s\n"),

5012 short_commit_name(r, commit), item->arg_len, arg);

5013 }

5004 - return error_with_patch(r, commit,

5005 - arg, item->arg_len, opts, res, !res);

5014 + return error_with_patch(r, commit, arg, item->arg_len, opts,

5015 + res, to_amend);

5016 } else if (pick_res == PICK_RESULT_OK) {

5017 record_in_rewritten(&item->commit->object.oid,

5018 peek_command(todo_list, 1));

5019 return 0;

5020 + } else if (pick_res == PICK_RESULT_DROPPED) {

5021 + if (is_final_fixup(todo_list))

5022 + flush_rewritten_pending();

5023 + return 0;

5024 } else if (pick_res == PICK_RESULT_CONFLICTS &&

5025 is_fixup(item->command)) {

5026 return error_failed_squash(r, item->commit, opts,

t/t3400-rebase.sh

+12

@@ -276,6 +276,18 @@ test_expect_success 'rebase --apply can copy notes' '

276 test "a note" = "$(git notes show HEAD)"

277 '

278

279 +test_expect_success 'rebase drops notes of dropped commits' '

280 + git checkout n1 &&

281 + echo n3 >n3.t &&

282 + echo n4 >n4.t &&

283 + git add n3.t n4.t &&

284 + git commit -m n34 &&

285 + git rebase HEAD n3 &&

286 + test_commit_message HEAD -m n2 &&

287 + test_must_fail git notes list HEAD >actual &&

288 + test_must_be_empty actual

289 +'

290 +

291 test_expect_success 'rebase commit with an ancient timestamp' '

292 git reset --hard &&

293

t/t5407-post-rewrite-hook.sh

+23

@@ -310,4 +310,27 @@ test_expect_success 'git rebase -i (exec)' '

310 verify_hook_input

311 '

312

313 +test_expect_success 'rebase with commits that become empty' '

314 + cat >todo <<-\EOF &&

315 + pick H

316 + pick E

317 + fixup I

318 + fixup H

319 + pick G

320 + pick I

321 + EOF

322 + (

323 + set_replace_editor todo &&

324 + git rebase -i --empty=drop A A

325 + ) &&

326 + echo rebase >expected.args &&

327 + cat >expected.data <<-EOF &&

328 + $(git rev-parse H) $(git rev-parse HEAD~2)

329 + $(git rev-parse E) $(git rev-parse HEAD~1)

330 + $(git rev-parse I) $(git rev-parse HEAD~1)

331 + $(git rev-parse G) $(git rev-parse HEAD)

332 + EOF

333 + verify_hook_input

334 +'

335 +

336 test_done