Re: Confusing git pull error message
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Oct 5, 2009, 19:36 UTC
- Message-ID
- <7vd451ab16.fsf@alter.siamese.dyndns.org>
- In-Reply-To
- <20091005191257.GA24305@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
Show 31 quoted lines
> 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