Re: [PATCH] upload-pack: Optionally allow fetching reachable sha1

From: Junio C Hamano <gitster@pobox.com>
Date: May 3, 2015, 17:40 UTC
Message-ID: <xmqqy4l5v9tm.fsf@gitster.dls.corp.google.com>
In-Reply-To: <1430604075-5951-1-git-send-email-fredrik.medley@gmail.com>

Fredrik Medley <fredrik.medley@gmail.com> writes:

> For you who don't remember the email discussion, look through the references.

Please don't do this. Always describe the background yourself in the log message so that "git log" can be read offline. "describe yourself" can be done by summarizing earlier discussion, borrowing others' words, of course. And it is a very good idea to give references like you did after your summary to optionally allow people to verify your summary is correct.

I do not quite understand the early part of the following:

> Since before, it is allowed to ask for a sha1, but most git servers refuse
> the request[1].

That makes it sound like "then talk to the server operator to reconfigure and your problem is solved", no? I do not think the exchange [2] and [3] are about that---your wish will not be granted, even if we extended the system to allow non-tip to be fetched, if the server is configured not use that feature.

> This patch is based on a suggested location for
> implementation[2] which has been updated according to ideas in the reply[3].

I quite am not sure what the suggested "location" for implemenation is.

Show 5 quoted lines

> [1] http://thread.gmane.org/gmane.comp.version-control.git/257809
> [2] http://thread.gmane.org/gmane.comp.version-control.git/258002
> [3] http://thread.gmane.org/gmane.comp.version-control.git/258080
>
> With uploadpack.allowreachablesha1inwant configuration option set,

"option set" on the server side, I presume?

> "git fetch" can make a request with a "want" line that names an object
> that has not been advertised (likely to have been obtained out of band or
> from a submodule pointer). Only objects reachable from the branch tips,
> hidden by transfer.hiderefs or not, will be processed.

I am not sure if I am reading the last sentence correctly. If there are two branches, 'master' (exposed) and 'occult' (hidden),

    ---o---o---2---1---x master
        \
         o---2---1---y occult

you can ask for the 40-hex object name of occult~2, but you cannot ask for the 40-hex object name of master~2? Why such a restriction?

 ... wait for an answer that justfies the restriction ...

Does that justification applies for allowing occult~4 but not master~4?

Show 10 quoted lines

> @@ -456,8 +457,11 @@ static void check_non_tip(void)
>  	char namebuf[42]; /* ^ + SHA-1 + LF */
>  	int i;
>  
> -	/* In the normal in-process case non-tip request can never happen */
> -	if (!stateless_rpc)
> +	/*
> +	 * In the normal in-process case without
> +	 * uploadpack.allowreachablesha1inwant, non-tip requests can never happen.
> +	 */

That's quite an overlong line; if you switched it to multi-line, then fold the line once more, perhaps before "non-tip"?

Show 5 quoted lines

> @@ -728,6 +732,7 @@ static int send_ref(const char *refname, const unsigned char *sha1, int flag, vo
>  			     sha1_to_hex(sha1), refname_nons,
>  			     0, capabilities,
>  			     allow_tip_sha1_in_want ? " allow-tip-sha1-in-want" : "",
> +			     allow_reachable_sha1_in_want ? " allow-reachable-sha1-in-want" : "",

Which printf format specifier does this new element correspond to?