git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [RFC PATCH 3/7] rebase -i: Stop on root commits with empty log messages

From
Eric Sunshine <sunshine@sunshineco.com>
Date
Jun 21, 2014, 00:33 UTC
Message-ID
<CAPig+cS48D1GBX9duHLVWnsW+g5_8rBGOdU=UBRuxrS9JpUh_A@mail.gmail.com>
In-Reply-To
<53A258E2.501@gmail.com>
On Wed, Jun 18, 2014 at 11:28 PM, Fabian Ruch <bafain@gmail.com> wrote:
Show 78 quoted lines
> When `rebase` is executed with `--root` but no `--onto` is specified,
> `rebase` creates a sentinel commit which is replaced with the root
> commit in three steps. This combination of options results never in a
> fast-forward.
>
>  1. The sentinel commit is forced into the authorship of the root
>     commit.
>
>  2. The changes introduced by the root commit are applied to the index
>     but not committed. If this step fails for whatever reason, all
>     commit information will be there and the user can safely run
>     `git-commit --amend` after resolving the problems.
>
>  3. The new root commit is created by squashing the changes into the
>     sentinel commit which already carries the authorship of the
>     cherry-picked root commit.
>
> The command line used to create the commit in the third step specifies
> effectless and erroneous options. Remove those.
>
>  - `--allow-empty-message` is erroneous: If the root's commit message is
>    empty, the rebase shall fail like for any other commit that is on the
>    to-do list and has an empty commit message.
>
>    Fix the bug that git-rebase does not fail when the initial commit has
>    an empty log message but is replayed using `--root` is specified.
>    Add test.
>
>  - `-C` is effectless: The commit being amended, which is the sentinel
>    commit, already carries the authorship and log message of the
>    cherry-picked root commit. The committer email and commit date fields
>    are reset either way.
>
> After all, if step two fails, `rebase --continue` won't include these
> flags in the git-commit command line either.
>
> Signed-off-by: Fabian Ruch <bafain@gmail.com>
> ---
>  git-rebase--interactive.sh |  4 ++--
>  t/t3412-rebase-root.sh     | 39 +++++++++++++++++++++++++++++++++++++++
>  2 files changed, 41 insertions(+), 2 deletions(-)
>
> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> index fffdfa5..f09eeae 100644
> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -539,8 +539,8 @@ do_pick () {
>                 git commit --allow-empty --allow-empty-message --amend \
>                            --no-post-rewrite -n -q -C $1 &&
>                         pick_one -n $1 &&
> -                       git commit --allow-empty --allow-empty-message \
> -                                  --amend --no-post-rewrite -n -q -C $1 \
> +                       git commit --allow-empty --amend \
> +                                  --no-post-rewrite -n -q \
>                                    ${gpg_sign_opt:+"$gpg_sign_opt"} ||
>                         die_with_patch $1 "Could not apply $1... $2"
>         else
> diff --git a/t/t3412-rebase-root.sh b/t/t3412-rebase-root.sh
> index 0b52105..3608db4 100755
> --- a/t/t3412-rebase-root.sh
> +++ b/t/t3412-rebase-root.sh
> @@ -278,4 +278,43 @@ test_expect_success 'rebase -i -p --root with conflict (second part)' '
>         test_cmp expect-conflict-p out
>  '
>
> +test_expect_success 'stop rebase --root on empty root log message' '
> +       # create a root commit with a non-empty tree so that rebase does
> +       # not fail because of an empty commit, and an empty log message
> +       echo root-commit >file &&
> +       git add file &&
> +       tree=$(git write-tree) &&
> +       root=$(git commit-tree $tree </dev/null) &&
> +       git checkout -b no-message-root-commit $root &&
> +       # do not ff because otherwise neither the patch nor the message
> +       # are looked at and checked for emptiness
> +       test_must_fail env EDITOR=true git rebase -i --force-rebase --root &&
> +       echo root-commit >file.expected &&
> +       test_cmp file file.expected &&

It is customary, and provides nicer diagnostic output upon failure, to have the "expected" file mentioned first:

    test_cmp file.expected file &&
> +       git rebase --abort

Do you want to place this under control of test_when_finished somewhere above the "git rebase" invocation to ensure cleanup if something fails before this point?

Show 23 quoted lines
> +'
> +
> +test_expect_success 'stop rebase --root on empty child log message' '
> +       # create a root commit with a non-empty tree and provide a log
> +       # message so that rebase does not fail until the root commit is
> +       # successfully replayed
> +       echo root-commit >file &&
> +       git add file &&
> +       tree=$(git write-tree) &&
> +       root=$(git commit-tree $tree -m root-commit) &&
> +       git checkout -b no-message-child-commit $root &&
> +       # create a child commit with a non-empty patch so that rebase
> +       # does not fail because of an empty commit, but an empty log
> +       # message
> +       echo child-commit >file &&
> +       git add file &&
> +       git commit --allow-empty-message --no-edit &&
> +       # do not ff because otherwise neither the patch nor the message
> +       # are looked at and checked for emptiness
> +       test_must_fail env EDITOR=true git rebase -i --force-rebase --root &&
> +       echo child-commit >file.expected &&
> +       test_cmp file file.expected &&
> +       git rebase --abort
Same two comments as for previous test.
Show 5 quoted lines
> +'
> +
>  test_done
> --
> 2.0.0
Previous: Fabian RuchNext: Fabian Ruch
Message 11 of 16 in “rebase -i: Make option handling in pick_one more flexible”
  1. 1/7 rebase -i: Make option handling in pick_one more flexibleFabian Ruch, Jun 19, 2014
  2. Michael HaggertyJun 20, 2014
  3. Junio C HamanoJun 20, 2014
  4. Fabian RuchJun 23, 2014
  5. Fabian RuchJun 21, 2014
  6. Johannes SchindelinJun 23, 2014
  7. 2/7 rebase -i: Teach do_pick the option --editFabian Ruch, Jun 19, 2014
  8. Michael HaggertyJun 20, 2014
  9. Fabian RuchJun 22, 2014
  10. 3/7 rebase -i: Stop on root commits with empty log messagesFabian Ruch, Jun 19, 2014
  11. Eric SunshineJun 21, 2014
  12. Fabian RuchJun 22, 2014
  13. 4/7 rebase -i: Commit only once when rewriting picksFabian Ruch, Jun 19, 2014
  14. 5/7 rebase -i: Do not die in do_pickFabian Ruch, Jun 19, 2014
  15. 6/7 rebase -i: Prepare for squash in terms of do_pick --amendFabian Ruch, Jun 19, 2014
  16. 7/7 rebase -i: Teach do_pick the options --amend and --fileFabian Ruch, Jun 19, 2014

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.