kernel
/
git
mirror of https://git.kernel.org/pub/scm/git/git.git
1
0
Fork 0
Code Releases Activity

Merge branch 'pw/rebase-drop-notes-with-commit' into jch 

The rebase post-rewrite notes-copying logic has been corrected. When a
commit is dropped during rebase (e.g., because its changes are already
upstream), we no longer record it as rewritten, preventing its notes
from being copied to an unrelated commit.

* pw/rebase-drop-notes-with-commit:
  amend! sequencer: simplify pick_one_commit()
  amend! sequencer: remove unnecessary "or" in pick_one_commit()
  fixup! sequencer: never reschedule on failed commit
  fixup! sequencer: be more careful with external merge
  sequencer: do not record dropped commits as rewritten
  sequencer: use an enum to represent result of picking a commit
  sequencer: return early from pick_one_commit() on success
  sequencer: simplify pick_one_commit()
  sequencer: remove unnecessary condition in pick_one_commit()
  sequencer: simplify handing of fixup with conflicts
  sequencer: remove unnecessary "or" in pick_one_commit()
  sequencer: never reschedule on failed commit
  sequencer: be more careful with external merge
  sequencer: move definition of is_final_fixup()
  t3400: restore coverage for note copying with apply backend
jch
Junio C Hamano 2026-07-01 10:48:38 -07:00
parent f3ffec5adb 20a5c340cd
commit fae0b71834
4 changed files with 155 additions and 49 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

154
sequencer.c
View File

@ -2260,10 +2260,17 @@ static const char *reflog_message(struct replay_opts *opts,
return buf.buf;
}

static int do_pick_commit(struct repository *r,
struct todo_item *item,
struct replay_opts *opts,
int final_fixup, int *check_todo)
enum pick_result {
PICK_RESULT_ERROR = -1,
PICK_RESULT_OK,
PICK_RESULT_CONFLICTS,
PICK_RESULT_DROPPED,
};

static enum pick_result do_pick_commit(struct repository *r,
struct todo_item *item,
struct replay_opts *opts,
int final_fixup, int *check_todo)
{
struct replay_ctx *ctx = opts->ctx;
unsigned int flags = should_edit(opts) ? EDIT_MSG : 0;
@ -2273,7 +2280,7 @@ static int do_pick_commit(struct repository *r,
const char *base_label, *next_label, *reflog_action;
char *author = NULL;
struct commit_message msg = { NULL, NULL, NULL, NULL };
int res, unborn = 0, reword = 0, allow, drop_commit;
int res, unborn = 0, reword = 0, allow, drop_commit = 0;
enum todo_command command = item->command;
struct commit *commit = item->commit;

@ -2453,14 +2460,25 @@ static int do_pick_commit(struct repository *r,
struct commit_list *common = NULL;
struct commit_list *remotes = NULL;

res = write_message(ctx->message.buf, ctx->message.len,
git_path_merge_msg(r), 0);
if (write_message(ctx->message.buf, ctx->message.len,
git_path_merge_msg(r), 0)) {
res = -1;
goto leave;
}

commit_list_insert(base, &common);
commit_list_insert(next, &remotes);
res |= try_merge_command(r, opts->strategy,
opts->xopts.nr, opts->xopts.v,
res = try_merge_command(r, opts->strategy,
opts->xopts.nr, opts->xopts.v,
common, oid_to_hex(&head), remotes);
/*
* If there were conflicts, try_merge_command() returns 1,
* any other no-zero return code means that either the merge
* command could not be run, or it failed to merge.
*/
if (res && res != 1)
res = -1;

commit_list_free(common);
commit_list_free(remotes);
}
@ -2492,7 +2510,6 @@ static int do_pick_commit(struct repository *r,
goto leave;
}

drop_commit = 0;
allow = allow_empty(r, opts, commit);
if (allow < 0) {
res = allow;
@ -2531,6 +2548,12 @@ fast_forward_edit:
res = run_git_commit(NULL, reflog_action, opts, flags);
*check_todo = 1;
}
/*
* If "git commit" failed to run then res == -1, but we don't
* want reschedule the last command because the picking the
* commit was successful.
*/
res = !!res;
}


@ -2547,7 +2570,14 @@ leave:
free(author);
update_abort_safety_file();

return res;
if (res < 0)
return PICK_RESULT_ERROR;
else if (res > 0)
return PICK_RESULT_CONFLICTS;
else if (drop_commit)
return PICK_RESULT_DROPPED;
else
return PICK_RESULT_OK;
}

static int prepare_revs(struct replay_opts *opts)
@ -3872,7 +3902,7 @@ static int error_failed_squash(struct repository *r,
return error(_("could not copy '%s' to '%s'"),
rebase_path_message(),
git_path_merge_msg(r));
return error_with_patch(r, commit, subject, subject_len, opts, 1, 0);
return error_with_patch(r, commit, subject, subject_len, opts, 1, 1);
}

static int do_exec(struct repository *r, const char *command_line, int quiet)
@ -4644,21 +4674,6 @@ static int do_update_refs(struct repository *r, int quiet)
return res;
}

