Re: [RFC/PATCH 0/5] stop installing old libexec aliases like "git-init"

From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: git@vger.kernel.org
Cc: Junio C Hamano <gitster@pobox.com>,
	Daniel Jacques <dnj@google.com>,
	Johannes Schindelin <Johannes.Schindelin@gmx.de>,
	Steffen Prohaska <prohaska@zib.de>,
	John Keeping <john@keeping.me.uk>, Stan Hu <stanhu@gmail.com>,
	Richard Clamp <richardc@unixbeard.net>, Jeff King <peff@peff.net>,
	Michael Haggerty <mhagger@alum.mit.edu>
Subject: Re: [RFC/PATCH 0/5] stop installing old libexec aliases like "git-init"
Date: Fri, 16 Nov 2018 11:38:18 +0100	[thread overview]
Message-ID: <87k1ld1f51.fsf@evledraar.gmail.com> (raw)
In-Reply-To: <20181102223743.4331-1-avarab@gmail.com>

On Fri, Nov 02 2018, Ævar Arnfjörð Bjarmason wrote:

> I think up to patch 4 here should be near a state that's ready for
> inclusion.
>
> Although I'm on the fence with the approach in 1/5. Should this be a
> giant getopt switch statement like that in a helper script? An
> alternative would be to write out a shell file similar to
> GIT-BUILD-OPTIONS and source that from this thing. I don't know, what
> do you all think?
>
> The idea with 4/5 was to make this symlink mode the default in
> config.mak.uname and have a blacklist of systems like Windows that
> couldn't deal with it.
>
> Since my ad874608d8 ("Makefile: optionally symlink libexec/git-core
> binaries to bin/git", 2018-03-13) I see that e.g. Debian and GitLab
> have started shipping with the INSTALL_SYMLINKS flag, so making that
> unconditional is the next logical step.
>
> The 5th one is more radical. See
> https://public-inbox.org/git/87woyfdkoi.fsf@evledraar.gmail.com/ from
> back in March for context.
>
> I'd like to say it's ready, but I've spotted some fallout:
>
>  * Help like "git ninit" suggesting "git init" doesn't work, this is
>    because load_command_list() in help.c doesn't look out our
>    in-memory idea of builtins, it reads the libexecdir, so if we don't
>    have the programs there it doesn't know about it.

A follow-up on this: We should really fix this for other
reasons. I.e. compile in some "this is stuff we ourselves think is in
git".

There's other manifestations of this, e.g.:

    git-sizer --help # => shows you help
    git sizer --help # => says it doesn't have a manpage

Because we aren't aware that git-sizer is some external tool, and that
we should route --help to it.

Non-withstanding the arguable bug that things like git-sizer shouldn't
be allowing themselves to be invoked by "git" like that without
guaranteeing that it can consume all the options 'git' expects. When I
had to deal with a similar problem in an external git-* command I was
maintaining I simply made it an error to invoke it as "git mything"
instead of "git-mything".

>  * GIT_TEST_INSTALLED breaks entirely under this, as early as the
>    heuristic for "are we built?" being "do we have git-init in
>    libexecdir?". I tried a bit to make this work, but there's a lot of
>    dependencies there.
>
>  * We still (and this is also true of my ad874608d8) hardlink
>    everything in the build dir via a different part of the Makefile,
>    ideally we should do exactly the same thing there so also normal
>    tests and not just GIT_TEST_INSTALLED (if that worked) would test
>    in the same mode.
>
>    I gave making that work a bit of a try and gave up in the Makefile
>    jungle.
>
> Ævar Arnfjörð Bjarmason (5):
>   Makefile: move long inline shell loops in "install" into helper
>   Makefile: conform some of the code to our coding standards
>   Makefile: stop hiding failures during "install"
>   Makefile: add NO_INSTALL_SYMLINKS_FALLBACK switch
>   Makefile: Add a NO_INSTALL_BUILTIN_EXECDIR_ALIASES flag
>
>  Makefile         |  65 +++++++++++--------------
>  install_programs | 124 +++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 151 insertions(+), 38 deletions(-)
>  create mode 100755 install_programs