meson vs make: missing/inconsistent ENV
I noticed warnings:
Use of uninitialized value $ENV{"with_icu"} in string eq at /home/pryzbyj/src/postgres/src/bin/pg_dump/t/002_pg_dump.pl line 56.
and looked through: git grep ^export '*/Makefile'
and found that:
src/bin/pg_dump/meson.build is missing with_icu since 396d348b0
Also, e6927270c added ZSTD to src/bin/pg_basebackup/meson.build, but
it's not in ./Makefile ?? Maybe that was for consistency with other
places, or pre-emptive in case the tap tests want to do tests involving
the ZSTD tool. But it'd be better if ./Makefile had it too.
The rest I think are not errors:
src/test/meson.build is missing PG_TEST_EXTRA
src/bin/pg_upgrade/meson.build and ../src/test/recovery/meson.build
are missing REGRESS_SHLIB
Is there any consideration of promoting these or other warnings to
fatal?
--
Justin
Hi,
On 2023-02-26 16:52:39 -0600, Justin Pryzby wrote:
I noticed warnings:
Use of uninitialized value $ENV{"with_icu"} in string eq at /home/pryzbyj/src/postgres/src/bin/pg_dump/t/002_pg_dump.pl line 56.and looked through: git grep ^export '*/Makefile'
and found that:
src/bin/pg_dump/meson.build is missing with_icu since 396d348b0
Looks like it.
Also, e6927270c added ZSTD to src/bin/pg_basebackup/meson.build, but
it's not in ./Makefile ?? Maybe that was for consistency with other
places, or pre-emptive in case the tap tests want to do tests involving
the ZSTD tool. But it'd be better if ./Makefile had it too.
I suspect I just over-eagerly added it when the pg_basebackup zstd support
went in, using the GZIP_PROGRAM/LZ4 cases as a template. And foolishly
assuming a newly added compression method would be tested.
The rest I think are not errors:
src/test/meson.build is missing PG_TEST_EXTRA
src/bin/pg_upgrade/meson.build and ../src/test/recovery/meson.build
are missing REGRESS_SHLIB
Yep, these are added in the top-level meson.build.
Is there any consideration of promoting these or other warnings to
fatal?
You mean the perl warnings?
Greetings,
Andres Freund
On Sun, Feb 26, 2023 at 03:21:04PM -0800, Andres Freund wrote:
Is there any consideration of promoting these or other warnings to
fatal?You mean the perl warnings?
Yes - it'd be nice if the warnings caused an obvious failure to allow
addressing the issue. I noticed the icu warning while looking at a bug
in 0da243fed, and updating to add ZSTD.
Justin Pryzby <pryzby@telsasoft.com> writes:
On Sun, Feb 26, 2023 at 03:21:04PM -0800, Andres Freund wrote:
Is there any consideration of promoting these or other warnings to
fatal?You mean the perl warnings?
Yes - it'd be nice if the warnings caused an obvious failure to allow
addressing the issue. I noticed the icu warning while looking at a bug
in 0da243fed, and updating to add ZSTD.
Perl warnings can be made fatal with `use warnings FATAL =>
<categories>;`, but one should be careful about which categories to
fatalise, per <https://metacpan.org/pod/warnings#Fatal-Warnings>.
Some categories are inherently unsafe to fatalise, as documented in
<https://metacpan.org/pod/strictures#CATEGORY-SELECTIONS>.
- ilmari
Dagfinn Ilmari Mannsåker <ilmari@ilmari.org> writes:
Justin Pryzby <pryzby@telsasoft.com> writes:
On Sun, Feb 26, 2023 at 03:21:04PM -0800, Andres Freund wrote:
Is there any consideration of promoting these or other warnings to
fatal?You mean the perl warnings?
Yes - it'd be nice if the warnings caused an obvious failure to allow
addressing the issue. I noticed the icu warning while looking at a bug
in 0da243fed, and updating to add ZSTD.Perl warnings can be made fatal with `use warnings FATAL =>
<categories>;`, but one should be careful about which categories to
fatalise, per <https://metacpan.org/pod/warnings#Fatal-Warnings>.Some categories are inherently unsafe to fatalise, as documented in
<https://metacpan.org/pod/strictures#CATEGORY-SELECTIONS>.
One disadvantage of making the warnings fatal is that it immediately
aborts the test. Another option would be to to turn warnings into test
failures, à la https://metacpan.org/pod/Test::Warnings or
https://metacpan.org/pod/Test::FailWarnings. Both those modules support
all the Perl versions we do, and have no non-core dependencies, but if
we don't want to add any more dependencies we can incorporate the logic
into one of our own testing modules.
- ilmari
On 2023-02-27 Mo 06:17, Dagfinn Ilmari Mannsåker wrote:
Justin Pryzby<pryzby@telsasoft.com> writes:
On Sun, Feb 26, 2023 at 03:21:04PM -0800, Andres Freund wrote:
Is there any consideration of promoting these or other warnings to
fatal?You mean the perl warnings?
Yes - it'd be nice if the warnings caused an obvious failure to allow
addressing the issue. I noticed the icu warning while looking at a bug
in 0da243fed, and updating to add ZSTD.Perl warnings can be made fatal with `use warnings FATAL =>
<categories>;`, but one should be careful about which categories to
fatalise, per<https://metacpan.org/pod/warnings#Fatal-Warnings>.Some categories are inherently unsafe to fatalise, as documented in
<https://metacpan.org/pod/strictures#CATEGORY-SELECTIONS>.
Yeah.
It would be nice if there were some fuller explanation of the various
categories, but I don't know of one.
cheers
andrew
--
Andrew Dunstan
EDB:https://www.enterprisedb.com
On 2023-02-27 Mo 07:33, Andrew Dunstan wrote:
On 2023-02-27 Mo 06:17, Dagfinn Ilmari Mannsåker wrote:
Justin Pryzby<pryzby@telsasoft.com> writes:
On Sun, Feb 26, 2023 at 03:21:04PM -0800, Andres Freund wrote:
Is there any consideration of promoting these or other warnings to
fatal?You mean the perl warnings?
Yes - it'd be nice if the warnings caused an obvious failure to allow
addressing the issue. I noticed the icu warning while looking at a bug
in 0da243fed, and updating to add ZSTD.Perl warnings can be made fatal with `use warnings FATAL =>
<categories>;`, but one should be careful about which categories to
fatalise, per<https://metacpan.org/pod/warnings#Fatal-Warnings>.Some categories are inherently unsafe to fatalise, as documented in
<https://metacpan.org/pod/strictures#CATEGORY-SELECTIONS>.Yeah.
It would be nice if there were some fuller explanation of the various
categories, but I don't know of one.
Looks like the explanations are in the perldiag manual page.
<https://perldoc.perl.org/perldiag>
cheers
andrew
--
Andrew Dunstan
EDB:https://www.enterprisedb.com
On Sun, Feb 26, 2023 at 03:21:04PM -0800, Andres Freund wrote:
On 2023-02-26 16:52:39 -0600, Justin Pryzby wrote:
Also, e6927270c added ZSTD to src/bin/pg_basebackup/meson.build, but
it's not in ./Makefile ?? Maybe that was for consistency with other
places, or pre-emptive in case the tap tests want to do tests involving
the ZSTD tool. But it'd be better if ./Makefile had it too.I suspect I just over-eagerly added it when the pg_basebackup zstd support
went in, using the GZIP_PROGRAM/LZ4 cases as a template. And foolishly
assuming a newly added compression method would be tested.
leaving the discussion with the perl warnings aside for the moment,
these still need to be adjusted. Justin, would you like to write a
patch with everything you have found?
--
Michael
Hi,
On 2023-03-09 09:36:52 +0900, Michael Paquier wrote:
On Sun, Feb 26, 2023 at 03:21:04PM -0800, Andres Freund wrote:
On 2023-02-26 16:52:39 -0600, Justin Pryzby wrote:
Also, e6927270c added ZSTD to src/bin/pg_basebackup/meson.build, but
it's not in ./Makefile ?? Maybe that was for consistency with other
places, or pre-emptive in case the tap tests want to do tests involving
the ZSTD tool. But it'd be better if ./Makefile had it too.I suspect I just over-eagerly added it when the pg_basebackup zstd support
went in, using the GZIP_PROGRAM/LZ4 cases as a template. And foolishly
assuming a newly added compression method would be tested.leaving the discussion with the perl warnings aside for the moment,
these still need to be adjusted. Justin, would you like to write a
patch with everything you have found?
I now pushed a fix for the two obvious cases pointed out by Justin.
Greetings,
Andres Freund