Horton

Thread

Commits

  1. Remove PostgreSQL::Test::Utils::perl2host completely

  2. Remove most msys special processing in TAP tests

  3. Ensure the right perl is used for TAP tests on msys

  4. Disable perl2host() processing in TAP tests

  1. killing perl2host

    Andrew Dunstan <andrew@dunslane.net> — 2022-02-16T20:46:28Z

    Largely following a recipe from Andres, I have migrated buildfarm
animals fairywren and jacana to a setup that shouldn't need (and in fact
won't be able to use) PostgreSQL::Test:Utils::perl2host(). AFAICT these
two are the only buildfarm animals that run TAP tests under msys.

See discussion at
<https://postgr.es/m/20220125023609.5ohu3nslxgoygihl@alap3.anarazel.de>

The lesson to be learned, incidentally, is "Don't use the msys targeted
perl to run TAP tests".

I suggest that we apply this patch:


diff --git a/src/test/perl/PostgreSQL/Test/Utils.pm
b/src/test/perl/PostgreSQL/Test/Utils.pm
index 57fcb24089..31e2b0315e 100644
--- a/src/test/perl/PostgreSQL/Test/Utils.pm
+++ b/src/test/perl/PostgreSQL/Test/Utils.pm
@@ -311,7 +311,7 @@ The returned path uses forward slashes but has no
trailing slash.
 sub perl2host
 {
    my ($subject) = @_;
-   return $subject unless $Config{osname} eq 'msys';
+   return $subject;
    if ($is_msys2)
    {
        # get absolute, windows type path


and if nothing breaks in a few days I will set about a more thorough
removal of perl2host() and adjusting everywhere it's called, and we can
forget that the whole sorry mess ever happened :-) I know a number of
people who will be overjoyed.


cheers


andrew

-- 

Andrew Dunstan
EDB: https://www.enterprisedb.com

  2. Re: killing perl2host

    Andres Freund <andres@anarazel.de> — 2022-02-16T21:01:41Z

    Hi,

On 2022-02-16 15:46:28 -0500, Andrew Dunstan wrote:
> I suggest that we apply this patch:

+1

> and if nothing breaks in a few days I will set about a more thorough
> removal of perl2host() and adjusting everywhere it's called, and we can
> forget that the whole sorry mess ever happened :-)

I think we would need an error check against using an msys perl when targeting
native windows, otherwise this'll be too hard to debug.

And we probably should set that environment variable that might fix in-tree
tests? Or reject in-tree builds?

Greetings,

Andres Freund

  3. Re: killing perl2host

    Andrew Dunstan <andrew@dunslane.net> — 2022-02-16T21:10:35Z

    On 2/16/22 16:01, Andres Freund wrote:
> Hi,
>
> On 2022-02-16 15:46:28 -0500, Andrew Dunstan wrote:
>> I suggest that we apply this patch:
> +1
>
>> and if nothing breaks in a few days I will set about a more thorough
>> removal of perl2host() and adjusting everywhere it's called, and we can
>> forget that the whole sorry mess ever happened :-)
> I think we would need an error check against using an msys perl when targeting
> native windows, otherwise this'll be too hard to debug.


So something like this in Utils.pm:


die "Msys targeted perl is unsupported for running TAP tests" if
$Config{osname}eq 'msys';


>
> And we probably should set that environment variable that might fix in-tree
> tests? Or reject in-tree builds?


I think that's an orthogonal issue, but yes we should probably set it.
Have you tested to make sure it does what we want? I certainly don't
want to reject in-tree builds.


cheers


andrew


--
Andrew Dunstan
EDB: https://www.enterprisedb.com

  4. Re: killing perl2host

    Andres Freund <andres@anarazel.de> — 2022-02-16T22:42:30Z

    Hi, 

On February 16, 2022 1:10:35 PM PST, Andrew Dunstan <andrew@dunslane.net> wrote:
>
>On 2/16/22 16:01, Andres Freund wrote:
>> Hi,
>>
>> On 2022-02-16 15:46:28 -0500, Andrew Dunstan wrote:
>>> I suggest that we apply this patch:
>> +1
>>
>>> and if nothing breaks in a few days I will set about a more thorough
>>> removal of perl2host() and adjusting everywhere it's called, and we can
>>> forget that the whole sorry mess ever happened :-)
>> I think we would need an error check against using an msys perl when targeting
>> native windows, otherwise this'll be too hard to debug.
>
>
>So something like this in Utils.pm:
>
>
>die "Msys targeted perl is unsupported for running TAP tests" if
>$Config{osname}eq 'msys';

