git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 3/6] Makefile: refactor out "ln || ln -s || cp" pattern

From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date
Mar 29, 2021, 16:31 UTC
Message-ID
<patch-3.7-bde9de756b4-20210329T162327Z-avarab@gmail.com>
In-Reply-To
<cover-0.7-00000000000-20210329T162327Z-avarab@gmail.com>

Refactor out the hard-to-read and maintain "ln || ln -s || cp" pattern.

This was initially added in 3e073dc5611 (Makefile: always provide a fallback when hardlinks fail, 2008-08-25), but since then it's become a lot more complex as we've added:

 * 3426e34fedd (Add NO_CROSS_DIRECTORY_HARDLINKS support to the
   Makefile, 2009-05-11)
 * NO_INSTALL_HARDLINKS in 70de5e65e8c (Makefile:
   NO_INSTALL_HARDLINKS, 2012-05-02)
 * INSTALL_SYMLINKS in ad874608d8c (Makefile: optionally symlink
   libexec/git-core binaries to bin/git, 2018-03-13)
 * SKIP_DASHED_BUILT_INS 179227d6e21 (Optionally skip linking/copying
   the built-ins, 2020-09-21)

Each of those commits had to add a new special-case to this code, resulting in quite an unmaintainable mess for adding any sort of new option.

Let's use the newly introduced ln-or-cp.sh script instead, note that we only sometimes pass the --no-cross-directory-hardlinks option, per the previous behavior. The target of the "ln -s" is also another special snowflake, but we're careful to carry that forward.

As in an earlier commit this also changes the behavior to emit any errors to stdout. In that earlier case that was done to simplify the script so that it can use one "ln -s" instead of the two, likewise we're now unconditionally emitting to stderr if ln (without -s, to create the hardlink) fails. We always emitted to stderr if "cp" failed.

As in that earlier commit let's let that pass for now, yes it might be very verbose in some scenarios, but we're working our way towards a simpler end-state here.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Makefile    | 41 +++++++++++++++++-----------------
 ln-or-cp.sh | 63 ++++++++++++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 81 insertions(+), 23 deletions(-)
diff --git a/Makefile b/Makefile
index cfc87d7734d..a4784f28f5b 100644
--- a/Makefile
+++ b/Makefile
@@ -3007,40 +3007,41 @@ endif
 	{ test "$$bindir/" = "$$execdir/" || \
 	  for p in git$X $(filter $(install_bindir_programs),$(ALL_PROGRAMS)); do \
 		$(RM) "$$execdir/$$p" && \
-		test -n "$(INSTALL_SYMLINKS)" && \
-		ln -s "$$destdir_from_execdir_SQ/$(bindir_relative_SQ)/$$p" "$$execdir/$$p" || \
-		{ test -z "$(NO_INSTALL_HARDLINKS)$(NO_CROSS_DIRECTORY_HARDLINKS)" && \
-		  ln "$$bindir/$$p" "$$execdir/$$p" 2>/dev/null || \
-		  cp "$$bindir/$$p" "$$execdir/$$p" || exit; } \
+		./ln-or-cp.sh \
+			--install-symlinks "$(INSTALL_SYMLINKS)" \
+			--no-install-hardlinks "$(NO_INSTALL_HARDLINKS)" \
+			--no-cross-directory-hardlinks "$(NO_CROSS_DIRECTORY_HARDLINKS)" \
+			--symlink-target "$$destdir_from_execdir_SQ/$(bindir_relative_SQ)/$$p" \
+			"$$bindir/$$p" "$$execdir/$$p"; \
 	  done; \
 	} && \
 	for p in $(filter $(install_bindir_programs),$(BUILT_INS)); do \
 		$(RM) "$$bindir/$$p" && \
