Re: [RFC PATCH 3/7] rebase -i: Stop on root commits with empty log messages
Hi Eric,
On 06/21/2014 02:33 AM, Eric Sunshine wrote:
On Wed, Jun 18, 2014 at 11:28 PM, Fabian Ruch baf...@gmail.com wrote:
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 baf...@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
Ok.
+ 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?
Thanks a lot for the hint, I will place the test_when_finished directly
above the rebase -i command line because it is not relevant before, the
exact position shouldn't matter though. I didn't know that test_run_
skips the cleanup code if -i (for immediate mode) is passed to the test
driver and the test case fails.
Fabian
+'
+
+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
Re: [RFC PATCH 3/7] rebase -i: Stop on root commits with empty log messages
On Wed, Jun 18, 2014 at 11:28 PM, Fabian Ruch baf...@gmail.com wrote:
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 baf...@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?
+'
+
+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
+
