All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Cc: Jeff King <peff@peff.net>, git@vger.kernel.org
Subject: Re: [PATCH 1/4] Makefile: don't re-define PERL_DEFINES
Date: Thu, 06 May 2021 17:42:41 +0900	[thread overview]
Message-ID: <xmqq7dkcz20u.fsf@gitster.g> (raw)
In-Reply-To: <87y2csv0qm.fsf@evledraar.gmail.com> ("Ævar Arnfjörð Bjarmason"'s message of "Thu, 06 May 2021 08:23:28 +0200")

Ævar Arnfjörð Bjarmason <avarab@gmail.com> 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?

  reply	other threads:[~2021-05-06  8:42 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-05 12:21 [PATCH 0/4] Makefile/perl: correctly re-generate build/* + speed up under NO_GETTEXT=Y Ævar Arnfjörð Bjarmason
2021-05-05 12:21 ` [PATCH 1/4] Makefile: don't re-define PERL_DEFINES Ævar Arnfjörð Bjarmason
2021-05-05 14:08   ` Jeff King
2021-05-06  1:05     ` Junio C Hamano
2021-05-06 16:55       ` Jeff King
2021-05-06  6:23     ` Ævar Arnfjörð Bjarmason
2021-05-06  8:42       ` Junio C Hamano [this message]
2021-05-06  9:04         ` Ævar Arnfjörð Bjarmason
2021-05-06 16:59           ` Jeff King
2021-05-07  8:42           ` Junio C Hamano
2021-05-05 12:21 ` [PATCH 2/4] Makefile: regenerate perl/build/* if GIT-PERL-DEFINES changes Ævar Arnfjörð Bjarmason
2021-05-05 12:21 ` [PATCH 3/4] Makefile: regenerate *.pm on NO_PERL_CPAN_FALLBACKS change Ævar Arnfjörð Bjarmason
2021-05-05 12:21 ` [PATCH 4/4] perl: use mock i18n functions under NO_GETTEXT=Y Ævar Arnfjörð Bjarmason
2021-05-10 10:50 ` [PATCH v2 0/6] Makefile/perl: correctly re-generate build/* + speed up " Ævar Arnfjörð Bjarmason
2021-05-10 10:50   ` [PATCH v2 1/6] Makefile: regenerate perl/build/* if GIT-PERL-DEFINES changes Ævar Arnfjörð Bjarmason
2021-05-10 10:50   ` [PATCH v2 2/6] Makefile: don't re-define PERL_DEFINES Ævar Arnfjörð Bjarmason
2021-05-10 10:50   ` [PATCH v2 3/6] Makefile: make PERL_DEFINES recursively expanded Ævar Arnfjörð Bjarmason
2021-05-10 10:50   ` [PATCH v2 4/6] Makefile: split up the deceleration of PERL_DEFINES Ævar Arnfjörð Bjarmason
2021-05-10 10:50   ` [PATCH v2 5/6] Makefile: regenerate *.pm on NO_PERL_CPAN_FALLBACKS change Ævar Arnfjörð Bjarmason
2021-05-10 10:50   ` [PATCH v2 6/6] perl: use mock i18n functions under NO_GETTEXT=Y Ævar Arnfjörð Bjarmason
2021-05-10 18:17   ` [PATCH v2 0/6] Makefile/perl: correctly re-generate build/* + speed up " Junio C Hamano
2021-05-11  6:56     ` Ævar Arnfjörð Bjarmason
2021-05-11  7:05       ` Junio C Hamano
2021-05-12  9:49         ` [PATCH] Makefile: make PERL_DEFINES recursively expanded Ævar Arnfjörð Bjarmason
2021-05-12 19:53           ` Jeff King
2021-05-12 22:45             ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqq7dkcz20u.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.