Re: [PATCH] handle http urls with query string ("?foo") correctly

[Junio C Hamano <gitster@pobox.com>]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Usually, this comes before the "---", and your comments/answers after it.  
> And the first line would be the subject:
>
> -- snip --
> Handle http urls with query string ("?foo") correctly
>
> Git breaks when a repository is cloned from an http url that contains a
> query string. This patch fixes this behaviour be inserting the name of
> the requested object (like "/info/refs") before the query string.
> -- snap --
>
> And of course, you usually sign off your patches.

Please wait a minute and step back.

Before going into the presentation, I have a strong doubt about what this
is trying to solve.

Without reading the patch at all (and the lazyness is only half the reason
for not reading the patch before thinking about the issue --- it is also a
good lithmus test to make sure that the commit log explains what is done
well), my understanding is that this peculiar http-hosted git repository
takes:

	http://foo.bar.xz/serve.cgi?repo=foo.git/

as the base URL, and the patch author wants us to ask for (for example)
"info/alternates" as

	http://foo.bar.xz/serve.cgi/info/alternates?repo=foo.git/

or something like that, not the usual:

	http://foo.bar.xz/serve.cgi?repo=foo.git/info/alternates

Two comments.

 (1) If that is not the problem being tackled, the commit log needs to
     explain the issue much better.  "git breaks" is obviously not good
     enough to convey the issue to me, and if the description was not
     clear for me to understand what is being fixed, it has no hope to
     explain the fix to other people.

 (2) If that is indeed the issue being tackled, sorry, it is not how "dumb
     protocol" http server is expected to behave.  Your server needs
     fixing.

If the protocol being used is still the "dumb commit walker" protocol,
then, given the base URL of the repository $URL, "info/refs" must exist at
"$URL/info/refs", and a loose object deadbeef... must exist at
"$URL/objects/de/adbeef...".  That's how the protocol is defined.

If we want to have a CGI on the server side, the client _could_ even talk
"git native" protocol or something similar to it.  But that is not what is
attempted with this patch as far as I can tell.