git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 1/4] Makefile: don't re-define PERL_DEFINES

From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date
May 6, 2021, 06:23 UTC
Message-ID
<87y2csv0qm.fsf@evledraar.gmail.com>
In-Reply-To
<YJKm0dnwHBwQuTi+@coredump.intra.peff.net>
On Wed, May 05 2021, Jeff King wrote:
Show 37 quoted lines
> On Wed, May 05, 2021 at 02:21:38PM +0200, Ævar Arnfjörð Bjarmason wrote:
>
>> Since 07d90eadb50 (Makefile: add Perl runtime prefix support,
>> 2018-04-10) we have been declaring PERL_DEFINES right after assigning
>> to it, with the effect that the first PERL_DEFINES was ignored.
>> 
>> That bug didn't matter in practice since the first line had all the
>> same variables as the second, so we'd correctly re-generate
>> everything. It just made for confusing reading.
>> 
>> Let's remove that first assignment, and while we're at it split these
>> across lines to make them more maintainable.
>
> This and the other three patches all look sensible to me.
>
> I did have one question:
>
>> diff --git a/Makefile b/Makefile
>> index 93664d67146..1d4c02e59d9 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -2270,9 +2270,10 @@ perl_localedir_SQ = $(localedir_SQ)
>>  
>>  ifndef NO_PERL
>>  PERL_HEADER_TEMPLATE = perl/header_templates/fixed_prefix.template.pl
>> -PERL_DEFINES = $(PERL_PATH_SQ):$(PERLLIB_EXTRA_SQ):$(perllibdir_SQ)
>> -
>> -PERL_DEFINES := $(PERL_PATH_SQ) $(PERLLIB_EXTRA_SQ) $(perllibdir_SQ)
>> +PERL_DEFINES :=
>> +PERL_DEFINES += $(PERL_PATH_SQ)
>> +PERL_DEFINES += $(PERLLIB_EXTRA_SQ)
>> +PERL_DEFINES += $(perllibdir_SQ)
>>  PERL_DEFINES += $(RUNTIME_PREFIX)
>
> I don't think we generally use simply-expanded variables in our Makefile
> unless there's a reason. Do we actually need it here? Obviously not new
> in your patch, but just a curiosity I noticed while reading it.

I didn't notice it at the time. I suppose it could be changed to not do expansion, but per-se unrelated to the more narrorw bugfix in this patch.

Previous: Jeff KingNext: Junio C Hamano
Message 6 of 26 in “Makefile/perl: correctly re-generate build/* + speed up under NO_GETTEXT=Y”
  1. 0/4 Makefile/perl: correctly re-generate build/* + speed up under NO_GETTEXT=YÆvar Arnfjörð Bjarmason, May 5, 2021
  2. 1/4 Makefile: don't re-define PERL_DEFINESÆvar Arnfjörð Bjarmason, May 5, 2021
  3. Jeff KingMay 5, 2021
  4. Junio C HamanoMay 6, 2021
  5. Jeff KingMay 6, 2021
  6. Ævar Arnfjörð BjarmasonMay 6, 2021
  7. Junio C HamanoMay 6, 2021
  8. Ævar Arnfjörð BjarmasonMay 6, 2021
  9. Jeff KingMay 6, 2021
  10. Junio C HamanoMay 7, 2021
  11. 3/4 Makefile: regenerate *.pm on NO_PERL_CPAN_FALLBACKS changeÆvar Arnfjörð Bjarmason, May 5, 2021
  12. 2/4 Makefile: regenerate perl/build/* if GIT-PERL-DEFINES changesÆvar Arnfjörð Bjarmason, May 5, 2021
  13. 4/4 perl: use mock i18n functions under NO_GETTEXT=YÆvar Arnfjörð Bjarmason, May 5, 2021
  14. 0/6 Makefile/perl: correctly re-generate build/* + speed up under NO_GETTEXT=YÆvar Arnfjörð Bjarmason, May 10, 2021
  15. 1/6 Makefile: regenerate perl/build/* if GIT-PERL-DEFINES changesÆvar Arnfjörð Bjarmason, May 10, 2021
  16. 2/6 Makefile: don't re-define PERL_DEFINESÆvar Arnfjörð Bjarmason, May 10, 2021
  17. 4/6 Makefile: split up the deceleration of PERL_DEFINESÆvar Arnfjörð Bjarmason, May 10, 2021
  18. 3/6 Makefile: make PERL_DEFINES recursively expandedÆvar Arnfjörð Bjarmason, May 10, 2021
  19. 6/6 perl: use mock i18n functions under NO_GETTEXT=YÆvar Arnfjörð Bjarmason, May 10, 2021
  20. 5/6 Makefile: regenerate *.pm on NO_PERL_CPAN_FALLBACKS changeÆvar Arnfjörð Bjarmason, May 10, 2021
  21. Junio C HamanoMay 10, 2021
  22. Ævar Arnfjörð BjarmasonMay 11, 2021
  23. Junio C HamanoMay 11, 2021
  24. Makefile: make PERL_DEFINES recursively expandedÆvar Arnfjörð Bjarmason, May 12, 2021
  25. Jeff KingMay 12, 2021
  26. Junio C HamanoMay 12, 2021

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.