Confusing git pull error message

>   So anyway, I tried to do a "git pull" and it gives me the misleading error:
> 
> You asked me to pull without telling me which branch you
> want to merge with, and 'branch.master.merge' in
> your configuration file does not tell me either.  Please
> name which branch you want to merge on the command line and
> try again (e.g. 'git pull <repository> <refspec>').
> See git-pull(1) for details on the refspec.
>
> [...]
>
> But the actual problem is that "master" doesn't exist on origin.  So
> basically I _have_ told it what branch.master.remote and
> branch.master.merge etc is, it's just that they don't appear to exist.

> 2009/9/13 Jeff King <peff@peff.net>:
> 
> > +       else
> > +               echo "Your configuration specified for us to pull the ref"
> > +               echo "'$upstream_short', but we were unable to fetch it from"
> > +               echo "the remote."
> 
> Thanks!
> 
> But why was it 'unable' ?  Are there cases other than it not existing?
>  Maybe something less cryptic:
> 
> "Attempted to pull from ref $upstream_short but this branch does not
> exist on remote."
> 
> Or something?

>        if [ -z "$curr_branch" ]; then
>                echo "You are not currently on a branch, so I cannot use any"
> @@ -96,7 +99,7 @@ error_on_no_merge_candidates () {
>                echo "Please specify which branch you want to merge on the command"
>                echo "line and try again (e.g. 'git pull <repository> <refspec>')."
>                echo "See git-pull(1) for details."
> -       else
> +       elif [ -z "$upstream" ]; then
>                echo "You asked me to pull without telling me which branch you"
>                echo "want to merge with, and 'branch.${curr_branch}.merge' in"
>                echo "your configuration file does not tell me either.  Please"
> @@ -114,6 +117,10 @@ error_on_no_merge_candidates () {
>                echo "    remote.<nickname>.fetch = <refspec>"
>                echo
>                echo "See git-config(1) for details."
> +       else
> +               echo "Your configuration specified for us to pull the ref"
> +               echo "'$upstream_short', but we were unable to fetch it from"
> +               echo "the remote."
>        fi
>        exit 1
>  }

> > +               echo "Your configuration specified for us to pull the ref"
> > +               echo "'$upstream_short', but we were unable to fetch it from"
> > +               echo "the remote."
> >        fi
> >        exit 1
> >  }
> 
> Pretty bad case of multiple personality disorder here? At first
> there's a "me", then again a "me", and then all of a sudden a "we"?

> > What you have here is precisely what we observed. But I think one of the
> > complaints was to say more explicitly "that ref doesn't exist on the
> > remote", which I think should be the case if we have got to this point
> > (anything else would have triggered an error in fetch).
> 
> Wouldn't you get into the situation with this?
> 
> 	[remote "origin"]
>         	fetch = refs/heads/master:refs/heads/master
> 	[branch "master"]
>         	remote = origin
>                 merge = refs/heads/next
> 
> I think saying "does not exist" will repeat the same mistake of
> overguessing you are trying to rectify.

