Fix use of openssl.path() if openssl isn't found

Started by Tristan Partinabout 2 years ago5 messages
#1Tristan Partin
tristan@neon.tech
1 attachment(s)

Found this issue during my Fedora 39 upgrade. Tested that uninstalling
openssl still allows the various ssl tests to run and succeed.

--
Tristan Partin
Neon (https://neon.tech)

Attachments:

v1-0001-Fix-use-of-openssl.path-if-openssl-isn-t-found.patchtext/x-patch; charset=utf-8; name=v1-0001-Fix-use-of-openssl.path-if-openssl-isn-t-found.patchDownload
From d684d6fc1546335804d2ed82ff909991965a61a8 Mon Sep 17 00:00:00 2001
From: Tristan Partin <tristan@neon.tech>
Date: Tue, 7 Nov 2023 15:59:04 -0600
Subject: [PATCH v1] Fix use of openssl.path() if openssl isn't found

openssl(1) is an optional dependency of the Postgres Meson build, but
was inadvertantly required when defining some SSL tests.
---
 src/test/ssl/meson.build | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/test/ssl/meson.build b/src/test/ssl/meson.build
index 4cda81f3bc..c3ffcaa032 100644
--- a/src/test/ssl/meson.build
+++ b/src/test/ssl/meson.build
@@ -1,5 +1,10 @@
 # Copyright (c) 2022-2023, PostgreSQL Global Development Group
 
+openssl_path = ''
+if openssl.found()
+  openssl_path = openssl.path()
+endif
+
 tests += {
   'name': 'ssl',
   'sd': meson.current_source_dir(),
@@ -7,7 +12,7 @@ tests += {
   'tap': {
     'env': {
       'with_ssl': ssl_library,
-      'OPENSSL': openssl.path(),
+      'OPENSSL': openssl_path,
     },
     'tests': [
       't/001_ssltests.pl',
-- 
Tristan Partin
Neon (https://neon.tech)

#2Michael Paquier
michael@paquier.xyz
In reply to: Tristan Partin (#1)
Re: Fix use of openssl.path() if openssl isn't found

On Tue, Nov 07, 2023 at 04:06:56PM -0600, Tristan Partin wrote:

Found this issue during my Fedora 39 upgrade. Tested that uninstalling
openssl still allows the various ssl tests to run and succeed.

Good catch. You are right that this is inconsistent with what we
expect in the test.

+openssl_path = ''
+if openssl.found()
+ openssl_path = openssl.path()
+endif
+
tests += {
'name': 'ssl',
'sd': meson.current_source_dir(),
@@ -7,7 +12,7 @@ tests += {
'tap': {
'env': {
'with_ssl': ssl_library,
- 'OPENSSL': openssl.path(),
+ 'OPENSSL': openssl_path,
},
'tests': [
't/001_ssltests.pl',

Okay, that's a nit and it leads to the same result, but why not using
the same one-liner style like all the other meson.build files that
rely on optional commands? See pg_verifybackup, pg_dump, etc. That
would be more consistent.
--
Michael

#3Tristan Partin
tristan@neon.tech
In reply to: Michael Paquier (#2)
1 attachment(s)
Re: Fix use of openssl.path() if openssl isn't found

On Tue Nov 7, 2023 at 11:53 PM CST, Michael Paquier wrote:

On Tue, Nov 07, 2023 at 04:06:56PM -0600, Tristan Partin wrote:

Found this issue during my Fedora 39 upgrade. Tested that uninstalling
openssl still allows the various ssl tests to run and succeed.

Good catch. You are right that this is inconsistent with what we
expect in the test.

+openssl_path = ''
+if openssl.found()
+ openssl_path = openssl.path()
+endif
+
tests += {
'name': 'ssl',
'sd': meson.current_source_dir(),
@@ -7,7 +12,7 @@ tests += {
'tap': {
'env': {
'with_ssl': ssl_library,
- 'OPENSSL': openssl.path(),
+ 'OPENSSL': openssl_path,
},
'tests': [
't/001_ssltests.pl',

Okay, that's a nit and it leads to the same result, but why not using
the same one-liner style like all the other meson.build files that
rely on optional commands? See pg_verifybackup, pg_dump, etc. That
would be more consistent.

Because I forgot there were ternary statements in Meson :). Thanks for
the review. Here is v2.

--
Tristan Partin
Neon (https://neon.tech)

Attachments:

v2-0001-Fix-use-of-openssl.path-if-openssl-isn-t-found.patchtext/x-patch; charset=utf-8; name=v2-0001-Fix-use-of-openssl.path-if-openssl-isn-t-found.patchDownload
From 5fc0460b0b85b8f04976182c0f6ec650c40df833 Mon Sep 17 00:00:00 2001
From: Tristan Partin <tristan@neon.tech>
Date: Tue, 7 Nov 2023 15:59:04 -0600
Subject: [PATCH v2] Fix use of openssl.path() if openssl isn't found

openssl(1) is an optional dependency of the Postgres Meson build, but
was inadvertantly required when defining some SSL tests.
---
 src/test/ssl/meson.build | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/test/ssl/meson.build b/src/test/ssl/meson.build
index 4cda81f3bc..ed83c438ef 100644
--- a/src/test/ssl/meson.build
+++ b/src/test/ssl/meson.build
@@ -7,7 +7,7 @@ tests += {
   'tap': {
     'env': {
       'with_ssl': ssl_library,
-      'OPENSSL': openssl.path(),
+      'OPENSSL': openssl.found() ? openssl.path : '',
     },
     'tests': [
       't/001_ssltests.pl',
-- 
Tristan Partin
Neon (https://neon.tech)

#4Michael Paquier
michael@paquier.xyz
In reply to: Tristan Partin (#3)
Re: Fix use of openssl.path() if openssl isn't found

On Wed, Nov 08, 2023 at 12:07:49AM -0600, Tristan Partin wrote:

'with_ssl': ssl_library,
-      'OPENSSL': openssl.path(),
+      'OPENSSL': openssl.found() ? openssl.path : '',

Except that this was incorrect. I've fixed the grammar and applied
that down to 16.
--
Michael

#5Tristan Partin
tristan@neon.tech
In reply to: Michael Paquier (#4)
Re: Fix use of openssl.path() if openssl isn't found

On Wed Nov 8, 2023 at 2:31 AM CST, Michael Paquier wrote:

On Wed, Nov 08, 2023 at 12:07:49AM -0600, Tristan Partin wrote:

'with_ssl': ssl_library,
-      'OPENSSL': openssl.path(),
+      'OPENSSL': openssl.found() ? openssl.path : '',

Except that this was incorrect. I've fixed the grammar and applied
that down to 16.

Coding at 12 in the morning is never conducive to coherent thought :).
Thanks. Sorry for the trouble.

--
Tristan Partin
Neon (https://neon.tech)