[RFC/PATCH,5/5] Makefile: Add a NO_INSTALL_BUILTIN_EXECDIR_ALIASES flag
diff mbox series

Message ID 20181102223743.4331-6-avarab@gmail.com
State New
Headers show
Series
  • [RFC/PATCH,1/5] Makefile: move long inline shell loops in "install" into helper
Related show

Commit Message

Ævar Arnfjörð Bjarmason Nov. 2, 2018, 10:37 p.m. UTC
Back when git was initially written the likes of "git-add", "git-init"
etc. were installed in the user's $PATH. A few years later everything,
with a few exceptions like git-upload-pack and git-receive-pack, was
expected to be invoked as "git $cmd".

Now something like a decade later we're still installing these old
commands in gitexecdir. This is so someone with a shellscript that
still targets e.g. "git-init" can add $(git --exec-path) to their
$PATH and not have to change their script.

Let's add an option to break this backwards compatibility. Now with
NO_INSTALL_BUILTIN_EXECDIR_ALIASES=YesPlease there's only 3 programs
in the bindir that are hardlinked to "git" (receive-pack,
upload-archive & upload-pack), and 3 in the
gitexecdir (git-remote-{ftp,ftps,https} linked to git-remote-http).

There's no cross-directory links anymore, so the
"NO_CROSS_DIRECTORY_HARDLINKS" flag becomes redundant under this new
option.

1. https://public-inbox.org/git/87woyfdkoi.fsf@evledraar.gmail.com/

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Makefile         |  8 ++++++++
 install_programs | 36 +++++++++++++++++++++---------------
 2 files changed, 29 insertions(+), 15 deletions(-)

Comments

Eric Sunshine Nov. 4, 2018, 1:04 a.m. UTC | #1
On Fri, Nov 2, 2018 at 6:38 PM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:
> Let's add an option to break this backwards compatibility. Now with
> NO_INSTALL_BUILTIN_EXECDIR_ALIASES=YesPlease there's only 3 programs
> in the bindir that are hardlinked to "git" (receive-pack,
> upload-archive & upload-pack), and 3 in the
> gitexecdir (git-remote-{ftp,ftps,https} linked to git-remote-http).
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
> diff --git a/Makefile b/Makefile
> @@ -346,6 +346,13 @@ all::
> +# Define NO_INSTALL_BUILTIN_EXECDIR_ALIASES if you'd like to skip
> +# installing legacy such as "git-init" and "git-add" in the
> +# gitexecdir. Unless you're on a system where "which git-init" is
> +# expected to returns something set this. Users have been expected to

s/returns/return/
s/something/&,/

Although, it's not clear what "return something" means. Perhaps rephrase it as:

   ...git-init is expected to exist, set this.

> +# use the likes of "git init" for ages now, these programs were only
> +# provided for legacy compatibility.
> +#
Johannes Schindelin Nov. 12, 2018, 2:14 p.m. UTC | #2
Hi Ævar,

On Fri, 2 Nov 2018, Ævar Arnfjörð Bjarmason wrote:

> Back when git was initially written the likes of "git-add", "git-init"
> etc. were installed in the user's $PATH. A few years later everything,
> with a few exceptions like git-upload-pack and git-receive-pack, was
> expected to be invoked as "git $cmd".
> 
> Now something like a decade later we're still installing these old
> commands in gitexecdir. This is so someone with a shellscript that
> still targets e.g. "git-init" can add $(git --exec-path) to their
> $PATH and not have to change their script.
> 
> Let's add an option to break this backwards compatibility. Now with
> NO_INSTALL_BUILTIN_EXECDIR_ALIASES=YesPlease there's only 3 programs
> in the bindir that are hardlinked to "git" (receive-pack,
> upload-archive & upload-pack), and 3 in the
> gitexecdir (git-remote-{ftp,ftps,https} linked to git-remote-http).
> 
> There's no cross-directory links anymore, so the
> "NO_CROSS_DIRECTORY_HARDLINKS" flag becomes redundant under this new
> option.
> 
> 1. https://public-inbox.org/git/87woyfdkoi.fsf@evledraar.gmail.com/
> 
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---

I like the idea.

With my suggested refactoring that avoids the non-DRY code, this patch
would also become much simpler (as would 2/5 -- 4/5).

However, I would not call these "aliases". That's just confusing. Maybe
NO_INSTALL_DASHED_BUILTINS would be better? It certainly would not have
confused me.

Ciao,
Dscho