-		test -n "$(INSTALL_SYMLINKS)" -o "$(NO_INSTALL_HARDLINKS)" && \
-		ln -s "git$X" "$$bindir/$$p" || \
-		{ test -z "$(NO_INSTALL_HARDLINKS)" && \
-		  ln "$$bindir/git$X" "$$bindir/$$p" 2>/dev/null || \
-		  cp "$$bindir/git$X" "$$bindir/$$p" || exit; }; \
+		./ln-or-cp.sh \
+			--install-symlinks "$(INSTALL_SYMLINKS)" \
+			--no-install-hardlinks "$(NO_INSTALL_HARDLINKS)" \
+			--symlink-target "git$X" \
+			"$$bindir/git$X" "$$bindir/$$p"; \
 	done && \
 	for p in $(BUILT_INS); do \
 		$(RM) "$$execdir/$$p" && \
 		if test -z "$(SKIP_DASHED_BUILT_INS)"; \
 		then \
-			test -n "$(INSTALL_SYMLINKS)" -o "$(NO_INSTALL_HARDLINKS)" && \
-			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 || \
-			  cp "$$execdir/git$X" "$$execdir/$$p" || exit; }; \
+			./ln-or-cp.sh \
+				--install-symlinks "$(INSTALL_SYMLINKS)" \
+				--no-install-hardlinks "$(NO_INSTALL_HARDLINKS)" \
+				--symlink-target "$$destdir_from_execdir_SQ/$(bindir_relative_SQ)/git$X" \
+				"$$execdir/git$X" "$$execdir/$$p"; \
 		fi \
 	done && \
 	remote_curl_aliases="$(REMOTE_CURL_ALIASES)" && \
 	for p in $$remote_curl_aliases; do \
 		$(RM) "$$execdir/$$p" && \
-		test -n "$(INSTALL_SYMLINKS)" -o "$(NO_INSTALL_HARDLINKS)" && \
-		ln -s "git-remote-http$X" "$$execdir/$$p" || \
-		{ test -z "$(NO_INSTALL_HARDLINKS)" && \
-		  ln "$$execdir/git-remote-http$X" "$$execdir/$$p" 2>/dev/null || \
-		  cp "$$execdir/git-remote-http$X" "$$execdir/$$p" || exit; } \
+		./ln-or-cp.sh \
+			--install-symlinks "$(INSTALL_SYMLINKS)" \
+			--no-install-hardlinks "$(NO_INSTALL_HARDLINKS)" \
+			--symlink-target "git-remote-http$X" \
+			"$$execdir/git-remote-http$X" "$$execdir/$$p"; \
 	done && \
 	./check_bindir "z$$bindir" "z$$execdir" "$$bindir/git-add$X"
 
diff --git a/ln-or-cp.sh b/ln-or-cp.sh
index de79cd85a81..663ffd0489d 100755
--- a/ln-or-cp.sh
+++ b/ln-or-cp.sh
@@ -1,8 +1,65 @@
 #!/bin/sh
 
+install_symlinks=
+no_install_hardlinks=
+no_cross_directory_hardlinks=
+symlink_target=
+while test $# != 0
+do
+	case "$1" in
+	--install-symlinks)
+		install_symlinks="$2"
+		shift
+		;;
+	--no-install-hardlinks)
+		no_install_hardlinks="$2"
+		shift
+		;;
+	--no-cross-directory-hardlinks)
+		no_cross_directory_hardlinks="$2"
+		shift
+		;;
+	--symlink-target)
+		symlink_target="$2"
+		shift
+		;;
+	*)
+		break
+		;;
+	esac
+	shift
+done
+
 target="$1"
+if test -z "$symlink_target"
+then
+	symlink_target="$target"
+fi
 link="$2"
 
