Обсуждение: "openssl" should not be optional
Debian's reproducible-builds machinery has discovered a problem in the SSL tests: When building with SSL support, but /usr/bin/openssl missing (i.e "libssl-dev" installed, but "openssl" missing), the tests fail in subtle ways: checking for openssl... no configure: using openssl: openssl not found checking for openssl/ssl.h... yes checking for openssl/err.h... yes build/src/test/ssl/tmp_check/log/regress_log_001_ssltests: Can't exec "x509": No such file or directory at t/001_ssltests.pl line 751. couldn't run " x509" to get client cert serialno at t/001_ssltests.pl line 775. build/src/test/ssl/tmp_check/log/regress_log_003_sslinfo: [08:42:02.209](0.029s) ok 11 - ssl_client_serial() compared with pg_stat_ssl psql:<stdin>:1: ERROR: invalid X.509 field name: "invalid" [08:42:02.238](0.029s) ok 12 - ssl_client_dn_field() for an invalid field Full build log: https://reproduce.debian.net/amd64-pull184/api/v1/builds/66623/log The problem does not show up on the normal Debian build daemons. While the build environment there is fairly minimal, it does have "openssl" preinstalled. So I cannot yet say if this problem is new in PG18, or just never got detected in older branches. While it is probably possible to skip the tests when the configure probe did not find the openssl binary, IMHO the configure check should already fail. That's more robust and easier. Attached is a WIP patch that implements that for autoconf. Christoph
Вложения
> On 24 Sep 2025, at 13:14, Christoph Berg <myon@debian.org> wrote: > While it is probably possible to skip the tests when the configure > probe did not find the openssl binary, IMHO the configure check should > already fail. That's more robust and easier. It seems a bit restrictive to require the openssl binary which is test-only, since we allow building with ssl but without TAP support (which is where the openssl binary is used). -- Daniel Gustafsson
Re: Daniel Gustafsson > It seems a bit restrictive to require the openssl binary which is test-only, > since we allow building with ssl but without TAP support (which is where the > openssl binary is used). Ok, but then the error messages should be better. This was found because a fellow Debian developer was smart enough to spot the extra space in that " x509" error message... (And another one knew about this difference between the different build environments.) Christoph
> On 24 Sep 2025, at 13:37, Christoph Berg <myon@debian.org> wrote: > > Re: Daniel Gustafsson >> It seems a bit restrictive to require the openssl binary which is test-only, >> since we allow building with ssl but without TAP support (which is where the >> openssl binary is used). > > Ok, but then the error messages should be better. This was found > because a fellow Debian developer was smart enough to spot the extra > space in that " x509" error message... (And another one knew about > this difference between the different build environments.) If we make it optional and skip the relevant tests then there wouldn't be any errors messages? I do agree that all messaging around should be very clear though, so it's obvious why tests were skipped. Do you feel like expanding your patch or should I? -- Daniel Gustafsson
Re: Daniel Gustafsson > If we make it optional and skip the relevant tests then there wouldn't be any > errors messages? Or that, sure. > Do you feel like expanding your patch or should I? TBH I know very little about how TAP interfaces with the build system, so that's better with you. Thanks, Christoph
> On 24 Sep 2025, at 13:51, Christoph Berg <myon@debian.org> wrote: >> Do you feel like expanding your patch or should I? > > TBH I know very little about how TAP interfaces with the build system, > so that's better with you. Looking at this I was reminded that we already handle this by using a fallback and the test worked all along. The message for this was quite poorly worded though, and used a warning instead of a note. The attached will try to detect openssl being missing before trying to run it, and will skip the warning message if the fallback is used (which really isn't a warning in the first place). The ERROR in 003_sslinfo is intentional, we are testing that processing fails by passing an invalid value. -- Daniel Gustafsson
Вложения
Re: Daniel Gustafsson > Looking at this I was reminded that we already handle this by using a fallback > and the test worked all along. The message for this was quite poorly worded > though, and used a warning instead of a note. The attached will try to detect > openssl being missing before trying to run it, and will skip the warning > message if the fallback is used (which really isn't a warning in the first > place). Thanks, I just built the postgresql-18 again with this patch (and openssl not installed [*]). It passes fine now. In the meantime, I also got the report that postgresql-17 is not failing in that environment, so the problem is new in 18. > The ERROR in 003_sslinfo is intentional, we are testing that processing fails > by passing an invalid value. Ah, I was mentioning that in the original report because it only showed up in the failing log, but that's just because the non-failing build does not go scraping the test log files. That made the problem look bigger than it actually was. Thanks, Christoph [*] future builds will have openssl as build-dependency.
> On 24 Sep 2025, at 17:13, Christoph Berg <myon@debian.org> wrote: > > Re: Daniel Gustafsson >> Looking at this I was reminded that we already handle this by using a fallback >> and the test worked all along. The message for this was quite poorly worded >> though, and used a warning instead of a note. The attached will try to detect >> openssl being missing before trying to run it, and will skip the warning >> message if the fallback is used (which really isn't a warning in the first >> place). > > Thanks, I just built the postgresql-18 again with this patch (and > openssl not installed [*]). It passes fine now. > > In the meantime, I also got the report that postgresql-17 is not > failing in that environment, so the problem is new in 18. That's odd, off the cuff I don't see anything materially different around this but I'll do some more digging. It will at least be fixed by 18.1. -- Daniel Gustafsson
Re: Daniel Gustafsson > > In the meantime, I also got the report that postgresql-17 is not > > failing in that environment, so the problem is new in 18. > > That's odd, off the cuff I don't see anything materially different around this > but I'll do some more digging. It will at least be fixed by 18.1. Because I had already added openssl to 17's build-dependencies and forgot to port that change to 18 *blush*: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1096243 Christoph