>  Makefile         |  8 ++++++++
>  install_programs | 36 +++++++++++++++++++++---------------
>  2 files changed, 29 insertions(+), 15 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 07c8b74353..a849a7b6d1 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -346,6 +346,13 @@ all::
>  # INSTALL_SYMLINKS if you'd prefer not to have the install procedure
>  # fallack on hardlinking or copying if "ln -s" fails.
>  #
> +# Define NO_INSTALL_BUILTIN_EXECDIR_ALIASES if you'd like to skip
> +# installing legacy such as "git-init" and "git-add" in the
> +# gitexecdir. Unless you're on a system where "which git-init" is
> +# expected to returns something set this. Users have been expected to
> +# use the likes of "git init" for ages now, these programs were only
> +# provided for legacy compatibility.
> +#
>  # Define NO_CROSS_DIRECTORY_HARDLINKS if you plan to distribute the installed
>  # programs as a tar, where bin/ and libexec/ might be on different file systems.
>  #
> @@ -2823,6 +2830,7 @@ endif
>  		--flag-no-install-hardlinks="$(NO_INSTALL_HARDLINKS)" \
>  		--flag-no-cross-directory-hardlinks="$(NO_CROSS_DIRECTORY_HARDLINKS)" \
>  		--flag-no-install-symlinks-fallback="$(NO_INSTALL_SYMLINKS_FALLBACK)" \
> +		--flag-no-install-builtin-execdir-aliases="$(NO_INSTALL_BUILTIN_EXECDIR_ALIASES)" \
>  		--list-bindir-standalone="git$X $(filter $(install_bindir_programs),$(ALL_PROGRAMS))" \
>  		--list-bindir-git-dashed="$(filter $(install_bindir_programs),$(BUILT_INS))" \
>  		--list-execdir-git-dashed="$(BUILT_INS)" \
> diff --git a/install_programs b/install_programs
> index 51e08019dd..8d89cd9984 100755
> --- a/install_programs
> +++ b/install_programs
> @@ -33,6 +33,9 @@ do
>  	--flag-no-install-symlinks-fallback=*)
>  		NO_INSTALL_SYMLINKS_FALLBACK="${1#--flag-no-install-symlinks-fallback=}"
>  		;;
> +	--flag-no-install-builtin-execdir-aliases=*)
> +		NO_INSTALL_BUILTIN_EXECDIR_ALIASES="${1#--flag-no-install-builtin-execdir-aliases=}"
> +		;;
>  	--list-bindir-standalone=*)
>  		list_bindir_standalone="${1#--list-bindir-standalone=}"
>  		;;
> @@ -54,7 +57,7 @@ do
>  	shift
>  done &&
>  
> -if test "$bindir/" != "$execdir/"
> +if test "$bindir/" != "$execdir/" -a -z "$NO_INSTALL_BUILTIN_EXECDIR_ALIASES"
>  then
>  	for p in $list_bindir_standalone; do
>  		$RM "$execdir/$p" &&
> @@ -87,20 +90,23 @@ do
>  	fi
>  done &&
>  
> -for p in $list_execdir_git_dashed; do
> -	$RM "$execdir/$p" &&
> -	if test -n "$INSTALL_SYMLINKS" -a -n "$NO_INSTALL_SYMLINKS_FALLBACK"
> -	then
> -		ln -s "$destdir_from_execdir/$bindir_relative/git$X" "$execdir/$p"
> -	else
> -		test -n "$INSTALL_SYMLINKS" &&
> -		ln -s "$destdir_from_execdir/$bindir_relative/git$X" "$execdir/$p" ||
> -		{ test -z "$NO_INSTALL_HARDLINKS" &&
> -		  ln "$execdir/git$X" "$execdir/$p" ||
> -		  ln -s "git$X" "$execdir/$p" ||
> -		  cp "$execdir/git$X" "$execdir/$p" || exit; }
> -	fi
> -done &&
> +if test -z "$NO_INSTALL_BUILTIN_EXECDIR_ALIASES"
> +then
> +	for p in $list_execdir_git_dashed; do
> +		$RM "$execdir/$p" &&
> +		if test -n "$INSTALL_SYMLINKS" -a -n "$NO_INSTALL_SYMLINKS_FALLBACK"
> +		then
> +			ln -s "$destdir_from_execdir/$bindir_relative/git$X" "$execdir/$p"
> +		else
> +			test -n "$INSTALL_SYMLINKS" &&
> +			ln -s "$destdir_from_execdir/$bindir_relative/git$X" "$execdir/$p" ||
> +			{ test -z "$NO_INSTALL_HARDLINKS" &&
> +			  ln "$execdir/git$X" "$execdir/$p" ||
> +			  ln -s "git$X" "$execdir/$p" ||
> +			  cp "$execdir/git$X" "$execdir/$p" || exit; }
> +		fi
> +	done
> +fi &&
>  
>  for p in $list_execdir_curl_aliases; do
>  	$RM "$execdir/$p" &&
> -- 
> 2.19.1.930.g4563a0d9d0
> 
>

