git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: git@vger.kernel.org
Cc: "Junio C Hamano" <gitster@pobox.com>, "Jeff King" <peff@peff.net>,
	"Andreas Färber" <andreas.faerber@web.de>,
	"Johannes Schindelin" <Johannes.Schindelin@gmx.de>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Subject: [PATCH 3/6] Makefile: refactor out "ln || ln -s || cp" pattern
Date: Mon, 29 Mar 2021 18:31:41 +0200	[thread overview]
Message-ID: <patch-3.7-bde9de756b4-20210329T162327Z-avarab@gmail.com> (raw)
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


  parent reply	other threads:[~2021-03-29 16:33 UTC|newest]

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-07 13:20 [PATCH] Makefile: generate 'git' as 'cc [...] -o git+ && mv git+ git' Ævar Arnfjörð Bjarmason
2021-03-07 20:41 ` Junio C Hamano
2021-03-08 12:38   ` Ævar Arnfjörð Bjarmason
2021-03-08 17:21     ` Junio C Hamano
2021-03-08 18:26     ` Jeff King
2021-03-29 16:20 ` [PATCH v2 0/5] Makefile: don't die on AIX with open ./git Ævar Arnfjörð Bjarmason
2021-03-29 16:20   ` [PATCH v2 1/5] Makefile: rename objects in-place, don't clobber Ævar Arnfjörð Bjarmason
2021-03-29 18:21     ` Junio C Hamano
2021-03-29 18:26       ` Junio C Hamano
2021-03-29 23:24       ` Ævar Arnfjörð Bjarmason
2021-03-30  0:21         ` Junio C Hamano
2021-03-31 14:17           ` Ævar Arnfjörð Bjarmason
2021-03-31 18:50             ` Junio C Hamano
2021-03-29 16:20   ` [PATCH v2 2/5] Makefile: rename scripts " Ævar Arnfjörð Bjarmason
2021-03-29 18:40     ` Junio C Hamano
2021-03-29 23:28       ` Ævar Arnfjörð Bjarmason
2021-03-29 16:20   ` [PATCH v2 3/5] Makefile: don't needlessly "rm $@ $@+" before "mv $@+ $@" Ævar Arnfjörð Bjarmason
2021-03-29 18:46     ` Junio C Hamano
2021-03-29 16:20   ` [PATCH v2 4/5] Makefile: add the ".DELETE_ON_ERROR" flag Ævar Arnfjörð Bjarmason
2021-03-29 18:51     ` Junio C Hamano
2021-03-29 23:31       ` Ævar Arnfjörð Bjarmason
2021-03-29 23:58         ` Junio C Hamano
2021-03-30 15:11         ` Jeff King
2021-03-30 18:36           ` Junio C Hamano
2021-03-31  6:58             ` Jeff King
2021-03-31 18:36               ` Junio C Hamano
2021-03-31 22:29                 ` Johannes Schindelin
2021-03-29 16:20   ` [PATCH v2 5/5] Makefile: don't "rm configure" before generating it Ævar Arnfjörð Bjarmason
2021-03-29 16:31   ` [PATCH 0/6] Makefile: make non-symlink & non-hardlink install sane Ævar Arnfjörð Bjarmason
2021-03-29 16:31     ` [PATCH 1/6] Makefile: symlink the same way under "symlinks" and "no hardlinks" Ævar Arnfjörð Bjarmason
2021-03-29 22:17       ` Junio C Hamano
2021-03-29 16:31     ` [PATCH 2/6] Makefile: begin refactoring out "ln || ln -s || cp" pattern Ævar Arnfjörð Bjarmason
2021-03-29 22:20       ` Junio C Hamano
2021-03-29 16:31     ` Ævar Arnfjörð Bjarmason [this message]
2021-03-29 22:24       ` [PATCH 3/6] Makefile: refactor " Junio C Hamano
2021-03-30 15:20         ` Jeff King
2021-03-30 18:36           ` Junio C Hamano
2021-03-29 16:31     ` [PATCH 4/6] Makefile: make INSTALL_SYMLINKS affect the build directory Ævar Arnfjörð Bjarmason
2021-03-29 22:31       ` Junio C Hamano
2021-03-31 14:04         ` Ævar Arnfjörð Bjarmason
2021-03-29 16:31     ` [PATCH 5/6] Makefile: use "ln -f" instead of "rm && ln" Ævar Arnfjörð Bjarmason
2021-03-29 22:46       ` Junio C Hamano
2021-03-29 16:31     ` [PATCH 6/6] Makefile: add a INSTALL_FALLBACK_LN_CP mode Ævar Arnfjörð Bjarmason
2021-03-29 22:53       ` Junio C Hamano
2021-03-31 14:03         ` Ævar Arnfjörð Bjarmason
2021-03-31 18:45           ` Junio C Hamano
2021-03-31 19:01             ` Ævar Arnfjörð Bjarmason
2021-03-29 23:08     ` [PATCH 0/6] Makefile: make non-symlink & non-hardlink install sane Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=patch-3.7-bde9de756b4-20210329T162327Z-avarab@gmail.com \
    --to=avarab@gmail.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=andreas.faerber@web.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).