[PATCH] rebase: do not fail silently when the HEAD could not be detached

[PATCH] rebase: do not fail silently when the HEAD could not be detached

[Johannes Schindelin]

Since 6fd2f5e(rebase: operate on a detached HEAD), rebase operates on a
detached HEAD.  However, it used "checkout -q" to detach the HEAD, which
hid an error message when, say, an untracked file would have to be
overwritten to detach the HEAD.

Instead of hiding the output of the checkout in all cases, only hide it
when no error occurred.

While at it, add a test that "rebase" actually operated on a detached
HEAD.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 git-rebase.sh     |    2 +-
 t/t3400-rebase.sh |   13 ++++++++++++-
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/git-rebase.sh b/git-rebase.sh
index 6d3eddb..1526fcb 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -434,7 +434,7 @@ fi
 
 # Detach HEAD and reset the tree
 echo "First, rewinding head to replay your work on top of it..."
-git checkout -q "$onto^0" || die "could not detach HEAD"
+error=$(git checkout "$onto" 2>&1) || die "${error}could not detach HEAD"
 git update-ref ORIG_HEAD $branch
 
 # If the $onto is a proper descendant of the tip of the branch, then
diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
index b7a670e..8c0c5f5 100755
--- a/t/t3400-rebase.sh
+++ b/t/t3400-rebase.sh
@@ -14,7 +14,8 @@ export GIT_AUTHOR_EMAIL
 
 test_expect_success \
     'prepare repository with topic branches' \
-    'echo First > A &&
+    'git config core.logAllRefUpdates true &&
+     echo First > A &&
      git update-index --add A &&
      git commit -m "Add A." &&
      git checkout -b my-topic-branch &&
@@ -84,4 +85,14 @@ test_expect_success 'rebase a single mode change' '
      GIT_TRACE=1 git rebase master
 '
 
+test_expect_success 'HEAD was detached during rebase' '
+     test $(git rev-parse HEAD@{1}) != $(git rev-parse modechange@{1})
+'
+
+test_expect_success 'Show verbose error when HEAD could not be detached' '
+     : > B &&
+     test_must_fail git rebase topic 2> output.err > output.out &&
+     grep "Untracked working tree file .B. would be overwritten" output.err
+'
+
 test_done
-- 
1.6.1.2.655.g3f285

Re: [PATCH] rebase: do not fail silently when the HEAD could not be detached

[Junio C Hamano]

Sounds sensible to be in 1.6.2.

Re: [PATCH] rebase: do not fail silently when the HEAD could not be detached

[Björn Steinbrink]

On 2009.02.07 18:20:09 +0100, Johannes Schindelin wrote:
> Since 6fd2f5e(rebase: operate on a detached HEAD), rebase operates on a
> detached HEAD.  However, it used "checkout -q" to detach the HEAD, which
> hid an error message when, say, an untracked file would have to be
> overwritten to detach the HEAD.

Hm, in 324c2c317 "git-rebase: report checkout failure", that was
switched from a stdout/stderr redirect to -q, so that those error
messages are shown. And with a quick test, rebase/checkout complains
just fine:

doener@atjola:g (master) $ git checkout -q bar
error: Untracked working tree file 'file' would be overwritten by merge.
doener@atjola:g (master) $ git rebase bar
First, rewinding head to replay your work on top of it...
error: Untracked working tree file 'file' would be overwritten by merge.
could not detach HEAD

doener@atjola:g (master) $ git --version 
git version 1.6.2.rc0

Same with next and pu.

Björn

Re: [PATCH] rebase: do not fail silently when the HEAD could not be detached

[Junio C Hamano]

Björn Steinbrink <B.Steinbrink@gmx.de> writes:

> On 2009.02.07 18:20:09 +0100, Johannes Schindelin wrote:
>> Since 6fd2f5e(rebase: operate on a detached HEAD), rebase operates on a
>> detached HEAD.  However, it used "checkout -q" to detach the HEAD, which
>> hid an error message when, say, an untracked file would have to be
>> overwritten to detach the HEAD.
>
> Hm, in 324c2c317 "git-rebase: report checkout failure", that was
> switched from a stdout/stderr redirect to -q, so that those error
> messages are shown. And with a quick test, rebase/checkout complains
> just fine:

Very interesting.

Dscho, how did you get the "important part of error information is hidden"
failure that led to this patch?  Did it come from code inspection and
speculation alone, or was there an actual failure case?

If I apply the patch t/t3400-rebase.sh from your message alone to
'master', it does seem to pass.

Re: [PATCH] rebase: do not fail silently when the HEAD could not be detached

[Johannes Schindelin]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1271 bytes --]

Hi,

On Sat, 7 Feb 2009, Junio C Hamano wrote:

> Björn Steinbrink <B.Steinbrink@gmx.de> writes:
> 
> > On 2009.02.07 18:20:09 +0100, Johannes Schindelin wrote:
> >> Since 6fd2f5e(rebase: operate on a detached HEAD), rebase operates on a
> >> detached HEAD.  However, it used "checkout -q" to detach the HEAD, which
> >> hid an error message when, say, an untracked file would have to be
> >> overwritten to detach the HEAD.
> >
> > Hm, in 324c2c317 "git-rebase: report checkout failure", that was
> > switched from a stdout/stderr redirect to -q, so that those error
> > messages are shown. And with a quick test, rebase/checkout complains
> > just fine:
> 
> Very interesting.
> 
> Dscho, how did you get the "important part of error information is hidden"
> failure that led to this patch?  Did it come from code inspection and
> speculation alone, or was there an actual failure case?

Hmm, sorry, I thought I got this with "my" Git, but it appears that due to 
some strange reason I do not understand completely, I was not actually 
using the Git binary from $HOME/bin/, but /usr/bin/.

> If I apply the patch t/t3400-rebase.sh from your message alone to
> 'master', it does seem to pass.

Well, please ignore the patch, then.  Or take the test alone.

Ciao,
Dscho