killing perl2host

Started by Andrew Dunstanabout 4 years ago22 messageshackers
Jump to latest
#1Andrew Dunstan
andrew@dunslane.net

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
</messages/by-id/20220125023609.5ohu3nslxgoygihl@alap3.anarazel.de&gt;

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

#2Andres Freund
andres@anarazel.de
In reply to: Andrew Dunstan (#1)
Re: killing perl2host

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

#3Andrew Dunstan
andrew@dunslane.net
In reply to: Andres Freund (#2)
Re: killing perl2host

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

#4Andres Freund
andres@anarazel.de
In reply to: Andrew Dunstan (#3)
Re: killing perl2host

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.

#5Andres Freund
andres@anarazel.de
In reply to: Andres Freund (#4)
Re: killing perl2host

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

#6Andrew Dunstan
andrew@dunslane.net
In reply to: Andres Freund (#5)
Re: killing perl2host

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

#7Andres Freund
andres@anarazel.de
In reply to: Andrew Dunstan (#6)
Re: killing perl2host

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

#8Andrew Dunstan
andrew@dunslane.net
In reply to: Andres Freund (#7)
Re: killing perl2host

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

#9Andres Freund
andres@anarazel.de
In reply to: Andrew Dunstan (#8)
Re: killing perl2host

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

#10Andrew Dunstan
andrew@dunslane.net
In reply to: Andres Freund (#9)
Re: killing perl2host

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

#11Andres Freund
andres@anarazel.de
In reply to: Andrew Dunstan (#10)
Re: killing perl2host

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

#12Andrew Dunstan
andrew@dunslane.net
In reply to: Andres Freund (#11)
Re: killing perl2host

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

#13Andres Freund
andres@anarazel.de
In reply to: Andrew Dunstan (#12)
Re: killing perl2host

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.

#14Andrew Dunstan
andrew@dunslane.net
In reply to: Andres Freund (#13)
Re: killing perl2host

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

Attachments:

msys-prove-detection.patchtext/x-patch; charset=UTF-8; name=msys-prove-detection.patchDownload+7-0
#15Andrew Dunstan
andrew@dunslane.net
In reply to: Andres Freund (#7)
Re: killing perl2host

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

#16Andrew Dunstan
andrew@dunslane.net
In reply to: Andrew Dunstan (#15)
Re: killing perl2host

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
</messages/by-id/6b467edc-4018-521f-ab18-171f098557ca@2ndquadrant.com&gt;
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

#17Andrew Dunstan
andrew@dunslane.net
In reply to: Andrew Dunstan (#1)
Re: killing perl2host

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

Attachments:

0001-remove-perl2host-completely.patchtext/x-patch; charset=UTF-8; name=0001-remove-perl2host-completely.patchDownload+25-95
0002-remove-most-msys-special-processing.patchtext/x-patch; charset=UTF-8; name=0002-remove-most-msys-special-processing.patchDownload+2-42
#18Andrew Dunstan
andrew@dunslane.net
In reply to: Andrew Dunstan (#17)
Re: killing perl2host

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

#19Andres Freund
andres@anarazel.de
In reply to: Andrew Dunstan (#17)
Re: killing perl2host

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

#20Andrew Dunstan
andrew@dunslane.net
In reply to: Andres Freund (#19)
Re: killing perl2host

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

#21Andres Freund
andres@anarazel.de
In reply to: Andrew Dunstan (#20)
#22Thomas Munro
thomas.munro@gmail.com
In reply to: Andrew Dunstan (#20)