I don't think we should reject msys general - it's fine as long as the target is msys, no? Msys includes postgres fwiw...


>> And we probably should set that environment variable that might fix in-tree
>> tests? Or reject in-tree builds?
>
>
>I think that's an orthogonal issue, but yes we should probably set it.
>Have you tested to make sure it does what we want? I certainly don't
>want to reject in-tree builds.

I don't think it's quite orthogonal - msys perl uses sane PATH semantics and thus doesn't have this problem.

Andres
-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

  5. Re: killing perl2host

    Andres Freund <andres@anarazel.de> — 2022-02-17T02:17:52Z

    Hi,

On 2022-02-16 14:42:30 -0800, Andres Freund wrote:
> On February 16, 2022 1:10:35 PM PST, Andrew Dunstan <andrew@dunslane.net> wrote:
> >So something like this in Utils.pm:
> >
> >
> >die "Msys targeted perl is unsupported for running TAP tests" if
> >$Config{osname}eq 'msys';
> 
> I don't think we should reject msys general - it's fine as long as the target is msys, no? Msys includes postgres fwiw...

I think this means we should do the msys test in configure, inside

if test "$enable_tap_tests" = yes; then

and verify that $host_os != msys.

Greetings,

Andres Freund

  6. Re: killing perl2host

    Andrew Dunstan <andrew@dunslane.net> — 2022-02-17T14:20:56Z

    On 2/16/22 21:17, Andres Freund wrote:
> Hi,
>
> On 2022-02-16 14:42:30 -0800, Andres Freund wrote:
>> On February 16, 2022 1:10:35 PM PST, Andrew Dunstan <andrew@dunslane.net> wrote:
>>> So something like this in Utils.pm:
>>>
>>>
>>> die "Msys targeted perl is unsupported for running TAP tests" if
>>> $Config{osname}eq 'msys';
>> I don't think we should reject msys general - it's fine as long as the target is msys, no? Msys includes postgres fwiw...



I don't think we have or have ever had a buildfarm animal targeting
msys. In general I think of msys as a build environment to create native
binaries. But if we want to support targeting msys we should have an
animal doing that.


> I think this means we should do the msys test in configure, inside
>
> if test "$enable_tap_tests" = yes; then
>
> and verify that $host_os != msys.
>


config/check_modules.pl is probably the right place for the test, as it
will be running with the perl we should be testing, and is only called
if we configure with TAP tests enabled.

perhaps something like:


    my $msystem = $ENV{MSYSTEM} || 'undef';

    die "incompatible perl" if $Config{osname} eq 'msys' && $msystem ne
'MSYS';


cheers


andrew


--
Andrew Dunstan
EDB: https://www.enterprisedb.com

  7. Re: killing perl2host

    Andres Freund <andres@anarazel.de> — 2022-02-17T17:12:09Z

    Hi,

On 2022-02-17 09:20:56 -0500, Andrew Dunstan wrote:
> I don't think we have or have ever had a buildfarm animal targeting
> msys. In general I think of msys as a build environment to create native
> binaries. But if we want to support targeting msys we should have an
> animal doing that.

It's pretty much cygwin. Wouldn't hurt to have a dedicated animal though, I
agree. We do have a dedicated path for it in configure.ac:

case $host_os in
...
  cygwin*|msys*) template=cygwin ;;



> > I think this means we should do the msys test in configure, inside
> >
> > if test "$enable_tap_tests" = yes; then
> >
> > and verify that $host_os != msys.

> config/check_modules.pl is probably the right place for the test, as it
> will be running with the perl we should be testing, and is only called
> if we configure with TAP tests enabled.

Makes sense.


> perhaps something like:
> 
> 
>     my $msystem = $ENV{MSYSTEM} || 'undef';
> 
>     die "incompatible perl" if $Config{osname} eq 'msys' && $msystem ne
> 'MSYS';

Why tests MSYSTEM instead of $host_os?

Greetings,

Andres Freund

  8. Re: killing perl2host

    Andrew Dunstan <andrew@dunslane.net> — 2022-02-17T18:08:01Z

    On 2/17/22 12:12, Andres Freund wrote:
