Re: [RFC/PATCH 1/5] Makefile: move long inline shell loops in "install" into helper

2018-11-16 Thread Ævar Arnfjörð Bjarmason 


On Mon, Nov 12 2018, Johannes Schindelin wrote:

> Hi Ævar,
>
> On Mon, 12 Nov 2018, Ævar Arnfjörð Bjarmason wrote:
>
>> On Mon, Nov 12 2018, Johannes Schindelin wrote:
>>
>> > 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 
>> >> ---
>> >>  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 00..e287108112
>> >> --- /dev/null
>> >> +++ b/install_programs
>> >> @@ -0,0 +1,89 @@
>> >> +#!/bin/sh
>> >> +
>> >> +while test $# != 0
>> >> +do
>> >> + case "$1" in
>> >> + --X=*)
>> >> +

Re: [RFC/PATCH 1/5] Makefile: move long inline shell loops in "install" into helper

2018-11-12 Thread Johannes Schindelin 
Hi Ævar,

On Mon, 12 Nov 2018, Ævar Arnfjörð Bjarmason wrote:

> On Mon, Nov 12 2018, Johannes Schindelin wrote:
> 
> > 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 
> >> ---
> >>  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 00..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=}"
> >> +  ;;

Re: [RFC/PATCH 1/5] Makefile: move long inline shell loops in "install" into helper

2018-11-12 Thread Ævar Arnfjörð Bjarmason 


On Mon, Nov 12 2018, Johannes Schindelin wrote:

> 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 
>> ---
>>  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 00..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=*)
>> +

Re: [RFC/PATCH 1/5] Makefile: move long inline shell loops in "install" into helper

2018-11-12 Thread Johannes Schindelin 
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 
> ---
>  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 00..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=}"
> +

Re: [RFC/PATCH 1/5] Makefile: move long inline shell loops in "install" into helper

2018-11-03 Thread Eric Sunshine 
'sb/filenames-with-dashes'On Fri, Nov 2, 2018 at 6:38 PM Æ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 
> ---
>  install_programs | 89 

Pure nitpick: Earlier this year, Stefan made an effort[1] to eradicate
filenames with underscores and replace them with hyphenated filenames.
Perhaps name this "install-programs", instead.

[1]: sb/filenames-with-dashes

> diff --git a/install_programs b/install_programs
> @@ -0,0 +1,89 @@
> +while test $# != 0
> +do
> +   case "$1" in
> +   --X=*)
> +   X="${1#--X=}"
> +   ;;
> +   --RM=*)
> +   RM="${1#--RM=}"
> +   ;;
> +   --bindir=*)
> +   bindir="${1#--bindir=}"
> +   ;;

Is the intention that the user might have X, RM, 'bindir', etc.
already in the environment, and the switches in this script merely
override those values? Or is the intention that X, RM, 'bindir, etc.
should all start out unset? If the latter, perhaps start the script
with an initialization block which clears all these variables first:

X=
RM=
bindir=
...

[RFC/PATCH 1/5] Makefile: move long inline shell loops in "install" into helper

2018-11-02 Thread Ævar Arnfjörð Bjarmason 
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 
---
 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 && \
+   ./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 00..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=*)
+