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. -Peff
