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

From: Junio C Hamano <[email-masked]>
Date: Thu, 06 May 2021 17:42:41 +0900

Ævar Arnfjörð Bjarmason <[email-masked]> writes:

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

Actually, strictly speaking there was *no* bug because assigning
three items with := made sure the previous recursively expanded one
to be ineffective.  In other words, there was a valid reason to use
":=" there in the original version.

Now your patch removed the recursively expanded one that was
immediately invalidated, there no longer is a reason to use :=
there.  So "unrelated to the more narrow bugfix" is a rather lame
excuse to do only half a task.  If we remove that extra one (which
is a good thing), then we should correct := into = because the
original used := only because there was the unwanted extra one, no?