From a8b5355d808ba7f13af94e4446dab980c39b054d Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Mon, 21 Sep 2020 22:28:15 +0000 Subject: msvc: copy the correct `.pdb` files in the Makefile target `install` There is a hard-coded list of `.pdb` files to copy. But we are about to introduce the `SKIP_DASHED_BUILT_INS` knob in the `Makefile`, which might make this hard-coded list incorrect. Let's switch to a dynamically-generated list instead. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- Makefile | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/Makefile b/Makefile index f1b1bc8aa0..ce072c4fb8 100644 --- a/Makefile +++ b/Makefile @@ -2921,15 +2921,8 @@ ifdef MSVC # have already been rolled up into the exe's pdb file. # We DO NOT have pdb files for the builtin commands (like git-status.exe) # because it is just a copy/hardlink of git.exe, rather than a unique binary. - $(INSTALL) git.pdb '$(DESTDIR_SQ)$(bindir_SQ)' - $(INSTALL) git-shell.pdb '$(DESTDIR_SQ)$(bindir_SQ)' - $(INSTALL) git-daemon.pdb '$(DESTDIR_SQ)$(gitexec_instdir_SQ)' - $(INSTALL) git-http-backend.pdb '$(DESTDIR_SQ)$(gitexec_instdir_SQ)' - $(INSTALL) git-http-fetch.pdb '$(DESTDIR_SQ)$(gitexec_instdir_SQ)' - $(INSTALL) git-http-push.pdb '$(DESTDIR_SQ)$(gitexec_instdir_SQ)' - $(INSTALL) git-imap-send.pdb '$(DESTDIR_SQ)$(gitexec_instdir_SQ)' - $(INSTALL) git-remote-http.pdb '$(DESTDIR_SQ)$(gitexec_instdir_SQ)' - $(INSTALL) git-sh-i18n--envsubst.pdb '$(DESTDIR_SQ)$(gitexec_instdir_SQ)' + $(INSTALL) $(patsubst %.exe,%.pdb,$(filter-out $(BUILT_INS),$(patsubst %,%$X,$(BINDIR_PROGRAMS_NEED_X)))) '$(DESTDIR_SQ)$(bindir_SQ)' + $(INSTALL) $(patsubst %.exe,%.pdb,$(filter-out $(BUILT_INS) $(REMOTE_CURL_ALIASES),$(PROGRAMS))) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)' ifndef DEBUG $(INSTALL) $(vcpkg_rel_bin)/*.dll '$(DESTDIR_SQ)$(bindir_SQ)' $(INSTALL) $(vcpkg_rel_bin)/*.pdb '$(DESTDIR_SQ)$(bindir_SQ)' -- cgit v1.2.3 From 179227d6e212373019f6a05ee235b3d4e7e2982e Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Mon, 21 Sep 2020 22:28:16 +0000 Subject: Optionally skip linking/copying the built-ins 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. However, we *do* care about keeping people's scripts working (even if they were written before the non-dashed form started to be recommended). Keeping this backwards-compatibility is not necessarily cheap, though: even so much as amending the tip commit in a git.git checkout will require re-linking all of those dashed commands. On this developer's laptop, this makes a noticeable difference: $ touch version.c && time make CC version.o AR libgit.a LINK git-bugreport.exe [... 11 similar lines ...] LN/CP git-remote-https.exe LN/CP git-remote-ftp.exe LN/CP git-remote-ftps.exe LINK git.exe BUILTIN git-add.exe [... 123 similar lines ...] BUILTIN all SUBDIR git-gui SUBDIR gitk-git SUBDIR templates LINK t/helper/test-fake-ssh.exe LINK t/helper/test-line-buffer.exe LINK t/helper/test-svn-fe.exe LINK t/helper/test-tool.exe real 0m36.633s user 0m3.794s sys 0m14.141s $ touch version.c && time make SKIP_DASHED_BUILT_INS=1 CC version.o AR libgit.a LINK git-bugreport.exe [... 11 similar lines ...] LN/CP git-remote-https.exe LN/CP git-remote-ftp.exe LN/CP git-remote-ftps.exe LINK git.exe BUILTIN git-receive-pack.exe BUILTIN git-upload-archive.exe BUILTIN git-upload-pack.exe BUILTIN all SUBDIR git-gui SUBDIR gitk-git SUBDIR templates LINK t/helper/test-fake-ssh.exe LINK t/helper/test-line-buffer.exe LINK t/helper/test-svn-fe.exe LINK t/helper/test-tool.exe real 0m23.717s user 0m1.562s sys 0m5.210s Also, `.zip` files do not have any standardized support for hard-links, therefore "zipping up" the executables will result in inflated disk usage. (To keep down the size of the "MinGit" variant of Git for Windows, which is distributed as a `.zip` file, the hard-links are excluded specifically.) 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. See e.g. https://github.com/msysgit/msysgit/issues/58. To save on the time needed to hard-link these dashed commands, with the plan 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 Signed-off-by: Junio C Hamano --- Makefile | 55 +++++++++++++++++++++++++++++++++++++------------------ 1 file changed, 37 insertions(+), 18 deletions(-) diff --git a/Makefile b/Makefile index ce072c4fb8..6931ecd45e 100644 --- a/Makefile +++ b/Makefile @@ -348,6 +348,9 @@ all:: # Define NO_INSTALL_HARDLINKS if you prefer to use either symbolic links or # copies to install built-in git commands e.g. git-cat-file. # +# Define SKIP_DASHED_BUILT_INS if you do not need the dashed versions of the +# built-ins to be linked/copied at all. +# # Define USE_NED_ALLOCATOR if you want to replace the platforms default # memory allocators with the nedmalloc allocator written by Niall Douglas. # @@ -774,6 +777,16 @@ 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_COMMANDS_TO_INSTALL = $(ALL_PROGRAMS) +ifeq (,$(SKIP_DASHED_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_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 OTHER_PROGRAMS = git$X @@ -2088,9 +2101,9 @@ profile-fast: profile-clean $(MAKE) PROFILE=USE all -all:: $(ALL_PROGRAMS) $(SCRIPT_LIB) $(BUILT_INS) $(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_COMMANDS_TO_INSTALL) git$X)), test -d '$p' -o '$p' -ef '$p$X' || $(RM) '$p';) endif all:: @@ -2950,7 +2963,7 @@ ifndef NO_TCLTK $(MAKE) -C git-gui gitexecdir='$(gitexec_instdir_SQ)' install 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_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) && \ @@ -2968,21 +2981,27 @@ endif } && \ for p in $(filter $(install_bindir_programs),$(BUILT_INS)); do \ $(RM) "$$bindir/$$p" && \ - test -n "$(INSTALL_SYMLINKS)" && \ - ln -s "git$X" "$$bindir/$$p" || \ - { test -z "$(NO_INSTALL_HARDLINKS)" && \ - ln "$$bindir/git$X" "$$bindir/$$p" 2>/dev/null || \ - ln -s "git$X" "$$bindir/$$p" 2>/dev/null || \ - cp "$$bindir/git$X" "$$bindir/$$p" || exit; } \ + if test -z "$(SKIP_DASHED_BUILT_INS)"; \ + then \ + test -n "$(INSTALL_SYMLINKS)" && \ + ln -s "git$X" "$$bindir/$$p" || \ + { test -z "$(NO_INSTALL_HARDLINKS)" && \ + ln "$$bindir/git$X" "$$bindir/$$p" 2>/dev/null || \ + ln -s "git$X" "$$bindir/$$p" 2>/dev/null || \ + cp "$$bindir/git$X" "$$bindir/$$p" || exit; }; \ + fi \ done && \ for p in $(BUILT_INS); do \ $(RM) "$$execdir/$$p" && \ - test -n "$(INSTALL_SYMLINKS)" && \ - ln -s "$$destdir_from_execdir_SQ/$(bindir_relative_SQ)/git$X" "$$execdir/$$p" || \ - { test -z "$(NO_INSTALL_HARDLINKS)" && \ - ln "$$execdir/git$X" "$$execdir/$$p" 2>/dev/null || \ - ln -s "git$X" "$$execdir/$$p" 2>/dev/null || \ - cp "$$execdir/git$X" "$$execdir/$$p" || exit; } \ + if test -z "$(SKIP_DASHED_BUILT_INS)"; \ + then \ + test -n "$(INSTALL_SYMLINKS)" && \ + ln -s "$$destdir_from_execdir_SQ/$(bindir_relative_SQ)/git$X" "$$execdir/$$p" || \ + { test -z "$(NO_INSTALL_HARDLINKS)" && \ + ln "$$execdir/git$X" "$$execdir/$$p" 2>/dev/null || \ + ln -s "git$X" "$$execdir/$$p" 2>/dev/null || \ + cp "$$execdir/git$X" "$$execdir/$$p" || exit; }; \ + fi \ done && \ remote_curl_aliases="$(REMOTE_CURL_ALIASES)" && \ for p in $$remote_curl_aliases; do \ @@ -3076,7 +3095,7 @@ ifneq ($(INCLUDE_DLLS_IN_ARTIFACTS),) OTHER_PROGRAMS += $(shell echo *.dll t/helper/*.dll) endif -artifacts-tar:: $(ALL_PROGRAMS) $(SCRIPT_LIB) $(BUILT_INS) $(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) \ @@ -3171,7 +3190,7 @@ endif ### Check documentation # -ALL_COMMANDS = $(ALL_PROGRAMS) $(SCRIPT_LIB) $(BUILT_INS) +ALL_COMMANDS = $(ALL_COMMANDS_TO_INSTALL) $(SCRIPT_LIB) ALL_COMMANDS += git ALL_COMMANDS += git-citool ALL_COMMANDS += git-gui @@ -3211,7 +3230,7 @@ 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; \ -- cgit v1.2.3 From ef60e9f74b2d3638281da8affd4c854eead258b1 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Mon, 21 Sep 2020 22:28:17 +0000 Subject: ci: stop linking built-ins to the dashed versions Since e4597aae6590 (run test suite without dashed git-commands in PATH, 2009-12-02), we stopped running our tests with `git-foo` binaries found at the top-level directory of a freshly built source tree; instead we have placed only `git` and selected `git-foo` commands that must be on `$PATH` in `bin-wrappers/` and prepended that `bin-wrappers/` to the `PATH` used in the test suite. We did that to catch the tests and scripted Git commands that still try to use the dashed form. Since CI jobs will not install the built Git to anywhere, and the hardlinks we make at the top-level of the source tree for `git-add` and friends are not even used during tests, they are pure waste of resources these days. Thanks to the newly invented `SKIP_DASHED_BUILT_INS` knob, we can now skip creating these links in the source tree. So let's do that. Note that this change introduces a subtle change of behavior: when Git's `cmd_main()` calls `setup_path()`, it inserts the value of `GIT_EXEC_PATH` (defaulting to `/libexec/git-core`) at the beginning of the environment variable `PATH`. This is necessary to find e.g. scripted commands that are installed in that location. For the purposes of Git's test suite, the `bin-wrappers/` scripts override `GIT_EXEC_PATH` to point to the top-level directory of the source code. In other words, if a scripted command had used a dashed invocation of a built-in Git command, it would not have been caught previously, which is fixed by this change. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- ci/lib.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/ci/lib.sh b/ci/lib.sh index 3eefec500d..821e3660d6 100755 --- a/ci/lib.sh +++ b/ci/lib.sh @@ -178,6 +178,7 @@ fi export DEVELOPER=1 export DEFAULT_TEST_TARGET=prove export GIT_TEST_CLONE_2GB=true +export SKIP_DASHED_BUILT_INS=YesPlease case "$jobname" in linux-clang|linux-gcc) -- cgit v1.2.3