@matheusbsbs / matheus-bagliano / commits / 450efe2d53

rebase -i: always update HEAD before rewording

If the user runs git log while rewording a commit it is confusing if sometimes we're amending the commit that's being reworded and at other times we're creating a new commit depending on whether we could fast-forward or not[1]. Fix this inconsistency by always committing the picked commit and then running 'git commit --amend' to do the reword. The first commit is performed by the sequencer without forking git commit and does not impact on the speed of rebase. In a test rewording 100 commits with GIT_EDITOR=true GIT_SEQUENCE_EDITOR='sed -i s/pick/reword/' \ ../bin-wrappers/git rebase -i --root and taking the best of three runs the current master took 957ms and with this patch it took 961ms. This change fixes rewording the new root commit when rearranging commits with --root. Note that the new code no longer updates CHERRY_PICK_HEAD after creating a root commit - I'm not sure why the old code was that creating that ref after a successful commit, everywhere else it is removed after a successful commit. [1] https://public-inbox.org/git/xmqqlfvu4be3.fsf@gitster-ct.c.googlers.com/T/#m133009cb91cf0917bcf667300f061178be56680a Reported-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Phillip Wood committed Aug 19, 2019 at 02:18 UTC 450efe2d53b67f67b5efc11172ed0706cbd4f776

4 files changed +27 -19

sequencer.c

+11 -8

@@ -986,12 +986,10 @@ static int run_git_commit(struct repository *r,

986

987 strbuf_release(&msg);

988 strbuf_release(&script);

989 - if (!res) {

990 - update_ref(NULL, "CHERRY_PICK_HEAD", &root_commit, NULL,

991 - REF_NO_DEREF, UPDATE_REFS_MSG_ON_ERR);

989 + if (!res)

990 res = update_ref(NULL, "HEAD", &root_commit, NULL, 0,

991 UPDATE_REFS_MSG_ON_ERR);

994 - }

992 +

993 return res < 0 ? error(_("writing root commit")) : 0;

994 }

995

@@ -1785,7 +1783,7 @@ static int do_pick_commit(struct repository *r,

1783 char *author = NULL;

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

1785 struct strbuf msgbuf = STRBUF_INIT;

1788 - int res, unborn = 0, allow;

1786 + int res, unborn = 0, reword = 0, allow;

1787

1788 if (opts->no_commit) {

1789 /*

@@ -1855,7 +1853,7 @@ static int do_pick_commit(struct repository *r,

1853 opts);

1854 if (res || command != TODO_REWORD)

1855 goto leave;

1858 - flags |= EDIT_MSG | AMEND_MSG | VERIFY_MSG;

1856 + reword = 1;

1857 msg_file = NULL;

1858 goto fast_forward_edit;

1859 }

@@ -1913,7 +1911,7 @@ static int do_pick_commit(struct repository *r,

1911 }

1912

1913 if (command == TODO_REWORD)

1916 - flags |= EDIT_MSG | VERIFY_MSG;

1914 + reword = 1;

1915 else if (is_fixup(command)) {

1916 if (update_squash_messages(r, command, commit, opts))

1917 return -1;

@@ -1997,13 +1995,18 @@ static int do_pick_commit(struct repository *r,

1995 } else if (allow)

1996 flags |= ALLOW_EMPTY;

1997 if (!opts->no_commit) {

2000 -fast_forward_edit:

1998 if (author || command == TODO_REVERT || (flags & AMEND_MSG))

1999 res = do_commit(r, msg_file, author, opts, flags);

2000 else

2001 res = error(_("unable to parse commit author"));

2002 + if (!res && reword)

2003 +fast_forward_edit:

2004 + res = run_git_commit(r, NULL, opts, EDIT_MSG |

2005 + VERIFY_MSG | AMEND_MSG |

2006 + (flags & ALLOW_EMPTY));

2007 }

2008

2009 +

2010 if (!res && final_fixup) {

2011 unlink(rebase_path_fixup_msg());

2012 unlink(rebase_path_squash_msg());

t/t3404-rebase-interactive.sh

+13 -3

@@ -1016,9 +1016,9 @@ test_expect_success 'rebase -i --root fixup root commit' '

1016 test 0 = $(git cat-file commit HEAD | grep -c ^parent\ )

1017 '

1018

1019 -test_expect_success 'rebase -i --root reword root commit' '

1019 +test_expect_success 'rebase -i --root reword original root commit' '

1020 test_when_finished "test_might_fail git rebase --abort" &&

1021 - git checkout -b reword-root-branch master &&

1021 + git checkout -b reword-original-root-branch master &&

1022 set_fake_editor &&

1023 FAKE_LINES="reword 1 2" FAKE_COMMIT_MESSAGE="A changed" \

1024 git rebase -i --root &&

@@ -1026,6 +1026,16 @@ test_expect_success 'rebase -i --root reword root commit' '

1026 test -z "$(git show -s --format=%p HEAD^)"

1027 '

1028

1029 +test_expect_success 'rebase -i --root reword new root commit' '

1030 + test_when_finished "test_might_fail git rebase --abort" &&

1031 + git checkout -b reword-now-root-branch master &&

1032 + set_fake_editor &&

1033 + FAKE_LINES="reword 3 1" FAKE_COMMIT_MESSAGE="C changed" \

1034 + git rebase -i --root &&

1035 + git show HEAD^ | grep "C changed" &&

1036 + test -z "$(git show -s --format=%p HEAD^)"

1037 +'

1038 +

1039 test_expect_success 'rebase -i --root when root has untracked file conflict' '

1040 test_when_finished "reset_rebase" &&

1041 git checkout -b failing-root-pick A &&

@@ -1054,7 +1064,7 @@ test_expect_success 'rebase -i --root reword root when root has untracked file c

1064 '

1065

1066 test_expect_success C_LOCALE_OUTPUT 'rebase --edit-todo does not work on non-interactive rebase' '

1057 - git checkout reword-root-branch &&

1067 + git checkout reword-original-root-branch &&

1068 git reset --hard &&

1069 git checkout conflict-branch &&

1070 set_fake_editor &&

t/t7505-prepare-commit-msg-hook.sh

+1 -7

@@ -241,13 +241,7 @@ test_rebase () {

241 git add b &&

242 git rebase --continue

243 ) &&

244 - if test "$mode" = -p # reword amended after pick

245 - then

246 - n=18

247 - else

248 - n=17

249 - fi &&

250 - git log --pretty=%s -g -n$n HEAD@{1} >actual &&

244 + git log --pretty=%s -g -n18 HEAD@{1} >actual &&

245 test_cmp "$TEST_DIRECTORY/t7505/expected-rebase${mode:--i}" actual

246 '

247 }

t/t7505/expected-rebase-i

+2 -1

@@ -7,7 +7,8 @@ message (no editor) [edit rebase-10]

7 message [fixup rebase-9]

8 message (no editor) [fixup rebase-8]

9 message (no editor) [squash rebase-7]

10 -message [reword rebase-6]

10 +HEAD [reword rebase-6]

11 +message (no editor) [reword rebase-6]

12 message [squash rebase-5]

13 message (no editor) [fixup rebase-4]

14 message (no editor) [pick rebase-3]