On Fri, Sep 16, 2016 at 01:53:15PM -0700, Junio C Hamano wrote:
> Kevin Wern <kevin.m.w...@gmail.com> writes:
>
> > 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.w...@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?
The short answer is yes, it could be done that way. Both methods--extending
upload-pack and creating a new service--were suggested in different
discussions.
However, my thought was to implement the separate service because:
- It is much easier for an admin trying the feature to sanity check the
output of an executable, compared to passing messages to upload-pack.
- In the other scenario, upload-pack might get too expansive in size
and scope--not only codewise, but in terms of config namespace if
"uploadpack" concerns too many things that are only tangentially
related (the properties of the primer resource).
- The transport_prime_clone API can be called independent of other
transport API functions, which might prove useful when revisiting or
refactoring code.
- You favored the creation of a service in our original discussion [1].
I'm not sure if your reasoning was similar to mine.
It definitely was a tight decision--for me, ultimately weighing the value added
in usability (point 1) against the need for a "failsafe" implementation. All
the other points are more speculative, IMO, but the first was strong enough for
me.
What do you think?
[1] http://www.spinics.net/lists/git/msg269992.html
> 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.
These are fair points. Changing for the revised version.
- Kevin
