Re: [PATCH 2/3] Optionally skip linking/copying the built-ins

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

[-- Attachment #1: Type: text/plain, Size: 2318 bytes --]

Hi Junio,

On Mon, 17 Aug 2020, Junio C Hamano wrote:

> Johannes Schindelin <gitgitgadget@gmail.com> writes:
>
> > 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.

You're right. My frustration with related Git for Windows tickets got the
better of me. I hope that you'll like v2's commit message much better.

> >  ### 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"?)

Right, that's a very good point. I had assumed that `check-docs` would be
exercised by CI, but it wasn't... It's only exercised in the
`Documentation` job, which is run without Makefile knobs.

I fixed it in preparation for v2 of this patch series.

Ciao,
Dscho