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 Wed, Dec 4, 2024 at 3:48 PM Daniel Gustafsson <daniel@yesql.se> wrote:

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

wrote:
[...]

-    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.

Good catch - indeed, you are right. Due to some other changes, the flow in
our internal builds went through
https://github.com/postgres/postgres/blob/master/meson.build#L1358
instead of
https://github.com/postgres/postgres/blob/master/meson.build#L1346
and I thought I fixed the issue by making the build green. But it wasn't
the case :-P

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'
+
+    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'
+    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'
+    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)

for easier reading, it is also available as a pull-request (closed
automatically as expected)
https://github.com/postgres/postgres/pull/191

--
marines() {
return Darek_Slusarczyk;
}

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

Hi,

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

diff --git a/meson.build b/meson.build
index e5ce437a5c7..37d4e9ca741 100644
--- a/meson.build
+++ b/meson.build
@@ -1343,14 +1343,41 @@ if sslopt in ['auto', 'openssl']

   # via library + headers
   if not ssl.found()
+    is_windows = host_system == 'windows'
+
+    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
+    )
+    # before openssl version 1.1.0, there was a different naming
convention for libraries on win
+    # the counterpart for libssl.[lib|dll] was ssleay32.[lib|dll]
+    # more details in
https://github.com/openssl/openssl/issues/10332#issuecomment-549027653
+    # hence if 'ssl' is not found and it's windows, try 'ssleay32'
+    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)
+    # before openssl version 1.1.0, there was a different naming
convention for libraries on win
+    # the counterpart for libcrypto.[lib|dll] was libeay32.[lib|dll]
+    # more details in
https://github.com/openssl/openssl/issues/10332#issuecomment-549027653
+    # hence if 'crypto' is not found and it's windows, try 'libeay32'
+    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)

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

thnx,
ds
--
marines() {
return Darek_Slusarczyk;
}

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

On Fri, Feb 7, 2025 at 3:14 PM Daniel Gustafsson <daniel@yesql.se> wrote:

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.

Great news, thnx!

--
marines() {
return Darek_Slusarczyk;
}