# [PATCH] git-rebase.sh: Display error output from git-checkout when detaching HEAD fails. 4 messages from 2008-07-11 to 2008-07-15. Participants: Robert Shearman, Junio C Hamano, Rob Shearman. Thread: https://gitlist.dev/t/14415 ## Robert Shearman, 2008-07-11 21:26 Subject: [PATCH] git-rebase.sh: Display error output from git-checkout when detaching HEAD fails. Message-ID: <1215811619-28512-1-git-send-email-robertshearman@gmail.com> URL: https://gitlist.dev/e/1215811619-28512-1-git-send-email-robertshearman%40gmail.com ``` The "git checkout" command executed could fail if, for example, upstream contains a file that would overrwrite a local, untracked file. The output redirection didn't work as stderr was redirected to /dev/null, as was stdout. This appears to be not what was intended so the order of redirections is fixed so that stderr is redirected to stdout instead. --- git-rebase.sh | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/git-rebase.sh b/git-rebase.sh index e2d85ee..0da2210 100755 --- a/git-rebase.sh +++ b/git-rebase.sh @@ -376,7 +376,7 @@ fi # Detach HEAD and reset the tree echo "First, rewinding head to replay your work on top of it..." -git checkout "$onto^0" >/dev/null 2>&1 || +git checkout "$onto^0" 2>&1 >/dev/null || die "could not detach HEAD" # git reset --hard "$onto^0" -- 1.5.6.2.225.g4596.dirty ``` ## Junio C Hamano, 2008-07-11 22:57 Subject: Re: [PATCH] git-rebase.sh: Display error output from git-checkout when detaching HEAD fails. Message-ID: <7vr6a0hvvu.fsf@gitster.siamese.dyndns.org> URL: https://gitlist.dev/e/7vr6a0hvvu.fsf%40gitster.siamese.dyndns.org In-Reply-To: <1215811619-28512-1-git-send-email-robertshearman@gmail.com> ``` Robert Shearman writes: > The "git checkout" command executed could fail if, for example, upstream contains a file that would overrwrite a local, untracked file. The output redirection didn't work as stderr was redirected to /dev/null, as was stdout. This appears to be not what was intended so the order of redirections is fixed so that stderr is redirected to stdout instead. Very long lines, lacks sign-off. > --- > git-rebase.sh | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/git-rebase.sh b/git-rebase.sh > index e2d85ee..0da2210 100755 > --- a/git-rebase.sh > +++ b/git-rebase.sh > @@ -376,7 +376,7 @@ fi > > # Detach HEAD and reset the tree > echo "First, rewinding head to replay your work on top of it..." > -git checkout "$onto^0" >/dev/null 2>&1 || I think this very much is done deliberately by somebody who knows the shell to discard everything. > +git checkout "$onto^0" 2>&1 >/dev/null || And if it is beneficial to show the error, you just do not touch fd #2, like this: git checkout "$onto^0" >/dev/null As I do not see any reason to send the error message to stdout like you did. I also suspect that this part of the script predates 6124aee (add a quiet option to git-checkout, 2007-02-01) where the command learned to be more quiet during the normal operation. Perhaps you can replace the line with git checkout -q "$onto^0" and be done with it. I haven't tested it, though. ``` ## Rob Shearman, 2008-07-14 19:57 Subject: Re: [PATCH] git-rebase.sh: Display error output from git-checkout when detaching HEAD fails. Message-ID: <1096648c0807141257i76f70322p5a16c86c1c826a66@mail.gmail.com> URL: https://gitlist.dev/e/1096648c0807141257i76f70322p5a16c86c1c826a66%40mail.gmail.com In-Reply-To: <7vr6a0hvvu.fsf@gitster.siamese.dyndns.org> ``` 2008/7/11 Junio C Hamano : > Robert Shearman writes: >> diff --git a/git-rebase.sh b/git-rebase.sh >> index e2d85ee..0da2210 100755 >> --- a/git-rebase.sh >> +++ b/git-rebase.sh >> @@ -376,7 +376,7 @@ fi >> >> # Detach HEAD and reset the tree >> echo "First, rewinding head to replay your work on top of it..." >> -git checkout "$onto^0" >/dev/null 2>&1 || > > I think this very much is done deliberately by somebody who knows the > shell to discard everything. Why wasn't "git checkout "$onto^0" &> /dev/null" used then? Then only reason I can come up with would be portability, but it seems surprising to me. >> +git checkout "$onto^0" 2>&1 >/dev/null || > > And if it is beneficial to show the error, you just do not touch fd #2, > like this: > > git checkout "$onto^0" >/dev/null Absolutely. I was just trying to fix the statement to what I thought was the original intent. > As I do not see any reason to send the error message to stdout like you > did. > > I also suspect that this part of the script predates 6124aee (add a quiet > option to git-checkout, 2007-02-01) where the command learned to be more > quiet during the normal operation. Perhaps you can replace the line with > > git checkout -q "$onto^0" > > and be done with it. I haven't tested it, though. I just tested it and it solves the original issue whilst not displaying unnecessary messages during a rebase. For reference, the attached script reproduces the issue that I was trying to solve. Should I resend the patch (like the following) now that it is effectively completely your work? -git checkout "$onto^0" >/dev/null 2>&1 || +git checkout -q "$onto^0" || -- Rob Shearman ``` ## Junio C Hamano, 2008-07-15 15:22 Subject: Re: [PATCH] git-rebase.sh: Display error output from git-checkout when detaching HEAD fails. Message-ID: <7vmykj2mvi.fsf@gitster.siamese.dyndns.org> URL: https://gitlist.dev/e/7vmykj2mvi.fsf%40gitster.siamese.dyndns.org In-Reply-To: <1096648c0807141257i76f70322p5a16c86c1c826a66@mail.gmail.com> ``` "Rob Shearman" writes: > 2008/7/11 Junio C Hamano : > ... >> I think this very much is done deliberately by somebody who knows the >> shell to discard everything. > > Why wasn't "git checkout "$onto^0" &> /dev/null" used then? Then only > reason I can come up with would be portability,... Yes, we are quite old fashioned when it comes to shell scripting. >> ... Perhaps you can replace the line with >> >> git checkout -q "$onto^0" >> >> and be done with it. I haven't tested it, though. > > I just tested it and it solves the original issue whilst not > displaying unnecessary messages during a rebase. For reference, the > attached script reproduces the issue that I was trying to solve. > > Should I resend the patch (like the following) now that it is > effectively completely your work? Your choice. The issues you might want to consider when making that choice are: - Resending something that is trivial may seem waste of time on your part; - Bringing the issue up is more than half of solving it, and you deserve the credit. By resending with a clear commit log message you assure this; - I am handling many patches for git.git that are readily applicable immediately after I read them in my mbox, while creating a commit out of this discussion is something I need to "work on". When I get around to do some git work tonight, I may not even recall this dialogue, and the solution may be forgotten. - I am lazy and may forget to pass --author="Rob Shearman " even if I remember this conversation and when I make a commit myself; I just made a commit out of this discussion before I forget, so no need to resend for this one, but in general I'd appreciate a resend in general ;-) -- >8 -- From: Junio C Hamano Date: Mon, 14 Jul 2008 14:05:35 -0700 Subject: [PATCH] git-rebase: report checkout failure When detaching the HEAD to the base commit, the "git checkout" command could fail if, for example, upstream contains a file that would overrwrite a local, untracked file. Unconditionally discarding the standard error stream was done to squelch the progress and notices back when checkout did not have -q option, but there is no reason to keep doing it anymore. Noticed by Robert Shearman. Signed-off-by: Junio C Hamano --- git-rebase.sh | 4 +--- 1 files changed, 1 insertions(+), 3 deletions(-) diff --git a/git-rebase.sh b/git-rebase.sh index e2d85ee..7825f88 100755 --- a/git-rebase.sh +++ b/git-rebase.sh @@ -376,9 +376,7 @@ fi # Detach HEAD and reset the tree echo "First, rewinding head to replay your work on top of it..." -git checkout "$onto^0" >/dev/null 2>&1 || - die "could not detach HEAD" -# git reset --hard "$onto^0" +git checkout -q "$onto^0" || die "could not detach HEAD" # If the $onto is a proper descendant of the tip of the branch, then # we just fast forwarded. -- 1.5.6.3.473.gc658e ```