On Wed, Mar 29, 2017 at 8:12 PM, Jeff King <peff@xxxxxxxx> wrote:
> On Wed, Mar 29, 2017 at 01:57:03PM +0000, Ævar Arnfjörð Bjarmason wrote:
>
>> Change the perl/perl.mak build process so that the file is regenerated
>> if the output of "perl -V" changes.
>>
>> Before this change updating e.g. /usr/bin/perl to a new major version
>> would cause the next "make" command to fail, since perl.mak has
>> hardcoded paths to perl library paths retrieved from its first run.
>
> This is one of those things that has been bugging me for years, but it
> comes up so rarely that I have never dug into it.
Glad to help. I've only run into this once a couple of days ago, made
a mental note to fix it, and then I saw that thread...
>> Now the logic added in commit ee9be06770 ("perl: detect new files in
>> MakeMaker builds", 2012-07-27) is extended to regenerate
>> perl/perl.mak if there's any change to "perl -V".
>
> Nice. This fix is way simpler than I feared.
>
>> This will in some cases redundantly trigger perl/perl.mak to be
>> re-made, e.g. if @INC is modified in ways the build process doesn't
>> care about through sitecustomize.pl, but the common case is that we
>> just do the right thing and re-generate perl/perl.mak when needed.
>
> I think that's fine. There's a related bug that the generation of
> perl/perl.mak via recursive-make is sometimes racy. So that _might_
> trigger more often as a result of this, but I think the solution is to
> fix that race, not try to pretend it won't happen. :)
We'll also redundantly trigger if you upgrade to a minor new perl
version, but I think that's squarely in "who cares" territory. This'll
only impact people working on git, and *occasionally* they might get a
100 ms hit when running make, as opposed to a cryptic error where
they'll likely stare at it for a bit before running "make clean".
If we were being more pedantic we could only bust the cache on major
perl version upgrades:
perl -e 'print substr($], 0, 5), "\n"' >>PM.stamp+
Or use Config.pm:
perl -MConfig -e 'print @Config{qw(api_revision api_version)},
"\n"' >>PM.stamp+
But I think overall leaning on the side of busting the cache more
often to avoid cryptic errors is the right choice, and we should use
"perl -V".