Thread: enable certain TAP tests for MSVC builds
Certain TAP tests rely on settings that the Make files provide for them. However vcregress.pl doesn't provide those settings. This patch remedies that, and I propose to apply it shortly (when we have a fix for the SSL tests that I will write about separately) and backpatch it appropriately. cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com
Attachment
On Sun, Dec 05, 2021 at 11:57:31AM -0500, Andrew Dunstan wrote: > Certain TAP tests rely on settings that the Make files provide for them. > However vcregress.pl doesn't provide those settings. This patch remedies > that, and I propose to apply it shortly (when we have a fix for the SSL > tests that I will write about separately) and backpatch it appropriately. > --- a/src/tools/msvc/vcregress.pl > +++ b/src/tools/msvc/vcregress.pl > @@ -59,6 +59,12 @@ copy("$Config/autoinc/autoinc.dll", "src/test/regress"); > copy("$Config/regress/regress.dll", "src/test/regress"); > copy("$Config/dummy_seclabel/dummy_seclabel.dll", "src/test/regress"); > > +# Settings used by TAP tests > +$ENV{with_ssl} = $config->{openssl} ? 'openssl' : 'no'; > +$ENV{with_ldap} = $config->{ldap} ? 'yes' : 'no'; > +$ENV{with_icu} = $config->{icu} ? 'yes' : 'no'; > +$ENV{with_gssapi} = $config->{gss} ? 'yes' : 'no'; That's appropriate. There are more variables to cover: $ git grep -n ^export ':(glob)**/Makefile' src/bin/pg_basebackup/Makefile:22:export LZ4 src/bin/pg_basebackup/Makefile:23:export TAR src/bin/pg_basebackup/Makefile:27:export GZIP_PROGRAM=$(GZIP) src/bin/psql/Makefile:20:export with_readline src/test/kerberos/Makefile:16:export with_gssapi with_krb_srvnam src/test/ldap/Makefile:16:export with_ldap src/test/modules/ssl_passphrase_callback/Makefile:3:export with_ssl src/test/recovery/Makefile:20:export REGRESS_SHLIB src/test/ssl/Makefile:18:export with_ssl src/test/subscription/Makefile:18:export with_icu
On 12/5/21 14:47, Noah Misch wrote: > On Sun, Dec 05, 2021 at 11:57:31AM -0500, Andrew Dunstan wrote: >> Certain TAP tests rely on settings that the Make files provide for them. >> However vcregress.pl doesn't provide those settings. This patch remedies >> that, and I propose to apply it shortly (when we have a fix for the SSL >> tests that I will write about separately) and backpatch it appropriately. >> --- a/src/tools/msvc/vcregress.pl >> +++ b/src/tools/msvc/vcregress.pl >> @@ -59,6 +59,12 @@ copy("$Config/autoinc/autoinc.dll", "src/test/regress"); >> copy("$Config/regress/regress.dll", "src/test/regress"); >> copy("$Config/dummy_seclabel/dummy_seclabel.dll", "src/test/regress"); >> >> +# Settings used by TAP tests >> +$ENV{with_ssl} = $config->{openssl} ? 'openssl' : 'no'; >> +$ENV{with_ldap} = $config->{ldap} ? 'yes' : 'no'; >> +$ENV{with_icu} = $config->{icu} ? 'yes' : 'no'; >> +$ENV{with_gssapi} = $config->{gss} ? 'yes' : 'no'; > That's appropriate. There are more variables to cover: > > $ git grep -n ^export ':(glob)**/Makefile' > src/bin/pg_basebackup/Makefile:22:export LZ4 > src/bin/pg_basebackup/Makefile:23:export TAR > src/bin/pg_basebackup/Makefile:27:export GZIP_PROGRAM=$(GZIP) > src/bin/psql/Makefile:20:export with_readline > src/test/kerberos/Makefile:16:export with_gssapi with_krb_srvnam > src/test/ldap/Makefile:16:export with_ldap > src/test/modules/ssl_passphrase_callback/Makefile:3:export with_ssl > src/test/recovery/Makefile:20:export REGRESS_SHLIB > src/test/ssl/Makefile:18:export with_ssl > src/test/subscription/Makefile:18:export with_icu LZ4/TAR/GZIP_PROGAM: It's not clear what these should be set to. The TAP tests skip tests that use them if they are not set. with_readline: we don't build with readline on Windows, period. I guess we could just set it to "no". REGRESS_SHLIB: already set in vcregress.pl with_krb_srvnam: the default is "postgres", we could just set it to that I guess. cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com
On Sun, Dec 05, 2021 at 06:00:08PM -0500, Andrew Dunstan wrote: > On 12/5/21 14:47, Noah Misch wrote: > > On Sun, Dec 05, 2021 at 11:57:31AM -0500, Andrew Dunstan wrote: > >> Certain TAP tests rely on settings that the Make files provide for them. > >> However vcregress.pl doesn't provide those settings. This patch remedies > >> that, and I propose to apply it shortly (when we have a fix for the SSL > >> tests that I will write about separately) and backpatch it appropriately. > >> --- a/src/tools/msvc/vcregress.pl > >> +++ b/src/tools/msvc/vcregress.pl > >> @@ -59,6 +59,12 @@ copy("$Config/autoinc/autoinc.dll", "src/test/regress"); > >> copy("$Config/regress/regress.dll", "src/test/regress"); > >> copy("$Config/dummy_seclabel/dummy_seclabel.dll", "src/test/regress"); > >> > >> +# Settings used by TAP tests > >> +$ENV{with_ssl} = $config->{openssl} ? 'openssl' : 'no'; > >> +$ENV{with_ldap} = $config->{ldap} ? 'yes' : 'no'; > >> +$ENV{with_icu} = $config->{icu} ? 'yes' : 'no'; > >> +$ENV{with_gssapi} = $config->{gss} ? 'yes' : 'no'; > > That's appropriate. There are more variables to cover: > > > > $ git grep -n ^export ':(glob)**/Makefile' > > src/bin/pg_basebackup/Makefile:22:export LZ4 > > src/bin/pg_basebackup/Makefile:23:export TAR > > src/bin/pg_basebackup/Makefile:27:export GZIP_PROGRAM=$(GZIP) > > src/bin/psql/Makefile:20:export with_readline > > src/test/kerberos/Makefile:16:export with_gssapi with_krb_srvnam > > src/test/ldap/Makefile:16:export with_ldap > > src/test/modules/ssl_passphrase_callback/Makefile:3:export with_ssl > > src/test/recovery/Makefile:20:export REGRESS_SHLIB > > src/test/ssl/Makefile:18:export with_ssl > > src/test/subscription/Makefile:18:export with_icu > > LZ4/TAR/GZIP_PROGAM: It's not clear what these should be set to. The TAP > tests skip tests that use them if they are not set. Could add config.pl entries for those. Preventing those skips on Windows may or may not be worth making config.pl readers think about them. > with_readline: we don't build with readline on Windows, period. I guess > we could just set it to "no". > with_krb_srvnam: the default is "postgres", we could just set it to that > I guess. Works for me.
On 12/5/21 18:32, Noah Misch wrote: > On Sun, Dec 05, 2021 at 06:00:08PM -0500, Andrew Dunstan wrote: >> On 12/5/21 14:47, Noah Misch wrote: >>> On Sun, Dec 05, 2021 at 11:57:31AM -0500, Andrew Dunstan wrote: >>>> Certain TAP tests rely on settings that the Make files provide for them. >>>> However vcregress.pl doesn't provide those settings. This patch remedies >>>> that, and I propose to apply it shortly (when we have a fix for the SSL >>>> tests that I will write about separately) and backpatch it appropriately. >>>> --- a/src/tools/msvc/vcregress.pl >>>> +++ b/src/tools/msvc/vcregress.pl >>>> @@ -59,6 +59,12 @@ copy("$Config/autoinc/autoinc.dll", "src/test/regress"); >>>> copy("$Config/regress/regress.dll", "src/test/regress"); >>>> copy("$Config/dummy_seclabel/dummy_seclabel.dll", "src/test/regress"); >>>> >>>> +# Settings used by TAP tests >>>> +$ENV{with_ssl} = $config->{openssl} ? 'openssl' : 'no'; >>>> +$ENV{with_ldap} = $config->{ldap} ? 'yes' : 'no'; >>>> +$ENV{with_icu} = $config->{icu} ? 'yes' : 'no'; >>>> +$ENV{with_gssapi} = $config->{gss} ? 'yes' : 'no'; >>> That's appropriate. There are more variables to cover: >>> >>> $ git grep -n ^export ':(glob)**/Makefile' >>> src/bin/pg_basebackup/Makefile:22:export LZ4 >>> src/bin/pg_basebackup/Makefile:23:export TAR >>> src/bin/pg_basebackup/Makefile:27:export GZIP_PROGRAM=$(GZIP) >>> src/bin/psql/Makefile:20:export with_readline >>> src/test/kerberos/Makefile:16:export with_gssapi with_krb_srvnam >>> src/test/ldap/Makefile:16:export with_ldap >>> src/test/modules/ssl_passphrase_callback/Makefile:3:export with_ssl >>> src/test/recovery/Makefile:20:export REGRESS_SHLIB >>> src/test/ssl/Makefile:18:export with_ssl >>> src/test/subscription/Makefile:18:export with_icu >> LZ4/TAR/GZIP_PROGAM: It's not clear what these should be set to. The TAP >> tests skip tests that use them if they are not set. > Could add config.pl entries for those. Preventing those skips on Windows may > or may not be worth making config.pl readers think about them. > >> with_readline: we don't build with readline on Windows, period. I guess >> we could just set it to "no". >> with_krb_srvnam: the default is "postgres", we could just set it to that >> I guess. > Works for me. All done. cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com
On Tue, Dec 07, 2021 at 03:40:29PM -0500, Andrew Dunstan wrote: > All done. bowerbird is complaining here with the tests of pg_basebackup: https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=bowerbird&dt=2021-12-08%2004%3A52%3A27 tar: Cannot execute remote shell: No such file or directory tar: H\\:/prog/bf/root/HEAD/pgsql.build/src/bin/pg_basebackup/tmp_check/t_010_pg_basebackup_main_data/backup/tarbackup2/base.tar: Cannot open: I/O error tar: Error is not recoverable: exiting now This comes from init_from_backup() via the tar_command. I am not sure what to think about this error. Perhaps this is related to the environment. +$ENV{TAR} ||= 'tar'; +$ENV{LZ4} ||= 'lz4'; +$ENV{GZIP_PROGRAM} ||= 'gzip'; This means that the default is to assume that those commands will be always present, no? But that may not be the case, which would cause the tests of pg_basebackup to fail in 010_pg_basebackup.pl. Shouldn't we at least check that the command can be executed if defined and skip the tests if not, in the same fashion as LZ4 and GZIP? -- Michael
Attachment
On 12/8/21 03:14, Michael Paquier wrote: > On Tue, Dec 07, 2021 at 03:40:29PM -0500, Andrew Dunstan wrote: >> All done. > bowerbird is complaining here with the tests of pg_basebackup: > https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=bowerbird&dt=2021-12-08%2004%3A52%3A27 > > tar: Cannot execute remote shell: No such file or directory > tar: H\\:/prog/bf/root/HEAD/pgsql.build/src/bin/pg_basebackup/tmp_check/t_010_pg_basebackup_main_data/backup/tarbackup2/base.tar: > Cannot open: I/O error > tar: Error is not recoverable: exiting now > > This comes from init_from_backup() via the tar_command. I am not sure > what to think about this error. Perhaps this is related to the > environment. > > +$ENV{TAR} ||= 'tar'; > +$ENV{LZ4} ||= 'lz4'; > +$ENV{GZIP_PROGRAM} ||= 'gzip'; > This means that the default is to assume that those commands will be > always present, no? But that may not be the case, which would cause > the tests of pg_basebackup to fail in 010_pg_basebackup.pl. Shouldn't > we at least check that the command can be executed if defined and skip > the tests if not, in the same fashion as LZ4 and GZIP? Yes, I'll fix it. cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com