>> I think saying "does not exist" will repeat the same mistake of
>> overguessing you are trying to rectify.
>
> You are right, of course. I think your wording makes sense, then
> (otherwise we get stuck with "well, we didn't fetch it. Maybe because it
> didn't exist. Or maybe because your configuration didn't include it.").

> On Sun, Sep 13, 2009 at 01:38:48PM -0700, Junio C Hamano wrote:
>
>> I saw some discussion on improving the wording.  Here is what I plan to
>> commit.
>
> Thanks for picking this up, I meant to re-post with improvements.
>
>> +     else
>> +             echo "Your configuration specifies to merge the ref"
>> +             echo "'${upstream#refs/heads/}' from the remote, but no such ref"
>> +             echo "was fetched."
>
> What you have here is precisely what we observed. But I think one of the
> complaints was to say more explicitly "that ref doesn't exist on the
> remote", which I think should be the case if we have got to this point
> (anything else would have triggered an error in fetch).

> On Sun, Sep 13, 2009 at 01:38:48PM -0700, Junio C Hamano wrote:
>
>> I saw some discussion on improving the wording.  Here is what I plan to
>> commit.
>
> Thanks for picking this up, I meant to re-post with improvements.

> An observation I'd like to make is that this is way too ugly:
> 
> 	[advice]
>         	pullNoMergeFound = false
>                 pushNonFastForward = false
>                 statusHints = false
> 
> than
> 
> 	[IKnowWhatIAmDoingThankYouVeryMuch]
>         	pullNoMergeFound
>                 pushNonFastForward
>                 statusHints
> 
> but this feature is for people who know what they are doing, so I guess
> the current set-up would be fine.

> I saw some discussion on improving the wording.  Here is what I plan to
> commit.
> 
> diff --git a/git-pull.sh b/git-pull.sh
> index 0bbd5bf..2c2fa79 100755
> --- a/git-pull.sh
> +++ b/git-pull.sh
> @@ -89,6 +89,8 @@ error_on_no_merge_candidates () {
>  	done
>  
>  	curr_branch=${curr_branch#refs/heads/}
> +	upstream=$(git config "branch.$curr_branch.merge" ||
> +			git config "branch.$curr_branch.rebase")

> Jeff King <peff@peff.net> writes:
>> I think it is enough for git-pull to just check whether the config
>> exists, and if so, guess that the ref was simply not fetched. IOW,
>> this:
> 
> Thanks.
> 
> I saw some discussion on improving the wording.  Here is what I plan to
> commit.
> 
> diff --git a/git-pull.sh b/git-pull.sh
> index 0bbd5bf..2c2fa79 100755
> --- a/git-pull.sh
> +++ b/git-pull.sh
> @@ -89,6 +89,8 @@ error_on_no_merge_candidates () {
>  	done
>  
>  	curr_branch=${curr_branch#refs/heads/}
> +	upstream=$(git config "branch.$curr_branch.merge" ||
> +			git config "branch.$curr_branch.rebase")
>  
>  	if [ -z "$curr_branch" ]; then
>  		echo "You are not currently on a branch, so I cannot use any"
> @@ -96,7 +98,7 @@ error_on_no_merge_candidates () {
>  		echo "Please specify which branch you want to merge on the command"
>  		echo "line and try again (e.g. 'git pull <repository> <refspec>')."
>  		echo "See git-pull(1) for details."
> -	else
> +	elif [ -z "$upstream" ]; then
>  		echo "You asked me to pull without telling me which branch you"
>  		echo "want to merge with, and 'branch.${curr_branch}.merge' in"
>  		echo "your configuration file does not tell me either.	Please"
> @@ -114,6 +116,10 @@ error_on_no_merge_candidates () {
>  		echo "    remote.<nickname>.fetch = <refspec>"
>  		echo
>  		echo "See git-config(1) for details."
> +	else
> +		echo "Your configuration specifies to merge the ref"
> +		echo "'${upstream#refs/heads/}' from the remote, but no such ref"
> +		echo "was fetched."
>  	fi
>  	exit 1
>  }

> Unfortunately, this is not water-tight. See what I just observed:
> 
>   $ git pull hk
>   From /exports/repos/hk/viscovery
>      9455552..6429037  master     -> hk/master
>   Your configuration specifies to merge the ref
>   'master' from the remote, but no such ref
>   was fetched.
> 
> The message is confusing when it says "'master' was not fetched" when
> clearly master _was_ fetched.
> 
> More importantly, the message is wrong to say that "Your configuration
> specifies to merge the ref 'master' from the remote", because I have this
> configuration:

> On Mon, Oct 05, 2009 at 01:32:34PM +0200, Johannes Sixt wrote:
> Ah, yeah. Looking at %(upstream) from for-each-ref (which is how we
> determine to show that message) always uses the configured remote. But
> if we have asked for another remote, then that doesn't make much sense.
> 
> 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.
> 
> diff --git a/git-pull.sh b/git-pull.sh
> index edf3ce3..a831db5 100755
> --- a/git-pull.sh
> +++ b/git-pull.sh
> @@ -97,6 +97,18 @@ error_on_no_merge_candidates () {
>  		echo "Please specify which branch you want to merge on the command"
>  		echo "line and try again (e.g. 'git pull <repository> <refspec>')."
>  		echo "See git-pull(1) for details."
> +	elif [ -n "$1" ]; then
> +		if [ $# -gt 1 ]; then
> +			echo "You asked to pull from the remote '$1', but none"
> +			echo "of the things you asked to fetch were candidates"
> +			echo "for merging."
> +		else
> +			echo "You asked to pull from the remote '$1', but did"
> +			echo "not specify a branch to merge. Because this is"
> +			echo "not the default configured remote for your current"
> +			echo "branch, you must specify a branch on the command"
> +			echo "line."
> +		fi
>  	elif [ -z "$upstream" ]; then
>  		echo "You asked me to pull without telling me which branch you"
>  		echo "want to merge with, and 'branch.${curr_branch}.merge' in"

> 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.

> diff --git a/git-pull.sh b/git-pull.sh
> index edf3ce3..a831db5 100755
> --- a/git-pull.sh
> +++ b/git-pull.sh
> @@ -97,6 +97,18 @@ error_on_no_merge_candidates () {
>  		echo "Please specify which branch you want to merge on the command"
>  		echo "line and try again (e.g. 'git pull <repository> <refspec>')."
>  		echo "See git-pull(1) for details."
> +	elif [ -n "$1" ]; then
> +		if [ $# -gt 1 ]; then
> +			echo "You asked to pull from the remote '$1', but none"
> +			echo "of the things you asked to fetch were candidates"
> +			echo "for merging."
> +		else
> +			echo "You asked to pull from the remote '$1', but did"
> +			echo "not specify a branch to merge. Because this is"
> +			echo "not the default configured remote for your current"
> +			echo "branch, you must specify a branch on the command"
> +			echo "line."
> +		fi
>  	elif [ -z "$upstream" ]; then
>  		echo "You asked me to pull without telling me which branch you"
>  		echo "want to merge with, and 'branch.${curr_branch}.merge' in"
>

> 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.

> 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.

> On Mon, Oct 05, 2009 at 03:12:57PM -0400, Jeff King wrote:
>
>> > 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.
>
> OK, here it is, which I think covers all of the cases. I also re-wrapped
> the text, as I agree with JSixt that it was pretty ugly. I also
> re-wrapped some of the existing text, as it gave the very choppy:
>
>   Your configuration specifies to merge the ref
>   'foo' from the remote, but no such ref
>   was fetched.
>
> It would be really nice to just pipe it through 'fmt', but I suspect
> that will create portability problems.
>
> -- >8 --
> Subject: [PATCH] pull: improve advice for unconfigured error case
>
> There are several reasons a git-pull invocation might not
> have anything marked for merge:
>
>   1. We're not on a branch, so there is no branch
>      configuration.
>
>   2. We're on a branch, but there is no configuration for
>      this branch.
>
>   3. We fetched from the configured remote, but the
>      configured branch to merge didn't get fetched (either
>      it doesn't exist, or wasn't part of the fetch refspec).
>
>   4. We fetched from the non-default remote, but didn't
>      specify a branch to merge. We can't use the configured
>      one because it applies to the default remote.
>
>   5. We fetched from a specified remote, and a refspec was
>      given, but it ended up not fetching anything (this is
>      actually hard to do; if the refspec points to a remote
>      branch and it doesn't exist, then fetch will fail and
>      we never make it to this code path. But if you provide
>      a wildcard refspec like
>
>        refs/bogus/*:refs/remotes/origin/*
>
>      then you can see this failure).
>
> We have handled (1) and (2) for some time. Recently, commit
> a6dbf88 added code to handle case (3).
>
> This patch handles cases (4) and (5), which previously just
> fell under other cases, producing a confusing message.
>
> While we're at it, let's rewrap the text for case (3), which
> looks terribly ugly as it is.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
>  git-pull.sh |   16 ++++++++++++----
>  1 files changed, 12 insertions(+), 4 deletions(-)

> Quoting Jeff King <peff@peff.net>
>
>> Subject: [PATCH] pull: improve advice for unconfigured error case
>> ...
> Junio, may I ask what happened to this patch?

> Nanako Shiraishi <nanako3@lavabit.com> writes:
>
>> Quoting Jeff King <peff@peff.net>
>>
>>> Subject: [PATCH] pull: improve advice for unconfigured error case
>>> ...
>> Junio, may I ask what happened to this patch?
>
> Thanks for prodding.  Unfortunately I lost track.  Will look at it again
> but probably not tonight.

> 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.