Hi,
On Fri, 2 Nov 2018, Ævar Arnfjörð Bjarmason wrote:
> Move a 37 line for-loop mess out of "install" and into a helper
> script. This started out fairly innocent but over the years has grown
> into a hard-to-maintain monster, and my recent ad874608d8 ("Makefile:
> optionally symlink libexec/git-core binaries to bin/git", 2018-03-13)
> certainly didn't help.
>
> The shell code is ported pretty much as-is (with getopts added), it'll
> be fixed & prettified in subsequent commits.
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx>
> ---
> Makefile | 52 ++++++++--------------------
> install_programs | 89 ++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 103 insertions(+), 38 deletions(-)
> create mode 100755 install_programs
>
> diff --git a/Makefile b/Makefile
> index bbfbb4292d..aa6ca1fa68 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -2808,44 +2808,20 @@ endif
> bindir=$$(cd '$(DESTDIR_SQ)$(bindir_SQ)' && pwd) && \
> execdir=$$(cd '$(DESTDIR_SQ)$(gitexec_instdir_SQ)' && pwd) && \
> destdir_from_execdir_SQ=$$(echo '$(gitexecdir_relative_SQ)' | sed -e 's|[^/][^/]*|..|g') && \
> - { 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; } \
> - done; \
> - } && \
> - 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; } \
> - 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; } \
> - done && \
> - remote_curl_aliases="$(REMOTE_CURL_ALIASES)" && \
> - for p in $$remote_curl_aliases; do \
> - $(RM) "$$execdir/$$p" && \
> - test -n "$(INSTALL_SYMLINKS)" && \
> - ln -s "git-remote-http$X" "$$execdir/$$p" || \
> - { test -z "$(NO_INSTALL_HARDLINKS)" && \
> - ln "$$execdir/git-remote-http$X" "$$execdir/$$p" 2>/dev/null || \
> - ln -s "git-remote-http$X" "$$execdir/$$p" 2>/dev/null || \
> - cp "$$execdir/git-remote-http$X" "$$execdir/$$p" || exit; } \
> - done && \
This indeed looks like a mess...
> + ./install_programs \
> + --X="$$X" \
> + --RM="$(RM)" \
> + --bindir="$$bindir" \
> + --bindir-relative="$(bindir_relative_SQ)" \
> + --execdir="$$execdir" \
> + --destdir-from-execdir="$$destdir_from_execdir_SQ" \
> + --flag-install-symlinks="$(INSTALL_SYMLINKS)" \
> + --flag-no-install-hardlinks="$(NO_INSTALL_HARDLINKS)" \
> + --flag-no-cross-directory-hardlinks="$(NO_CROSS_DIRECTORY_HARDLINKS)" \
> + --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)" \
> + --list-execdir-curl-aliases="$(REMOTE_CURL_ALIASES)" && \
> ./check_bindir "z$$bindir" "z$$execdir" "$$bindir/git-add$X"
>
> .PHONY: install-gitweb install-doc install-man install-man-perl install-html install-info install-pdf
> diff --git a/install_programs b/install_programs
> new file mode 100755
> index 0000000000..e287108112
> --- /dev/null
> +++ b/install_programs
> @@ -0,0 +1,89 @@
> +#!/bin/sh
> +
> +while test $# != 0
> +do
> + case "$1" in
> + --X=*)
> + X="${1#--X=}"
> + ;;
> + --RM=*)
> + RM="${1#--RM=}"
> + ;;
> + --bindir=*)
> + bindir="${1#--bindir=}"
> + ;;
> + --bindir-relative=*)
> + bindir_relative="${1#--bindir-relative=}"
> + ;;
> + --execdir=*)
> + execdir="${1#--execdir=}"
> + ;;
> + --destdir-from-execdir=*)
> + destdir_from_execdir="${1#--destdir-from-execdir=}"
> + ;;
> + --flag-install-symlinks=*)
> + INSTALL_SYMLINKS="${1#--flag-install-symlinks=}"
> + ;;
> + --flag-no-install-hardlinks=*)
> + NO_INSTALL_HARDLINKS="${1#--flag-no-install-hardlinks=}"
> + ;;
> + --flag-no-cross-directory-hardlinks=*)
> + NO_CROSS_DIRECTORY_HARDLINKS="${1#--flag-no-cross-directory-hardlinks=}"
> + ;;
> + --list-bindir-standalone=*)
> + list_bindir_standalone="${1#--list-bindir-standalone=}"
> + ;;
> + --list-bindir-git-dashed=*)
> + list_bindir_git_dashed="${1#--list-bindir-git-dashed=}"
> + ;;
> + --list-execdir-git-dashed=*)
> + list_execdir_git_dashed="${1#--list-execdir-git-dashed=}"
> + ;;
> + --list-execdir-curl-aliases=*)
> + list_execdir_curl_aliases="${1#--list-execdir-curl-aliases=}"
> + ;;
> +
> + *)
> + echo "Unknown option $1"
> + exit 1
> + ;;
> + esac
> + shift
> +done &&
> +{ test "$bindir/" = "$execdir/" ||
> + for p in $list_bindir_standalone; do
> + $RM "$execdir/$p" &&
> + test -n "$INSTALL_SYMLINKS" &&
> + ln -s "$destdir_from_execdir/$bindir_relative/$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; }
> + done;
> +} &&
> +for p in $list_bindir_git_dashed; 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; }
> +done &&
> +for p in $list_execdir_git_dashed; do
> + $RM "$execdir/$p" &&
> + 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" 2>/dev/null ||
> + ln -s "git$X" "$execdir/$p" 2>/dev/null ||
> + cp "$execdir/git$X" "$execdir/$p" || exit; }
> +done &&
> +for p in $list_execdir_curl_aliases; do
> + $RM "$execdir/$p" &&
> + test -n "$INSTALL_SYMLINKS" &&
> + ln -s "git-remote-http$X" "$execdir/$p" ||
> + { test -z "$NO_INSTALL_HARDLINKS" &&
> + ln "$execdir/git-remote-http$X" "$execdir/$p" 2>/dev/null ||
> + ln -s "git-remote-http$X" "$execdir/$p" 2>/dev/null ||
> + cp "$execdir/git-remote-http$X" "$execdir/$p" || exit; }
> +done
... but so does this. I would be very surprised if these four very
similar-looking constructs could not be refactored into a single shell
script that is then called four times with different parameters.
Something like
#!/bin/sh
from=
while case "$1" in
--no-hardlinks)
NO_INSTALL_HARDLINKS=t
;;
--from=*)
from="${1#*=}"
;;
*)
break
;;
esac; do
shift
done
test $# -gt 3 || {
echo "Usage: $0 [--no-hardlinks] <from-dir> <to-dir> <file>..." >&2
exit 1
}
fromdir="$1"
todir="$2"
shift
shift
for p in "$@"
do
$RM "$todir/$p" &&
test -n "$INSTALL_SYMLINKS" &&
ln -s "$fromdir/${from:-$p}" "$todir/$p" ||
{ test -z "$NO_INSTALL_HARDLINKS" &&
ln "$fromdir/${from:-$p}" "$todir/$p" ||
ln -s "$fromdir/${from:-$p}" "$todir/$p" ||
cp "$fromdir/${from:-$p}" "$todir/$p" || exit; }
done
and then calling it using
test "$bindir/" = "$execdir/" ||
link-or-copy ${NO_CROSS_DIRECTORY_HARDLINKS:+--no-hardlinks} \
"$bindir" "$execdir" $list_bindir_standalone
link-or-copy --from=git$X "$bindir" "$bindir" $list_bindir_git_dashed
link-or-copy --from=git$X "$bindir" "$execdir" $list_bindir_git_dashed
link-or-copy --from=git-remote-http$X "$bindir" "$execdir" $list_execdir_curl_aliases
That would at least DRY up this mess a bit.
Ciao,
Dscho
> --
> 2.19.1.930.g4563a0d9d0
>
>
