Re: [PATCH v2] remove the impression of unexpectedness when access is denied
Hi,
Heiko Voigt wrote:
> --- a/connect.c
> +++ b/connect.c
> @@ -49,6 +49,16 @@ static void add_extra_have(struct extra_have_objects
> *extra, unsigned char *sha1
> extra->nr++;
> }
>
> +static void die_initial_contact(int got_at_least_one_head)
> +{
> + if (got_at_least_one_head)
> + die("The remote end hung up upon initial contact");
> + else
> + die("Could not read from remote repository.\n\n"
> + "Please make sure you have the correct access rights\n"
> + "and the repository exists.");
> +}
[...]
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?
* 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.
* 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.
Thoughts?
Jonathan
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] remove the impression of unexpectedness when access is denied
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. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] remove the impression of unexpectedness when access is denied
On Mon, May 06, 2013 at 07:02:41AM -0700, Junio C Hamano wrote: > >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. Yeah, that went into v1.7.8. Do we have any simple way to find out which version kernel.org is running? They should probably also turn on the --informative-errors option, as they do not (AFAIK) have any private repos whose information could be leaked by better error messages. If they are running v1.7.8 and it is not producing an ERR message, then I think there is a bug. > > * 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. I made a stab at this some time ago: http://article.gmane.org/gmane.comp.version-control.git/112189 There were some follow-up comments, and I remember trying to make something work with processing remote stderr, but running into complications. Alas, I don't remember any more details than that. But maybe it helps. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
