Re: [PATCH 2/3] Optionally skip linking/copying the built-ins
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Aug 17, 2020, 18:19 UTC
- Message-ID
- <xmqq7dtx2kzf.fsf@gitster.c.googlers.com>
- In-Reply-To
- <647f49d62e910fe8392219c3a5c6d86ac98a88e6.1597655273.git.gitgitgadget@gmail.com>
Johannes Schindelin <gitgitgadget@gmail.com> writes:
Show 9 quoted lines
> The dashed form of the built-ins is so passé. To save on development > time, and to support the idea of eventually dropping the dashed form > altogether, let's introduce a Makefile knob to skip generating those > hard-links. > > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> > --- > Makefile | 53 ++++++++++++++++++++++++++++++++++++----------------- > 1 file changed, 36 insertions(+), 17 deletions(-)
I do not know passé is a good adjective to use for the past effort of keeping the promise we made to our users, but I think in general this as an optional installation knob is an excellent idea.
Show 7 quoted lines
> ### Check documentation > # > -ALL_COMMANDS = $(ALL_PROGRAMS) $(SCRIPT_LIB) $(BUILT_INS) > +ALL_COMMANDS = $(ALL_PROGRAMS_AND_BUILT_INS) $(SCRIPT_LIB) > ALL_COMMANDS += git > ALL_COMMANDS += git-citool > ALL_COMMANDS += git-gui
This stops "make check-docs" from ensuring that the built-in commands are documented when skip-dashed is requested, no? The first action in check-docs target that runs lint-docs in the Documentation directory may notice a missing documentation when it is referenced by somebody else, but the check in the target itself are told that these built-ins no longer exist and triggers "removed but listed" errors.
A mistake clike the above an become harder to make if ALL_PROGRAMS_AND_BUILT_INS is renamed to indicate what it really is (which would also help its primary target, the installation step). It obviously does NOT always include $(BUILT_INS), so it is not "all programs and built-ins" but something else (perhaps "all programs and built-ins that are installed on a filesystem as separate executable files"?)
Thanks.