Re: BUG #15065: ActivePerl 5.24.3 breaks PG compilation on Windows

On Wed, Feb 14, 2018 at 2:27 PM, Heath Lord <heath(dot)lord(at)crunchydata(dot)com>
wrote:

>
> On Wed, Feb 14, 2018 at 8:19 AM, PG Bug reporting form <
> noreply(at)postgresql(dot)org> wrote:
>
>> The following bug has been logged on the website:
>>
>> Bug reference: 15065
>> Logged by: Heath Lord
>> Email address: heath(dot)lord(at)crunchydata(dot)com
>> PostgreSQL version: 9.6.7
>> Operating system: Windows
>> Description:
>>
>> With the latest release of ActivePerl 5.24.3 they are installing both a
>> .a and a .lib with their installation. While this normally wouldn't be an
>> issue, the logic that is being used in the PG source checks for one or the
>> other and if it finds both it will fail with an error stating that it
>> could
>> not find them.
>> In the postgres/src/tools/msvc/Mkvcbuild.pm file the following logic
>> exists:
>>
>> # ActivePerl 5.16 provided perl516.lib; 5.18 provided libperl518.a
>> my @perl_libs =
>> grep { /perl\d+\.lib$|libperl\d+\.a$/ }
>> glob($perl_path);
>> if (@perl_libs == 1)
>> {
>> $plperl->AddLibrary($perl_libs[0]);
>> }
>>
>> The issue is that with the latest ActivePerl release the if statement
>> results in a value of 2 which causes this to fail and it falls through to
>> the else which is the failure case. This causes all of the Windows builds
>> to fail if using the latest version of ActivePerl.
>> There are two possible solutions that I am proposing:
>>
>> 1) [msvc_perl_lib_check_for_two_use_second.patch]
>> Add an elsif clause and check if @perl_libs is 2 and then use
>> the libperl###.a which I have tested and everything compiles fine and
>> passes
>> all tests.
>>
>> 2) [msvc_perl_lib_check_inclusive_use_first_found.patch]
>> Change the if statement to check for 1 or more perl libs found and
>> then always use the first one that was found. I have also built and
>> tested
>> that this passes all regression.
>>
>> Both of these options work and have been built and tested. The
>> resulting
>> generated binaries are of identical size as well.
>>
>> I am not sure given the fact that this breaks all windows builds using
>> this version of ActivePerl if this change needs to be backpatched or not?
>> Thank you in advance for any help or assistance given to get one of these
>> fixes into the PG code base.
>>
>> I will be attaching the patches referenced above in a follow up reply
>> after the bug has been submitted.
>>
>> -Heath
>>
>>
> I am attaching the patch files that I have referenced above for your
> review. Thank you.
>
>
I think the second one is definitely the cleaner way to do it. Thus, I've
applied that one, after moving the comments around a bit.

I also removed the part where you changed the whitespace in the error
message :) perltidy would likely just put that right back.

--
Magnus Hagander
Me: https://www.hagander.net/ <http://www.hagander.net/>
Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/>