Patch
diff mbox series

diff --git a/Makefile b/Makefile
index 07c8b74353..a849a7b6d1 100644
--- a/Makefile
+++ b/Makefile
@@ -346,6 +346,13 @@  all::
 # INSTALL_SYMLINKS if you'd prefer not to have the install procedure
 # fallack on hardlinking or copying if "ln -s" fails.
 #
+# Define NO_INSTALL_BUILTIN_EXECDIR_ALIASES if you'd like to skip
+# installing legacy such as "git-init" and "git-add" in the
+# gitexecdir. Unless you're on a system where "which git-init" is
+# expected to returns something set this. Users have been expected to
+# use the likes of "git init" for ages now, these programs were only
+# provided for legacy compatibility.
+#
 # Define NO_CROSS_DIRECTORY_HARDLINKS if you plan to distribute the installed
 # programs as a tar, where bin/ and libexec/ might be on different file systems.
 #
@@ -2823,6 +2830,7 @@  endif
 		--flag-no-install-hardlinks="$(NO_INSTALL_HARDLINKS)" \
 		--flag-no-cross-directory-hardlinks="$(NO_CROSS_DIRECTORY_HARDLINKS)" \
 		--flag-no-install-symlinks-fallback="$(NO_INSTALL_SYMLINKS_FALLBACK)" \
+		--flag-no-install-builtin-execdir-aliases="$(NO_INSTALL_BUILTIN_EXECDIR_ALIASES)" \
 		--list-bindir-standalone="git$X $(filter $(install_bindir_programs),$(ALL_PROGRAMS))" \
 		--list-bindir-git-dashed="$(filter $(install_bindir_programs),$(BUILT_INS))" \
 		--list-execdir-git-dashed="$(BUILT_INS)" \
diff --git a/install_programs b/install_programs
index 51e08019dd..8d89cd9984 100755
--- a/install_programs
+++ b/install_programs
@@ -33,6 +33,9 @@  do
 	--flag-no-install-symlinks-fallback=*)
 		NO_INSTALL_SYMLINKS_FALLBACK="${1#--flag-no-install-symlinks-fallback=}"
 		;;
+	--flag-no-install-builtin-execdir-aliases=*)
+		NO_INSTALL_BUILTIN_EXECDIR_ALIASES="${1#--flag-no-install-builtin-execdir-aliases=}"
+		;;
 	--list-bindir-standalone=*)
 		list_bindir_standalone="${1#--list-bindir-standalone=}"
 		;;
@@ -54,7 +57,7 @@  do
 	shift
 done &&
 
-if test "$bindir/" != "$execdir/"
+if test "$bindir/" != "$execdir/" -a -z "$NO_INSTALL_BUILTIN_EXECDIR_ALIASES"
 then
 	for p in $list_bindir_standalone; do
 		$RM "$execdir/$p" &&
@@ -87,20 +90,23 @@  do
 	fi
 done &&
 
-for p in $list_execdir_git_dashed; do
-	$RM "$execdir/$p" &&
-	if test -n "$INSTALL_SYMLINKS" -a -n "$NO_INSTALL_SYMLINKS_FALLBACK"
-	then
-		ln -s "$destdir_from_execdir/$bindir_relative/git$X" "$execdir/$p"
-	else
-		test -n "$INSTALL_SYMLINKS" &&
-		ln -s "$destdir_from_execdir/$bindir_relative/git$X" "$execdir/$p" ||
-		{ test -z "$NO_INSTALL_HARDLINKS" &&
-		  ln "$execdir/git$X" "$execdir/$p" ||
-		  ln -s "git$X" "$execdir/$p" ||
-		  cp "$execdir/git$X" "$execdir/$p" || exit; }
-	fi
-done &&
+if test -z "$NO_INSTALL_BUILTIN_EXECDIR_ALIASES"
+then
+	for p in $list_execdir_git_dashed; do
+		$RM "$execdir/$p" &&
+		if test -n "$INSTALL_SYMLINKS" -a -n "$NO_INSTALL_SYMLINKS_FALLBACK"
+		then
+			ln -s "$destdir_from_execdir/$bindir_relative/git$X" "$execdir/$p"
+		else
+			test -n "$INSTALL_SYMLINKS" &&
+			ln -s "$destdir_from_execdir/$bindir_relative/git$X" "$execdir/$p" ||
+			{ test -z "$NO_INSTALL_HARDLINKS" &&
+			  ln "$execdir/git$X" "$execdir/$p" ||
+			  ln -s "git$X" "$execdir/$p" ||
+			  cp "$execdir/git$X" "$execdir/$p" || exit; }
+		fi
+	done
+fi &&
 
 for p in $list_execdir_curl_aliases; do
 	$RM "$execdir/$p" &&