From: Junio C Hamano Date: Mon, 06 May 2013 14:02:41 GMT Subject: Re: [PATCH v2] remove the impression of unexpectedness when access is denied Message-ID: <7vr4hktddq.fsf@alter.siamese.dyndns.org> In-Reply-To: <20130504031001.GA3734@elie.Belkin> Jonathan Nieder writes: > 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. > * 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.