diff mbox series

[v2,1/3] Makefile: factor sed-powered '#!/bin/sh' munging into a variable

Message ID patch-v2-1.3-fc6c5a6a8df-20221026T143534Z-avarab@gmail.com (mailing list archive)
State New, archived
Headers show
Series Makefile: fix issues with bin-wrappers/% rule | expand

Commit Message

Ævar Arnfjörð Bjarmason Oct. 26, 2022, 2:42 p.m. UTC
Reduce the amount of magical copy/pasting in the Makefile by factoring
the munging of "#!/bin/sh" on the first line of a shellscript into a
variable we can re-use in the various rules that need to do so.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Makefile | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

Comments

Junio C Hamano Oct. 26, 2022, 5:51 p.m. UTC | #1
Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> Reduce the amount of magical copy/pasting in the Makefile by factoring
> the munging of "#!/bin/sh" on the first line of a shellscript into a
> variable we can re-use in the various rules that need to do so.

At least when taken standalone, this looks more like replacing one
magical copy pasting with another magical one, the difference being
that the latter is not immediately obvious without referring back to
the definition of the variable.

If we need to change the replacement wholesale in a later step, then
it might give us a good trade off, but otherwise I am not sure why
this is a good idea that is worth the churn.  

When adding or updating these actions, in the original, you can typo
SHELL_PATH_SQ, but you can typo cmd_munge_script_sed_shell_path_arg
in the updated to break it the same way.  So...?

> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>  Makefile | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 85f03c6aed1..45b22d33513 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -2344,8 +2344,12 @@ GIT-SCRIPT-DEFINES: FORCE
>  		echo "$$FLAGS" >$@; \
>              fi
>  
> +define cmd_munge_script_sed_shell_path_arg
> +'1s|#!.*/sh|#!$(SHELL_PATH_SQ)|'
> +endef
> +
>  define cmd_munge_script
> -sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \
> +sed -e $(call cmd_munge_script_sed_shell_path_arg) \
>      -e 's|@SHELL_PATH@|$(SHELL_PATH_SQ)|' \
>      -e 's|@@DIFF@@|$(DIFF_SQ)|' \
>      -e 's|@@LOCALEDIR@@|$(localedir_SQ)|g' \
> @@ -2447,7 +2451,7 @@ git-instaweb: git-instaweb.sh GIT-SCRIPT-DEFINES
>  else # NO_PERL
>  $(SCRIPT_PERL_GEN) git-instaweb: % : unimplemented.sh
>  	$(QUIET_GEN) \
> -	sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \
> +	sed -e $(call cmd_munge_script_sed_shell_path_arg) \
>  	    -e 's|@@REASON@@|NO_PERL=$(NO_PERL)|g' \
>  	    unimplemented.sh >$@+ && \
>  	chmod +x $@+ && \
> @@ -2468,7 +2472,7 @@ $(SCRIPT_PYTHON_GEN): % : %.py
>  else # NO_PYTHON
>  $(SCRIPT_PYTHON_GEN): % : unimplemented.sh
>  	$(QUIET_GEN) \
> -	sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \
> +	sed -e $(call cmd_munge_script_sed_shell_path_arg) \
>  	    -e 's|@@REASON@@|NO_PYTHON=$(NO_PYTHON)|g' \
>  	    unimplemented.sh >$@+ && \
>  	chmod +x $@+ && \
> @@ -3061,7 +3065,7 @@ all:: $(TEST_PROGRAMS) $(test_bindir_programs)
>  
>  bin-wrappers/%: wrap-for-bin.sh
>  	$(call mkdir_p_parent_template)
> -	$(QUIET_GEN)sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \
> +	$(QUIET_GEN)sed -e $(call cmd_munge_script_sed_shell_path_arg) \
>  	     -e 's|@@BUILD_DIR@@|$(shell pwd)|' \
>  	     -e 's|@@PROG@@|$(patsubst test-%,t/helper/test-%,$(@F))$(if $(filter-out $(BINDIR_PROGRAMS_NO_X),$(@F)),$(X),)|' < $< > $@ && \
>  	chmod +x $@
diff mbox series

Patch

diff --git a/Makefile b/Makefile
index 85f03c6aed1..45b22d33513 100644
--- a/Makefile
+++ b/Makefile
@@ -2344,8 +2344,12 @@  GIT-SCRIPT-DEFINES: FORCE
 		echo "$$FLAGS" >$@; \
             fi
 
+define cmd_munge_script_sed_shell_path_arg
+'1s|#!.*/sh|#!$(SHELL_PATH_SQ)|'
+endef
+
 define cmd_munge_script
-sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \
+sed -e $(call cmd_munge_script_sed_shell_path_arg) \
     -e 's|@SHELL_PATH@|$(SHELL_PATH_SQ)|' \
     -e 's|@@DIFF@@|$(DIFF_SQ)|' \
     -e 's|@@LOCALEDIR@@|$(localedir_SQ)|g' \
@@ -2447,7 +2451,7 @@  git-instaweb: git-instaweb.sh GIT-SCRIPT-DEFINES
 else # NO_PERL
 $(SCRIPT_PERL_GEN) git-instaweb: % : unimplemented.sh
 	$(QUIET_GEN) \
-	sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \
+	sed -e $(call cmd_munge_script_sed_shell_path_arg) \
 	    -e 's|@@REASON@@|NO_PERL=$(NO_PERL)|g' \
 	    unimplemented.sh >$@+ && \
 	chmod +x $@+ && \
@@ -2468,7 +2472,7 @@  $(SCRIPT_PYTHON_GEN): % : %.py
 else # NO_PYTHON
 $(SCRIPT_PYTHON_GEN): % : unimplemented.sh
 	$(QUIET_GEN) \
-	sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \
+	sed -e $(call cmd_munge_script_sed_shell_path_arg) \
 	    -e 's|@@REASON@@|NO_PYTHON=$(NO_PYTHON)|g' \
 	    unimplemented.sh >$@+ && \
 	chmod +x $@+ && \
@@ -3061,7 +3065,7 @@  all:: $(TEST_PROGRAMS) $(test_bindir_programs)
 
 bin-wrappers/%: wrap-for-bin.sh
 	$(call mkdir_p_parent_template)
-	$(QUIET_GEN)sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \
+	$(QUIET_GEN)sed -e $(call cmd_munge_script_sed_shell_path_arg) \
 	     -e 's|@@BUILD_DIR@@|$(shell pwd)|' \
 	     -e 's|@@PROG@@|$(patsubst test-%,t/helper/test-%,$(@F))$(if $(filter-out $(BINDIR_PROGRAMS_NO_X),$(@F)),$(X),)|' < $< > $@ && \
 	chmod +x $@