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

From: Junio C Hamano <gitster@pobox.com>
Date: May 6, 2021, 08:42 UTC
Message-ID: <xmqq7dkcz20u.fsf@gitster.g>
In-Reply-To: <87y2csv0qm.fsf@evledraar.gmail.com>

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

Show 16 quoted lines

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