Re: [PATCH 04/16] Makefile: move Perl-only variable assignments under !NO_PERL
From: Junio C Hamano <hidden>
Date: 2021-11-09 23:23:01
Ævar Arnfjörð Bjarmason [off-list ref] 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.
quoted hunk
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 ...
quoted hunk
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?
quoted hunk
+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.