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

From: Junio C Hamano <[email-masked]>
Date: Sun, 03 May 2015 10:40:21 -0700

Fredrik Medley <[email-masked]> 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.

> [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?

> @@ -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"?

> @@ -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?