Re: [RFC PATCH v2 08/16] remote-helpers: Support custom transport options

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, 12 Oct 2009, Shawn O. Pearce wrote:

> Some transports, like the native pack transport implemented by
> fetch-pack, support useful features like depth or include tags.
> These should be exposed if the underlying helper knows how to
> use them and is based upon the same infrastructure.
>
> Helpers must advertise the options they support, any attempt
> to set an unsupported option will cause a failure.
> 
> Signed-off-by: Shawn O. Pearce <spearce@xxxxxxxxxxx>
> CC: Daniel Barkalow <barkalow@xxxxxxxxxxxx>
> ---
>  Documentation/git-remote-helpers.txt |   20 ++++++++++
>  remote-curl.c                        |   16 ++++++-
>  transport-helper.c                   |   70 ++++++++++++++++++++++++++++++++++
>  3 files changed, 103 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/git-remote-helpers.txt b/Documentation/git-remote-helpers.txt
> index e10ce99..334ab30 100644
> --- a/Documentation/git-remote-helpers.txt
> +++ b/Documentation/git-remote-helpers.txt
> @@ -46,6 +46,7 @@ Supported if the helper has the "fetch" capability.
>  'fetch-multiple'::
>  	Fetches multiple objects at once.  The fetch-multiple
>  	command is followed by one or more 'fetch' lines as above,
> +	zero or more 'option' lines for the supported options,
>  	and then a blank line to terminate the batch.  Outputs a
>  	single blank line when the entire batch is complete.
>  	Optionally may output a 'lock <file>' line indicating a
> @@ -69,6 +70,9 @@ CAPABILITIES
>  'fetch-multiple'::
>  	This helper supports the 'fetch-multiple' command.
>  
> +'option' <name>::
> +	This helper supports the option <name> under fetch-multiple.
> +

I'm a bit surprised that the options only apply in a fetch-multiple 
section, rather than getting set at the beginning and applying to 
everything for that run. At least, I think an "option" command should be 
useable outside of a fetch-multiple (or possible future grouping 
construct) and have global scope.

>  REF LIST ATTRIBUTES
>  -------------------
>  
> @@ -76,10 +80,26 @@ None are defined yet, but the caller must accept any which are supplied.
>  
>  FETCH OPTIONS
>  -------------
> +To enable an option the helper must list it in 'capabilities'.
>  
>  'option verbose'::
>  	Print more verbose activity messages to stderr.

I think you mis-split the above part; your previoud patch declared this 
option without declaring any way to use it. Might be worth allowing 
multiple "verboses" and "quiet" or "option verbosity quiet"/"option 
verbosity verbose verbose".

> +'option uploadpack' <command>::
> +	The program to use on the remote side to generate a pack.

I sort of feel like the helper ought to read this one out of the config 
file itself if it wants it. In general, it would be good to have 
transport.c and remote.c out of the business of knowing this sort of 
protocol-specific (albiet specific now to two protocols) information. (Of 
course, the native protocol's transport methods are in transport.c, so 
that's there, but I'd like to move that to a transport-native.c someday.)

> +'option depth' <depth>::
> +	Deepen the history of a shallow repository.
> +
> +'option keep'::
> +	Keep the transferred pack(s) with .keep files.
> +
> +'option followtags'::
> +	Aggressively fetch annotated tags if possible.

I assume this means to fetch tags which annotate objects we have or are 
fetching? (As opposed to fetching any annotated tag we could possibly 
fetch, even if we don't otherwise care about the tag or the thing it 
tags.) It's obvious in the context of git's config options, but I'd like 
this document to avoid assuming that context, and the option could apply 
more generally.

> +
> +'option thin'::
> +	Transfer the data as a thin pack if possible.

Does anyone still use non-default thinness? 

	-Daniel
*This .sig left intentionally blank*
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]