diff mbox series

[04/16] Makefile: move Perl-only variable assignments under !NO_PERL

Message ID patch-04.16-5affe94b05f-20211106T205717Z-avarab@gmail.com (mailing list archive)
State Superseded
Headers show
Series Makefiles: dependency correctness & speedup | expand

Commit Message

Ævar Arnfjörð Bjarmason Nov. 6, 2021, 9:03 p.m. UTC
Move the assignment to perl_localedir_SQ to be guarded by NO_PERL, and
furthermore only assign to it if RUNTIME_PREFIX is in effect. If the
latter isn't being used then we'll default to the empty string, so
there was no need for the second assignment added in
07d90eadb50 (Makefile: add Perl runtime prefix support, 2018-04-10).

Similarly, we can move the PERL_HEADER_TEMPLATE assignment inside the
"!NO_PERL" block, and having simplified all of this let's consolidate
three comments on this control flow into one.

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

Comments

Junio C Hamano Nov. 9, 2021, 11:22 p.m. UTC | #1
Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> -# Used for substitution in Perl modules. Disabled when using RUNTIME_PREFIX
> -# since the locale directory is injected.
> -perl_localedir_SQ = $(localedir_SQ)
> -

So, we used to unconditionally define this even if we had NO_PERL;
the patch is going to move it inside "ifndef NO_PERL", which makes
sense.

>  ifndef NO_PERL
> -PERL_HEADER_TEMPLATE = perl/header_templates/fixed_prefix.template.pl
>  PERL_DEFINES =
>  PERL_DEFINES += $(PERL_PATH_SQ)
>  PERL_DEFINES += $(PERLLIB_EXTRA_SQ)
> @@ -2305,16 +2300,15 @@ PERL_DEFINES += $(RUNTIME_PREFIX)
>  PERL_DEFINES += $(NO_PERL_CPAN_FALLBACKS)
>  PERL_DEFINES += $(NO_GETTEXT)
>  
> -# Support Perl runtime prefix. In this mode, a different header is installed
> -# into Perl scripts.
> +# Under RUNTIME_PREFIX we inject a header into the Perl scripts; If
> +# NO_GETTEXT is not defined we'll make use of the localedir.

 * The second sentence after ';' should not begin with capital.  

 * "if X we do Y" wants a comma before the latter sentence.

 * "if NO_FOO is not defined" has too many negations.

Taking them together, I wonder if this would be more readable.

    ... into the Perl scripts; unless NO_GETTEXT is defined, we'll
    make use of ...

>  ifdef RUNTIME_PREFIX
> -
>  PERL_HEADER_TEMPLATE = perl/header_templates/runtime_prefix.template.pl
> -
> -# Don't export a fixed $(localedir) path; it will be resolved by the Perl header
> -# at runtime.
> -perl_localedir_SQ =

Is it a good idea to lose this?  We would get affected by a stray
environment variable of the same name without it, no?

> +ifndef NO_GETTEXT
> +perl_localedir_SQ = $(localedir_SQ)
> +endif


> +else

This "else" pairs with... ah, RUNTIME_PREFIX.  And of course we will
use the "fixed" variant only when RUNTIME_PREFIX is not in use.

> +PERL_HEADER_TEMPLATE = perl/header_templates/fixed_prefix.template.pl
>  endif

Nice. now we have one HEADER_TEMPLATE definition on each branch
between "using runtime-prefix" and "not using runtime-prefix", which
makes sense.
diff mbox series

Patch

diff --git a/Makefile b/Makefile
index 009b08152b5..fd4fe6c1045 100644
--- a/Makefile
+++ b/Makefile
@@ -2291,12 +2291,7 @@  git.res: git.rc GIT-VERSION-FILE GIT-PREFIX
 # This makes sure we depend on the NO_PERL setting itself.
 $(SCRIPT_PERL_GEN): GIT-BUILD-OPTIONS
 
-# Used for substitution in Perl modules. Disabled when using RUNTIME_PREFIX
-# since the locale directory is injected.
-perl_localedir_SQ = $(localedir_SQ)
-
 ifndef NO_PERL
-PERL_HEADER_TEMPLATE = perl/header_templates/fixed_prefix.template.pl
 PERL_DEFINES =
 PERL_DEFINES += $(PERL_PATH_SQ)
 PERL_DEFINES += $(PERLLIB_EXTRA_SQ)
@@ -2305,16 +2300,15 @@  PERL_DEFINES += $(RUNTIME_PREFIX)
 PERL_DEFINES += $(NO_PERL_CPAN_FALLBACKS)
 PERL_DEFINES += $(NO_GETTEXT)
 
-# Support Perl runtime prefix. In this mode, a different header is installed
-# into Perl scripts.
+# Under RUNTIME_PREFIX we inject a header into the Perl scripts; If
+# NO_GETTEXT is not defined we'll make use of the localedir.
 ifdef RUNTIME_PREFIX
-
 PERL_HEADER_TEMPLATE = perl/header_templates/runtime_prefix.template.pl
-
-# Don't export a fixed $(localedir) path; it will be resolved by the Perl header
-# at runtime.
-perl_localedir_SQ =
-
+ifndef NO_GETTEXT
+perl_localedir_SQ = $(localedir_SQ)
+endif
+else
+PERL_HEADER_TEMPLATE = perl/header_templates/fixed_prefix.template.pl
 endif
 
 PERL_DEFINES += $(gitexecdir) $(perllibdir) $(localedir)