Skip to content

rebase -i: always update HEAD before rewording #312

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 3 commits into from
Closed

rebase -i: always update HEAD before rewording #312

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6289396
rebase -i: always update HEAD before rewording
phillipwood Aug 15, 2019
7839a84
rebase -i: check for updated todo after squash and reword
phillipwood Aug 17, 2019
3c3b459
sequencer: simplify root commit creation
phillipwood Aug 13, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
130 changes: 39 additions & 91 deletions sequencer.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -869,34 +869,6 @@ static char *get_author(const char *message)
return NULL;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Eric Sunshine wrote (reply to this):

On Mon, Aug 19, 2019 at 5:18 AM Phillip Wood via GitGitGadget
<[email protected]> wrote:
> Adapt try_to_commit() to create a new root commit rather than special
> casing this in run_git_commit(). The significantly reduces the amount of

s/The/This/

> special case code for creating the root commit and reduces the number of
> commit code paths we have to worry about.
>
> Signed-off-by: Phillip Wood <[email protected]>

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Phillip Wood wrote (reply to this):

On 19/08/2019 17:09, Eric Sunshine wrote:
> On Mon, Aug 19, 2019 at 5:18 AM Phillip Wood via GitGitGadget
> <[email protected]> wrote:
>> Adapt try_to_commit() to create a new root commit rather than special
>> casing this in run_git_commit(). The significantly reduces the amount of
> 
> s/The/This/

Thanks Eric - well spotted as ever. Thanks Junio for fixing this up in pu.

Best Wishes

Phillip

>> special case code for creating the root commit and reduces the number of
>> commit code paths we have to worry about.
>>
>> Signed-off-by: Phillip Wood <[email protected]>

}

/* Read author-script and return an ident line (author <email> timestamp) */
static const char *read_author_ident(struct strbuf *buf)
{
struct strbuf out = STRBUF_INIT;
char *name, *email, *date;

if (read_author_script(rebase_path_author_script(),
&name, &email, &date, 0))
return NULL;

/* validate date since fmt_ident() will die() on bad value */
if (parse_date(date, &out)){
warning(_("invalid date format '%s' in '%s'"),
date, rebase_path_author_script());
strbuf_release(&out);
return NULL;
}

strbuf_reset(&out);
strbuf_addstr(&out, fmt_ident(name, email, WANT_AUTHOR_IDENT, date, 0));
strbuf_swap(buf, &out);
strbuf_release(&out);
free(name);
free(email);
free(date);
return buf->buf;
}

