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

On Thu, May 06, 2021 at 11:04:34AM +0200, Ævar Arnfjörð Bjarmason wrote:

quoted

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.

Yes, there wasn't any bug with the the eventual value being
incorrect. I.e. both of these are equivalent in a Makefile:

    FOO = abc
    FOO := def
    FOO += ghi

And:

    FOO = abc
    FOO = def
    FOO += ghi

Both will yield "def ghi". They're just different in a case like:
    
    X = Y
    FOO = abc
    FOO := $(X)
    X = Z
    FOO += ghi

Where using := will echo "Y ghi", and using = will echo "Z ghi". As a
practical matter the distinction doesn't matter in this case.

Yeah, I don't think the ":=" was impacting the bug or no bug (not to
mention that even if we duplicated those entries in the variable, it
_still_ wouldn't be a bug, since the whole point of the variable is just
to notice when the content changes).

quoted

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?

I don't see how removing the stray line changes the reason to use ":="
or "=" there. I agree it should be removed, it's just unrelated to
removing the stay line. Looking at 07d90eadb50 it's clear that it's just
some copy/pasting error.

Yeah, I'd agree it is truly orthogonal. I don't mind seeing it cleaned
up in addition (or am even actively happy to see it cleaned up :) ), but
IMHO it would not need to hold up the series.

-Peff