{"thread":{"id":"55622","subject":"[PATCH 0/4] Makefile/perl: correctly re-generate build/* + speed up under NO_GETTEXT=Y","startedAt":"2021-05-05T12:21:51Z","lastAt":"2021-05-12T23:28:01Z","messageCount":26,"participants":["Ævar Arnfjörð Bjarmason","Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"423662","messageId":"cover-0.4-00000000000-20210505T121857Z-avarab@gmail.com","threadId":"55622","inReplyTo":null,"subject":"[PATCH 0/4] Makefile/perl: correctly re-generate build/* + speed up under NO_GETTEXT=Y","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-05-05T12:21:37Z","receivedAt":"2021-05-05T12:21:51Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"These patches are relatively trivial fixes for bugs noticed when I was\nworking on recent send-email patches. We don't re-build perl/build/*\nassets when variables that affect them are changed, and needlessly use\nour non-mock gettext Perl i18n framework under NO_GETTEXT=Y if the\nsystem happens to have Locale::Messages installed.\n\nÆvar Arnfjörð Bjarmason (4):\n Makefile: don't re-define PERL_DEFINES\n Makefile: regenerate perl/build/* if GIT-PERL-DEFINES changes\n Makefile: regenerate *.pm on NO_PERL_CPAN_FALLBACKS change\n perl: use mock i18n functions under NO_GETTEXT=Y\n\n Makefile | 13 +++++++++----\n perl/Git/I18N.pm | 10 ++++++++++\n 2 files changed, 19 insertions(+), 4 deletions(-)\n\n-- \n2.31.1.838.g7ac6e98bb53\n\n"},{"id":"423664","messageId":"patch-1.4-ed2005a2fbf-20210505T121857Z-avarab@gmail.com","threadId":"55622","inReplyTo":"cover-0.4-00000000000-20210505T121857Z-avarab@gmail.com","subject":"[PATCH 1/4] Makefile: don't re-define PERL_DEFINES","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-05-05T12:21:38Z","receivedAt":"2021-05-05T12:21:53Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Since 07d90eadb50 (Makefile: add Perl runtime prefix support,\n2018-04-10) we have been declaring PERL_DEFINES right after assigning\nto it, with the effect that the first PERL_DEFINES was ignored.\n\nThat bug didn't matter in practice since the first line had all the\nsame variables as the second, so we'd correctly re-generate\neverything. It just made for confusing reading.\n\nLet's remove that first assignment, and while we're at it split these\nacross lines to make them more maintainable.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n Makefile | 7 ++++---\n 1 file changed, 4 insertions(+), 3 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 93664d67146..1d4c02e59d9 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -2270,9 +2270,10 @@ perl_localedir_SQ = $(localedir_SQ)\n \n ifndef NO_PERL\n PERL_HEADER_TEMPLATE = perl/header_templates/fixed_prefix.template.pl\n-PERL_DEFINES = $(PERL_PATH_SQ):$(PERLLIB_EXTRA_SQ):$(perllibdir_SQ)\n-\n-PERL_DEFINES := $(PERL_PATH_SQ) $(PERLLIB_EXTRA_SQ) $(perllibdir_SQ)\n+PERL_DEFINES :=\n+PERL_DEFINES += $(PERL_PATH_SQ)\n+PERL_DEFINES += $(PERLLIB_EXTRA_SQ)\n+PERL_DEFINES += $(perllibdir_SQ)\n PERL_DEFINES += $(RUNTIME_PREFIX)\n \n # Support Perl runtime prefix. In this mode, a different header is installed\n-- \n2.31.1.838.g7ac6e98bb53\n\n"},{"id":"423663","messageId":"patch-3.4-06e25bc1db3-20210505T121857Z-avarab@gmail.com","threadId":"55622","inReplyTo":"cover-0.4-00000000000-20210505T121857Z-avarab@gmail.com","subject":"[PATCH 3/4] Makefile: regenerate *.pm on NO_PERL_CPAN_FALLBACKS change","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-05-05T12:21:40Z","receivedAt":"2021-05-05T12:21:56Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Regenerate the *.pm files in perl/build/* if the\nNO_PERL_CPAN_FALLBACKS flag added to the *.pm files in\n1aca69c0195 (perl Git::LoadCPAN: emit better errors under\nNO_PERL_CPAN_FALLBACKS, 2018-03-03) is changed.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n Makefile | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/Makefile b/Makefile\nindex a15f39e40ff..574b25512e7 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -2275,6 +2275,7 @@ PERL_DEFINES += $(PERL_PATH_SQ)\n PERL_DEFINES += $(PERLLIB_EXTRA_SQ)\n PERL_DEFINES += $(perllibdir_SQ)\n PERL_DEFINES += $(RUNTIME_PREFIX)\n+PERL_DEFINES += $(NO_PERL_CPAN_FALLBACKS)\n \n # Support Perl runtime prefix. In this mode, a different header is installed\n # into Perl scripts.\n-- \n2.31.1.838.g7ac6e98bb53\n\n"},{"id":"423665","messageId":"patch-2.4-49339028db4-20210505T121857Z-avarab@gmail.com","threadId":"55622","inReplyTo":"cover-0.4-00000000000-20210505T121857Z-avarab@gmail.com","subject":"[PATCH 2/4] Makefile: regenerate perl/build/* if GIT-PERL-DEFINES changes","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-05-05T12:21:39Z","receivedAt":"2021-05-05T12:21:58Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change the logic to generate perl/build/* to regenerate those files if\nGIT-PERL-DEFINES changes. This ensures that e.g. changing localedir\nwill result in correctly re-generated files.\n\nI don't think that ever worked. The brokenness pre-dates my\n20d2a30f8ff (Makefile: replace perl/Makefile.PL with simple make\nrules, 2017-12-10).\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n Makefile | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/Makefile b/Makefile\nindex 1d4c02e59d9..a15f39e40ff 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -2676,7 +2676,7 @@ endif\n NO_PERL_CPAN_FALLBACKS_SQ = $(subst ','\\'',$(NO_PERL_CPAN_FALLBACKS))\n endif\n \n-perl/build/lib/%.pm: perl/%.pm\n+perl/build/lib/%.pm: perl/%.pm GIT-PERL-DEFINES\n \t$(QUIET_GEN)mkdir -p $(dir $@) && \\\n \tsed -e 's|@@LOCALEDIR@@|$(perl_localedir_SQ)|g' \\\n \t -e 's|@@NO_PERL_CPAN_FALLBACKS@@|$(NO_PERL_CPAN_FALLBACKS_SQ)|g' \\\n-- \n2.31.1.838.g7ac6e98bb53\n\n"},{"id":"423666","messageId":"patch-4.4-97247cb72a5-20210505T121857Z-avarab@gmail.com","threadId":"55622","inReplyTo":"cover-0.4-00000000000-20210505T121857Z-avarab@gmail.com","subject":"[PATCH 4/4] perl: use mock i18n functions under NO_GETTEXT=Y","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-05-05T12:21:41Z","receivedAt":"2021-05-05T12:21:59Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change the logic of the i18n functions I added in 5e9637c6297 (i18n:\nadd infrastructure for translating Git with gettext, 2011-11-18) to\nuse pass-through functions when NO_GETTEXT is defined.\n\nThis speeds up the compilation time of commands that use this library\nwhen NO_GETTEXT=Y is in effect. Loading it and POSIX.pm is around 20ms\non my machine, whereas it takes 2ms to just instantiate perl itself.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n Makefile | 3 +++\n perl/Git/I18N.pm | 10 ++++++++++\n 2 files changed, 13 insertions(+)\n\ndiff --git a/Makefile b/Makefile\nindex 574b25512e7..b8d6b313056 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1986,6 +1986,7 @@ ETC_GITCONFIG_SQ = $(subst ','\\'',$(ETC_GITCONFIG))\n ETC_GITATTRIBUTES_SQ = $(subst ','\\'',$(ETC_GITATTRIBUTES))\n \n DESTDIR_SQ = $(subst ','\\'',$(DESTDIR))\n+NO_GETTEXT_SQ = $(subst ','\\'',$(NO_GETTEXT))\n bindir_SQ = $(subst ','\\'',$(bindir))\n bindir_relative_SQ = $(subst ','\\'',$(bindir_relative))\n mandir_SQ = $(subst ','\\'',$(mandir))\n@@ -2276,6 +2277,7 @@ PERL_DEFINES += $(PERLLIB_EXTRA_SQ)\n PERL_DEFINES += $(perllibdir_SQ)\n PERL_DEFINES += $(RUNTIME_PREFIX)\n PERL_DEFINES += $(NO_PERL_CPAN_FALLBACKS)\n+PERL_DEFINES += $(NO_GETTEXT)\n \n # Support Perl runtime prefix. In this mode, a different header is installed\n # into Perl scripts.\n@@ -2680,6 +2682,7 @@ endif\n perl/build/lib/%.pm: perl/%.pm GIT-PERL-DEFINES\n \t$(QUIET_GEN)mkdir -p $(dir $@) && \\\n \tsed -e 's|@@LOCALEDIR@@|$(perl_localedir_SQ)|g' \\\n+\t -e 's|@@NO_GETTEXT@@|$(NO_GETTEXT_SQ)|g' \\\n \t -e 's|@@NO_PERL_CPAN_FALLBACKS@@|$(NO_PERL_CPAN_FALLBACKS_SQ)|g' \\\n \t< $< > $@\n \ndiff --git a/perl/Git/I18N.pm b/perl/Git/I18N.pm\nindex 2037f387c89..895e759c57a 100644\n--- a/perl/Git/I18N.pm\n+++ b/perl/Git/I18N.pm\n@@ -16,9 +16,19 @@ BEGIN\n our @EXPORT = qw(__ __n N__);\n our @EXPORT_OK = @EXPORT;\n \n+# See Git::LoadCPAN's NO_PERL_CPAN_FALLBACKS_STR for a description of\n+# this \"'@@' [...] '@@'\" pattern.\n+use constant NO_GETTEXT_STR => '@@' . 'NO_GETTEXT' . '@@';\n+use constant NO_GETTEXT => (\n+\tq[@@NO_GETTEXT@@] ne ''\n+\tand\n+\tq[@@NO_GETTEXT@@] ne NO_GETTEXT_STR\n+);\n+\n sub __bootstrap_locale_messages {\n \tour $TEXTDOMAIN = 'git';\n \tour $TEXTDOMAINDIR ||= $ENV{GIT_TEXTDOMAINDIR} || '@@LOCALEDIR@@';\n+\tdie \"NO_GETTEXT=\" . NO_GETTEXT_STR if NO_GETTEXT;\n \n \trequire POSIX;\n \tPOSIX->import(qw(setlocale));\n-- \n2.31.1.838.g7ac6e98bb53\n\n"},{"id":"423685","messageId":"YJKm0dnwHBwQuTi+@coredump.intra.peff.net","threadId":"55622","inReplyTo":"patch-1.4-ed2005a2fbf-20210505T121857Z-avarab@gmail.com","subject":"Re: [PATCH 1/4] Makefile: don't re-define PERL_DEFINES","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-05-05T14:08:17Z","receivedAt":"2021-05-05T14:08:19Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, May 05, 2021 at 02:21:38PM +0200, Ævar Arnfjörð Bjarmason wrote:\n\n> Since 07d90eadb50 (Makefile: add Perl runtime prefix support,\n> 2018-04-10) we have been declaring PERL_DEFINES right after assigning\n> to it, with the effect that the first PERL_DEFINES was ignored.\n> \n> That bug didn't matter in practice since the first line had all the\n> same variables as the second, so we'd correctly re-generate\n> everything. It just made for confusing reading.\n> \n> Let's remove that first assignment, and while we're at it split these\n> across lines to make them more maintainable.\n\nThis and the other three patches all look sensible to me.\n\nI did have one question:\n\n> diff --git a/Makefile b/Makefile\n> index 93664d67146..1d4c02e59d9 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -2270,9 +2270,10 @@ perl_localedir_SQ = $(localedir_SQ)\n> \n> ifndef NO_PERL\n> PERL_HEADER_TEMPLATE = perl/header_templates/fixed_prefix.template.pl\n> -PERL_DEFINES = $(PERL_PATH_SQ):$(PERLLIB_EXTRA_SQ):$(perllibdir_SQ)\n> -\n> -PERL_DEFINES := $(PERL_PATH_SQ) $(PERLLIB_EXTRA_SQ) $(perllibdir_SQ)\n> +PERL_DEFINES :=\n> +PERL_DEFINES += $(PERL_PATH_SQ)\n> +PERL_DEFINES += $(PERLLIB_EXTRA_SQ)\n> +PERL_DEFINES += $(perllibdir_SQ)\n> PERL_DEFINES += $(RUNTIME_PREFIX)\n\nI don't think we generally use simply-expanded variables in our Makefile\nunless there's a reason. Do we actually need it here? Obviously not new\nin your patch, but just a curiosity I noticed while reading it.\n\n-Peff\n"},{"id":"423726","messageId":"xmqq4kfg3c4v.fsf@gitster.g","threadId":"55622","inReplyTo":"YJKm0dnwHBwQuTi+@coredump.intra.peff.net","subject":"Re: [PATCH 1/4] Makefile: don't re-define PERL_DEFINES","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-05-06T01:05:20Z","receivedAt":"2021-05-06T01:05:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> I did have one question:\n>\n>> diff --git a/Makefile b/Makefile\n>> index 93664d67146..1d4c02e59d9 100644\n>> --- a/Makefile\n>> +++ b/Makefile\n>> @@ -2270,9 +2270,10 @@ perl_localedir_SQ = $(localedir_SQ)\n>> \n>> ifndef NO_PERL\n>> PERL_HEADER_TEMPLATE = perl/header_templates/fixed_prefix.template.pl\n>> -PERL_DEFINES = $(PERL_PATH_SQ):$(PERLLIB_EXTRA_SQ):$(perllibdir_SQ)\n>> -\n>> -PERL_DEFINES := $(PERL_PATH_SQ) $(PERLLIB_EXTRA_SQ) $(perllibdir_SQ)\n>> +PERL_DEFINES :=\n>> +PERL_DEFINES += $(PERL_PATH_SQ)\n>> +PERL_DEFINES += $(PERLLIB_EXTRA_SQ)\n>> +PERL_DEFINES += $(perllibdir_SQ)\n>> PERL_DEFINES += $(RUNTIME_PREFIX)\n>\n> I don't think we generally use simply-expanded variables in our Makefile\n> unless there's a reason. Do we actually need it here? Obviously not new\n> in your patch, but just a curiosity I noticed while reading it.\n\nSplitting the appending into multiple lines does make sense, and is\nin line with what 07d90ead (Makefile: add Perl runtime prefix\nsupport, 2018-04-10) introduced the \"first create a space separated\nlist and then redefine that same variable with spaces replaced with\ncolons\" strategy to reach the final value (i.e. colon separated\ntokens that lets us notice when build options changed) for.\n\nAs to the simply-expanded vs recursively-expanded variable, there is\naneed to use former, which comes from what the original commit\n07d90ead did outside the context of this patch, which is:\n\n PERL_DEFINES := $(subst $(space),:,$(PERL_DEFINES))\n GIT-PERL-DEFINES: FORCE\n @FLAGS='$(PERL_DEFINES)'; \\\n if test x\"$$FLAGS\" != x\"`cat $@ 2>/dev/null`\" ; then \\\n echo >&2 \" * new perl-specific parameters\"; \\\n echo \"$$FLAGS\" >$@; \\\n fi\n\nThat is, up to this point PERL_DEFINES accumulate various build-time\nsettings with += (i.e. space separated tokens), and at this point\nfinally it is turned into a colon separated tokens, which cannot be\nwritten with a recursively expanded variable.\n\nBut I tend to agree that you do not have to := clear the list in\nthis patch.\n"},{"id":"423745","messageId":"87y2csv0qm.fsf@evledraar.gmail.com","threadId":"55622","inReplyTo":"YJKm0dnwHBwQuTi+@coredump.intra.peff.net","subject":"Re: [PATCH 1/4] Makefile: don't re-define PERL_DEFINES","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-05-06T06:23:28Z","receivedAt":"2021-05-06T06:24:16Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Wed, May 05 2021, Jeff King wrote:\n\n> On Wed, May 05, 2021 at 02:21:38PM +0200, Ævar Arnfjörð Bjarmason wrote:\n>\n>> Since 07d90eadb50 (Makefile: add Perl runtime prefix support,\n>> 2018-04-10) we have been declaring PERL_DEFINES right after assigning\n>> to it, with the effect that the first PERL_DEFINES was ignored.\n>> \n>> That bug didn't matter in practice since the first line had all the\n>> same variables as the second, so we'd correctly re-generate\n>> everything. It just made for confusing reading.\n>> \n>> Let's remove that first assignment, and while we're at it split these\n>> across lines to make them more maintainable.\n>\n> This and the other three patches all look sensible to me.\n>\n> I did have one question:\n>\n>> diff --git a/Makefile b/Makefile\n>> index 93664d67146..1d4c02e59d9 100644\n>> --- a/Makefile\n>> +++ b/Makefile\n>> @@ -2270,9 +2270,10 @@ perl_localedir_SQ = $(localedir_SQ)\n>> \n>> ifndef NO_PERL\n>> PERL_HEADER_TEMPLATE = perl/header_templates/fixed_prefix.template.pl\n>> -PERL_DEFINES = $(PERL_PATH_SQ):$(PERLLIB_EXTRA_SQ):$(perllibdir_SQ)\n>> -\n>> -PERL_DEFINES := $(PERL_PATH_SQ) $(PERLLIB_EXTRA_SQ) $(perllibdir_SQ)\n>> +PERL_DEFINES :=\n>> +PERL_DEFINES += $(PERL_PATH_SQ)\n>> +PERL_DEFINES += $(PERLLIB_EXTRA_SQ)\n>> +PERL_DEFINES += $(perllibdir_SQ)\n>> PERL_DEFINES += $(RUNTIME_PREFIX)\n>\n> I don't think we generally use simply-expanded variables in our Makefile\n> unless there's a reason. Do we actually need it here? Obviously not new\n> in your patch, but just a curiosity I noticed while reading it.\n\nI didn't notice it at the time. I suppose it could be changed to not do\nexpansion, but per-se unrelated to the more narrorw bugfix in this\npatch.\n"},{"id":"423755","messageId":"xmqq7dkcz20u.fsf@gitster.g","threadId":"55622","inReplyTo":"87y2csv0qm.fsf@evledraar.gmail.com","subject":"Re: [PATCH 1/4] Makefile: don't re-define PERL_DEFINES","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-05-06T08:42:41Z","receivedAt":"2021-05-06T08:42:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n>>> -PERL_DEFINES = $(PERL_PATH_SQ):$(PERLLIB_EXTRA_SQ):$(perllibdir_SQ)\n>>> -\n>>> -PERL_DEFINES := $(PERL_PATH_SQ) $(PERLLIB_EXTRA_SQ) $(perllibdir_SQ)\n>>> +PERL_DEFINES :=\n>>> +PERL_DEFINES += $(PERL_PATH_SQ)\n>>> +PERL_DEFINES += $(PERLLIB_EXTRA_SQ)\n>>> +PERL_DEFINES += $(perllibdir_SQ)\n>>> PERL_DEFINES += $(RUNTIME_PREFIX)\n>>\n>> I don't think we generally use simply-expanded variables in our Makefile\n>> unless there's a reason. Do we actually need it here? Obviously not new\n>> in your patch, but just a curiosity I noticed while reading it.\n>\n> I didn't notice it at the time. I suppose it could be changed to not do\n> expansion, but per-se unrelated to the more narrorw bugfix in this\n> patch.\n\nActually, strictly speaking there was *no* bug because assigning\nthree items with := made sure the previous recursively expanded one\nto be ineffective. In other words, there was a valid reason to use\n\":=\" there in the original version.\n\nNow your patch removed the recursively expanded one that was\nimmediately invalidated, there no longer is a reason to use :=\nthere. So \"unrelated to the more narrow bugfix\" is a rather lame\nexcuse to do only half a task. If we remove that extra one (which\nis a good thing), then we should correct := into = because the\noriginal used := only because there was the unwanted extra one, no?\n"},{"id":"423762","messageId":"87sg30usm9.fsf@evledraar.gmail.com","threadId":"55622","inReplyTo":"xmqq7dkcz20u.fsf@gitster.g","subject":"Re: [PATCH 1/4] Makefile: don't re-define PERL_DEFINES","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-05-06T09:04:34Z","receivedAt":"2021-05-06T09:19:33Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, May 06 2021, Junio C Hamano wrote:\n\n> Ævar Arnfjörð Bjarmason writes:\n>\n>>>> -PERL_DEFINES = $(PERL_PATH_SQ):$(PERLLIB_EXTRA_SQ):$(perllibdir_SQ)\n>>>> -\n>>>> -PERL_DEFINES := $(PERL_PATH_SQ) $(PERLLIB_EXTRA_SQ) $(perllibdir_SQ)\n>>>> +PERL_DEFINES :=\n>>>> +PERL_DEFINES += $(PERL_PATH_SQ)\n>>>> +PERL_DEFINES += $(PERLLIB_EXTRA_SQ)\n>>>> +PERL_DEFINES += $(perllibdir_SQ)\n>>>> PERL_DEFINES += $(RUNTIME_PREFIX)\n>>>\n>>> I don't think we generally use simply-expanded variables in our Makefile\n>>> unless there's a reason. Do we actually need it here? Obviously not new\n>>> in your patch, but just a curiosity I noticed while reading it.\n>>\n>> I didn't notice it at the time. I suppose it could be changed to not do\n>> expansion, but per-se unrelated to the more narrorw bugfix in this\n>> patch.\n>\n> Actually, strictly speaking there was *no* bug because assigning\n> three items with := made sure the previous recursively expanded one\n> to be ineffective. In other words, there was a valid reason to use\n> \":=\" there in the original version.\n\nYes, there wasn't any bug with the the eventual value being\nincorrect. I.e. both of these are equivalent in a Makefile:\n\n FOO = abc\n FOO := def\n FOO += ghi\n\nAnd:\n\n FOO = abc\n FOO = def\n FOO += ghi\n\nBoth will yield \"def ghi\". They're just different in a case like:\n \n X = Y\n FOO = abc\n FOO := $(X)\n X = Z\n FOO += ghi\n\nWhere using := will echo \"Y ghi\", and using = will echo \"Z ghi\". As a\npractical matter the distinction doesn't matter in this case.\n\n> Now your patch removed the recursively expanded one that was\n> immediately invalidated, there no longer is a reason to use :=\n> there. So \"unrelated to the more narrow bugfix\" is a rather lame\n> excuse to do only half a task. If we remove that extra one (which\n> is a good thing), then we should correct := into = because the\n> original used := only because there was the unwanted extra one, no?\n\nI don't see how removing the stray line changes the reason to use \":=\"\nor \"=\" there. I agree it should be removed, it's just unrelated to\nremoving the stay line. Looking at 07d90eadb50 it's clear that it's just\nsome copy/pasting error.\n\nMaybe the confusion is that I'm using \"bug\" closer to a meaning of \"a\nthing nobody intended to be in the program\", not just \"a\nbehavior-changing issue observable from the outside\".\n\nIn any case. I can just submit a patch on top of this in a v2. I\ncontinue to find it hard to discover the line between superfluous\nwhile-we're-at-it fixes in your mind v.s. \"we should fix this while\nwe're at it\" though :)\n\nBut regarding the \"half a task\" it seems to me that these are different\nissues; I don't think that's a point worth arguing in this case\nspecifically (let's just fix it, and I will), but perhaps I'm missing\nsomething subtle with regards to Makefile semantics per my examples\nabove so it really is all one issue, and I'd like to understand how\nthey're entwined.\n"},{"id":"423809","messageId":"YJQfnviIeK7UdCBy@coredump.intra.peff.net","threadId":"55622","inReplyTo":"xmqq4kfg3c4v.fsf@gitster.g","subject":"Re: [PATCH 1/4] Makefile: don't re-define PERL_DEFINES","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-05-06T16:55:58Z","receivedAt":"2021-05-06T16:56:00Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, May 06, 2021 at 10:05:20AM +0900, Junio C Hamano wrote:\n\n> As to the simply-expanded vs recursively-expanded variable, there is\n> aneed to use former, which comes from what the original commit\n> 07d90ead did outside the context of this patch, which is:\n> \n> PERL_DEFINES := $(subst $(space),:,$(PERL_DEFINES))\n> GIT-PERL-DEFINES: FORCE\n> @FLAGS='$(PERL_DEFINES)'; \\\n> if test x\"$$FLAGS\" != x\"`cat $@ 2>/dev/null`\" ; then \\\n> echo >&2 \" * new perl-specific parameters\"; \\\n> echo \"$$FLAGS\" >$@; \\\n> fi\n> \n> That is, up to this point PERL_DEFINES accumulate various build-time\n> settings with += (i.e. space separated tokens), and at this point\n> finally it is turned into a colon separated tokens, which cannot be\n> written with a recursively expanded variable.\n> \n> But I tend to agree that you do not have to := clear the list in\n> this patch.\n\nOK, that matches my understanding. Thanks for laying out.\n\nIn general, I would say that the later use that you quoted above would\ndo better to use a second variable (because then there is no question of\nwhen PERL_DEFINES is space-separated and when it is colon-separated).\nBut that is not that big a deal, and certainly very orthogonal to Ævar's\npatch.\n\nI'd also question whether the colon transformation is even necessary.\nThe point of the file is to change when the values change. Being sloppy\nwith delimiters means we _could_ miss a change, but in practice I doubt\nit is very likely (and it is not like colons cannot appear in values,\neither, so they are not foolproof). But again, not really important for\nthis patch.\n\n-Peff\n\nP.S. I also wondered briefly if make would preserve spaces even for\nempty variables (since without that, we might miss delimiters and\nconfuse one of the variables for another). I.e., we know that:\n\n FOO = $(one):$(two):$(three)\n\nwill have two colons even if some of the variables are empty. But does\nit preserve them even for \"$(one) $(two) $(three)\", or more importantly,\nwhen using \"+=\" (which _would_ be relevant to this patch)? The answer is\nyes, they are all fine.\n\nYou can demonstrate it with the Makefile below, running \"make one=foo\nthree=bar\", \"make two=foo\", etc.\n\n-- >8 --\nempty :=\nspace := $(empty) $(empty)\n\nCOLONS = $(one):$(two):$(three)\n\nSPACES_SINGLE = $(one) $(two) $(three)\nSPACES_SINGLE := $(subst $(space),:,$(SPACES_SINGLE))\n\nSPACES_PLUS =\nSPACES_PLUS += $(one)\nSPACES_PLUS += $(two)\nSPACES_PLUS += $(three)\nSPACES_PLUS := $(subst $(space),:,$(SPACES_PLUS))\n\nall:\n\t@echo \"COLONS=$(COLONS)\"\n\t@echo \"SPACES_SINGLE=$(SPACES_SINGLE)\"\n\t@echo \"SPACES_PLUS=$(SPACES_PLUS)\"\n"},{"id":"423810","messageId":"YJQgVsKdb+gh5Sep@coredump.intra.peff.net","threadId":"55622","inReplyTo":"87sg30usm9.fsf@evledraar.gmail.com","subject":"Re: [PATCH 1/4] Makefile: don't re-define PERL_DEFINES","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-05-06T16:59:02Z","receivedAt":"2021-05-06T16:59:05Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, May 06, 2021 at 11:04:34AM +0200, Ævar Arnfjörð Bjarmason wrote:\n\n> > Actually, strictly speaking there was *no* bug because assigning\n> > three items with := made sure the previous recursively expanded one\n> > to be ineffective. In other words, there was a valid reason to use\n> > \":=\" there in the original version.\n> \n> Yes, there wasn't any bug with the the eventual value being\n> incorrect. I.e. both of these are equivalent in a Makefile:\n> \n> FOO = abc\n> FOO := def\n> FOO += ghi\n> \n> And:\n> \n> FOO = abc\n> FOO = def\n> FOO += ghi\n> \n> Both will yield \"def ghi\". They're just different in a case like:\n> \n> X = Y\n> FOO = abc\n> FOO := $(X)\n> X = Z\n> FOO += ghi\n> \n> Where using := will echo \"Y ghi\", and using = will echo \"Z ghi\". As a\n> practical matter the distinction doesn't matter in this case.\n\nYeah, I don't think the \":=\" was impacting the bug or no bug (not to\nmention that even if we duplicated those entries in the variable, it\n_still_ wouldn't be a bug, since the whole point of the variable is just\nto notice when the content changes).\n\n> > Now your patch removed the recursively expanded one that was\n> > immediately invalidated, there no longer is a reason to use :=\n> > there. So \"unrelated to the more narrow bugfix\" is a rather lame\n> > excuse to do only half a task. If we remove that extra one (which\n> > is a good thing), then we should correct := into = because the\n> > original used := only because there was the unwanted extra one, no?\n> \n> I don't see how removing the stray line changes the reason to use \":=\"\n> or \"=\" there. I agree it should be removed, it's just unrelated to\n> removing the stay line. Looking at 07d90eadb50 it's clear that it's just\n> some copy/pasting error.\n\nYeah, I'd agree it is truly orthogonal. I don't mind seeing it cleaned\nup in addition (or am even actively happy to see it cleaned up :) ), but\nIMHO it would not need to hold up the series.\n\n-Peff\n"},{"id":"423865","messageId":"xmqqwnsbue7i.fsf@gitster.g","threadId":"55622","inReplyTo":"87sg30usm9.fsf@evledraar.gmail.com","subject":"Re: [PATCH 1/4] Makefile: don't re-define PERL_DEFINES","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-05-07T08:42:57Z","receivedAt":"2021-05-07T08:43:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> Maybe the confusion is that I'm using \"bug\" closer to a meaning of \"a\n> thing nobody intended to be in the program\", not just \"a\n> behavior-changing issue observable from the outside\".\n\nOK, thanks for explaining where my confusion came from.\n\n"},{"id":"424012","messageId":"patch-1.6-8b899364916-20210510T104937Z-avarab@gmail.com","threadId":"55622","inReplyTo":"cover-0.6-00000000000-20210510T104937Z-avarab@gmail.com","subject":"[PATCH v2 1/6] Makefile: regenerate perl/build/* if GIT-PERL-DEFINES changes","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-05-10T10:50:17Z","receivedAt":"2021-05-10T11:20:20Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change the logic to generate perl/build/* to regenerate those files if\nGIT-PERL-DEFINES changes. This ensures that e.g. changing localedir\nwill result in correctly re-generated files.\n\nI don't think that ever worked. The brokenness pre-dates my\n20d2a30f8ff (Makefile: replace perl/Makefile.PL with simple make\nrules, 2017-12-10).\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n Makefile | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/Makefile b/Makefile\nindex 93664d67146..ad618cea33f 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -2675,7 +2675,7 @@ endif\n NO_PERL_CPAN_FALLBACKS_SQ = $(subst ','\\'',$(NO_PERL_CPAN_FALLBACKS))\n endif\n \n-perl/build/lib/%.pm: perl/%.pm\n+perl/build/lib/%.pm: perl/%.pm GIT-PERL-DEFINES\n \t$(QUIET_GEN)mkdir -p $(dir $@) && \\\n \tsed -e 's|@@LOCALEDIR@@|$(perl_localedir_SQ)|g' \\\n \t -e 's|@@NO_PERL_CPAN_FALLBACKS@@|$(NO_PERL_CPAN_FALLBACKS_SQ)|g' \\\n-- \n2.31.1.838.g924d365b763\n\n"},{"id":"424015","messageId":"cover-0.6-00000000000-20210510T104937Z-avarab@gmail.com","threadId":"55622","inReplyTo":"cover-0.4-00000000000-20210505T121857Z-avarab@gmail.com","subject":"[PATCH v2 0/6] Makefile/perl: correctly re-generate build/* + speed up under NO_GETTEXT=Y","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-05-10T10:50:16Z","receivedAt":"2021-05-10T11:20:20Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"The summary, from v1:\n\n These patches are relatively trivial fixes for bugs noticed when I was\n working on recent send-email patches. We don't re-build perl/build/*\n assets when variables that affect them are changed, and needlessly use\n our non-mock gettext Perl i18n framework under NO_GETTEXT=Y if the\n system happens to have Locale::Messages installed.\n\nChanges since v1:\n\nThe only change to the end-state is the trivial change-on-top of:\n\n -PERL_DEFINES :=\n +PERL_DEFINES =\n\nI.e. the PERL_DEFINES is now also refactored away from a\nsimply-expanded variable. The re-arrangement and split-up of patches\nin this v2 just makes for a more incremental way to get there, per the\ndiscussion on v1.\n\nÆvar Arnfjörð Bjarmason (6):\n Makefile: regenerate perl/build/* if GIT-PERL-DEFINES changes\n Makefile: don't re-define PERL_DEFINES\n Makefile: make PERL_DEFINES recursively expanded\n Makefile: split up the deceleration of PERL_DEFINES\n Makefile: regenerate *.pm on NO_PERL_CPAN_FALLBACKS change\n perl: use mock i18n functions under NO_GETTEXT=Y\n\n Makefile | 13 +++++++++----\n perl/Git/I18N.pm | 10 ++++++++++\n 2 files changed, 19 insertions(+), 4 deletions(-)\n\nRange-diff against v1:\n2: 49339028db4 = 1: 8b899364916 Makefile: regenerate perl/build/* if GIT-PERL-DEFINES changes\n-: ----------- > 2: c15887bbc93 Makefile: don't re-define PERL_DEFINES\n-: ----------- > 3: 7919ae0e546 Makefile: make PERL_DEFINES recursively expanded\n1: ed2005a2fbf ! 4: 2cdefbe920c Makefile: don't re-define PERL_DEFINES\n @@ Metadata\n Author: Ævar Arnfjörð Bjarmason \n \n ## Commit message ##\n - Makefile: don't re-define PERL_DEFINES\n + Makefile: split up the deceleration of PERL_DEFINES\n \n - Since 07d90eadb50 (Makefile: add Perl runtime prefix support,\n - 2018-04-10) we have been declaring PERL_DEFINES right after assigning\n - to it, with the effect that the first PERL_DEFINES was ignored.\n + Split the declaration of PERL_DEFINES across multiple line, making\n + this easier to read.\n \n - That bug didn't matter in practice since the first line had all the\n - same variables as the second, so we'd correctly re-generate\n - everything. It just made for confusing reading.\n -\n - Let's remove that first assignment, and while we're at it split these\n - across lines to make them more maintainable.\n + In 07d90eadb50 (Makefile: add Perl runtime prefix support, 2018-04-10)\n + when PERL_DEFINES was added only the RUNTIME_PREFIX was put on its own\n + line the rest weren't formatted like that for consistency. Let's do\n + that to make this consistent with most of the rest of this Makefile.\n \n Signed-off-by: Ævar Arnfjörð Bjarmason \n \n @@ Makefile: perl_localedir_SQ = $(localedir_SQ)\n \n ifndef NO_PERL\n PERL_HEADER_TEMPLATE = perl/header_templates/fixed_prefix.template.pl\n --PERL_DEFINES = $(PERL_PATH_SQ):$(PERLLIB_EXTRA_SQ):$(perllibdir_SQ)\n --\n --PERL_DEFINES := $(PERL_PATH_SQ) $(PERLLIB_EXTRA_SQ) $(perllibdir_SQ)\n -+PERL_DEFINES :=\n +-PERL_DEFINES = $(PERL_PATH_SQ) $(PERLLIB_EXTRA_SQ) $(perllibdir_SQ)\n ++PERL_DEFINES =\n +PERL_DEFINES += $(PERL_PATH_SQ)\n +PERL_DEFINES += $(PERLLIB_EXTRA_SQ)\n +PERL_DEFINES += $(perllibdir_SQ)\n3: 06e25bc1db3 = 5: 1171b73256e Makefile: regenerate *.pm on NO_PERL_CPAN_FALLBACKS change\n4: 97247cb72a5 = 6: 7a5e7cf39a5 perl: use mock i18n functions under NO_GETTEXT=Y\n-- \n2.31.1.838.g924d365b763\n\n"},{"id":"424013","messageId":"patch-2.6-c15887bbc93-20210510T104937Z-avarab@gmail.com","threadId":"55622","inReplyTo":"cover-0.6-00000000000-20210510T104937Z-avarab@gmail.com","subject":"[PATCH v2 2/6] Makefile: don't re-define PERL_DEFINES","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-05-10T10:50:18Z","receivedAt":"2021-05-10T11:20:21Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Since 07d90eadb50 (Makefile: add Perl runtime prefix support,\n2018-04-10) we have been declaring PERL_DEFINES right after assigning\nto it, with the effect that the first PERL_DEFINES was ignored.\n\nThis didn't matter in practice since the first line had all the same\nvariables as the second, so we'd correctly re-generate everything. It\njust made for confusing reading. Let's remove that first assignment.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n Makefile | 2 --\n 1 file changed, 2 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex ad618cea33f..ea387b431e1 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -2270,8 +2270,6 @@ perl_localedir_SQ = $(localedir_SQ)\n \n ifndef NO_PERL\n PERL_HEADER_TEMPLATE = perl/header_templates/fixed_prefix.template.pl\n-PERL_DEFINES = $(PERL_PATH_SQ):$(PERLLIB_EXTRA_SQ):$(perllibdir_SQ)\n-\n PERL_DEFINES := $(PERL_PATH_SQ) $(PERLLIB_EXTRA_SQ) $(perllibdir_SQ)\n PERL_DEFINES += $(RUNTIME_PREFIX)\n \n-- \n2.31.1.838.g924d365b763\n\n"},{"id":"424014","messageId":"patch-4.6-2cdefbe920c-20210510T104937Z-avarab@gmail.com","threadId":"55622","inReplyTo":"cover-0.6-00000000000-20210510T104937Z-avarab@gmail.com","subject":"[PATCH v2 4/6] Makefile: split up the deceleration of PERL_DEFINES","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-05-10T10:50:20Z","receivedAt":"2021-05-10T11:20:21Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Split the declaration of PERL_DEFINES across multiple line, making\nthis easier to read.\n\nIn 07d90eadb50 (Makefile: add Perl runtime prefix support, 2018-04-10)\nwhen PERL_DEFINES was added only the RUNTIME_PREFIX was put on its own\nline the rest weren't formatted like that for consistency. Let's do\nthat to make this consistent with most of the rest of this Makefile.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n Makefile | 5 ++++-\n 1 file changed, 4 insertions(+), 1 deletion(-)\n\ndiff --git a/Makefile b/Makefile\nindex 3ed6828de67..4f68f5e1dba 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -2270,7 +2270,10 @@ perl_localedir_SQ = $(localedir_SQ)\n \n ifndef NO_PERL\n PERL_HEADER_TEMPLATE = perl/header_templates/fixed_prefix.template.pl\n-PERL_DEFINES = $(PERL_PATH_SQ) $(PERLLIB_EXTRA_SQ) $(perllibdir_SQ)\n+PERL_DEFINES =\n+PERL_DEFINES += $(PERL_PATH_SQ)\n+PERL_DEFINES += $(PERLLIB_EXTRA_SQ)\n+PERL_DEFINES += $(perllibdir_SQ)\n PERL_DEFINES += $(RUNTIME_PREFIX)\n \n # Support Perl runtime prefix. In this mode, a different header is installed\n-- \n2.31.1.838.g924d365b763\n\n"},{"id":"424019","messageId":"patch-3.6-7919ae0e546-20210510T104937Z-avarab@gmail.com","threadId":"55622","inReplyTo":"cover-0.6-00000000000-20210510T104937Z-avarab@gmail.com","subject":"[PATCH v2 3/6] Makefile: make PERL_DEFINES recursively expanded","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-05-10T10:50:19Z","receivedAt":"2021-05-10T11:20:21Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Since 07d90eadb50 (Makefile: add Perl runtime prefix support,\n2018-04-10) PERL_DEFINES has been a simply-expanded variable, let's\nmake it recursively expanded instead.\n\nThis change doesn't matter for the correctness of the logic. Whether\nwe used simply-expanded or recursively expanded didn't change what we\nwrote out in GIT-PERL-DEFINES, but being consistent with other rules\nmakes this easier to understand.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n Makefile | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/Makefile b/Makefile\nindex ea387b431e1..3ed6828de67 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -2270,7 +2270,7 @@ perl_localedir_SQ = $(localedir_SQ)\n \n ifndef NO_PERL\n PERL_HEADER_TEMPLATE = perl/header_templates/fixed_prefix.template.pl\n-PERL_DEFINES := $(PERL_PATH_SQ) $(PERLLIB_EXTRA_SQ) $(perllibdir_SQ)\n+PERL_DEFINES = $(PERL_PATH_SQ) $(PERLLIB_EXTRA_SQ) $(perllibdir_SQ)\n PERL_DEFINES += $(RUNTIME_PREFIX)\n \n # Support Perl runtime prefix. In this mode, a different header is installed\n-- \n2.31.1.838.g924d365b763\n\n"},{"id":"424017","messageId":"patch-6.6-7a5e7cf39a5-20210510T104937Z-avarab@gmail.com","threadId":"55622","inReplyTo":"cover-0.6-00000000000-20210510T104937Z-avarab@gmail.com","subject":"[PATCH v2 6/6] perl: use mock i18n functions under NO_GETTEXT=Y","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-05-10T10:50:22Z","receivedAt":"2021-05-10T11:20:22Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change the logic of the i18n functions I added in 5e9637c6297 (i18n:\nadd infrastructure for translating Git with gettext, 2011-11-18) to\nuse pass-through functions when NO_GETTEXT is defined.\n\nThis speeds up the compilation time of commands that use this library\nwhen NO_GETTEXT=Y is in effect. Loading it and POSIX.pm is around 20ms\non my machine, whereas it takes 2ms to just instantiate perl itself.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n Makefile | 3 +++\n perl/Git/I18N.pm | 10 ++++++++++\n 2 files changed, 13 insertions(+)\n\ndiff --git a/Makefile b/Makefile\nindex aaa972c56aa..0705fc2d3fb 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1986,6 +1986,7 @@ ETC_GITCONFIG_SQ = $(subst ','\\'',$(ETC_GITCONFIG))\n ETC_GITATTRIBUTES_SQ = $(subst ','\\'',$(ETC_GITATTRIBUTES))\n \n DESTDIR_SQ = $(subst ','\\'',$(DESTDIR))\n+NO_GETTEXT_SQ = $(subst ','\\'',$(NO_GETTEXT))\n bindir_SQ = $(subst ','\\'',$(bindir))\n bindir_relative_SQ = $(subst ','\\'',$(bindir_relative))\n mandir_SQ = $(subst ','\\'',$(mandir))\n@@ -2276,6 +2277,7 @@ PERL_DEFINES += $(PERLLIB_EXTRA_SQ)\n PERL_DEFINES += $(perllibdir_SQ)\n PERL_DEFINES += $(RUNTIME_PREFIX)\n PERL_DEFINES += $(NO_PERL_CPAN_FALLBACKS)\n+PERL_DEFINES += $(NO_GETTEXT)\n \n # Support Perl runtime prefix. In this mode, a different header is installed\n # into Perl scripts.\n@@ -2680,6 +2682,7 @@ endif\n perl/build/lib/%.pm: perl/%.pm GIT-PERL-DEFINES\n \t$(QUIET_GEN)mkdir -p $(dir $@) && \\\n \tsed -e 's|@@LOCALEDIR@@|$(perl_localedir_SQ)|g' \\\n+\t -e 's|@@NO_GETTEXT@@|$(NO_GETTEXT_SQ)|g' \\\n \t -e 's|@@NO_PERL_CPAN_FALLBACKS@@|$(NO_PERL_CPAN_FALLBACKS_SQ)|g' \\\n \t< $< > $@\n \ndiff --git a/perl/Git/I18N.pm b/perl/Git/I18N.pm\nindex 2037f387c89..895e759c57a 100644\n--- a/perl/Git/I18N.pm\n+++ b/perl/Git/I18N.pm\n@@ -16,9 +16,19 @@ BEGIN\n our @EXPORT = qw(__ __n N__);\n our @EXPORT_OK = @EXPORT;\n \n+# See Git::LoadCPAN's NO_PERL_CPAN_FALLBACKS_STR for a description of\n+# this \"'@@' [...] '@@'\" pattern.\n+use constant NO_GETTEXT_STR => '@@' . 'NO_GETTEXT' . '@@';\n+use constant NO_GETTEXT => (\n+\tq[@@NO_GETTEXT@@] ne ''\n+\tand\n+\tq[@@NO_GETTEXT@@] ne NO_GETTEXT_STR\n+);\n+\n sub __bootstrap_locale_messages {\n \tour $TEXTDOMAIN = 'git';\n \tour $TEXTDOMAINDIR ||= $ENV{GIT_TEXTDOMAINDIR} || '@@LOCALEDIR@@';\n+\tdie \"NO_GETTEXT=\" . NO_GETTEXT_STR if NO_GETTEXT;\n \n \trequire POSIX;\n \tPOSIX->import(qw(setlocale));\n-- \n2.31.1.838.g924d365b763\n\n"},{"id":"424016","messageId":"patch-5.6-1171b73256e-20210510T104937Z-avarab@gmail.com","threadId":"55622","inReplyTo":"cover-0.6-00000000000-20210510T104937Z-avarab@gmail.com","subject":"[PATCH v2 5/6] Makefile: regenerate *.pm on NO_PERL_CPAN_FALLBACKS change","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-05-10T10:50:21Z","receivedAt":"2021-05-10T11:20:23Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Regenerate the *.pm files in perl/build/* if the\nNO_PERL_CPAN_FALLBACKS flag added to the *.pm files in\n1aca69c0195 (perl Git::LoadCPAN: emit better errors under\nNO_PERL_CPAN_FALLBACKS, 2018-03-03) is changed.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n Makefile | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/Makefile b/Makefile\nindex 4f68f5e1dba..aaa972c56aa 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -2275,6 +2275,7 @@ PERL_DEFINES += $(PERL_PATH_SQ)\n PERL_DEFINES += $(PERLLIB_EXTRA_SQ)\n PERL_DEFINES += $(perllibdir_SQ)\n PERL_DEFINES += $(RUNTIME_PREFIX)\n+PERL_DEFINES += $(NO_PERL_CPAN_FALLBACKS)\n \n # Support Perl runtime prefix. In this mode, a different header is installed\n # into Perl scripts.\n-- \n2.31.1.838.g924d365b763\n\n"},{"id":"424064","messageId":"xmqqzgx2pi5y.fsf@gitster.g","threadId":"55622","inReplyTo":"cover-0.6-00000000000-20210510T104937Z-avarab@gmail.com","subject":"Re: [PATCH v2 0/6] Makefile/perl: correctly re-generate build/* + speed up under NO_GETTEXT=Y","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-05-10T18:17:45Z","receivedAt":"2021-05-10T18:17:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> The summary, from v1:\n>\n> These patches are relatively trivial fixes for bugs noticed when I was\n> working on recent send-email patches. We don't re-build perl/build/*\n> assets when variables that affect them are changed, and needlessly use\n> our non-mock gettext Perl i18n framework under NO_GETTEXT=Y if the\n> system happens to have Locale::Messages installed.\n>\n> Changes since v1:\n\nHmph, didn't reviewers declare that the previous round good enough?\nI thought I merged it 'next' already.\n\n"},{"id":"424113","messageId":"87a6p1u5a6.fsf@evledraar.gmail.com","threadId":"55622","inReplyTo":"xmqqzgx2pi5y.fsf@gitster.g","subject":"Re: [PATCH v2 0/6] Makefile/perl: correctly re-generate build/* + speed up under NO_GETTEXT=Y","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-05-11T06:56:40Z","receivedAt":"2021-05-11T06:57:10Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Tue, May 11 2021, Junio C Hamano wrote:\n\n> Ævar Arnfjörð Bjarmason writes:\n>\n>> The summary, from v1:\n>>\n>> These patches are relatively trivial fixes for bugs noticed when I was\n>> working on recent send-email patches. We don't re-build perl/build/*\n>> assets when variables that affect them are changed, and needlessly use\n>> our non-mock gettext Perl i18n framework under NO_GETTEXT=Y if the\n>> system happens to have Locale::Messages installed.\n>>\n>> Changes since v1:\n>\n> Hmph, didn't reviewers declare that the previous round good enough?\n> I thought I merged it 'next' already.\n\nI sent this re-roll before I noticed that. Nevermind & sorry for the\nnoise.\n"},{"id":"424115","messageId":"xmqqr1idn41e.fsf@gitster.g","threadId":"55622","inReplyTo":"87a6p1u5a6.fsf@evledraar.gmail.com","subject":"Re: [PATCH v2 0/6] Makefile/perl: correctly re-generate build/* + speed up under NO_GETTEXT=Y","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-05-11T07:05:49Z","receivedAt":"2021-05-11T07:06:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> On Tue, May 11 2021, Junio C Hamano wrote:\n>\n>> Ævar Arnfjörð Bjarmason writes:\n>>\n>>> The summary, from v1:\n>>>\n>>> These patches are relatively trivial fixes for bugs noticed when I was\n>>> working on recent send-email patches. We don't re-build perl/build/*\n>>> assets when variables that affect them are changed, and needlessly use\n>>> our non-mock gettext Perl i18n framework under NO_GETTEXT=Y if the\n>>> system happens to have Locale::Messages installed.\n>>>\n>>> Changes since v1:\n>>\n>> Hmph, didn't reviewers declare that the previous round good enough?\n>> I thought I merged it 'next' already.\n>\n> I sent this re-roll before I noticed that. Nevermind & sorry for the\n> noise.\n\nMails cross all the time. A single-liner follow-up incremental\nwould be good, perhaps?\n\nThanks.\n"},{"id":"424309","messageId":"patch-1.1-92571a8cf7-20210512T094803Z-avarab@gmail.com","threadId":"55622","inReplyTo":"xmqqr1idn41e.fsf@gitster.g","subject":"[PATCH] Makefile: make PERL_DEFINES recursively expanded","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-05-12T09:49:44Z","receivedAt":"2021-05-12T09:49:59Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Since 07d90eadb50 (Makefile: add Perl runtime prefix support,\n2018-04-10) PERL_DEFINES has been a simply-expanded variable, let's\nmake it recursively expanded instead.\n\nThis change doesn't matter for the correctness of the logic. Whether\nwe used simply-expanded or recursively expanded didn't change what we\nwrote out in GIT-PERL-DEFINES, but being consistent with other rules\nmakes this easier to understand.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n\nThis is a trivial improvement-on-top for my ab/perl-makefile-cleanup\nalready in \"next\". I'd sent a v2 of that in [1] not seeing it was\nmerged down already, this incremental patch replaces that and results\nin the same end-state.\n\n1. https://lore.kernel.org/git/cover-0.6-00000000000-20210510T104937Z-avarab@gmail.com/\n\n Makefile | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/Makefile b/Makefile\nindex b8d6b31305..0705fc2d3f 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -2271,7 +2271,7 @@ perl_localedir_SQ = $(localedir_SQ)\n \n ifndef NO_PERL\n PERL_HEADER_TEMPLATE = perl/header_templates/fixed_prefix.template.pl\n-PERL_DEFINES :=\n+PERL_DEFINES =\n PERL_DEFINES += $(PERL_PATH_SQ)\n PERL_DEFINES += $(PERLLIB_EXTRA_SQ)\n PERL_DEFINES += $(perllibdir_SQ)\n-- \n2.31.1.909.g789bb6d90e\n\n"},{"id":"424372","messageId":"YJwyJKxXJ5Oe+EYD@coredump.intra.peff.net","threadId":"55622","inReplyTo":"patch-1.1-92571a8cf7-20210512T094803Z-avarab@gmail.com","subject":"Re: [PATCH] Makefile: make PERL_DEFINES recursively expanded","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-05-12T19:53:08Z","receivedAt":"2021-05-12T20:37:31Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, May 12, 2021 at 11:49:44AM +0200, Ævar Arnfjörð Bjarmason wrote:\n\n> Since 07d90eadb50 (Makefile: add Perl runtime prefix support,\n> 2018-04-10) PERL_DEFINES has been a simply-expanded variable, let's\n> make it recursively expanded instead.\n> \n> This change doesn't matter for the correctness of the logic. Whether\n> we used simply-expanded or recursively expanded didn't change what we\n> wrote out in GIT-PERL-DEFINES, but being consistent with other rules\n> makes this easier to understand.\n\nThanks for following up on this. I think it's worth doing.\n\n-Peff\n"},{"id":"424392","messageId":"xmqqzgwzefks.fsf@gitster.g","threadId":"55622","inReplyTo":"YJwyJKxXJ5Oe+EYD@coredump.intra.peff.net","subject":"Re: [PATCH] Makefile: make PERL_DEFINES recursively expanded","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-05-12T22:45:55Z","receivedAt":"2021-05-12T23:28:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Wed, May 12, 2021 at 11:49:44AM +0200, Ævar Arnfjörð Bjarmason wrote:\n>\n>> Since 07d90eadb50 (Makefile: add Perl runtime prefix support,\n>> 2018-04-10) PERL_DEFINES has been a simply-expanded variable, let's\n>> make it recursively expanded instead.\n>> \n>> This change doesn't matter for the correctness of the logic. Whether\n>> we used simply-expanded or recursively expanded didn't change what we\n>> wrote out in GIT-PERL-DEFINES, but being consistent with other rules\n>> makes this easier to understand.\n>\n> Thanks for following up on this. I think it's worth doing.\n>\n> -Peff\n\nYup, thanks, both.\n"}]}