-ln "$target" "$link" 2>/dev/null ||
-ln -s "$target" "$link" 2>/dev/null ||
-cp "$target" "$link"
+hardlink_or_cp () {
+	if test -z "$no_install_hardlinks" -a -z "$no_cross_directory_hardlinks"
+	then
+		if ! ln "$target" "$link"
+		then
+			cp "$target" "$link"
+		fi
+
+	else
+		cp "$target" "$link"
+	fi
+}
+
+main_with_fallbacks () {
+	if test -n "$install_symlinks" -o -n "$no_install_hardlinks"
+	then
+		if ! ln -s "$symlink_target" "$link"
+		then
+			hardlink_or_cp
+		fi
+	else
+		hardlink_or_cp
+	fi
+}
+
+main_with_fallbacks
-- 
2.31.1.461.gd47399f6574
Previous: Ævar Arnfjörð BjarmasonNext: Junio C Hamano
Message 42 of 48 in “Makefile: generate 'git' as 'cc [...] -o git+ && mv git+ git'”
  1. Makefile: generate 'git' as 'cc [...] -o git+ && mv git+ git'Ævar Arnfjörð Bjarmason, Mar 7, 2021
  2. Junio C HamanoMar 7, 2021
  3. Ævar Arnfjörð BjarmasonMar 8, 2021
  4. Junio C HamanoMar 8, 2021
  5. Jeff KingMar 8, 2021
  6. 0/5 Makefile: don't die on AIX with open ./gitÆvar Arnfjörð Bjarmason, Mar 29, 2021
  7. 1/5 Makefile: rename objects in-place, don't clobberÆvar Arnfjörð Bjarmason, Mar 29, 2021
  8. Junio C HamanoMar 29, 2021
  9. Junio C HamanoMar 29, 2021
  10. Ævar Arnfjörð BjarmasonMar 29, 2021
  11. Junio C HamanoMar 30, 2021
  12. Ævar Arnfjörð BjarmasonMar 31, 2021
  13. Junio C HamanoMar 31, 2021
  14. 4/5 Makefile: add the ".DELETE_ON_ERROR" flagÆvar Arnfjörð Bjarmason, Mar 29, 2021
  15. Junio C HamanoMar 29, 2021
  16. Ævar Arnfjörð BjarmasonMar 29, 2021
  17. Junio C HamanoMar 29, 2021
  18. Jeff KingMar 30, 2021
  19. Junio C HamanoMar 30, 2021
  20. Jeff KingMar 31, 2021
  21. Junio C HamanoMar 31, 2021
  22. Johannes SchindelinMar 31, 2021
  23. 3/5 Makefile: don't needlessly "rm $@ $@+" before "mv $@+ $@"Ævar Arnfjörð Bjarmason, Mar 29, 2021
  24. Junio C HamanoMar 29, 2021
  25. 2/5 Makefile: rename scripts in-place, don't clobberÆvar Arnfjörð Bjarmason, Mar 29, 2021
  26. Junio C HamanoMar 29, 2021
  27. Ævar Arnfjörð BjarmasonMar 29, 2021
  28. 5/5 Makefile: don't "rm configure" before generating itÆvar Arnfjörð Bjarmason, Mar 29, 2021
  29. 0/6 Makefile: make non-symlink & non-hardlink install saneÆvar Arnfjörð Bjarmason, Mar 29, 2021
  30. 1/6 Makefile: symlink the same way under "symlinks" and "no hardlinks"Ævar Arnfjörð Bjarmason, Mar 29, 2021
  31. Junio C HamanoMar 29, 2021
  32. 5/6 Makefile: use "ln -f" instead of "rm && ln"Ævar Arnfjörð Bjarmason, Mar 29, 2021
  33. Junio C HamanoMar 29, 2021
  34. 4/6 Makefile: make INSTALL_SYMLINKS affect the build directoryÆvar Arnfjörð Bjarmason, Mar 29, 2021
  35. Junio C HamanoMar 29, 2021
  36. Ævar Arnfjörð BjarmasonMar 31, 2021
  37. 6/6 Makefile: add a INSTALL_FALLBACK_LN_CP modeÆvar Arnfjörð Bjarmason, Mar 29, 2021
  38. Junio C HamanoMar 29, 2021
  39. Ævar Arnfjörð BjarmasonMar 31, 2021
  40. Junio C HamanoMar 31, 2021
  41. Ævar Arnfjörð BjarmasonMar 31, 2021
  42. 3/6 Makefile: refactor out "ln || ln -s || cp" patternÆvar Arnfjörð Bjarmason, Mar 29, 2021
  43. Junio C HamanoMar 29, 2021
  44. Jeff KingMar 30, 2021
  45. Junio C HamanoMar 30, 2021
  46. 2/6 Makefile: begin refactoring out "ln || ln -s || cp" patternÆvar Arnfjörð Bjarmason, Mar 29, 2021
  47. Junio C HamanoMar 29, 2021
  48. Junio C HamanoMar 29, 2021

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.