> Hi,
>
> On 2022-02-17 09:20:56 -0500, Andrew Dunstan wrote:
>> I don't think we have or have ever had a buildfarm animal targeting
>> msys. In general I think of msys as a build environment to create native
>> binaries. But if we want to support targeting msys we should have an
>> animal doing that.
> It's pretty much cygwin. Wouldn't hurt to have a dedicated animal though, I
> agree. We do have a dedicated path for it in configure.ac:
>
> case $host_os in
> ...
>   cygwin*|msys*) template=cygwin ;;
>
>
>
>>> I think this means we should do the msys test in configure, inside
>>>
>>> if test "$enable_tap_tests" = yes; then
>>>
>>> and verify that $host_os != msys.
>> config/check_modules.pl is probably the right place for the test, as it
>> will be running with the perl we should be testing, and is only called
>> if we configure with TAP tests enabled.
> Makes sense.
>
>
>> perhaps something like:
>>
>>
>>     my $msystem = $ENV{MSYSTEM} || 'undef';
>>
>>     die "incompatible perl" if $Config{osname} eq 'msys' && $msystem ne
>> 'MSYS';
> Why tests MSYSTEM instead of $host_os?



Is that available in check_modules.pl? AFAICT it's an unexported shell
variable.


cheers


andrew


--
Andrew Dunstan
EDB: https://www.enterprisedb.com

  9. Re: killing perl2host

    Andres Freund <andres@anarazel.de> — 2022-02-17T18:10:16Z

    On 2022-02-17 13:08:01 -0500, Andrew Dunstan wrote:
> >> perhaps something like:
> >>
> >>
> >>     my $msystem = $ENV{MSYSTEM} || 'undef';
> >>
> >>     die "incompatible perl" if $Config{osname} eq 'msys' && $msystem ne
> >> 'MSYS';
> > Why tests MSYSTEM instead of $host_os?

> Is that available in check_modules.pl? AFAICT it's an unexported shell
> variable.

No, but it could just be passed to it? Or the test just implemented in
configure, as I suggested.

Greetings,

Andres Freund

  10. Re: killing perl2host

    Andrew Dunstan <andrew@dunslane.net> — 2022-02-17T19:40:01Z

    On 2/17/22 13:10, Andres Freund wrote:
> On 2022-02-17 13:08:01 -0500, Andrew Dunstan wrote:
>>>> perhaps something like:
>>>>
>>>>
>>>>     my $msystem = $ENV{MSYSTEM} || 'undef';
>>>>
>>>>     die "incompatible perl" if $Config{osname} eq 'msys' && $msystem ne
>>>> 'MSYS';
>>> Why tests MSYSTEM instead of $host_os?
>> Is that available in check_modules.pl? AFAICT it's an unexported shell
>> variable.
> No, but it could just be passed to it? 


Sure, that could be done, but what's the issue? Msys2 normally defines
MSYSTEM for you - see /etc/msystem which is sourced by /etc/profile.


> Or the test just implemented in
> configure, as I suggested.
>

No, because we don't know which perl will be invoked by $PROVE. That's
why we set up check_modules.pl in the first place.


cheers


andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

  11. Re: killing perl2host

    Andres Freund <andres@anarazel.de> — 2022-02-17T20:09:07Z

    Hi,

On 2022-02-17 14:40:01 -0500, Andrew Dunstan wrote:
> Sure, that could be done, but what's the issue? Msys2 normally defines
> MSYSTEM for you - see /etc/msystem which is sourced by /etc/profile.

It seems not a great idea to me to use different sources of truth about build
target. And I think it's not hard to get the actual host_os and MSYSTEM to
disagree:
- afaics it's quite possible to run configure outside of a login msys shell
- you could do an msys build in a ucrt shell, but specify --host=x86_64-pc-msys,
  or the other way round

There's probably also some stuff about cross building from linux, but that
doesn't matter much, because right now wine doesn't get through even the base
regression tests (although it gets through initdb these days, which is nice).


> > Or the test just implemented in
> > configure, as I suggested.
> >
> 
> No, because we don't know which perl will be invoked by $PROVE. That's
> why we set up check_modules.pl in the first place.

Ah.

Greetings,

Andres Freund

  12. Re: killing perl2host

    Andrew Dunstan <andrew@dunslane.net> — 2022-02-17T20:23:36Z

    On 2/17/22 15:09, Andres Freund wrote:
> Hi,
>
> On 2022-02-17 14:40:01 -0500, Andrew Dunstan wrote:
>> Sure, that could be done, but what's the issue? Msys2 normally defines
>> MSYSTEM for you - see /etc/msystem which is sourced by /etc/profile.
> It seems not a great idea to me to use different sources of truth about build
> target. And I think it's not hard to get the actual host_os and MSYSTEM to
> disagree:
> - afaics it's quite possible to run configure outside of a login msys shell
> - you could do an msys build in a ucrt shell, but specify --host=x86_64-pc-msys,
>   or the other way round


