threads / patch / 17633

patchrebase: do not fail silently when the HEAD could not be detached

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

## tl;dr

5 messages between Feb 7, 2009 and Feb 8, 2009. Diffs are folded; open one to read it.

replies: 4people: 3as markdown or json

Johannes Schindelin· Feb 7, 2009, 17:20 UTC · lore

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(-)
Show changes to 2 files +13 −2

git-rebase.sh, t/t3400-rebase.sh

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
Björn Steinbrink· Feb 7, 2009, 23:35 UTC · re: Johannes Schindelin · lore

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

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
Junio C Hamano· Feb 8, 2009, 02:26 UTC · re: Björn Steinbrink · lore

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

Björn Steinbrink <B.Steinbrink@gmx.de> writes:
Show 10 quoted lines
> 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.

Johannes Schindelin· Feb 8, 2009, 10:27 UTC · re: Junio C Hamano · lore

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

Hi,
On Sat, 7 Feb 2009, Junio C Hamano wrote:
Show 18 quoted lines
> 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

← back to recent threads