Обсуждение: add support for the old naming libs convention on windows (ssleay32.lib and libeay32.lib)

> On 2 Dec 2024, at 22:12, Darek Ślusarczyk <dslusarczyk@splunk.com> wrote:

> According to the postgresql-17 requirements https://www.postgresql.org/docs/17/install-requirements.html
> the minimum required version of openssl is 1.0.2.
> In that version, the naming convention on windows is still ssleay32.[lib|dll] and
> libeay32.[lib|dll] instead of libssl.[lib|dll] and libcrypto.[lib|dll].
> It changed in version 1.1.0 https://github.com/openssl/openssl/issues/10332#issuecomment-549027653

Correct, nice catch.

> I also submitted a pull request on GitHub, which can be found here: https://github.com/postgres/postgres/pull/188

Thanks for submitting it here once the PR was auto-closed, we don't use Github
as I believe the bot stated.

> -    ssl_lib = cc.find_library('ssl',
> +    ssl_lib = cc.find_library(['ssl', 'ssleay32'],

I'm not a meson expert by any means but I was suprised to see this, libname is
defined as str and not list[str] in the meson documentaion so I didn't expect
that to work.  Is the list prioritized in order in case both libs exist on the
system?  On a non-Windows system I wouldn't want libssleay32 to be picked up
over libssl if it happened to exist etc.

--
Daniel Gustafsson

> On 9 Dec 2024, at 07:25, Darek Ślusarczyk <dslusarczyk@splunk.com> wrote:

> I've prepared another patch:
> - it prioritizes libssl and libcrypto over ssleay32 and libeay32
> - it checks ssleay32 and libeay32 on windows only
> - I tested it locally on both lnx/win enforcing various possible scenarios

I'm neither a Windows or Meson expert but this seems correct to me and matches
the autoconf stanza for OpenSSL libraries on Windows.  CC:ing one of the
project experts on Meson for a second opinion.

--
Daniel Gustafsson

Hi,

Thank you for working on this!

On Tue, 10 Dec 2024 at 00:15, Darek Ślusarczyk <dslusarczyk@splunk.com> wrote:
>
> I've prepared another patch:
> - it prioritizes libssl and libcrypto over ssleay32 and libeay32
> - it checks ssleay32 and libeay32 on windows only
> - I tested it locally on both lnx/win enforcing various possible scenarios
>
> diff --git a/meson.build b/meson.build
> index e5ce437a5c7..70b003a5f23 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -1343,14 +1343,35 @@ if sslopt in ['auto', 'openssl']
>
>    # via library + headers
>    if not ssl.found()
> +    is_windows = build_system == 'windows'

I think this should be host_system instead of build_system.

> +
> +    ssl_lib_common_params = {
> +      'dirs': test_lib_d,
> +      'header_include_directories': postgres_inc,
> +      'has_headers': ['openssl/ssl.h', 'openssl/err.h'],
> +    }
>      ssl_lib = cc.find_library('ssl',
> -      dirs: test_lib_d,
> -      header_include_directories: postgres_inc,
> -      has_headers: ['openssl/ssl.h', 'openssl/err.h'],
> -      required: openssl_required)
> +      kwargs: ssl_lib_common_params,
> +      required: openssl_required and not is_windows
> +    )
> +    # if 'ssl' is not found and it's windows, try 'ssleay32'

It would be nice to explain why we are trying another library for Windows.

> +    if not ssl_lib.found() and is_windows
> +      ssl_lib = cc.find_library('ssleay32',
> +        kwargs: ssl_lib_common_params,
> +        required: openssl_required
> +      )
> +    endif
> +
>      crypto_lib = cc.find_library('crypto',
>        dirs: test_lib_d,
> -      required: openssl_required)
> +      required: openssl_required and not is_windows)
> +    # if 'crypto' is not found and it's windows, try 'libeay32'

Same above.

> +    if not crypto_lib.found() and is_windows
> +      crypto_lib = cc.find_library('libeay32',
> +        dirs: test_lib_d,
> +        required: openssl_required)
> +    endif
> +
>      if ssl_lib.found() and crypto_lib.found()
>        ssl_int = [ssl_lib, crypto_lib]
>        ssl = declare_dependency(dependencies: ssl_int, include_directories: postgres_inc)

Other than these LGTM.

--
Regards,
Nazir Bilal Yavuz
Microsoft

> On 7 Jan 2025, at 19:25, Darek Ślusarczyk <dslusarczyk@splunk.com> wrote:

> Thanks for reviewing the patch - I've added a few changes and it looks as follows:

Thanks for the new version, I've tested it in CI against v17 and plan to commit
it soon.

--
Daniel Gustafsson

> On 9 Jan 2025, at 13:50, Daniel Gustafsson <daniel@yesql.se> wrote:
>
>> On 7 Jan 2025, at 19:25, Darek Ślusarczyk <dslusarczyk@splunk.com> wrote:
>
>> Thanks for reviewing the patch - I've added a few changes and it looks as follows:
>
> Thanks for the new version, I've tested it in CI against v17 and plan to commit
> it soon.

Committed to v17 and backpatched to v16 which is when the Meson support was
added.

--
Daniel Gustafsson