Subject: [PATCH v2 0/3] Optionally skip linking/copying the built-ins

From: "Johannes Schindelin via GitGitGadget" <[email-masked]>
Date: Mon, 24 Aug 2020 15:37:57 +0000

The dashed form of the built-ins is so passé.

Incidentally, this also handles the .pdb issue in MSVC's install Makefile
target that Peff pointed out in the context of the "slimming down" patch
series
[https://lore.kernel.org/git/[email-masked]/]
.

This addresses https://github.com/gitgitgadget/git/issues/406

Changes since v1:

 * Fixed check-docs under SKIP_DASHED_BUILT_INS
 * Renamed ALL_PROGRAMS_AND_BUILT_INS to ALL_COMMANDS_TO_INSTALL to reflect
   its purpose better.
 * Revamped the commit message of patch 2/3 and 3/3.

Johannes Schindelin (3):
  msvc: copy the correct `.pdb` files in the Makefile target `install`
  Optionally skip linking/copying the built-ins
  ci: stop linking built-ins to the dashed versions

 Makefile                  | 71 +++++++++++++++++++++------------------
 ci/run-build-and-tests.sh |  2 +-
 2 files changed, 40 insertions(+), 33 deletions(-)

base-commit: 878e727637ec5815ccb3301eb994a54df95b21b8
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-411%2Fdscho%2Foptionally-skip-dashed-built-ins-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-411/dscho/optionally-skip-dashed-built-ins-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/411

Range-diff vs v1:

 1:  120d2bb3e4 = 1:  1880a0e4bf msvc: copy the correct `.pdb` files in the Makefile target `install`
 2:  647f49d62e ! 2:  166bd0d8fb Optionally skip linking/copying the built-ins
     @@ Metadata
       ## Commit message ##
          Optionally skip linking/copying the built-ins
      
     -    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.
     +    For a long time already, the non-dashed form of the built-ins is the
     +    recommended way to write scripts, i.e. it is better to call `git merge
     +    [...]` than to call `git-merge [...]`.
     +
     +    While Git still supports the dashed form (by hard-linking the `git`
     +    executable to the dashed name in `libexec/git-core/`), in practice, it
     +    is probably almost irrelevant.
     +
     +    In fact, some platforms (such as Windows) only started gaining
     +    meaningful Git support _after_ the dashed form was deprecated, and
     +    therefore one would expect that all this hard-linking is unnecessary on
     +    those platforms.
     +
     +    In addition to that, some programs that are regularly used to assess
     +    disk usage fail to realize that those are hard-links, and heavily
     +    overcount disk usage. Most notably, this was the case with Windows
     +    Explorer up until the last couple of Windows 10 versions.
     +
     +    To save on the time needed to hard-link these dashed commands, and to
     +    eventually stop shipping with those hard-links on Windows, let's
     +    introduce a Makefile knob to skip generating them.
      
          Signed-off-by: Johannes Schindelin <[email-masked]>
      
     @@ Makefile: BUILT_INS += git-whatchanged$X
       # what 'all' will build and 'install' will install in gitexecdir,
       # excluding programs for built-in commands
       ALL_PROGRAMS = $(PROGRAMS) $(SCRIPTS)
     -+ALL_PROGRAMS_AND_BUILT_INS = $(ALL_PROGRAMS)
     ++ALL_COMMANDS_TO_INSTALL = $(ALL_PROGRAMS)
      +ifeq (,$(SKIP_DASHED_BUILT_INS))
     -+ALL_PROGRAMS_AND_BUILT_INS += $(BUILT_INS)
     ++ALL_COMMANDS_TO_INSTALL += $(BUILT_INS)
      +else
      +# git-upload-pack, git-receive-pack and git-upload-archive are special: they
      +# are _expected_ to be present in the `bin/` directory in their dashed form.
     -+ALL_PROGRAMS_AND_BUILT_INS += git-receive-pack$(X)
     -+ALL_PROGRAMS_AND_BUILT_INS += git-upload-archive$(X)
     -+ALL_PROGRAMS_AND_BUILT_INS += git-upload-pack$(X)
     ++ALL_COMMANDS_TO_INSTALL += git-receive-pack$(X)
     ++ALL_COMMANDS_TO_INSTALL += git-upload-archive$(X)
     ++ALL_COMMANDS_TO_INSTALL += git-upload-pack$(X)
      +endif
       
       # what 'all' will build but not install in gitexecdir
     @@ Makefile: profile-fast: profile-clean

      -all:: $(ALL_PROGRAMS) $(SCRIPT_LIB) $(BUILT_INS) $(OTHER_PROGRAMS) GIT-BUILD-OPTIONS
     -+all:: $(ALL_PROGRAMS_AND_BUILT_INS) $(SCRIPT_LIB) $(OTHER_PROGRAMS) GIT-BUILD-OPTIONS
     ++all:: $(ALL_COMMANDS_TO_INSTALL) $(SCRIPT_LIB) $(OTHER_PROGRAMS) GIT-BUILD-OPTIONS
       ifneq (,$X)
      -	$(QUIET_BUILT_IN)$(foreach p,$(patsubst %$X,%,$(filter %$X,$(ALL_PROGRAMS) $(BUILT_INS) git$X)), test -d '$p' -o '$p' -ef '$p$X' || $(RM) '$p';)
     -+	$(QUIET_BUILT_IN)$(foreach p,$(patsubst %$X,%,$(filter %$X,$(ALL_PROGRAMS_AND_BUILT_INS) git$X)), test -d '$p' -o '$p' -ef '$p$X' || $(RM) '$p';)
     ++	$(QUIET_BUILT_IN)$(foreach p,$(patsubst %$X,%,$(filter %$X,$(ALL_COMMANDS_TO_INSTALL) git$X)), test -d '$p' -o '$p' -ef '$p$X' || $(RM) '$p';)
       endif
       
       all::
     @@ Makefile: ifndef NO_TCLTK
       endif
       ifneq (,$X)
      -	$(foreach p,$(patsubst %$X,%,$(filter %$X,$(ALL_PROGRAMS) $(BUILT_INS) git$X)), test '$(DESTDIR_SQ)$(gitexec_instdir_SQ)/$p' -ef '$(DESTDIR_SQ)$(gitexec_instdir_SQ)/$p$X' || $(RM) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)/$p';)
     -+	$(foreach p,$(patsubst %$X,%,$(filter %$X,$(ALL_PROGRAMS_AND_BUILT_INS) git$X)), test '$(DESTDIR_SQ)$(gitexec_instdir_SQ)/$p' -ef '$(DESTDIR_SQ)$(gitexec_instdir_SQ)/$p$X' || $(RM) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)/$p';)
     ++	$(foreach p,$(patsubst %$X,%,$(filter %$X,$(ALL_COMMANDS_TO_INSTALL) git$X)), test '$(DESTDIR_SQ)$(gitexec_instdir_SQ)/$p' -ef '$(DESTDIR_SQ)$(gitexec_instdir_SQ)/$p$X' || $(RM) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)/$p';)
       endif
       
       	bindir=$$(cd '$(DESTDIR_SQ)$(bindir_SQ)' && pwd) && \
     @@ Makefile: ifneq ($(INCLUDE_DLLS_IN_ARTIFACTS),)
       endif
       
      -artifacts-tar:: $(ALL_PROGRAMS) $(SCRIPT_LIB) $(BUILT_INS) $(OTHER_PROGRAMS) \
     -+artifacts-tar:: $(ALL_PROGRAMS_AND_BUILT_INS) $(SCRIPT_LIB) $(OTHER_PROGRAMS) \
     ++artifacts-tar:: $(ALL_COMMANDS_TO_INSTALL) $(SCRIPT_LIB) $(OTHER_PROGRAMS) \
       		GIT-BUILD-OPTIONS $(TEST_PROGRAMS) $(test_bindir_programs) \
       		$(MOFILES)
       	$(QUIET_SUBDIR0)templates $(QUIET_SUBDIR1) \
     @@ Makefile: endif
       ### Check documentation
       #
      -ALL_COMMANDS = $(ALL_PROGRAMS) $(SCRIPT_LIB) $(BUILT_INS)
     -+ALL_COMMANDS = $(ALL_PROGRAMS_AND_BUILT_INS) $(SCRIPT_LIB)
     ++ALL_COMMANDS = $(ALL_COMMANDS_TO_INSTALL) $(SCRIPT_LIB)
       ALL_COMMANDS += git
       ALL_COMMANDS += git-citool
       ALL_COMMANDS += git-gui
     +@@ Makefile: check-docs::
     + 		    -e 's/\.txt//'; \
     + 	) | while read how cmd; \
     + 	do \
     +-		case " $(patsubst %$X,%,$(ALL_COMMANDS) $(EXCLUDED_PROGRAMS)) " in \
     ++		case " $(patsubst %$X,%,$(ALL_COMMANDS) $(BUILT_INS) $(EXCLUDED_PROGRAMS)) " in \
     + 		*" $$cmd "*)	;; \
     + 		*) echo "removed but $$how: $$cmd" ;; \
     + 		esac; \
 3:  1269d7ace8 ! 3:  ea23ba5e26 ci: stop linking built-ins to the dashed versions
     @@ Commit message
          This backwards-compatibility was needed to support scripts that called
          the dashed form, even if we deprecated that a _long_ time ago.
      
     -    In preparation for eventually dropping those hard-links, teach the CI
     +    For that reason, we just introduced a Makefile knob to skip linking
     +    them. TO make sure that this keeps working, teach the CI
          (and PR) builds to skip generating those hard-links.
      
          Signed-off-by: Johannes Schindelin <[email-masked]>

-- 
gitgitgadget