Horton

Thread

Commits

  1. Log OpenSSL version in ./configure output

  1. Show version of OpenSSL in ./configure output

    Michael Paquier <michael@paquier.xyz> — 2023-10-23T00:26:48Z

    Hi all,

5e4dacb9878c has reminded me that we don't show the version of OpenSSL
in the output of ./configure.  This would be useful to know when
looking at issues within the buildfarm, and I've wanted that a few
times.

How about the attached to use the openssl command, if available, to
display this information?  Libraries may be installed while the
command is not available, but in most cases I'd like to think that it
is around, and it is less complex than using something like
SSLeay_version() from libcrypto.

meson already shows this information, so no additions are required
there.  Also, LibreSSL uses `openssl`, right?

Thoughts or comments?
--
Michael

  2. Re: Show version of OpenSSL in ./configure output

    Tom Lane <tgl@sss.pgh.pa.us> — 2023-10-23T00:34:40Z

    Michael Paquier <michael@paquier.xyz> writes:
> 5e4dacb9878c has reminded me that we don't show the version of OpenSSL
> in the output of ./configure.  This would be useful to know when
> looking at issues within the buildfarm, and I've wanted that a few
> times.

+1, I've wished for that too.  It's not 100% clear that whatever
openssl is in your PATH matches the libraries we select, but this
will get it right in most cases and it seems like about the right
level of effort.

+  pgac_openssl_version="$($OPENSSL version 2> /dev/null || echo no)"

Maybe "echo 'openssl not found'" would be better.

			regards, tom lane

  3. Re: Show version of OpenSSL in ./configure output

    Michael Paquier <michael@paquier.xyz> — 2023-10-23T01:00:43Z

    On Sun, Oct 22, 2023 at 08:34:40PM -0400, Tom Lane wrote:
> +1, I've wished for that too.  It's not 100% clear that whatever
> openssl is in your PATH matches the libraries we select, but this
> will get it right in most cases and it seems like about the right
> level of effort.

Yes, I don't reallt want to add more macros for the sake of this
information.

> +  pgac_openssl_version="$($OPENSSL version 2> /dev/null || echo no)"
> 
> Maybe "echo 'openssl not found'" would be better.

Sure.
--
Michael

  4. Re: Show version of OpenSSL in ./configure output

    Peter Eisentraut <peter@eisentraut.org> — 2023-10-23T06:22:25Z

    On 23.10.23 02:26, Michael Paquier wrote:
> 5e4dacb9878c has reminded me that we don't show the version of OpenSSL
> in the output of ./configure.  This would be useful to know when
> looking at issues within the buildfarm, and I've wanted that a few
> times.
> 
> How about the attached to use the openssl command, if available, to
> display this information?  Libraries may be installed while the
> command is not available, but in most cases I'd like to think that it
> is around, and it is less complex than using something like
> SSLeay_version() from libcrypto.
> 
> meson already shows this information, so no additions are required
> there.  Also, LibreSSL uses `openssl`, right?

The problem is that the binary might not match the library, so this 
could be very misleading.  Also, meson gets the version via pkg-config, 
so the result would also be inconsistent with meson.  I am afraid this 
approach would be unreliable in the really interesting cases.

 > +  # Print version of OpenSSL, if command is available.
 > +  AC_ARG_VAR(OPENSSL, [path to openssl command])
 > +  PGAC_PATH_PROGS(OPENSSL, openssl)

There is already a call like this in configure.ac, so (if this approach 
is taken) you should rearrange things to make use of that one.

 > +  pgac_openssl_version="$($OPENSSL version 2> /dev/null || echo no)"
 > +  AC_MSG_NOTICE([using openssl $pgac_openssl_version])

  5. Re: Show version of OpenSSL in ./configure output

    Daniel Gustafsson <daniel@yesql.se> — 2023-10-23T07:18:58Z

    > On 23 Oct 2023, at 08:22, Peter Eisentraut <peter@eisentraut.org> wrote:
> 
> On 23.10.23 02:26, Michael Paquier wrote:
>> 5e4dacb9878c has reminded me that we don't show the version of OpenSSL
>> in the output of ./configure.  This would be useful to know when
>> looking at issues within the buildfarm, and I've wanted that a few
>> times.

Many +1's, this has been on my TODO for some time but has never bubbled to the
top. Thanks for working on this.

>> How about the attached to use the openssl command, if available, to
>> display this information?  Libraries may be installed while the
>> command is not available, but in most cases I'd like to think that it
>> is around, and it is less complex than using something like
>> SSLeay_version() from libcrypto.
>> meson already shows this information, so no additions are required
>> there.  Also, LibreSSL uses `openssl`, right?
> 
> The problem is that the binary might not match the library, so this could be very misleading.  Also, meson gets the version via pkg-config, so the result would also be inconsistent with meson.  I am afraid this approach would be unreliable in the really interesting cases.

I tend to agree with this, it would be preferrable to be consistent with meson
if possible/feasible.

--
Daniel Gustafsson

  6. Re: Show version of OpenSSL in ./configure output

    Tom Lane <tgl@sss.pgh.pa.us> — 2023-10-23T14:26:36Z

    Daniel Gustafsson <daniel@yesql.se> writes:
> On 23 Oct 2023, at 08:22, Peter Eisentraut <peter@eisentraut.org> wrote:
>> The problem is that the binary might not match the library, so this could be very misleading.  Also, meson gets the version via pkg-config, so the result would also be inconsistent with meson.  I am afraid this approach would be unreliable in the really interesting cases.

> I tend to agree with this, it would be preferrable to be consistent with meson
> if possible/feasible.

The configure script doesn't use pkg-config to find OpenSSL, so that
would also risk a misleading result.  I'm inclined to guess that
believing "openssl version" would be less likely to give a wrong
answer than believing "pkg-config --modversion openssl".  The former
amounts to assuming that your PATH is consistent with whatever you
set as the include and library search paths.  The latter, well,
hasn't got any principle at all, because we aren't consulting
pkg-config for this.

Also, since "PGAC_PATH_PROGS(OPENSSL, openssl)" prints the full path
to what it found, you can at least tell after the fact that you
are being misled, because you can cross-check that path against
the -L switches being used for libraries.  I don't think there's
any equivalent sanity check available for what pkg-config tells us,
if we're not consulting that for the library location.

meson.build may be fine here --- I suppose the reason that gets
the version via pkg-config is that it also gets other build details
from there.  It's slightly worrisome that the autoconf and meson
build systems might choose different openssl installations, but
that's possibly true of lots of our dependencies.

Another angle worth considering is that "openssl version" provides
more information.  On my RHEL8 box:

$ pkg-config --modversion openssl
1.1.1k
$ openssl version
OpenSSL 1.1.1k  FIPS 25 Mar 2021

On my laptop using MacPorts:

$ pkg-config --modversion openssl
3.1.3
$ openssl version
OpenSSL 3.1.3 19 Sep 2023 (Library: OpenSSL 3.1.3 19 Sep 2023)

			regards, tom lane

  7. Re: Show version of OpenSSL in ./configure output

    Peter Eisentraut <peter@eisentraut.org> — 2023-10-23T16:06:02Z

    On 23.10.23 16:26, Tom Lane wrote:
> Also, since "PGAC_PATH_PROGS(OPENSSL, openssl)" prints the full path to 
> what it found, you can at least tell after the fact that you are being 
> misled, because you can cross-check that path against the -L switches 
> being used for libraries.

Yeah, that seems ok.

  8. Re: Show version of OpenSSL in ./configure output

    Daniel Gustafsson <daniel@yesql.se> — 2023-10-23T17:58:43Z

    > On 23 Oct 2023, at 18:06, Peter Eisentraut <peter@eisentraut.org> wrote:
> 
> On 23.10.23 16:26, Tom Lane wrote:
>> Also, since "PGAC_PATH_PROGS(OPENSSL, openssl)" prints the full path to what it found, you can at least tell after the fact that you are being misled, because you can cross-check that path against the -L switches being used for libraries.
> 
> Yeah, that seems ok.

+1, all good points raised, thanks!

--
Daniel Gustafsson

  9. Re: Show version of OpenSSL in ./configure output

    Michael Paquier <michael@paquier.xyz> — 2023-10-23T23:33:30Z

    On Mon, Oct 23, 2023 at 06:06:02PM +0200, Peter Eisentraut wrote:
> On 23.10.23 16:26, Tom Lane wrote:
>> Also, since "PGAC_PATH_PROGS(OPENSSL, openssl)" prints the full path to
>> what it found, you can at least tell after the fact that you are being
>> misled, because you can cross-check that path against the -L switches
>> being used for libraries.
>
> Yeah, that seems ok.

FWIW, I was also contemplating this one yesterday:
+PKG_CHECK_MODULES(OPENSSL, openssl)

Still, when I link my builds to a custom OpenSSL one, I force PATH to
point to a command of openssl related to the libs used so
PGAC_PATH_PROGS is more useful.  I guess that everybody here does the
same.  It could be of course possible to show both the command from
PATH and from pkg-config, but that's just confusing IMO.

There may be a point in doing the same for other commands like LZ4 and
Zstandard but these have been less of a pain in the buildfarm, even if
we don't use them for that long, so I cannot get excited about
spending more ./configure cycles for these.

Please find attached a patch to move the version call close to the
existing PGAC_PATH_PROGS.  And of course, I'd like to do a backpatch.
Is that OK?
--
Michael

  10. Re: Show version of OpenSSL in ./configure output

    Tom Lane <tgl@sss.pgh.pa.us> — 2023-10-24T00:44:01Z

    Michael Paquier <michael@paquier.xyz> writes:
> Please find attached a patch to move the version call close to the
> existing PGAC_PATH_PROGS.  And of course, I'd like to do a backpatch.
> Is that OK?

OK by me.

			regards, tom lane

  11. Re: Show version of OpenSSL in ./configure output

    Michael Paquier <michael@paquier.xyz> — 2023-10-25T00:28:51Z

    On Mon, Oct 23, 2023 at 08:44:01PM -0400, Tom Lane wrote:
> OK by me.

Cool, done down to 16 as this depends on c8e4030d1bdd.
--
Michael