Re: [PATCH v2] remove the impression of unexpectedness when access is denied

From: Junio C Hamano <gitster@pobox.com>
Date: May 6, 2013, 14:02 UTC
Message-ID: <7vr4hktddq.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20130504031001.GA3734@elie.Belkin>

Jonathan Nieder <jrnieder@gmail.com> writes:

Show 30 quoted lines

> I ran into this message for the first time today.
>
>  $ git fetch --all
>  Fetching origin
>  remote: Counting objects: 368, done.
> [...]
>  Fetching gitk
>  fatal: Could not read from remote repository.
>
>  Please make sure you have the correct access rights
>  and the repository exists.
>  error: Could not fetch gitk
>  Fetching debian
>  Fetching pape
> [...]
>
> The "gitk" remote refers to git://git.kernel.org/pub/scm/gitk/gitk.
> Using ls-remote to contact it produces the same result.  The message
> is correct: the repository does not exist.
>
> Impressions:
>
>  * Looking at "Could not read", it is not clear what could not read
>    and why.  GIT_TRACE_PACKET tells me the interaction was
>
> 	me> git-upload-pack /pub/scm/gitk/gitk\0host=git.kernel.org\0
> 	them> (hangup)
>
>    Would it make sense for the server to send an "ERR" packet to give
>    a more helpful diagnosis?

I think git-daemon does so (or at least attempts to do so); path_ok() uses enter_repo() to check if the given path is a repository, returns NULL to run_service(), whichh in turn calls daemon_error() that does the ERR thing.

Show 13 quoted lines

>  * The spacing and capitalization is odd and makes it not flow well
>    with the rest of the output.  I suspect it would be easier to read
>    with the error separated from hints:
>
> 	Fetching gitk
> 	fatal: the remote server sent an empty response
> 	hint: does the repository exist?
> 	hint: do you have the correct access rights?
> 	error: Could not fetch gitk
> 	Fetching debian
>
>    If a server is misconfigured and just decides to send an empty
>    response for no good reason, the output would still be true.

It does sound better. Also s/Could not fetch/could not fetch/.

>  * The error message is the same whether the server returned no
>    response or an incomplete pkt-line.  Maybe in the latter case it
>    should print the "hung up unexpectedly" thing.

OK.