Thread: killing perl2host
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
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
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
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.
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
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
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
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
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
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
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
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
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.
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
Attachment
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
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
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
Attachment
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
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
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
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.
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.