On 02/22, Jeff King wrote: > On Wed, Feb 21, 2018 at 01:44:22PM -0800, Jonathan Tan wrote: > > > On Tue, 6 Feb 2018 17:12:41 -0800 > > Brandon Williams <bmwill@xxxxxxxxxx> wrote: > > > > > In order to allow for code sharing with the server-side of fetch in > > > protocol-v2 convert upload-pack to be a builtin. > > > > > > Signed-off-by: Brandon Williams <bmwill@xxxxxxxxxx> > > > > As Stefan mentioned in [1], also mention in the commit message that this > > means that the "git-upload-pack" invocation gains additional > > capabilities (for example, invoking a pager for --help). > > And possibly respecting pager.upload-pack, which would violate our rule > that it is safe to run upload-pack in untrusted repositories. And this isn't an issue with receive-pack because this same guarantee doesn't exist? > > (This actually doesn't work right now because pager.* is broken for > builtins that don't specify RUN_SETUP; but I think with the fixes last > year to the config code, we can now drop that restriction). > > Obviously we can work around this with an extra RUN_NO_PAGER_CONFIG > flag. But I think it points to a general danger in making upload-pack a > builtin. I'm not sure what other features it would want to avoid (or > what might grow in the future). > > > Having said that, the main purpose of this patch seems to be to libify > > upload-pack, and the move to builtin is just a way of putting the > > program somewhere - we could have easily renamed upload-pack.c and > > created a new upload-pack.c containing the main(), preserving the > > non-builtin-ness of upload-pack, while still gaining the benefits of > > libifying upload-pack. > > Yeah, this seems like a better route to me. > > -Peff -- Brandon Williams