Very well. I think the easiest way will be to stash $host_os in the
environment and let the script pick it up similarly to what I suggested
with MSYSTEM.


cheers


andrew


--
Andrew Dunstan
EDB: https://www.enterprisedb.com

  13. Re: killing perl2host

    Andres Freund <andres@anarazel.de> — 2022-02-17T20:46:27Z

    On 2022-02-17 15:23:36 -0500, Andrew Dunstan wrote:
> Very well. I think the easiest way will be to stash $host_os in the
> environment and let the script pick it up similarly to what I suggested
> with MSYSTEM.

WFM.

  14. Re: killing perl2host

    Andrew Dunstan <andrew@dunslane.net> — 2022-02-18T14:03:17Z

    On 2/17/22 15:46, Andres Freund wrote:
> On 2022-02-17 15:23:36 -0500, Andrew Dunstan wrote:
>> Very well. I think the easiest way will be to stash $host_os in the
>> environment and let the script pick it up similarly to what I suggested
>> with MSYSTEM.
> WFM.


OK, here's a patch.


cheers


andrew


--
Andrew Dunstan
EDB: https://www.enterprisedb.com

  15. Re: killing perl2host

    Andrew Dunstan <andrew@dunslane.net> — 2022-02-18T22:34:39Z

    On 2/17/22 12:12, Andres Freund wrote:
> Hi,
>
> On 2022-02-17 09:20:56 -0500, Andrew Dunstan wrote:
>> I don't think we have or have ever had a buildfarm animal targeting
>> msys. In general I think of msys as a build environment to create native
>> binaries. But if we want to support targeting msys we should have an
>> animal doing that.
> It's pretty much cygwin. Wouldn't hurt to have a dedicated animal though, I
> agree. We do have a dedicated path for it in configure.ac:
>
> case $host_os in
> ...
>   cygwin*|msys*) template=cygwin ;;



FYI I tested it while in wait mode for something else, and it fell over
at the first hurdle:


running bootstrap script ... 2022-02-18 22:25:45.119 UTC [34860] FATAL: 
could not create shared memory segment: Function not implemented
2022-02-18 22:25:45.119 UTC [34860] DETAIL:  Failed system call was
shmget(key=1407374884304065, size=56, 03600).
child process exited with exit code 1


I'm not intending to put any more effort into supporting it.


cheers


andrew


--
Andrew Dunstan
EDB: https://www.enterprisedb.com

  16. Re: killing perl2host

    Andrew Dunstan <andrew@dunslane.net> — 2022-02-19T13:48:14Z

    On 2/18/22 17:34, Andrew Dunstan wrote:
> On 2/17/22 12:12, Andres Freund wrote:
>> Hi,
>>
>> On 2022-02-17 09:20:56 -0500, Andrew Dunstan wrote:
>>> I don't think we have or have ever had a buildfarm animal targeting
>>> msys. In general I think of msys as a build environment to create native
>>> binaries. But if we want to support targeting msys we should have an
>>> animal doing that.
>> It's pretty much cygwin. Wouldn't hurt to have a dedicated animal though, I
>> agree. We do have a dedicated path for it in configure.ac:
>>
>> case $host_os in
>> ...
>>   cygwin*|msys*) template=cygwin ;;
>
>
> FYI I tested it while in wait mode for something else, and it fell over
> at the first hurdle:
>
>
> running bootstrap script ... 2022-02-18 22:25:45.119 UTC [34860] FATAL: 
> could not create shared memory segment: Function not implemented
> 2022-02-18 22:25:45.119 UTC [34860] DETAIL:  Failed system call was
> shmget(key=1407374884304065, size=56, 03600).
> child process exited with exit code 1
>
>
> I'm not intending to put any more effort into supporting it.


See also
<https://postgr.es/m/6b467edc-4018-521f-ab18-171f098557ca@2ndquadrant.com>
where Peter says:


    MSYS2 doesn't ship with cygserver AFAICT, so you can't run a
    PostgreSQL server, but everything else should work.


which explains this perfectly. If we can't run a server then any sort of
buildfarm/CI support seems moot.


cheers


andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

  17. Re: killing perl2host

    Andrew Dunstan <andrew@dunslane.net> — 2022-02-19T18:04:05Z

    On 2/16/22 15:46, Andrew Dunstan wrote:
