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
next prev parent 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.