static int is_final_fixup(struct todo_list *todo_list)
{
int i = todo_list->current;

if (!is_fixup(todo_list->items[i].command))
return 0;

while (++i < todo_list->nr)
if (is_fixup(todo_list->items[i].command))
return 0;
else if (!is_noop(todo_list->items[i].command))
break;
return 1;
}

static enum todo_command peek_command(struct todo_list *todo_list, int offset)
{
int i;
@ -4942,6 +4957,21 @@ static int reread_todo_if_changed(struct repository *r,
return 0;
}

static int is_final_fixup(struct todo_list *todo_list)
{
int i = todo_list->current;

if (!is_fixup(todo_list->items[i].command))
return 0;

while (++i < todo_list->nr)
if (is_fixup(todo_list->items[i].command))
return 0;
else if (!is_noop(todo_list->items[i].command))
break;
return 1;
}

static const char rescheduled_advice[] =
N_("Could not execute the todo command\n"
"\n"
@ -4958,37 +4988,59 @@ static int pick_one_commit(struct repository *r,
struct replay_opts *opts,
int *check_todo, int* reschedule)
{
int res;
enum pick_result pick_res;
struct todo_item *item = todo_list->items + todo_list->current;
const char *arg = todo_item_get_arg(todo_list, item);

res = do_pick_commit(r, item, opts, is_final_fixup(todo_list),
check_todo);
if (is_rebase_i(opts) && res < 0) {
pick_res = do_pick_commit(r, item, opts, is_final_fixup(todo_list),
check_todo);
if (!is_rebase_i(opts))
switch (pick_res) {
case PICK_RESULT_ERROR:
return -1;
case PICK_RESULT_CONFLICTS:
return 1;
default:
return 0;
}

if (pick_res == PICK_RESULT_ERROR) {
/* Reschedule */
*reschedule = 1;
return -1;
}
if (item->command == TODO_EDIT) {
} else if (item->command == TODO_EDIT) {
struct commit *commit = item->commit;
if (!res) {
int res = pick_res == PICK_RESULT_CONFLICTS;
int to_amend = pick_res != PICK_RESULT_CONFLICTS &&
pick_res != PICK_RESULT_DROPPED;

/*
* NEEDSWORK: Do not record the commit as rewritten when
* continuing if it was dropped. Does it even make sense
* to stop if the commit was dropped?
*/
if (pick_res == PICK_RESULT_OK ||
pick_res == PICK_RESULT_DROPPED) {
if (!opts->verbose)
term_clear_line();
fprintf(stderr, _("Stopped at %s... %.*s\n"),
short_commit_name(r, commit), item->arg_len, arg);
}
return error_with_patch(r, commit,
arg, item->arg_len, opts, res, !res);
}
if (is_rebase_i(opts) && !res)
return error_with_patch(r, commit, arg, item->arg_len, opts,
res, to_amend);
} else if (pick_res == PICK_RESULT_OK) {
record_in_rewritten(&item->commit->object.oid,
peek_command(todo_list, 1));
if (res && is_fixup(item->command)) {
if (res == 1)
intend_to_amend();
return 0;
} else if (pick_res == PICK_RESULT_DROPPED) {
if (is_final_fixup(todo_list))
flush_rewritten_pending();
return 0;
} else if (pick_res == PICK_RESULT_CONFLICTS &&
is_fixup(item->command)) {
return error_failed_squash(r, item->commit, opts,
item->arg_len, arg);
} else if (res && is_rebase_i(opts) && item->commit) {
} else if (pick_res == PICK_RESULT_CONFLICTS) {
int to_amend = 0;
struct object_id oid;

@ -5005,11 +5057,11 @@ static int pick_one_commit(struct repository *r,
oideq(&opts->squash_onto, &oid))))
to_amend = 1;

return res | error_with_patch(r, item->commit,
arg, item->arg_len, opts,
res, to_amend);
return error_with_patch(r, item->commit, arg, item->arg_len,
opts, 1, to_amend);
}
return res;

BUG("Unhandled return value from do_pick_commit()");
}

static int pick_commits(struct repository *r,
@ -5545,7 +5597,15 @@ static int single_pick(struct repository *r,
TODO_PICK : TODO_REVERT;
item.commit = cmit;

return do_pick_commit(r, &item, opts, 0, &check_todo);
switch (do_pick_commit(r, &item, opts, 0, &check_todo)) {
case PICK_RESULT_ERROR:
return -1;
case PICK_RESULT_CONFLICTS:
return 1;
default:
return 0;
}

}

int sequencer_pick_revisions(struct repository *r,

16
t/t3400-rebase.sh
View File

@ -270,12 +270,24 @@ test_expect_success 'rebase can copy notes' '
test "a note" = "$(git notes show HEAD)"
'

test_expect_success 'rebase -m can copy notes' '
test_expect_success 'rebase --apply can copy notes' '
git reset --hard n3 &&
git rebase -m --onto n1 n2 &&
git rebase --apply --onto n1 n2 &&
test "a note" = "$(git notes show HEAD)"
'

test_expect_success 'rebase drops notes of dropped commits' '
git checkout n1 &&
echo n3 >n3.t &&
echo n4 >n4.t &&
git add n3.t n4.t &&
git commit -m n34 &&
git rebase HEAD n3 &&
test_commit_message HEAD -m n2 &&
test_must_fail git notes list HEAD >actual &&
test_must_be_empty actual
'

test_expect_success 'rebase commit with an ancient timestamp' '
git reset --hard &&


11
t/t3404-rebase-interactive.sh
View File

@ -1251,6 +1251,17 @@ test_expect_success 'interrupted rebase -i with --strategy and -X' '
test $(cat file1) = Z
'

test_expect_success 'failing pick with --strategy is rescheduled' '
test_when_finished "rm -rf bin; test_might_fail git rebase --abort" &&
mkdir bin &&
echo exit 2 | write_script bin/git-merge-fail &&
git log -1 --format="pick %H # %s" HEAD >expect &&
test_must_fail env PATH="$PWD/bin:$PATH" \
git rebase --no-ff --strategy fail HEAD^ &&
test_cmp expect .git/rebase-merge/git-rebase-todo &&
test_cmp expect .git/rebase-merge/done
'

test_expect_success 'rebase -i error on commits with \ in message' '
current_head=$(git rev-parse HEAD) &&
test_when_finished "git rebase --abort; git reset --hard $current_head; rm -f error" &&

23
t/t5407-post-rewrite-hook.sh
View File

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

test_expect_success 'rebase with commits that become empty' '
cat >todo <<-\EOF &&
pick H
pick E
fixup I
fixup H
pick G
pick I
EOF
(
set_replace_editor todo &&
git rebase -i --empty=drop A A
) &&
echo rebase >expected.args &&
cat >expected.data <<-EOF &&
$(git rev-parse H) $(git rev-parse HEAD~2)
$(git rev-parse E) $(git rev-parse HEAD~1)
$(git rev-parse I) $(git rev-parse HEAD~1)
$(git rev-parse G) $(git rev-parse HEAD)
EOF
verify_hook_input
'

test_done