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.

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

Conversation

phillipwood
Copy link

This series contains a couple of patches to make the C version of
rebase --interactive behave more like the scripted version. The third
patch is not strictly related to the first two but is included here to
avoid merge conflicts.

Cc: SZEDER Gábor [email protected], Johannes Schindelin [email protected]

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/[email protected]/T/#m133009cb91cf0917bcf667300f061178be56680a

Reported-by: SZEDER Gábor <[email protected]>
Signed-off-by: Phillip Wood <[email protected]>
While a rebase is stopped for the user to edit a commit message it can
be convenient for them to also edit the todo list. The scripted version
of rebase supported this but the C version does not. We already check to
see if the todo list has been updated by an exec command so extend this
to rewords and squashes. It only costs a single stat call to do this so
it should not affect the speed of the rebase (especially as it has just
stopped for the user to edit a message)

Note that for squashes the editor may be opened on a different pick to
the squash itself as we edit the message at the end of a chain fixups
and squashes.

Signed-off-by: Phillip Wood <[email protected]>
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
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]>
@phillipwood phillipwood force-pushed the wip/rebase-reword-update-head branch from a737c6b to 3c3b459 Compare August 18, 2019 17:40
@phillipwood
Copy link
Author

/submit

@gitgitgadget
Copy link

gitgitgadget bot commented Aug 19, 2019

Submitted as [email protected]

@@ -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]>

@gitgitgadget
Copy link

gitgitgadget bot commented Aug 19, 2019

This branch is now known as pw/rebase-i-show-HEAD-to-reword.

@gitgitgadget
Copy link

gitgitgadget bot commented Aug 19, 2019

This patch series was integrated into pu via git@88a9d4a.

@gitgitgadget gitgitgadget bot added the pu label Aug 19, 2019
@gitgitgadget
Copy link

gitgitgadget bot commented Aug 22, 2019

This patch series was integrated into pu via git@3335578.

@gitgitgadget
Copy link

gitgitgadget bot commented Aug 22, 2019

This patch series was integrated into pu via git@d4d2c7a.

@gitgitgadget
Copy link

gitgitgadget bot commented Aug 24, 2019

This patch series was integrated into pu via git@80bb26d.

@gitgitgadget
Copy link

gitgitgadget bot commented Aug 26, 2019

This patch series was integrated into pu via git@5dc69ff.

@gitgitgadget
Copy link

gitgitgadget bot commented Sep 3, 2019

This patch series was integrated into pu via git@627a1df.

@gitgitgadget
Copy link

gitgitgadget bot commented Sep 5, 2019

This patch series was integrated into pu via git@283c8d8.

@gitgitgadget
Copy link

gitgitgadget bot commented Sep 7, 2019

This patch series was integrated into pu via git@0e20c75.

@gitgitgadget
Copy link

gitgitgadget bot commented Sep 9, 2019

This patch series was integrated into pu via git@c46b85f.

@gitgitgadget
Copy link

gitgitgadget bot commented Sep 16, 2019

This patch series was integrated into pu via git@c6efc8a.

@gitgitgadget
Copy link

gitgitgadget bot commented Sep 17, 2019

This patch series was integrated into pu via git@aa02063.

@gitgitgadget
Copy link

gitgitgadget bot commented Sep 18, 2019

This patch series was integrated into pu via git@8b56832.

@gitgitgadget
Copy link

gitgitgadget bot commented Sep 18, 2019

This patch series was integrated into pu via git@a054d6f.

@gitgitgadget
Copy link

gitgitgadget bot commented Sep 30, 2019

This patch series was integrated into pu via git@a008757.

@gitgitgadget
Copy link

gitgitgadget bot commented Oct 2, 2019

This patch series was integrated into pu via git@baa8c84.

@gitgitgadget
Copy link

gitgitgadget bot commented Oct 3, 2019

This patch series was integrated into pu via git@cb3eed2.

@gitgitgadget
Copy link

gitgitgadget bot commented Oct 4, 2019

This patch series was integrated into pu via git@c438e26.

@gitgitgadget
Copy link

gitgitgadget bot commented Oct 4, 2019

This patch series was integrated into next via git@ab3d7ee.

@gitgitgadget gitgitgadget bot added the next label Oct 4, 2019
@gitgitgadget
Copy link

gitgitgadget bot commented Oct 6, 2019

This patch series was integrated into pu via git@3492bec.

@gitgitgadget
Copy link

gitgitgadget bot commented Oct 7, 2019

This patch series was integrated into pu via git@ec65760.

@gitgitgadget
Copy link

gitgitgadget bot commented Oct 8, 2019

This patch series was integrated into pu via git@660494b.

@gitgitgadget
Copy link

gitgitgadget bot commented Oct 9, 2019

This patch series was integrated into pu via git@144d3f8.

@gitgitgadget
Copy link

gitgitgadget bot commented Oct 9, 2019

This patch series was integrated into pu via git@5072b4b.

@gitgitgadget
Copy link

gitgitgadget bot commented Oct 11, 2019

This patch series was integrated into pu via git@4608a02.

@gitgitgadget
Copy link

gitgitgadget bot commented Oct 11, 2019

This patch series was integrated into next via git@4608a02.

@gitgitgadget
Copy link

gitgitgadget bot commented Oct 11, 2019

This patch series was integrated into master via git@4608a02.

@gitgitgadget gitgitgadget bot added the master label Oct 11, 2019
@gitgitgadget gitgitgadget bot closed this Oct 11, 2019
@gitgitgadget
Copy link

gitgitgadget bot commented Oct 11, 2019

Closed via 4608a02.

@phillipwood phillipwood deleted the wip/rebase-reword-update-head branch August 12, 2021 13:35
@phillipwood phillipwood restored the wip/rebase-reword-update-head branch August 15, 2021 15:23
@phillipwood phillipwood deleted the wip/rebase-reword-update-head branch August 1, 2023 15:46
@phillipwood phillipwood restored the wip/rebase-reword-update-head branch August 1, 2023 17:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant