git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jonathan Nieder <jrnieder@gmail.com>
To: Jeff King <peff@peff.net>
Cc: Junio C Hamano <gitster@pobox.com>,
	Michael Haggerty <mhagger@alum.mit.edu>,
	git@vger.kernel.org, Matthieu Moy <Matthieu.Moy@imag.fr>
Subject: Re: [PATCH] t0090: mark add-interactive test with PERL prerequisite
Date: Tue, 18 Nov 2014 10:38:38 -0800	[thread overview]
Message-ID: <20141118183838.GD6527@google.com> (raw)
In-Reply-To: <20141118174309.GB31672@peff.net>

Jeff King wrote:

> Subject: Makefile: have perl scripts depend on NO_PERL setting
[...]
> ---
>  Makefile | 3 +++
>  1 file changed, 3 insertions(+)

Gah.  Good catch.

Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>

[...]
> --- a/Makefile
> +++ b/Makefile
> @@ -1676,6 +1676,9 @@ git.res: git.rc GIT-VERSION-FILE
>  	  $(join -DMAJOR= -DMINOR=, $(wordlist 1,2,$(subst -, ,$(subst ., ,$(GIT_VERSION))))) \
>  	  -DGIT_VERSION="\\\"$(GIT_VERSION)\\\"" $< -o $@
>  
> +# This makes sure we depend on the NO_PERL setting itself.
> +$(patsubst %.perl,%,$(SCRIPT_PERL)): GIT-BUILD-OPTIONS
> +
>  ifndef NO_PERL
>  $(patsubst %.perl,%,$(SCRIPT_PERL)): perl/perl.mak

Why do these repeat the 'patsubst ...' expression instead of using
SCRIPT_PERL_GEN, by the way?

-- >8 --
Subject: Makefile: simplify by using SCRIPT_{PERL,SH}_GEN macros

SCRIPT_PERL_GEN is defined as $(patsubst %.perl,%,$(SCRIPT_PERL)) for
use in targets like build-perl-script used by makefiles in
subdirectories that override SCRIPT_PERL (see v1.8.2-rc0~17^2,
"git-remote-mediawiki: use toplevel's Makefile", 2013-02-08).

The same expression is used in the rules that actually write the
generated perl scripts, and since this rules were introduced before
SCRIPT_PERL_GEN, they use the longhand instead of that macro.  Use the
macro to make reading easier.

Likewise for SCRIPT_SH_GEN.  The Python rules already got the same
simplification in v1.8.4-rc0~162^2~8 (2013-05-24).

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 Makefile | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/Makefile b/Makefile
index 0fa02ff..8f980e0 100644
--- a/Makefile
+++ b/Makefile
@@ -1662,7 +1662,7 @@ GIT-SCRIPT-DEFINES: FORCE
             fi
 
 
-$(patsubst %.sh,%,$(SCRIPT_SH)) : % : %.sh GIT-SCRIPT-DEFINES
+$(SCRIPT_SH_GEN) : % : %.sh GIT-SCRIPT-DEFINES
 	$(QUIET_GEN)$(cmd_munge_script) && \
 	chmod +x $@+ && \
 	mv $@+ $@
@@ -1677,10 +1677,10 @@ git.res: git.rc GIT-VERSION-FILE
 	  -DGIT_VERSION="\\\"$(GIT_VERSION)\\\"" $< -o $@
 
 # This makes sure we depend on the NO_PERL setting itself.
-$(patsubst %.perl,%,$(SCRIPT_PERL)): GIT-BUILD-OPTIONS
+$(SCRIPT_PERL_GEN): GIT-BUILD-OPTIONS
 
 ifndef NO_PERL
-$(patsubst %.perl,%,$(SCRIPT_PERL)): perl/perl.mak
+$(SCRIPT_PERL_GEN): perl/perl.mak
 
 perl/perl.mak: perl/PM.stamp
 
@@ -1693,7 +1693,7 @@ perl/perl.mak: GIT-CFLAGS GIT-PREFIX perl/Makefile perl/Makefile.PL
 	$(QUIET_SUBDIR0)perl $(QUIET_SUBDIR1) PERL_PATH='$(PERL_PATH_SQ)' prefix='$(prefix_SQ)' $(@F)
 
 PERL_DEFINES = $(PERL_PATH_SQ):$(PERLLIB_EXTRA_SQ)
-$(patsubst %.perl,%,$(SCRIPT_PERL)): % : %.perl perl/perl.mak GIT-PERL-DEFINES GIT-VERSION-FILE
+$(SCRIPT_PERL_GEN): % : %.perl perl/perl.mak GIT-PERL-DEFINES GIT-VERSION-FILE
 	$(QUIET_GEN)$(RM) $@ $@+ && \
 	INSTLIBDIR=`MAKEFLAGS= $(MAKE) -C perl -s --no-print-directory instlibdir` && \
 	INSTLIBDIR_EXTRA='$(PERLLIB_EXTRA_SQ)' && \
@@ -1727,7 +1727,7 @@ git-instaweb: git-instaweb.sh gitweb GIT-SCRIPT-DEFINES
 	chmod +x $@+ && \
 	mv $@+ $@
 else # NO_PERL
-$(patsubst %.perl,%,$(SCRIPT_PERL)) git-instaweb: % : unimplemented.sh
+$(SCRIPT_PERL_GEN) git-instaweb: % : unimplemented.sh
 	$(QUIET_GEN)$(RM) $@ $@+ && \
 	sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \
 	    -e 's|@@REASON@@|NO_PERL=$(NO_PERL)|g' \
-- 
2.1.0.rc2.206.gedb03e5

  reply	other threads:[~2014-11-18 18:38 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-11-18 17:22 [PATCH] t0090: mark add-interactive test with PERL prerequisite Jeff King
2014-11-18 17:29 ` [PATCH] t960[34]: mark cvsimport tests as requiring perl Jeff King
2014-11-18 18:56   ` Jonathan Nieder
2014-11-18 19:15     ` Jeff King
2014-11-18 17:43 ` [PATCH] t0090: mark add-interactive test with PERL prerequisite Jeff King
2014-11-18 18:38   ` Jonathan Nieder [this message]
2014-11-18 18:43     ` Jonathan Nieder
2014-11-18 18:49       ` Jeff King
2014-11-18 23:10       ` Pete Wyckoff
2014-11-18 18:44     ` Jeff King
2014-11-18 18:51 ` Jonathan Nieder

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

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

  git send-email \
    --in-reply-to=20141118183838.GD6527@google.com \
    --to=jrnieder@gmail.com \
    --cc=Matthieu.Moy@imag.fr \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=mhagger@alum.mit.edu \
    --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.
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 NNTP newsgroup(s).