> Largely following a recipe from Andres, I have migrated buildfarm
> animals fairywren and jacana to a setup that shouldn't need (and in fact
> won't be able to use) PostgreSQL::Test:Utils::perl2host(). AFAICT these
> two are the only buildfarm animals that run TAP tests under msys.

[...]


> I suggest that we apply this patch:
>
>
[...]
>
> and if nothing breaks in a few days I will set about a more thorough
> removal of perl2host() and adjusting everywhere it's called, and we can
> forget that the whole sorry mess ever happened :-) I know a number of
> people who will be overjoyed.
>
>


OK, nothing broke, so here are two more invasive patches. The first
removes perl2host completely and adjusts  all the places that use it.
The second removes almost all the remaining msys special processing in
TAP tests.


I have tested these and they appear to work fine.


cheers


andrew


--
Andrew Dunstan
EDB: https://www.enterprisedb.com

  18. Re: killing perl2host

    Andrew Dunstan <andrew@dunslane.net> — 2022-02-19T18:47:41Z

    On 2/19/22 13:04, Andrew Dunstan wrote:
> On 2/16/22 15:46, Andrew Dunstan wrote:
>> Largely following a recipe from Andres, I have migrated buildfarm
>> animals fairywren and jacana to a setup that shouldn't need (and in fact
>> won't be able to use) PostgreSQL::Test:Utils::perl2host(). AFAICT these
>> two are the only buildfarm animals that run TAP tests under msys.
> [...]
>
>
>> I suggest that we apply this patch:
>>
>>
> [...]
>> and if nothing breaks in a few days I will set about a more thorough
>> removal of perl2host() and adjusting everywhere it's called, and we can
>> forget that the whole sorry mess ever happened :-) I know a number of
>> people who will be overjoyed.
>>
>>
>
> OK, nothing broke, so here are two more invasive patches. The first
> removes perl2host completely and adjusts  all the places that use it.
> The second removes almost all the remaining msys special processing in
> TAP tests.
>
>
> I have tested these and they appear to work fine.
>
>

It turns out we can also remove the skip code in
src/bin/pg_dump/t/010_dump_connstr.pl


cheers


andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

  19. Re: killing perl2host

    Andres Freund <andres@anarazel.de> — 2022-02-19T21:34:56Z

    Hi,

On 2022-02-19 13:04:05 -0500, Andrew Dunstan wrote:
> OK, nothing broke, so here are two more invasive patches.

Great!


> The first
> removes perl2host completely and adjusts  all the places that use it.
> The second removes almost all the remaining msys special processing in
> TAP tests.

Hm. Do we have to worry about backpatched bugfixes breaking in the
backbranches? Or are you planning to backpatch this stuff?

Greetings,

Andres Freund

  20. Re: killing perl2host

    Andrew Dunstan <andrew@dunslane.net> — 2022-02-19T21:51:18Z

    On 2/19/22 16:34, Andres Freund wrote:
> Hi,
>
> On 2022-02-19 13:04:05 -0500, Andrew Dunstan wrote:
>> OK, nothing broke, so here are two more invasive patches.
> Great!
>
>
>> The first
>> removes perl2host completely and adjusts  all the places that use it.
>> The second removes almost all the remaining msys special processing in
>> TAP tests.
> Hm. Do we have to worry about backpatched bugfixes breaking in the
> backbranches? Or are you planning to backpatch this stuff?
>


I will backpatch it. That'll be a bit of fun given how much the TAP
tests change, but I think it's worth it.


cheers


andrew


--
Andrew Dunstan
EDB: https://www.enterprisedb.com

  21. Re: killing perl2host

    Andres Freund <andres@anarazel.de> — 2022-02-19T21:54:46Z

    On 2022-02-19 16:51:18 -0500, Andrew Dunstan wrote:
> I will backpatch it. That'll be a bit of fun given how much the TAP
> tests change, but I think it's worth it.

Agreed. And thanks.

  22. Re: killing perl2host

    Thomas Munro <thomas.munro@gmail.com> — 2022-02-20T20:55:09Z

    On Sun, Feb 20, 2022 at 10:51 AM Andrew Dunstan <andrew@dunslane.net> wrote:
> On 2/19/22 16:34, Andres Freund wrote:
> >> The first
> >> removes perl2host completely and adjusts  all the places that use it.
> >> The second removes almost all the remaining msys special processing in
> >> TAP tests.

Very nice improvement.  Thanks for sorting this out.  I didn't enjoy
playing "Wordle" with the build farm.