Re: Confusing git pull error message

[Junio C Hamano <gitster@pobox.com>]

Jeff King <peff@peff.net> writes:

> On Mon, Oct 05, 2009 at 12:08:38PM -0700, Junio C Hamano wrote:
>
>> Jeff King <peff@peff.net> writes:
>> 
>> > So I think we need something like this. I wasn't able to figure out a
>> > test case to trigger the first code path below, though. It may not be
>> > possible; if we give a refspec on the command line, either it will be a
>> > candidate for merging or, if it does not exist, fetch will barf. So it
>> > may be that we can just collapse it down to a single case.
>> 
>> I think you are right.
>
> Nope, I'm not. I figured out one more case that it needs to handle.
> Revised patch coming up in a few minutes.
>
>> By the way, I think the other case arms in the case statement that has the
>> sole caller of this function are never reached, no?
>> 
>> Back when you added the check in a74b170 (git-pull: disallow implicit
>> merging to detached HEAD, 2007-01-15), $? referred to the error status of
>> reading HEAD as a symbolic-ref so the check did make sense, but cd67e4d
>> (Teach 'git pull' about --rebase, 2007-11-28) made a stupid mistake that
>> nobody noticed.
>
> Hmm. I'm not sure. I don't see how $? could not be zero, though, because
> the last thing we run is a subshell with sed and tr. But beyond that, we
> actually handle the detached case in error_on_no_merge_candidates
> already. So I think that case statement can simply be collapsed to the
> first case.
>
> -Peff

-- >8 --
Subject: [PATCH] git-pull: dead code removal

Back when a74b170 (git-pull: disallow implicit merging to detached HEAD,
2007-01-15) added this check, $? referred to the error status of reading
HEAD as a symbolic-ref; but cd67e4d (Teach 'git pull' about --rebase,
2007-11-28) moved the command away from where the check is, and nobody
noticed the breakage.  Ever since, $? has always been 0 (tr at the end of
the pipe to find merge_head never fails) and other case arms were never
reached.

These days, error_on_no_merge_candidates function is prepared to handle a
detached HEAD case, which was what the code this patch removes used to
handle.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 git-pull.sh |    9 +--------
 1 files changed, 1 insertions(+), 8 deletions(-)

diff --git a/git-pull.sh b/git-pull.sh
index edf3ce3..66d73eb 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -182,14 +182,7 @@ merge_head=$(sed -e '/	not-for-merge	/d' \
 
 case "$merge_head" in
 '')
-	case $? in
-	0) error_on_no_merge_candidates "$@";;
-	1) echo >&2 "You are not currently on a branch; you must explicitly"
-	   echo >&2 "specify which branch you wish to merge:"
-	   echo >&2 "  git pull <remote> <branch>"
-	   exit 1;;
-	*) exit $?;;
-	esac
+	error_on_no_merge_candidates "$@"
 	;;
 ?*' '?*)
 	if test -z "$orig_head"
-- 
1.6.5.rc2.77.g2ea4a