Re: [WIP 04/15] upload-pack: convert to a builtin
Johannes Schindelinwrites: > On Wed, 6 Dec 2017, Junio C Hamano wrote: > ... >> I vaguely recalled and feared that we on purpose kept this program >> separate from the rest of the system for a reason, but my mailing >> list search is coming up empty. > > I only recall that we kept it in the bin/ directory (as opposed to > mlibexec/git-core/) to help with fetching via SSH. Yes, that is about where it is installed (i.e. on $PATH), which is a different issue. My vague recollection was about what is (and what is not) included in and linked into the program built, with some reason that is different from but similar to the reason why remote helpers that link to curl and openssl libraries are excluded from the builtin deliberately. I know we exclude remote-helpers from builtin in order to save the start-up overhead for other more important built-in commands by not having to link these heavyweight libs. I suspect there was some valid reason why we didn't make upload-pack a built-in, but am failing to recall what the reason was.
Re: [WIP 04/15] upload-pack: convert to a builtin
On 12/06, Junio C Hamano wrote: > Brandon Williamswrites: > > > 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 > > --- > > This looks obvious and straight-forward to a cursory look. > > I vaguely recalled and feared that we on purpose kept this program > separate from the rest of the system for a reason, but my mailing > list search is coming up empty. > > > Makefile | 3 ++- > > builtin.h | 1 + > > git.c | 1 + > > upload-pack.c | 2 +- > > 4 files changed, 5 insertions(+), 2 deletions(-) > > ... > > diff --git a/upload-pack.c b/upload-pack.c > > index ef99a029c..2d16952a3 100644 > > --- a/upload-pack.c > > +++ b/upload-pack.c > > @@ -1033,7 +1033,7 @@ static int upload_pack_config(const char *var, const > > char *value, void *unused) > > return parse_hide_refs_config(var, value, "uploadpack"); > > } > > > > -int cmd_main(int argc, const char **argv) > > +int cmd_upload_pack(int argc, const char **argv, const char *prefix) > > { > > const char *dir; > > int strict = 0; > > Shouldn't this file be moved to builtin/ directory, though? I can definitely move the file to builtin if you would prefer. -- Brandon Williams
Re: [WIP 04/15] upload-pack: convert to a builtin
Hi Junio, On Wed, 6 Dec 2017, Junio C Hamano wrote: > Brandon Williamswrites: > > > 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 > > --- > > This looks obvious and straight-forward to a cursory look. > > I vaguely recalled and feared that we on purpose kept this program > separate from the rest of the system for a reason, but my mailing > list search is coming up empty. I only recall that we kept it in the bin/ directory (as opposed to mlibexec/git-core/) to help with fetching via SSH. Ciao, Dscho
Re: [WIP 04/15] upload-pack: convert to a builtin
Brandon Williamswrites: > 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 > --- This looks obvious and straight-forward to a cursory look. I vaguely recalled and feared that we on purpose kept this program separate from the rest of the system for a reason, but my mailing list search is coming up empty. > Makefile | 3 ++- > builtin.h | 1 + > git.c | 1 + > upload-pack.c | 2 +- > 4 files changed, 5 insertions(+), 2 deletions(-) > ... > diff --git a/upload-pack.c b/upload-pack.c > index ef99a029c..2d16952a3 100644 > --- a/upload-pack.c > +++ b/upload-pack.c > @@ -1033,7 +1033,7 @@ static int upload_pack_config(const char *var, const > char *value, void *unused) > return parse_hide_refs_config(var, value, "uploadpack"); > } > > -int cmd_main(int argc, const char **argv) > +int cmd_upload_pack(int argc, const char **argv, const char *prefix) > { > const char *dir; > int strict = 0; Shouldn't this file be moved to builtin/ directory, though?