static const char staged_changes_advice[] =
N_("you have staged changes in your working tree\n"
"If these changes are meant to be squashed into the previous commit, run:\n"
Expand Down Expand Up @@ -954,47 +926,6 @@ static int run_git_commit(struct repository *r,
{
struct child_process cmd = CHILD_PROCESS_INIT;

if ((flags & CREATE_ROOT_COMMIT) && !(flags & AMEND_MSG)) {
struct strbuf msg = STRBUF_INIT, script = STRBUF_INIT;
const char *author = NULL;
struct object_id root_commit, *cache_tree_oid;
int res = 0;

if (is_rebase_i(opts)) {
author = read_author_ident(&script);
if (!author) {
strbuf_release(&script);
return -1;
}
}

if (!defmsg)
BUG("root commit without message");

if (!(cache_tree_oid = get_cache_tree_oid(r->index)))
res = -1;

if (!res)
res = strbuf_read_file(&msg, defmsg, 0);

if (res <= 0)
res = error_errno(_("could not read '%s'"), defmsg);
else
res = commit_tree(msg.buf, msg.len, cache_tree_oid,
NULL, &root_commit, author,
opts->gpg_sign);

strbuf_release(&msg);
strbuf_release(&script);
if (!res) {
update_ref(NULL, "CHERRY_PICK_HEAD", &root_commit, NULL,
REF_NO_DEREF, UPDATE_REFS_MSG_ON_ERR);
res = update_ref(NULL, "HEAD", &root_commit, NULL, 0,
UPDATE_REFS_MSG_ON_ERR);
}
return res < 0 ? error(_("writing root commit")) : 0;
}

cmd.git_cmd = 1;

if (is_rebase_i(opts) && read_env_script(&cmd.env_array)) {
Expand Down Expand Up @@ -1378,7 +1309,7 @@ static int try_to_commit(struct repository *r,
struct object_id *oid)
{
struct object_id tree;
struct commit *current_head;
struct commit *current_head = NULL;
struct commit_list *parents = NULL;
struct commit_extra_header *extra = NULL;
struct strbuf err = STRBUF_INIT;
Expand Down Expand Up @@ -1413,7 +1344,8 @@ static int try_to_commit(struct repository *r,
}
parents = copy_commit_list(current_head->parents);
extra = read_commit_extra_headers(current_head, exclude_gpgsig);
} else if (current_head) {
} else if (current_head &&
(!(flags & CREATE_ROOT_COMMIT) || (flags & AMEND_MSG))) {
commit_list_insert(current_head, &parents);
}

Expand Down Expand Up @@ -1490,8 +1422,7 @@ static int do_commit(struct repository *r,
{
int res = 1;

if (!(flags & EDIT_MSG) && !(flags & VERIFY_MSG) &&
!(flags & CREATE_ROOT_COMMIT)) {
if (!(flags & EDIT_MSG) && !(flags & VERIFY_MSG)) {
struct object_id oid;
struct strbuf sb = STRBUF_INIT;

Expand Down Expand Up @@ -1775,7 +1706,7 @@ static int do_pick_commit(struct repository *r,
enum todo_command command,
struct commit *commit,
struct replay_opts *opts,
int final_fixup)
int final_fixup, int *check_todo)
{
unsigned int flags = opts->edit ? EDIT_MSG : 0;
const char *msg_file = opts->edit ? NULL : git_path_merge_msg(r);
Expand All @@ -1785,7 +1716,7 @@ static int do_pick_commit(struct repository *r,
char *author = NULL;
struct commit_message msg = { NULL, NULL, NULL, NULL };
struct strbuf msgbuf = STRBUF_INIT;
int res, unborn = 0, allow;
int res, unborn = 0, reword = 0, allow;

if (opts->no_commit) {
/*
Expand Down Expand Up @@ -1855,7 +1786,7 @@ static int do_pick_commit(struct repository *r,
opts);
if (res || command != TODO_REWORD)
goto leave;
flags |= EDIT_MSG | AMEND_MSG | VERIFY_MSG;
reword = 1;
msg_file = NULL;
goto fast_forward_edit;
}
Expand Down Expand Up @@ -1913,7 +1844,7 @@ static int do_pick_commit(struct repository *r,
}

if (command == TODO_REWORD)
flags |= EDIT_MSG | VERIFY_MSG;
reword = 1;
else if (is_fixup(command)) {
if (update_squash_messages(r, command, commit, opts))
return -1;
Expand Down Expand Up @@ -1997,13 +1928,21 @@ static int do_pick_commit(struct repository *r,
} else if (allow)
flags |= ALLOW_EMPTY;
if (!opts->no_commit) {
fast_forward_edit:
if (author || command == TODO_REVERT || (flags & AMEND_MSG))
res = do_commit(r, msg_file, author, opts, flags);
else
res = error(_("unable to parse commit author"));
*check_todo = !!(flags & EDIT_MSG);
if (!res && reword) {
fast_forward_edit:
res = run_git_commit(r, NULL, opts, EDIT_MSG |
VERIFY_MSG | AMEND_MSG |
(flags & ALLOW_EMPTY));
*check_todo = 1;
}
}


if (!res && final_fixup) {
unlink(rebase_path_fixup_msg());
unlink(rebase_path_squash_msg());
Expand Down Expand Up @@ -3818,6 +3757,7 @@ static int pick_commits(struct repository *r,
while (todo_list->current < todo_list->nr) {
struct todo_item *item = todo_list->items + todo_list->current;
const char *arg = todo_item_get_arg(todo_list, item);
int check_todo = 0;

if (save_todo(todo_list, opts))
return -1;
Expand Down Expand Up @@ -3856,7 +3796,8 @@ static int pick_commits(struct repository *r,
command_to_string(item->command), NULL),
1);
res = do_pick_commit(r, item->command, item->commit,
opts, is_final_fixup(todo_list));
opts, is_final_fixup(todo_list),
&check_todo);
if (is_rebase_i(opts) && res < 0) {
/* Reschedule */
advise(_(rescheduled_advice),
Expand Down Expand Up @@ -3913,7 +3854,6 @@ static int pick_commits(struct repository *r,
} else if (item->command == TODO_EXEC) {
char *end_of_arg = (char *)(arg + item->arg_len);
int saved = *end_of_arg;
struct stat st;

if (!opts->verbose)
term_clear_line();
Expand All @@ -3924,17 +3864,8 @@ static int pick_commits(struct repository *r,
if (res) {
if (opts->reschedule_failed_exec)
reschedule = 1;
} else if (stat(get_todo_path(opts), &st))
res = error_errno(_("could not stat '%s'"),
get_todo_path(opts));
else if (match_stat_data(&todo_list->stat, &st)) {
/* Reread the todo file if it has changed. */
todo_list_release(todo_list);
if (read_populate_todo(r, todo_list, opts))
res = -1; /* message was printed */
/* `current` will be incremented below */
todo_list->current = -1;
}
check_todo = 1;
} else if (item->command == TODO_LABEL) {
if ((res = do_label(r, arg, item->arg_len)))
reschedule = 1;
Expand Down Expand Up @@ -3970,6 +3901,20 @@ static int pick_commits(struct repository *r,
item->commit,
arg, item->arg_len,
opts, res, 0);
} else if (check_todo && !res) {
struct stat st;

if (stat(get_todo_path(opts), &st)) {
res = error_errno(_("could not stat '%s'"),
get_todo_path(opts));
} else if (match_stat_data(&todo_list->stat, &st)) {
/* Reread the todo file if it has changed. */
todo_list_release(todo_list);
if (read_populate_todo(r, todo_list, opts))
res = -1; /* message was printed */
/* `current` will be incremented below */
todo_list->current = -1;
}
}

todo_list->current++;
Expand Down Expand Up @@ -4296,9 +4241,12 @@ static int single_pick(struct repository *r,
struct commit *cmit,
struct replay_opts *opts)
{
int check_todo;

setenv(GIT_REFLOG_ACTION, action_name(opts), 0);
return do_pick_commit(r, opts->action == REPLAY_PICK ?
TODO_PICK : TODO_REVERT, cmit, opts, 0);
TODO_PICK : TODO_REVERT, cmit, opts, 0,
&check_todo);
}

int sequencer_pick_revisions(struct repository *r,
Expand Down
16 changes: 13 additions & 3 deletions t/t3404-rebase-interactive.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1016,16 +1016,26 @@ test_expect_success 'rebase -i --root fixup root commit' '
test 0 = $(git cat-file commit HEAD | grep -c ^parent\ )
'

test_expect_success 'rebase -i --root reword root commit' '
test_expect_success 'rebase -i --root reword original root commit' '
test_when_finished "test_might_fail git rebase --abort" &&
git checkout -b reword-root-branch master &&
git checkout -b reword-original-root-branch master &&
set_fake_editor &&
FAKE_LINES="reword 1 2" FAKE_COMMIT_MESSAGE="A changed" \
git rebase -i --root &&
git show HEAD^ | grep "A changed" &&
test -z "$(git show -s --format=%p HEAD^)"
'

test_expect_success 'rebase -i --root reword new root commit' '
test_when_finished "test_might_fail git rebase --abort" &&
git checkout -b reword-now-root-branch master &&
set_fake_editor &&
FAKE_LINES="reword 3 1" FAKE_COMMIT_MESSAGE="C changed" \
git rebase -i --root &&
git show HEAD^ | grep "C changed" &&
test -z "$(git show -s --format=%p HEAD^)"
'

test_expect_success 'rebase -i --root when root has untracked file conflict' '
test_when_finished "reset_rebase" &&
git checkout -b failing-root-pick A &&
Expand Down Expand Up @@ -1054,7 +1064,7 @@ test_expect_success 'rebase -i --root reword root when root has untracked file c
'

test_expect_success C_LOCALE_OUTPUT 'rebase --edit-todo does not work on non-interactive rebase' '
git checkout reword-root-branch &&
git checkout reword-original-root-branch &&
git reset --hard &&
git checkout conflict-branch &&
set_fake_editor &&
Expand Down
21 changes: 20 additions & 1 deletion t/t3429-rebase-edit-todo.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,15 @@
test_description='rebase should reread the todo file if an exec modifies it'

. ./test-lib.sh
. "$TEST_DIRECTORY"/lib-rebase.sh

test_expect_success 'setup' '
test_commit first file &&
test_commit second file &&
test_commit third file
'

test_expect_success 'rebase exec modifies rebase-todo' '
test_commit initial &&
todo=.git/rebase-merge/git-rebase-todo &&
git rebase HEAD -x "echo exec touch F >>$todo" &&
test -e F
Expand Down Expand Up @@ -33,4 +39,17 @@ test_expect_success SHA1 'loose object cache vs re-reading todo list' '
git rebase HEAD -x "./append-todo.sh 5 6"
'

test_expect_success 'todo is re-read after reword and squash' '
write_script reword-editor.sh <<-\EOS &&
GIT_SEQUENCE_EDITOR="echo \"exec echo $(cat file) >>actual\" >>" \
git rebase --edit-todo
EOS

test_write_lines first third >expected &&
set_fake_editor &&
GIT_SEQUENCE_EDITOR="$EDITOR" FAKE_LINES="reword 1 squash 2 fixup 3" \
GIT_EDITOR=./reword-editor.sh git rebase -i --root third &&
test_cmp expected actual
'

test_done
8 changes: 1 addition & 7 deletions t/t7505-prepare-commit-msg-hook.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,13 +241,7 @@ test_rebase () {
git add b &&
git rebase --continue
) &&
if test "$mode" = -p # reword amended after pick
then
n=18
else
n=17
fi &&
git log --pretty=%s -g -n$n HEAD@{1} >actual &&
git log --pretty=%s -g -n18 HEAD@{1} >actual &&
test_cmp "$TEST_DIRECTORY/t7505/expected-rebase${mode:--i}" actual
'
}
Expand Down
3 changes: 2 additions & 1 deletion t/t7505/expected-rebase-i
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@ message (no editor) [edit rebase-10]
message [fixup rebase-9]
message (no editor) [fixup rebase-8]
message (no editor) [squash rebase-7]
message [reword rebase-6]
HEAD [reword rebase-6]
message (no editor) [reword rebase-6]
message [squash rebase-5]
message (no editor) [fixup rebase-4]
message (no editor) [pick rebase-3]
Expand Down