Re: [RFC v2 11/16] Add explanatory comment for transport-helpers refs mapping.

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Jul 30, 2012, 17:08 UTC
Message-ID: <20120730170842.GC8515@burratino>
In-Reply-To: <1343658683-10713-12-git-send-email-florian.achleitner.2.6.31@gmail.com>

Hi Junio,

Florian Achleitner wrote:

Show 5 quoted lines

> transport-helpers can advertise the 'refspec' capability,
> if not a default refspec *:* is assumed. This explains
> the post-processing of refs after fetching with fast-import.
>
> Signed-off-by: Florian Achleitner <florian.achleitner.2.6.31@gmail.com>

The patch below adds a comment to fetch_with_import() explaining the loop that saves the fetched commit names after 'git fast-import' has done its work. It avoids some confusion that Florian encountered on first reading about which refs the fast-import stream is supposed to use to write its result.

(By the way, I guess I find the above paragraph clearer than Florian's commit message. But aside from that, the patch seems good to me.)

I would like to see the patch applied so the remote-svn series without it gets shorter and easier to review. ;-) Munging the two context lines ending with argv_array_clear(&importer_argv); to

 	free(fastimport.argv);
 	fastimport.argv = NULL;

makes this patch apply against master. Does it look ready for application to you? If you'd like, I can send a copy rebased against 'master'.

Patch left unsnipped for reference.

Thanks, Jonathan

Show 33 quoted lines

> ---
>  transport-helper.c |   15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/transport-helper.c b/transport-helper.c
> index d6daad5..e10fd6b 100644
> --- a/transport-helper.c
> +++ b/transport-helper.c
> @@ -478,6 +478,21 @@ static int fetch_with_import(struct transport *transport,
>  
>  	argv_array_clear(&importer_argv);
>  
> +	/*
> +	 * If the remote helper advertised the "refspec" capability,
> +	 * it will have the written result of the import to the refs
> +	 * named on the right hand side of the first refspec matching
> +	 * each ref we were fetching.
> +	 *
> +	 * (If no "refspec" capability was specified, for historical
> +	 * reasons we default to *:*.)
> +	 *
> +	 * Store the result in to_fetch[i].old_sha1.  Callers such
> +	 * as "git fetch" can use the value to write feedback to the
> +	 * terminal, populate FETCH_HEAD, and determine what new value
> +	 * should be written to peer_ref if the update is a
> +	 * fast-forward or this is a forced update.
> +	 */
>  	for (i = 0; i < nr_heads; i++) {
>  		char *private;
>  		posn = to_fetch[i];
> -- 
> 1.7.9.5
>