Re: [PATCH 01/11] Resumable clone: create service git-prime-clone

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 16, 2016, 20:53 UTC
Message-ID: <xmqqzin7in2c.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <1473984742-12516-2-git-send-email-kevin.m.wern@gmail.com>

Kevin Wern <kevin.m.wern@gmail.com> writes:

Show 11 quoted lines

> Create git-prime-clone, a program to be executed on the server that
> returns the location and type of static resource to download before
> performing the rest of a clone.
>
> Additionally, as this executable's location will be configurable (see:
> upload-pack and receive-pack), add the program to
> BINDIR_PROGRAMS_NEED_X, in addition to the usual builtin places. Add
> git-prime-clone executable to gitignore, as well
>
> Signed-off-by: Kevin Wern <kevin.m.wern@gmail.com>
> ---

I wonder if we even need a separate service like this.

Wouldn't a new protocol capability that is advertised from upload-pack sufficient to tell the "git clone" that it can and should consider priming from this static resource?

Show 18 quoted lines

> +static void prime_clone(void)
> +{
> +	if (!enabled) {
> +		fprintf(stderr, _("prime-clone not enabled\n"));
> +	}
> +	else if (url && filetype){
> +		packet_write(1, "%s %s\n", filetype, url);
> +	}
> +	else if (url || filetype) {
> +		if (filetype)
> +			fprintf(stderr, _("prime-clone not properly "
> +					  "configured: missing url\n"));
> +		else if (url)
> +			fprintf(stderr, _("prime-clone not properly "
> +					  "configured: missing filetype\n"));
> +	}
> +	packet_flush(1);
> +}

Two minor comments:

 - For whom are you going to localize these strings?  This program
   is running on the server side and we do not know the locale
   preferred by the end-user who is sitting on the other end of the
   connection, no?

 - Turn "}\n\s+else " into "} else ", please.