meson: Non-feature feature options
Most meson options (meson_options.txt) that enable an external
dependency (e.g., icu, ldap) are of type 'feature'. Most of these have
a default value of 'auto', which means they are pulled in automatically
if found. Some have a default value of 'disabled' for specific reasons
(e.g., selinux). This is all good.
Two options deviate from this in annoying ways:
option('ssl', type : 'combo', choices : ['none', 'openssl'],
value : 'none',
description: 'use LIB for SSL/TLS support (openssl)')
option('uuid', type : 'combo', choices : ['none', 'bsd', 'e2fs', 'ossp'],
value : 'none',
description: 'build contrib/uuid-ossp using LIB')
These were moved over from configure like that.
The problem is that these features now cannot be automatically enabled
and behave annoyingly different from other feature options.
For the 'ssl' option, we have deprecated the --with-openssl option in
configure and replaced it with --with-ssl, in anticipation of other SSL
implementations. None of that ever happened or is currently planned
AFAICT. So I suggest that we semi-revert this, so that we can make
'openssl' an auto option in meson.
For the 'uuid' option, I'm not sure what the best way to address this
would. We could establish a search order of libraries that is used if
no specific one is set (similar to libreadline, libedit, in a way). So
we'd have one option 'uuid' that is of type feature with default 'auto'
and another option, say, 'uuid-library' of type 'combo'.
Thoughts?
Hi,
On 2/8/23 13:45, Peter Eisentraut wrote:
The problem is that these features now cannot be automatically enabled
and behave annoyingly different from other feature options.
Agreed.
For the 'ssl' option, we have deprecated the --with-openssl option in
configure and replaced it with --with-ssl, in anticipation of other
SSL implementations. None of that ever happened or is currently
planned AFAICT. So I suggest that we semi-revert this, so that we can
make 'openssl' an auto option in meson.
+1
For the 'uuid' option, I'm not sure what the best way to address this
would. We could establish a search order of libraries that is used if
no specific one is set (similar to libreadline, libedit, in a way).
So we'd have one option 'uuid' that is of type feature with default
'auto' and another option, say, 'uuid-library' of type 'combo'.
Your suggestion looks good and TCL already has a similar implementation
with what you suggested:
option('pltcl', type : 'feature', value: 'auto',
description: 'build with TCL support')
option('tcl_version', type : 'string', value : 'tcl',
description: 'specify TCL version')
Regards,
Nazir Bilal Yavuz
Microsoft
Hi,
On 2023-02-08 11:45:05 +0100, Peter Eisentraut wrote:
Most meson options (meson_options.txt) that enable an external dependency
(e.g., icu, ldap) are of type 'feature'. Most of these have a default value
of 'auto', which means they are pulled in automatically if found. Some have
a default value of 'disabled' for specific reasons (e.g., selinux). This is
all good.Two options deviate from this in annoying ways:
option('ssl', type : 'combo', choices : ['none', 'openssl'],
value : 'none',
description: 'use LIB for SSL/TLS support (openssl)')option('uuid', type : 'combo', choices : ['none', 'bsd', 'e2fs', 'ossp'],
value : 'none',
description: 'build contrib/uuid-ossp using LIB')These were moved over from configure like that.
The problem is that these features now cannot be automatically enabled and
behave annoyingly different from other feature options.
Oh, yes, this has been bothering me too.
For the 'ssl' option, we have deprecated the --with-openssl option in
configure and replaced it with --with-ssl, in anticipation of other SSL
implementations. None of that ever happened or is currently planned AFAICT.
So I suggest that we semi-revert this, so that we can make 'openssl' an auto
option in meson.
Hm. I'm inclined to leave it there - I do think it's somewhat likely that
we'll eventually end up with some platform native library. I think it's likely
the NSS patch isn't going anywhere, but I'm not sure that's true for
e.g. using the windows encryption library. IIRC Heikki had a patch at some
point.
I'd probably just add a 'auto' option, and manually make it behave like a
feature option.
For the 'uuid' option, I'm not sure what the best way to address this would.
We could establish a search order of libraries that is used if no specific
one is set (similar to libreadline, libedit, in a way). So we'd have one
option 'uuid' that is of type feature with default 'auto' and another
option, say, 'uuid-library' of type 'combo'.
Or add 'auto' as a combo option, and handle the value of the auto_features
option ourselves?
Greetings,
Andres Freund
Hi,
On 2/8/23 19:23, Andres Freund wrote:
For the 'uuid' option, I'm not sure what the best way to address this would.
We could establish a search order of libraries that is used if no specific
one is set (similar to libreadline, libedit, in a way). So we'd have one
option 'uuid' that is of type feature with default 'auto' and another
option, say, 'uuid-library' of type 'combo'.Or add 'auto' as a combo option, and handle the value of the auto_features
option ourselves?
If we do it like this, meson's --auto-features option won't work for
uuid. Is this something we want to consider?
Regards,
Nazir Bilal Yavuz
Microsoft
Hi,
I added SSL and UUID patches. UUID patch has two different fixes:
1 - v1-0002-meson-Refactor-UUID-option.patch: Adding 'auto' choice to
'uuid' combo option.
2 - v1-0002-meson-Refactor-UUID-option-with-uuid_library.patch: Making
'uuid' feature option and adding new 'uuid_library' combo option with
['auto', 'bsd', 'e2fs', 'ossp'] choices. If 'uuid_library' is set other
than 'auto' and it can't be found, build throws an error.
What do you think?
Regards,
Nazir Bilal Yavuz
Microsoft
Attachments:
v1-0001-meson-Refactor-SSL-option.patchtext/plain; charset=UTF-8; name=v1-0001-meson-Refactor-SSL-option.patchDownload
From e607f8e6de8bdd65cbaede200e184d7f908991c5 Mon Sep 17 00:00:00 2001
From: Nazir Bilal Yavuz <byavuz81@gmail.com>
Date: Mon, 13 Feb 2023 15:21:32 +0300
Subject: [PATCH v1 1/2] meson: Refactor SSL option
---
.cirrus.yml | 7 +++----
meson.build | 6 +++++-
meson_options.txt | 3 +--
src/interfaces/libpq/meson.build | 2 +-
src/makefiles/meson.build | 2 +-
src/test/modules/ssl_passphrase_callback/meson.build | 2 +-
src/test/ssl/meson.build | 2 +-
7 files changed, 13 insertions(+), 11 deletions(-)
diff --git a/.cirrus.yml b/.cirrus.yml
index f212978752..aaf4066366 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -181,7 +181,7 @@ task:
su postgres <<-EOF
meson setup \
--buildtype=debug \
- -Dcassert=true -Dssl=openssl -Duuid=bsd -Dtcl_version=tcl86 -Ddtrace=auto \
+ -Dcassert=true -Duuid=bsd -Dtcl_version=tcl86 -Ddtrace=auto \
-DPG_TEST_EXTRA="$PG_TEST_EXTRA" \
-Dextra_lib_dirs=/usr/local/lib -Dextra_include_dirs=/usr/local/include/ \
build
@@ -243,7 +243,6 @@ LINUX_CONFIGURE_FEATURES: &LINUX_CONFIGURE_FEATURES >-
LINUX_MESON_FEATURES: &LINUX_MESON_FEATURES >-
-Dllvm=enabled
- -Dssl=openssl
-Duuid=e2fs
@@ -497,7 +496,7 @@ task:
-Dextra_include_dirs=${brewpath}/include \
-Dextra_lib_dirs=${brewpath}/lib \
-Dcassert=true \
- -Dssl=openssl -Duuid=e2fs -Ddtrace=auto \
+ -Duuid=e2fs -Ddtrace=auto \
-Dsegsize_blocks=6 \
-DPG_TEST_EXTRA="$PG_TEST_EXTRA" \
build
@@ -568,7 +567,7 @@ task:
# Use /DEBUG:FASTLINK to avoid high memory usage during linking
configure_script: |
vcvarsall x64
- meson setup --backend ninja --buildtype debug -Dc_link_args=/DEBUG:FASTLINK -Dcassert=true -Db_pch=true -Dssl=openssl -Dextra_lib_dirs=c:\openssl\1.1\lib -Dextra_include_dirs=c:\openssl\1.1\include -DTAR=%TAR% -DPG_TEST_EXTRA="%PG_TEST_EXTRA%" build
+ meson setup --backend ninja --buildtype debug -Dc_link_args=/DEBUG:FASTLINK -Dcassert=true -Db_pch=true -Dextra_lib_dirs=c:\openssl\1.1\lib -Dextra_include_dirs=c:\openssl\1.1\include -DTAR=%TAR% -DPG_TEST_EXTRA="%PG_TEST_EXTRA%" build
build_script: |
vcvarsall x64
diff --git a/meson.build b/meson.build
index f534704452..563589ac48 100644
--- a/meson.build
+++ b/meson.build
@@ -1171,7 +1171,9 @@ cdata.set('USE_SYSTEMD', systemd.found() ? 1 : false)
# Library: SSL
###############################################################
-if get_option('ssl') == 'openssl'
+ssl_type = 'none'
+sslopt = get_option('ssl')
+if not sslopt.disabled()
# Try to find openssl via pkg-config et al, if that doesn't work
# (e.g. because it's provided as part of the OS, like on FreeBSD), look for
@@ -1239,6 +1241,8 @@ if get_option('ssl') == 'openssl'
endif
endforeach
+ ssl_type = 'openssl'
+
cdata.set('USE_OPENSSL', 1,
description: 'Define to 1 to build with OpenSSL support. (-Dssl=openssl)')
cdata.set('OPENSSL_API_COMPAT', '0x10001000L',
diff --git a/meson_options.txt b/meson_options.txt
index 9d3ef4aa20..9c74cc6512 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -127,8 +127,7 @@ option('readline', type : 'feature', value : 'auto',
option('selinux', type : 'feature', value : 'disabled',
description: 'build with SELinux support')
-option('ssl', type : 'combo', choices : ['none', 'openssl'],
- value : 'none',
+option('ssl', type : 'feature', value : 'auto',
description: 'use LIB for SSL/TLS support (openssl)')
option('systemd', type : 'feature', value: 'auto',
diff --git a/src/interfaces/libpq/meson.build b/src/interfaces/libpq/meson.build
index 573fd9b6ea..f62dc3ac52 100644
--- a/src/interfaces/libpq/meson.build
+++ b/src/interfaces/libpq/meson.build
@@ -117,7 +117,7 @@ tests += {
't/001_uri.pl',
't/002_api.pl',
],
- 'env': {'with_ssl': get_option('ssl')},
+ 'env': {'with_ssl': ssl_type},
},
}
diff --git a/src/makefiles/meson.build b/src/makefiles/meson.build
index bf7303dc99..2b73a9e5dd 100644
--- a/src/makefiles/meson.build
+++ b/src/makefiles/meson.build
@@ -66,7 +66,7 @@ pgxs_kv = {
'SUN_STUDIO_CC': 'no', # not supported so far
# want the chosen option, rather than the library
- 'with_ssl' : get_option('ssl'),
+ 'with_ssl' : ssl_type,
'with_uuid': uuidopt,
'default_port': get_option('pgport'),
diff --git a/src/test/modules/ssl_passphrase_callback/meson.build b/src/test/modules/ssl_passphrase_callback/meson.build
index de016b0280..0d2d8804fa 100644
--- a/src/test/modules/ssl_passphrase_callback/meson.build
+++ b/src/test/modules/ssl_passphrase_callback/meson.build
@@ -54,6 +54,6 @@ tests += {
'tests': [
't/001_testfunc.pl',
],
- 'env': {'with_ssl': 'openssl'},
+ 'env': {'with_ssl': ssl_type},
},
}
diff --git a/src/test/ssl/meson.build b/src/test/ssl/meson.build
index a8d9a5424d..4612416da3 100644
--- a/src/test/ssl/meson.build
+++ b/src/test/ssl/meson.build
@@ -6,7 +6,7 @@ tests += {
'bd': meson.current_build_dir(),
'tap': {
'env': {
- 'with_ssl': get_option('ssl'),
+ 'with_ssl': ssl_type,
'OPENSSL': openssl.path(),
},
'tests': [
--
2.25.1
v1-0002-meson-Refactor-UUID-option.patchtext/plain; charset=UTF-8; name=v1-0002-meson-Refactor-UUID-option.patchDownload
From 0d94e30facc9e1bbd489afa563fd57c9ed002bba Mon Sep 17 00:00:00 2001
From: Nazir Bilal Yavuz <byavuz81@gmail.com>
Date: Mon, 13 Feb 2023 15:22:05 +0300
Subject: [PATCH v1 2/2] meson: Refactor UUID option
---
.cirrus.yml | 5 ++--
meson.build | 60 ++++++++++++++++++++++++++++++-----------------
meson_options.txt | 4 ++--
3 files changed, 43 insertions(+), 26 deletions(-)
diff --git a/.cirrus.yml b/.cirrus.yml
index aaf4066366..d696d5dc48 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -181,7 +181,7 @@ task:
su postgres <<-EOF
meson setup \
--buildtype=debug \
- -Dcassert=true -Duuid=bsd -Dtcl_version=tcl86 -Ddtrace=auto \
+ -Dcassert=true -Dtcl_version=tcl86 -Ddtrace=auto \
-DPG_TEST_EXTRA="$PG_TEST_EXTRA" \
-Dextra_lib_dirs=/usr/local/lib -Dextra_include_dirs=/usr/local/include/ \
build
@@ -243,7 +243,6 @@ LINUX_CONFIGURE_FEATURES: &LINUX_CONFIGURE_FEATURES >-
LINUX_MESON_FEATURES: &LINUX_MESON_FEATURES >-
-Dllvm=enabled
- -Duuid=e2fs
task:
@@ -496,7 +495,7 @@ task:
-Dextra_include_dirs=${brewpath}/include \
-Dextra_lib_dirs=${brewpath}/lib \
-Dcassert=true \
- -Duuid=e2fs -Ddtrace=auto \
+ -Ddtrace=auto \
-Dsegsize_blocks=6 \
-DPG_TEST_EXTRA="$PG_TEST_EXTRA" \
build
diff --git a/meson.build b/meson.build
index 563589ac48..26f48a4c12 100644
--- a/meson.build
+++ b/meson.build
@@ -1259,31 +1259,49 @@ endif
uuidopt = get_option('uuid')
if uuidopt != 'none'
- uuidname = uuidopt.to_upper()
- if uuidopt == 'e2fs'
- uuid = dependency('uuid', required: true)
- uuidfunc = 'uuid_generate'
- uuidheader = 'uuid/uuid.h'
- elif uuidopt == 'bsd'
- # libc should have uuid function
- uuid = declare_dependency()
- uuidfunc = 'uuid_to_string'
- uuidheader = 'uuid.h'
- elif uuidopt == 'ossp'
- uuid = dependency('ossp-uuid', required: true)
- uuidfunc = 'uuid_export'
- uuidheader = 'ossp/uuid.h'
- else
- error('huh')
- endif
- if not cc.has_header_symbol(uuidheader, uuidfunc, args: test_c_args, dependencies: uuid)
+ uuidtypes = {
+ 'e2fs': {
+ 'uuiddep': 'uuid',
+ 'uuidfunc': 'uuid_generate',
+ 'uuidheader': 'uuid/uuid.h',
+ },
+ 'bsd': {
+ # libc should have uuid function
+ 'uuiddep': '',
+ 'uuidfunc': 'uuid_to_string',
+ 'uuidheader': 'uuid.h',
+ },
+ 'ossp': {
+ 'uuiddep':'ossp-uuid',
+ 'uuidfunc': 'uuid_export',
+ 'uuidheader': 'ossp/uuid.h',
+ },
+ }
+
+ uuidfound = false
+ foreach uuidname, uuidelements : uuidtypes
+ if uuidopt == 'auto' or uuidopt == uuidname
+ uuiddep = uuidelements['uuiddep']
+ uuid = uuiddep != '' ? dependency(uuiddep, required: false) : declare_dependency()
+ uuidfunc = uuidelements['uuidfunc']
+ uuidheader = uuidelements['uuidheader']
+
+ if cc.has_header_symbol(uuidheader, uuidfunc, args: test_c_args, dependencies: uuid)
+ uuidfound = true
+ uuidname = uuidname.to_upper()
+ cdata.set('HAVE_@0@'.format(uuidheader.underscorify().to_upper()), 1)
+ cdata.set('HAVE_UUID_@0@'.format(uuidname), 1,
+ description: 'Define to 1 if you have @0@ UUID support.'.format(uuidname))
+ break
+ endif
+ endif
+ endforeach
+
+ if not uuidfound and uuidopt != 'auto'
error('uuid library @0@ missing required function @1@'.format(uuidopt, uuidfunc))
endif
- cdata.set('HAVE_@0@'.format(uuidheader.underscorify().to_upper()), 1)
- cdata.set('HAVE_UUID_@0@'.format(uuidname), 1,
- description: 'Define to 1 if you have @0@ UUID support.'.format(uuidname))
else
uuid = not_found_dep
endif
diff --git a/meson_options.txt b/meson_options.txt
index 9c74cc6512..9b7768a8e2 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -133,8 +133,8 @@ option('ssl', type : 'feature', value : 'auto',
option('systemd', type : 'feature', value: 'auto',
description: 'build with systemd support')
-option('uuid', type : 'combo', choices : ['none', 'bsd', 'e2fs', 'ossp'],
- value : 'none',
+option('uuid', type : 'combo', choices : ['auto', 'none', 'bsd', 'e2fs', 'ossp'],
+ value : 'auto',
description: 'build contrib/uuid-ossp using LIB')
option('zlib', type : 'feature', value: 'auto',
--
2.25.1
v1-0002-meson-Refactor-UUID-option-with-uuid_library.patchtext/plain; charset=UTF-8; name=v1-0002-meson-Refactor-UUID-option-with-uuid_library.patchDownload
From d0c30b6dad4769e75cbf012f1387bb54c3447f8b Mon Sep 17 00:00:00 2001
From: Nazir Bilal Yavuz <byavuz81@gmail.com>
Date: Mon, 13 Feb 2023 15:22:05 +0300
Subject: [PATCH v1] meson: Refactor UUID option with uuid_library
---
.cirrus.yml | 5 ++-
meson.build | 67 ++++++++++++++++++++++++++-------------
meson_options.txt | 9 ++++--
src/makefiles/meson.build | 2 +-
4 files changed, 54 insertions(+), 29 deletions(-)
diff --git a/.cirrus.yml b/.cirrus.yml
index aaf4066366..d696d5dc48 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -181,7 +181,7 @@ task:
su postgres <<-EOF
meson setup \
--buildtype=debug \
- -Dcassert=true -Duuid=bsd -Dtcl_version=tcl86 -Ddtrace=auto \
+ -Dcassert=true -Dtcl_version=tcl86 -Ddtrace=auto \
-DPG_TEST_EXTRA="$PG_TEST_EXTRA" \
-Dextra_lib_dirs=/usr/local/lib -Dextra_include_dirs=/usr/local/include/ \
build
@@ -243,7 +243,6 @@ LINUX_CONFIGURE_FEATURES: &LINUX_CONFIGURE_FEATURES >-
LINUX_MESON_FEATURES: &LINUX_MESON_FEATURES >-
-Dllvm=enabled
- -Duuid=e2fs
task:
@@ -496,7 +495,7 @@ task:
-Dextra_include_dirs=${brewpath}/include \
-Dextra_lib_dirs=${brewpath}/lib \
-Dcassert=true \
- -Duuid=e2fs -Ddtrace=auto \
+ -Ddtrace=auto \
-Dsegsize_blocks=6 \
-DPG_TEST_EXTRA="$PG_TEST_EXTRA" \
build
diff --git a/meson.build b/meson.build
index 563589ac48..8f4bbc979b 100644
--- a/meson.build
+++ b/meson.build
@@ -1257,33 +1257,56 @@ endif
# Library: uuid
###############################################################
+uuid_type = 'none'
uuidopt = get_option('uuid')
-if uuidopt != 'none'
- uuidname = uuidopt.to_upper()
- if uuidopt == 'e2fs'
- uuid = dependency('uuid', required: true)
- uuidfunc = 'uuid_generate'
- uuidheader = 'uuid/uuid.h'
- elif uuidopt == 'bsd'
- # libc should have uuid function
- uuid = declare_dependency()
- uuidfunc = 'uuid_to_string'
- uuidheader = 'uuid.h'
- elif uuidopt == 'ossp'
- uuid = dependency('ossp-uuid', required: true)
- uuidfunc = 'uuid_export'
- uuidheader = 'ossp/uuid.h'
- else
- error('huh')
- endif
+if not uuidopt.disabled()
+ uuid_library = get_option('uuid_library')
+
+ uuidtypes = {
+ 'e2fs': {
+ 'uuiddep': 'uuid',
+ 'uuidfunc': 'uuid_generate',
+ 'uuidheader': 'uuid/uuid.h',
+ },
+ 'bsd': {
+ # libc should have uuid function
+ 'uuiddep': '',
+ 'uuidfunc': 'uuid_to_string',
+ 'uuidheader': 'uuid.h',
+ },
+ 'ossp': {
+ 'uuiddep':'ossp-uuid',
+ 'uuidfunc': 'uuid_export',
+ 'uuidheader': 'ossp/uuid.h',
+ },
+ }
+
+ uuidfound = false
+ foreach uuidname, uuidelements : uuidtypes
+ if uuid_library == 'auto' or uuid_library == uuidname
+ uuiddep = uuidelements['uuiddep']
+ uuid = uuiddep != '' ? dependency(uuiddep, required: false) : declare_dependency()
+ uuidfunc = uuidelements['uuidfunc']
+ uuidheader = uuidelements['uuidheader']
+
+ if cc.has_header_symbol(uuidheader, uuidfunc, args: test_c_args, dependencies: uuid)
+ uuidfound = true
+ uuid_type = uuidname
+ uuidname = uuidname.to_upper()
+ cdata.set('HAVE_@0@'.format(uuidheader.underscorify().to_upper()), 1)
+ cdata.set('HAVE_UUID_@0@'.format(uuidname), 1,
+ description: 'Define to 1 if you have @0@ UUID support.'.format(uuidname))
+ break
+ endif
+ endif
+ endforeach
- if not cc.has_header_symbol(uuidheader, uuidfunc, args: test_c_args, dependencies: uuid)
+ # if uuid is enabled or uuid_library is set other than auto,
+ # and couldn't found any library throw an error
+ if not uuidfound and (uuidopt.enabled() or uuid_library != 'auto')
error('uuid library @0@ missing required function @1@'.format(uuidopt, uuidfunc))
endif
- cdata.set('HAVE_@0@'.format(uuidheader.underscorify().to_upper()), 1)
- cdata.set('HAVE_UUID_@0@'.format(uuidname), 1,
- description: 'Define to 1 if you have @0@ UUID support.'.format(uuidname))
else
uuid = not_found_dep
endif
diff --git a/meson_options.txt b/meson_options.txt
index 9c74cc6512..88493d5a9b 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -133,9 +133,12 @@ option('ssl', type : 'feature', value : 'auto',
option('systemd', type : 'feature', value: 'auto',
description: 'build with systemd support')
-option('uuid', type : 'combo', choices : ['none', 'bsd', 'e2fs', 'ossp'],
- value : 'none',
- description: 'build contrib/uuid-ossp using LIB')
+option('uuid', type : 'feature', value : 'auto',
+ description: 'whether to use uuid')
+
+option('uuid_library', type : 'combo', choices : ['auto', 'bsd', 'e2fs', 'ossp'],
+ value : 'auto',
+ description: 'specify uuid library')
option('zlib', type : 'feature', value: 'auto',
description: 'whether to use zlib')
diff --git a/src/makefiles/meson.build b/src/makefiles/meson.build
index 2b73a9e5dd..e03b721b13 100644
--- a/src/makefiles/meson.build
+++ b/src/makefiles/meson.build
@@ -67,7 +67,7 @@ pgxs_kv = {
# want the chosen option, rather than the library
'with_ssl' : ssl_type,
- 'with_uuid': uuidopt,
+ 'with_uuid': uuid_type,
'default_port': get_option('pgport'),
'with_system_tzdata': get_option('system_tzdata'),
--
2.25.1
On 20.02.23 13:33, Nazir Bilal Yavuz wrote:
I added SSL and UUID patches. UUID patch has two different fixes:
1 - v1-0002-meson-Refactor-UUID-option.patch: Adding 'auto' choice to
'uuid' combo option.2 - v1-0002-meson-Refactor-UUID-option-with-uuid_library.patch: Making
'uuid' feature option and adding new 'uuid_library' combo option with
['auto', 'bsd', 'e2fs', 'ossp'] choices. If 'uuid_library' is set other
than 'auto' and it can't be found, build throws an error.What do you think?
I like the second approach, with a 'uuid' feature option. As you wrote
earlier, adding an 'auto' choice to a combo option doesn't work fully
like a real feature option.
But what does uuid_library=auto do? Which one does it pick? This is
not a behavior we currently have, is it?
I would rename the ssl_type variable to ssl_library, so that if we ever
expose that as an option, it would be consistent with uuid_library.
Hi,
On Mon, 20 Feb 2023 at 21:53, Peter Eisentraut
<peter.eisentraut@enterprisedb.com> wrote:
But what does uuid_library=auto do? Which one does it pick? This is
not a behavior we currently have, is it?
Yes, we didn't have that behavior before. It checks uuid libs by the
order of 'e2fs', 'bsd' and 'ossp'. It uses the first one it finds and
doesn't try to find the rest but the build doesn't fail if it can't
find any library.
Regards,
Nazir Bilal Yavuz
Microsoft
On 20 Feb 2023, at 19:53, Peter Eisentraut <peter.eisentraut@enterprisedb.com> wrote:
I would rename the ssl_type variable to ssl_library, so that if we ever expose that as an option, it would be consistent with uuid_library.
+1, ssl_library is a better name.
--
Daniel Gustafsson
Hi,
On 2023-02-20 19:53:53 +0100, Peter Eisentraut wrote:
On 20.02.23 13:33, Nazir Bilal Yavuz wrote:
I added SSL and UUID patches. UUID patch has two different fixes:
1 - v1-0002-meson-Refactor-UUID-option.patch: Adding 'auto' choice to
'uuid' combo option.2 - v1-0002-meson-Refactor-UUID-option-with-uuid_library.patch: Making
'uuid' feature option and adding new 'uuid_library' combo option with
['auto', 'bsd', 'e2fs', 'ossp'] choices. If 'uuid_library' is set other
than 'auto' and it can't be found, build throws an error.What do you think?
I like the second approach, with a 'uuid' feature option. As you wrote
earlier, adding an 'auto' choice to a combo option doesn't work fully like a
real feature option.
But we can make it behave exactly like one, by checking the auto_features
option.
Greetings,
Andres Freund
Hi,
On Mon, 20 Feb 2023 at 22:42, Andres Freund <andres@anarazel.de> wrote:
On 2023-02-20 19:53:53 +0100, Peter Eisentraut wrote:
On 20.02.23 13:33, Nazir Bilal Yavuz wrote:
I added SSL and UUID patches. UUID patch has two different fixes:
1 - v1-0002-meson-Refactor-UUID-option.patch: Adding 'auto' choice to
'uuid' combo option.2 - v1-0002-meson-Refactor-UUID-option-with-uuid_library.patch: Making
'uuid' feature option and adding new 'uuid_library' combo option with
['auto', 'bsd', 'e2fs', 'ossp'] choices. If 'uuid_library' is set other
than 'auto' and it can't be found, build throws an error.What do you think?
I like the second approach, with a 'uuid' feature option. As you wrote
earlier, adding an 'auto' choice to a combo option doesn't work fully like a
real feature option.But we can make it behave exactly like one, by checking the auto_features
option.
Yes, we can set it like `uuidopt = get_option('auto_features')`.
However, if someone wants to set 'auto_features' to 'disabled' but
'uuid' to 'enabled'(to find at least one working uuid library); this
won't be possible. We can add 'enabled', 'disabled and 'auto' choices
to 'uuid' combo option to make all behaviours possible but adding
'uuid' feature option and 'uuid_library' combo option seems better to
me.
Regards,
Nazir Bilal Yavuz
Microsoft
On 21.02.23 17:32, Nazir Bilal Yavuz wrote:
I like the second approach, with a 'uuid' feature option. As you wrote
earlier, adding an 'auto' choice to a combo option doesn't work fully like a
real feature option.But we can make it behave exactly like one, by checking the auto_features
option.Yes, we can set it like `uuidopt = get_option('auto_features')`.
However, if someone wants to set 'auto_features' to 'disabled' but
'uuid' to 'enabled'(to find at least one working uuid library); this
won't be possible. We can add 'enabled', 'disabled and 'auto' choices
to 'uuid' combo option to make all behaviours possible but adding
'uuid' feature option and 'uuid_library' combo option seems better to
me.
I think the uuid side of this is making this way too complicated. I'm
content leaving this as a manual option for now.
There is much more value in making the ssl option work automatically.
So I would welcome a patch that just makes -Dssl=auto work smoothly,
perhaps using the "trick" that Andres described.
Hi,
On Wed, 22 Feb 2023 at 12:14, Peter Eisentraut
<peter.eisentraut@enterprisedb.com> wrote:
On 21.02.23 17:32, Nazir Bilal Yavuz wrote:
I like the second approach, with a 'uuid' feature option. As you wrote
earlier, adding an 'auto' choice to a combo option doesn't work fully like a
real feature option.But we can make it behave exactly like one, by checking the auto_features
option.Yes, we can set it like `uuidopt = get_option('auto_features')`.
However, if someone wants to set 'auto_features' to 'disabled' but
'uuid' to 'enabled'(to find at least one working uuid library); this
won't be possible. We can add 'enabled', 'disabled and 'auto' choices
to 'uuid' combo option to make all behaviours possible but adding
'uuid' feature option and 'uuid_library' combo option seems better to
me.I think the uuid side of this is making this way too complicated. I'm
content leaving this as a manual option for now.There is much more value in making the ssl option work automatically.
So I would welcome a patch that just makes -Dssl=auto work smoothly,
perhaps using the "trick" that Andres described.
Thanks for the feedback. I updated the ssl patch and if you like
changes, I can apply the same logic to uuid.
Regards,
Nazir Bilal Yavuz
Microsoft
Attachments:
v2-0001-meson-Refactor-SSL-option.patchapplication/octet-stream; name=v2-0001-meson-Refactor-SSL-option.patchDownload
From 09e4c80b3d325e739bdbe6ac02b46df24c9d0c29 Mon Sep 17 00:00:00 2001
From: Nazir Bilal Yavuz <byavuz81@gmail.com>
Date: Fri, 24 Feb 2023 15:35:32 +0300
Subject: [PATCH v2] meson: Refactor SSL option
---
.cirrus.yml | 7 +-
meson.build | 153 ++++++++++++++++++-------------
meson_options.txt | 4 +-
src/interfaces/libpq/meson.build | 2 +-
src/makefiles/meson.build | 2 +-
src/test/ssl/meson.build | 2 +-
6 files changed, 97 insertions(+), 73 deletions(-)
diff --git a/.cirrus.yml b/.cirrus.yml
index f212978752..aaf4066366 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -181,7 +181,7 @@ task:
su postgres <<-EOF
meson setup \
--buildtype=debug \
- -Dcassert=true -Dssl=openssl -Duuid=bsd -Dtcl_version=tcl86 -Ddtrace=auto \
+ -Dcassert=true -Duuid=bsd -Dtcl_version=tcl86 -Ddtrace=auto \
-DPG_TEST_EXTRA="$PG_TEST_EXTRA" \
-Dextra_lib_dirs=/usr/local/lib -Dextra_include_dirs=/usr/local/include/ \
build
@@ -243,7 +243,6 @@ LINUX_CONFIGURE_FEATURES: &LINUX_CONFIGURE_FEATURES >-
LINUX_MESON_FEATURES: &LINUX_MESON_FEATURES >-
-Dllvm=enabled
- -Dssl=openssl
-Duuid=e2fs
@@ -497,7 +496,7 @@ task:
-Dextra_include_dirs=${brewpath}/include \
-Dextra_lib_dirs=${brewpath}/lib \
-Dcassert=true \
- -Dssl=openssl -Duuid=e2fs -Ddtrace=auto \
+ -Duuid=e2fs -Ddtrace=auto \
-Dsegsize_blocks=6 \
-DPG_TEST_EXTRA="$PG_TEST_EXTRA" \
build
@@ -568,7 +567,7 @@ task:
# Use /DEBUG:FASTLINK to avoid high memory usage during linking
configure_script: |
vcvarsall x64
- meson setup --backend ninja --buildtype debug -Dc_link_args=/DEBUG:FASTLINK -Dcassert=true -Db_pch=true -Dssl=openssl -Dextra_lib_dirs=c:\openssl\1.1\lib -Dextra_include_dirs=c:\openssl\1.1\include -DTAR=%TAR% -DPG_TEST_EXTRA="%PG_TEST_EXTRA%" build
+ meson setup --backend ninja --buildtype debug -Dc_link_args=/DEBUG:FASTLINK -Dcassert=true -Db_pch=true -Dextra_lib_dirs=c:\openssl\1.1\lib -Dextra_include_dirs=c:\openssl\1.1\include -DTAR=%TAR% -DPG_TEST_EXTRA="%PG_TEST_EXTRA%" build
build_script: |
vcvarsall x64
diff --git a/meson.build b/meson.build
index 656777820c..7fae0e1c49 100644
--- a/meson.build
+++ b/meson.build
@@ -43,6 +43,7 @@ cc = meson.get_compiler('c')
not_found_dep = dependency('', required: false)
thread_dep = dependency('threads')
+auto_features = get_option('auto_features')
@@ -1171,80 +1172,104 @@ cdata.set('USE_SYSTEMD', systemd.found() ? 1 : false)
# Library: SSL
###############################################################
-if get_option('ssl') == 'openssl'
+ssl = not_found_dep
+ssl_library = 'none'
+sslopt = get_option('ssl')
- # Try to find openssl via pkg-config et al, if that doesn't work
- # (e.g. because it's provided as part of the OS, like on FreeBSD), look for
- # the library names that we know about.
+if (sslopt == 'auto' and auto_features.disabled())
+ sslopt = 'none'
+endif
- # via pkg-config et al
- ssl = dependency('openssl', required: false)
+if sslopt != 'none'
- # via library + headers
- if not ssl.found()
- ssl_lib = cc.find_library('ssl',
- dirs: test_lib_d,
- header_include_directories: postgres_inc,
- has_headers: ['openssl/ssl.h', 'openssl/err.h'])
- crypto_lib = cc.find_library('crypto',
- dirs: test_lib_d,
- header_include_directories: postgres_inc)
- ssl_int = [ssl_lib, crypto_lib]
+ if not ssl.found() and sslopt in ['auto', 'openssl']
+ openssl_required = sslopt == 'openssl'
- ssl = declare_dependency(dependencies: ssl_int,
- include_directories: postgres_inc)
- else
- cc.has_header('openssl/ssl.h', args: test_c_args, dependencies: ssl, required: true)
- cc.has_header('openssl/err.h', args: test_c_args, dependencies: ssl, required: true)
+ # Try to find openssl via pkg-config et al, if that doesn't work
+ # (e.g. because it's provided as part of the OS, like on FreeBSD), look for
+ # the library names that we know about.
+
+ # via pkg-config et al
+ ssl = dependency('openssl', required: false)
+ # via library + headers
+ if not ssl.found()
+ ssl_lib = cc.find_library('ssl',
+ dirs: test_lib_d,
+ header_include_directories: postgres_inc,
+ has_headers: ['openssl/ssl.h', 'openssl/err.h'])
+ crypto_lib = cc.find_library('crypto',
+ dirs: test_lib_d,
+ header_include_directories: postgres_inc)
+ ssl_int = [ssl_lib, crypto_lib]
+
+ ssl = declare_dependency(dependencies: ssl_int,
+ include_directories: postgres_inc)
+ elif cc.has_header('openssl/ssl.h', args: test_c_args, dependencies: ssl, required: openssl_required) and \
+ cc.has_header('openssl/err.h', args: test_c_args, dependencies: ssl, required: openssl_required)
ssl_int = [ssl]
- endif
+ endif
- check_funcs = [
- ['CRYPTO_new_ex_data', {'required': true}],
- ['SSL_new', {'required': true}],
-
- # Function introduced in OpenSSL 1.0.2.
- ['X509_get_signature_nid'],
-
- # Functions introduced in OpenSSL 1.1.0. We used to check for
- # OPENSSL_VERSION_NUMBER, but that didn't work with 1.1.0, because LibreSSL
- # defines OPENSSL_VERSION_NUMBER to claim version 2.0.0, even though it
- # doesn't have these OpenSSL 1.1.0 functions. So check for individual
- # functions.
- ['OPENSSL_init_ssl'],
- ['BIO_get_data'],
- ['BIO_meth_new'],
- ['ASN1_STRING_get0_data'],
- ['HMAC_CTX_new'],
- ['HMAC_CTX_free'],
-
- # OpenSSL versions before 1.1.0 required setting callback functions, for
- # thread-safety. In 1.1.0, it's no longer required, and CRYPTO_lock()
- # function was removed.
- ['CRYPTO_lock'],
-
- # Function introduced in OpenSSL 1.1.1
- ['X509_get_signature_info'],
- ]
+ if ssl.found()
+ check_funcs = [
+ ['CRYPTO_new_ex_data', {'required': true}],
+ ['SSL_new', {'required': true}],
+
+ # Function introduced in OpenSSL 1.0.2.
+ ['X509_get_signature_nid'],
+
+ # Functions introduced in OpenSSL 1.1.0. We used to check for
+ # OPENSSL_VERSION_NUMBER, but that didn't work with 1.1.0, because LibreSSL
+ # defines OPENSSL_VERSION_NUMBER to claim version 2.0.0, even though it
+ # doesn't have these OpenSSL 1.1.0 functions. So check for individual
+ # functions.
+ ['OPENSSL_init_ssl'],
+ ['BIO_get_data'],
+ ['BIO_meth_new'],
+ ['ASN1_STRING_get0_data'],
+ ['HMAC_CTX_new'],
+ ['HMAC_CTX_free'],
+
+ # OpenSSL versions before 1.1.0 required setting callback functions, for
+ # thread-safety. In 1.1.0, it's no longer required, and CRYPTO_lock()
+ # function was removed.
+ ['CRYPTO_lock'],
+
+ # Function introduced in OpenSSL 1.1.1
+ ['X509_get_signature_info'],
+ ]
- foreach c : check_funcs
- func = c.get(0)
- val = cc.has_function(func, args: test_c_args, dependencies: ssl_int)
- required = c.get(1, {}).get('required', false)
- if required and not val
- error('openssl function @0@ is required'.format(func))
- elif not required
- cdata.set('HAVE_' + func.to_upper(), val ? 1 : false)
+ are_openssl_funcs_complete = true
+ foreach c : check_funcs
+ func = c.get(0)
+ val = cc.has_function(func, args: test_c_args, dependencies: ssl_int)
+ required = c.get(1, {}).get('required', false)
+ if required and not val
+ are_openssl_funcs_complete = false
+ openssl_required ? error('openssl function @0@ is required'.format(func)) : \
+ message('openssl function @0@ is required'.format(func))
+ break
+ elif not required
+ cdata.set('HAVE_' + func.to_upper(), val ? 1 : false)
+ endif
+ endforeach
+
+ if are_openssl_funcs_complete
+ cdata.set('USE_OPENSSL', 1,
+ description: 'Define to 1 to build with OpenSSL support. (-Dssl=openssl)')
+ cdata.set('OPENSSL_API_COMPAT', '0x10001000L',
+ description: '''Define to the OpenSSL API version in use. This avoids deprecation warnings from newer OpenSSL versions.''')
+ ssl_library = 'openssl'
+ else
+ ssl = not_found_dep
+ endif
endif
- endforeach
+ endif
- cdata.set('USE_OPENSSL', 1,
- description: 'Define to 1 to build with OpenSSL support. (-Dssl=openssl)')
- cdata.set('OPENSSL_API_COMPAT', '0x10001000L',
- description: '''Define to the OpenSSL API version in use. This avoids deprecation warnings from newer OpenSSL versions.''')
-else
- ssl = not_found_dep
+ # At least one SSL library must be found, otherwise throw an error
+ if sslopt == 'auto' and auto_features.enabled()
+ error('SSL Library could not be found')
+ endif
endif
diff --git a/meson_options.txt b/meson_options.txt
index 9d3ef4aa20..3edd3e25df 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -127,8 +127,8 @@ option('readline', type : 'feature', value : 'auto',
option('selinux', type : 'feature', value : 'disabled',
description: 'build with SELinux support')
-option('ssl', type : 'combo', choices : ['none', 'openssl'],
- value : 'none',
+option('ssl', type : 'combo', choices : ['auto', 'none', 'openssl'],
+ value : 'auto',
description: 'use LIB for SSL/TLS support (openssl)')
option('systemd', type : 'feature', value: 'auto',
diff --git a/src/interfaces/libpq/meson.build b/src/interfaces/libpq/meson.build
index 573fd9b6ea..3cd0ddb494 100644
--- a/src/interfaces/libpq/meson.build
+++ b/src/interfaces/libpq/meson.build
@@ -117,7 +117,7 @@ tests += {
't/001_uri.pl',
't/002_api.pl',
],
- 'env': {'with_ssl': get_option('ssl')},
+ 'env': {'with_ssl': ssl_library},
},
}
diff --git a/src/makefiles/meson.build b/src/makefiles/meson.build
index bf7303dc99..98c2bf9ed1 100644
--- a/src/makefiles/meson.build
+++ b/src/makefiles/meson.build
@@ -66,7 +66,7 @@ pgxs_kv = {
'SUN_STUDIO_CC': 'no', # not supported so far
# want the chosen option, rather than the library
- 'with_ssl' : get_option('ssl'),
+ 'with_ssl' : ssl_library,
'with_uuid': uuidopt,
'default_port': get_option('pgport'),
diff --git a/src/test/ssl/meson.build b/src/test/ssl/meson.build
index a8d9a5424d..4cda81f3bc 100644
--- a/src/test/ssl/meson.build
+++ b/src/test/ssl/meson.build
@@ -6,7 +6,7 @@ tests += {
'bd': meson.current_build_dir(),
'tap': {
'env': {
- 'with_ssl': get_option('ssl'),
+ 'with_ssl': ssl_library,
'OPENSSL': openssl.path(),
},
'tests': [
--
2.25.1
On 24.02.23 14:01, Nazir Bilal Yavuz wrote:
Thanks for the feedback. I updated the ssl patch and if you like
changes, I can apply the same logic to uuid.
Maybe we can make some of the logic less nested. Right now there is
if sslopt != 'none'
if not ssl.found() and sslopt in ['auto', 'openssl']
I think at that point, ssl.found() is never true, so it can be removed.
And the two checks for sslopt are nearly redundant.
At the end of the block, there is
# At least one SSL library must be found, otherwise throw an error
if sslopt == 'auto' and auto_features.enabled()
error('SSL Library could not be found')
endif
endif
which also implies sslopt != 'none'. So I think the whole thing could be
if sslopt in ['auto', 'openssl']
...
endif
if sslopt == 'auto' and auto_features.enabled()
error('SSL Library could not be found')
endif
both at the top level.
Another issue, I think this is incorrect:
+ openssl_required ? error('openssl function @0@ is
required'.format(func)) : \
+ message('openssl function @0@ is
required'.format(func))We don't want to issue a message like this when a non-required function
is missing.
Hi,
Thanks for the review.
On Wed, 1 Mar 2023 at 18:52, Peter Eisentraut
<peter.eisentraut@enterprisedb.com> wrote:
Maybe we can make some of the logic less nested. Right now there is
if sslopt != 'none'
if not ssl.found() and sslopt in ['auto', 'openssl']
I think at that point, ssl.found() is never true, so it can be removed.
I agree, ssl.found() can be removed.
And the two checks for sslopt are nearly redundant.
At the end of the block, there is
# At least one SSL library must be found, otherwise throw an error
if sslopt == 'auto' and auto_features.enabled()
error('SSL Library could not be found')
endif
endifwhich also implies sslopt != 'none'. So I think the whole thing could be
if sslopt in ['auto', 'openssl']
...
endif
if sslopt == 'auto' and auto_features.enabled()
error('SSL Library could not be found')
endifboth at the top level.
I am kind of confused. I added these checks for considering other SSL
implementations in the future, for this reason I have two nested if
checks. The top one is for checking if we need to search an SSL
library and the nested one is for checking if we need to search this
specific SSL library. What do you think?
The other thing is(which I forgot before) I need to add "and not
ssl.found()" condition to the "if sslopt == 'auto' and
auto_features.enabled()" check.
Another issue, I think this is incorrect:
+ openssl_required ? error('openssl function @0@ is required'.format(func)) : \ + message('openssl function @0@ is required'.format(func))We don't want to issue a message like this when a non-required function
is missing.
I agree, the message part can be removed.
Regards,
Nazir Bilal Yavuz
Microsoft
On 02.03.23 11:41, Nazir Bilal Yavuz wrote:
I am kind of confused. I added these checks for considering other SSL
implementations in the future, for this reason I have two nested if
checks. The top one is for checking if we need to search an SSL
library and the nested one is for checking if we need to search this
specific SSL library. What do you think?
I suppose that depends on how you envision integrating other SSL
libraries into this logic. It's not that important right now; if the
structure makes sense to you, that's fine.
Please send an updated patch with the small changes that have been
mentioned.
Hi,
On Fri, 3 Mar 2023 at 12:16, Peter Eisentraut
<peter.eisentraut@enterprisedb.com> wrote:
On 02.03.23 11:41, Nazir Bilal Yavuz wrote:
I am kind of confused. I added these checks for considering other SSL
implementations in the future, for this reason I have two nested if
checks. The top one is for checking if we need to search an SSL
library and the nested one is for checking if we need to search this
specific SSL library. What do you think?I suppose that depends on how you envision integrating other SSL
libraries into this logic. It's not that important right now; if the
structure makes sense to you, that's fine.Please send an updated patch with the small changes that have been
mentioned.
The updated patch is attached.
Regards,
Nazir Bilal Yavuz
Microsoft
Attachments:
v3-0001-meson-Refactor-SSL-option.patchtext/x-diff; charset=US-ASCII; name=v3-0001-meson-Refactor-SSL-option.patchDownload
From 9cb9d50ba008e2a385a7b72219a759490a3de00e Mon Sep 17 00:00:00 2001
From: Nazir Bilal Yavuz <byavuz81@gmail.com>
Date: Fri, 3 Mar 2023 12:24:46 +0300
Subject: [PATCH v3] meson: Refactor SSL option
---
.cirrus.yml | 7 +-
meson.build | 121 ++++++++++++++++++-------------
meson_options.txt | 4 +-
src/interfaces/libpq/meson.build | 2 +-
src/makefiles/meson.build | 2 +-
src/test/ssl/meson.build | 2 +-
6 files changed, 80 insertions(+), 58 deletions(-)
diff --git a/.cirrus.yml b/.cirrus.yml
index f2129787529..aaf4066366c 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -181,7 +181,7 @@ task:
su postgres <<-EOF
meson setup \
--buildtype=debug \
- -Dcassert=true -Dssl=openssl -Duuid=bsd -Dtcl_version=tcl86 -Ddtrace=auto \
+ -Dcassert=true -Duuid=bsd -Dtcl_version=tcl86 -Ddtrace=auto \
-DPG_TEST_EXTRA="$PG_TEST_EXTRA" \
-Dextra_lib_dirs=/usr/local/lib -Dextra_include_dirs=/usr/local/include/ \
build
@@ -243,7 +243,6 @@ LINUX_CONFIGURE_FEATURES: &LINUX_CONFIGURE_FEATURES >-
LINUX_MESON_FEATURES: &LINUX_MESON_FEATURES >-
-Dllvm=enabled
- -Dssl=openssl
-Duuid=e2fs
@@ -497,7 +496,7 @@ task:
-Dextra_include_dirs=${brewpath}/include \
-Dextra_lib_dirs=${brewpath}/lib \
-Dcassert=true \
- -Dssl=openssl -Duuid=e2fs -Ddtrace=auto \
+ -Duuid=e2fs -Ddtrace=auto \
-Dsegsize_blocks=6 \
-DPG_TEST_EXTRA="$PG_TEST_EXTRA" \
build
@@ -568,7 +567,7 @@ task:
# Use /DEBUG:FASTLINK to avoid high memory usage during linking
configure_script: |
vcvarsall x64
- meson setup --backend ninja --buildtype debug -Dc_link_args=/DEBUG:FASTLINK -Dcassert=true -Db_pch=true -Dssl=openssl -Dextra_lib_dirs=c:\openssl\1.1\lib -Dextra_include_dirs=c:\openssl\1.1\include -DTAR=%TAR% -DPG_TEST_EXTRA="%PG_TEST_EXTRA%" build
+ meson setup --backend ninja --buildtype debug -Dc_link_args=/DEBUG:FASTLINK -Dcassert=true -Db_pch=true -Dextra_lib_dirs=c:\openssl\1.1\lib -Dextra_include_dirs=c:\openssl\1.1\include -DTAR=%TAR% -DPG_TEST_EXTRA="%PG_TEST_EXTRA%" build
build_script: |
vcvarsall x64
diff --git a/meson.build b/meson.build
index 26be83afb61..1e9411eb247 100644
--- a/meson.build
+++ b/meson.build
@@ -43,6 +43,7 @@ cc = meson.get_compiler('c')
not_found_dep = dependency('', required: false)
thread_dep = dependency('threads')
+auto_features = get_option('auto_features')
@@ -1171,7 +1172,16 @@ cdata.set('USE_SYSTEMD', systemd.found() ? 1 : false)
# Library: SSL
###############################################################
-if get_option('ssl') == 'openssl'
+ssl = not_found_dep
+ssl_library = 'none'
+sslopt = get_option('ssl')
+
+if (sslopt == 'auto' and auto_features.disabled())
+ sslopt = 'none'
+endif
+
+if sslopt in ['auto', 'openssl']
+ openssl_required = sslopt == 'openssl'
# Try to find openssl via pkg-config et al, if that doesn't work
# (e.g. because it's provided as part of the OS, like on FreeBSD), look for
@@ -1192,59 +1202,72 @@ if get_option('ssl') == 'openssl'
ssl_int = [ssl_lib, crypto_lib]
ssl = declare_dependency(dependencies: ssl_int,
- include_directories: postgres_inc)
- else
- cc.has_header('openssl/ssl.h', args: test_c_args, dependencies: ssl, required: true)
- cc.has_header('openssl/err.h', args: test_c_args, dependencies: ssl, required: true)
-
- ssl_int = [ssl]
+ include_directories: postgres_inc)
+ elif cc.has_header('openssl/ssl.h', args: test_c_args, dependencies: ssl, required: openssl_required) and \
+ cc.has_header('openssl/err.h', args: test_c_args, dependencies: ssl, required: openssl_required)
+ ssl_int = [ssl]
endif
- check_funcs = [
- ['CRYPTO_new_ex_data', {'required': true}],
- ['SSL_new', {'required': true}],
-
- # Function introduced in OpenSSL 1.0.2.
- ['X509_get_signature_nid'],
-
- # Functions introduced in OpenSSL 1.1.0. We used to check for
- # OPENSSL_VERSION_NUMBER, but that didn't work with 1.1.0, because LibreSSL
- # defines OPENSSL_VERSION_NUMBER to claim version 2.0.0, even though it
- # doesn't have these OpenSSL 1.1.0 functions. So check for individual
- # functions.
- ['OPENSSL_init_ssl'],
- ['BIO_get_data'],
- ['BIO_meth_new'],
- ['ASN1_STRING_get0_data'],
- ['HMAC_CTX_new'],
- ['HMAC_CTX_free'],
-
- # OpenSSL versions before 1.1.0 required setting callback functions, for
- # thread-safety. In 1.1.0, it's no longer required, and CRYPTO_lock()
- # function was removed.
- ['CRYPTO_lock'],
-
- # Function introduced in OpenSSL 1.1.1
- ['X509_get_signature_info'],
- ]
+ if ssl.found()
+ check_funcs = [
+ ['CRYPTO_new_ex_data', {'required': true}],
+ ['SSL_new', {'required': true}],
+
+ # Function introduced in OpenSSL 1.0.2.
+ ['X509_get_signature_nid'],
+
+ # Functions introduced in OpenSSL 1.1.0. We used to check for
+ # OPENSSL_VERSION_NUMBER, but that didn't work with 1.1.0, because LibreSSL
+ # defines OPENSSL_VERSION_NUMBER to claim version 2.0.0, even though it
+ # doesn't have these OpenSSL 1.1.0 functions. So check for individual
+ # functions.
+ ['OPENSSL_init_ssl'],
+ ['BIO_get_data'],
+ ['BIO_meth_new'],
+ ['ASN1_STRING_get0_data'],
+ ['HMAC_CTX_new'],
+ ['HMAC_CTX_free'],
+
+ # OpenSSL versions before 1.1.0 required setting callback functions, for
+ # thread-safety. In 1.1.0, it's no longer required, and CRYPTO_lock()
+ # function was removed.
+ ['CRYPTO_lock'],
+
+ # Function introduced in OpenSSL 1.1.1
+ ['X509_get_signature_info'],
+ ]
+
+ are_openssl_funcs_complete = true
+ foreach c : check_funcs
+ func = c.get(0)
+ val = cc.has_function(func, args: test_c_args, dependencies: ssl_int)
+ required = c.get(1, {}).get('required', false)
+ if required and not val
+ are_openssl_funcs_complete = false
+ if openssl_required
+ error('openssl function @0@ is required'.format(func))
+ endif
+ break
+ elif not required
+ cdata.set('HAVE_' + func.to_upper(), val ? 1 : false)
+ endif
+ endforeach
- foreach c : check_funcs
- func = c.get(0)
- val = cc.has_function(func, args: test_c_args, dependencies: ssl_int)
- required = c.get(1, {}).get('required', false)
- if required and not val
- error('openssl function @0@ is required'.format(func))
- elif not required
- cdata.set('HAVE_' + func.to_upper(), val ? 1 : false)
+ if are_openssl_funcs_complete
+ cdata.set('USE_OPENSSL', 1,
+ description: 'Define to 1 to build with OpenSSL support. (-Dssl=openssl)')
+ cdata.set('OPENSSL_API_COMPAT', '0x10001000L',
+ description: '''Define to the OpenSSL API version in use. This avoids deprecation warnings from newer OpenSSL versions.''')
+ ssl_library = 'openssl'
+ else
+ ssl = not_found_dep
endif
- endforeach
+ endif
+endif
- cdata.set('USE_OPENSSL', 1,
- description: 'Define to 1 to build with OpenSSL support. (-Dssl=openssl)')
- cdata.set('OPENSSL_API_COMPAT', '0x10001000L',
- description: '''Define to the OpenSSL API version in use. This avoids deprecation warnings from newer OpenSSL versions.''')
-else
- ssl = not_found_dep
+# At least one SSL library must be found, otherwise throw an error
+if sslopt == 'auto' and auto_features.enabled() and not ssl.found()
+ error('SSL Library could not be found')
endif
diff --git a/meson_options.txt b/meson_options.txt
index 7d33c9f1d4b..4402dd4299d 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -130,8 +130,8 @@ option('readline', type : 'feature', value : 'auto',
option('selinux', type : 'feature', value : 'disabled',
description: 'build with SELinux support')
-option('ssl', type : 'combo', choices : ['none', 'openssl'],
- value : 'none',
+option('ssl', type : 'combo', choices : ['auto', 'none', 'openssl'],
+ value : 'auto',
description: 'use LIB for SSL/TLS support (openssl)')
option('systemd', type : 'feature', value: 'auto',
diff --git a/src/interfaces/libpq/meson.build b/src/interfaces/libpq/meson.build
index 573fd9b6ea4..3cd0ddb4945 100644
--- a/src/interfaces/libpq/meson.build
+++ b/src/interfaces/libpq/meson.build
@@ -117,7 +117,7 @@ tests += {
't/001_uri.pl',
't/002_api.pl',
],
- 'env': {'with_ssl': get_option('ssl')},
+ 'env': {'with_ssl': ssl_library},
},
}
diff --git a/src/makefiles/meson.build b/src/makefiles/meson.build
index 5a0032ab0d2..7635771c5ae 100644
--- a/src/makefiles/meson.build
+++ b/src/makefiles/meson.build
@@ -66,7 +66,7 @@ pgxs_kv = {
'SUN_STUDIO_CC': 'no', # not supported so far
# want the chosen option, rather than the library
- 'with_ssl' : get_option('ssl'),
+ 'with_ssl' : ssl_library,
'with_uuid': uuidopt,
'default_port': get_option('pgport'),
diff --git a/src/test/ssl/meson.build b/src/test/ssl/meson.build
index a8d9a5424d4..4cda81f3bc2 100644
--- a/src/test/ssl/meson.build
+++ b/src/test/ssl/meson.build
@@ -6,7 +6,7 @@ tests += {
'bd': meson.current_build_dir(),
'tap': {
'env': {
- 'with_ssl': get_option('ssl'),
+ 'with_ssl': ssl_library,
'OPENSSL': openssl.path(),
},
'tests': [
--
2.39.2
On 03.03.23 11:01, Nazir Bilal Yavuz wrote:
On Fri, 3 Mar 2023 at 12:16, Peter Eisentraut
<peter.eisentraut@enterprisedb.com> wrote:On 02.03.23 11:41, Nazir Bilal Yavuz wrote:
I am kind of confused. I added these checks for considering other SSL
implementations in the future, for this reason I have two nested if
checks. The top one is for checking if we need to search an SSL
library and the nested one is for checking if we need to search this
specific SSL library. What do you think?I suppose that depends on how you envision integrating other SSL
libraries into this logic. It's not that important right now; if the
structure makes sense to you, that's fine.Please send an updated patch with the small changes that have been
mentioned.The updated patch is attached.
This seems to work well.
One flaw, the "External libraries" summary shows something like
ssl : YES 3.0.7
It would be nice if it showed "openssl".
How about we just hardcode "openssl" here instead? We could build that
array dynamically, of course, but maybe we leave that until we actually
have a need?
On 9 Mar 2023, at 14:45, Peter Eisentraut <peter.eisentraut@enterprisedb.com> wrote:
How about we just hardcode "openssl" here instead? We could build that array dynamically, of course, but maybe we leave that until we actually have a need?
At least for 16 keeping it hardcoded is an entirely safe bet so +1 for leaving
additional complexity for when needed.
--
Daniel Gustafsson
Hi,
On Thu, 9 Mar 2023 at 16:54, Daniel Gustafsson <daniel@yesql.se> wrote:
On 9 Mar 2023, at 14:45, Peter Eisentraut <peter.eisentraut@enterprisedb.com> wrote:
How about we just hardcode "openssl" here instead? We could build that array dynamically, of course, but maybe we leave that until we actually have a need?
At least for 16 keeping it hardcoded is an entirely safe bet so +1 for leaving
additional complexity for when needed.
We already have the 'ssl_library' variable. Can't we use that instead
of hardcoding 'openssl'? e.g:
summary(
{
'ssl': ssl.found() ? [ssl, '(@0@)'.format(ssl_library)] : ssl,
},
section: 'External libraries',
list_sep: ', ',
)
And it will output:
ssl : YES 3.0.8, (openssl)
I don't think that using 'ssl_library' will increase the complexity.
Regards,
Nazir Bilal Yavuz
Microsoft
On 9 Mar 2023, at 15:12, Nazir Bilal Yavuz <byavuz81@gmail.com> wrote:
Hi,
On Thu, 9 Mar 2023 at 16:54, Daniel Gustafsson <daniel@yesql.se> wrote:
On 9 Mar 2023, at 14:45, Peter Eisentraut <peter.eisentraut@enterprisedb.com> wrote:
How about we just hardcode "openssl" here instead? We could build that array dynamically, of course, but maybe we leave that until we actually have a need?
At least for 16 keeping it hardcoded is an entirely safe bet so +1 for leaving
additional complexity for when needed.We already have the 'ssl_library' variable. Can't we use that instead
of hardcoding 'openssl'? e.g:summary(
{
'ssl': ssl.found() ? [ssl, '(@0@)'.format(ssl_library)] : ssl,
},
section: 'External libraries',
list_sep: ', ',
)And it will output:
ssl : YES 3.0.8, (openssl)I don't think that using 'ssl_library' will increase the complexity.
That seems like a good idea.
--
Daniel Gustafsson
On 09.03.23 15:12, Nazir Bilal Yavuz wrote:
Hi,
On Thu, 9 Mar 2023 at 16:54, Daniel Gustafsson <daniel@yesql.se> wrote:
On 9 Mar 2023, at 14:45, Peter Eisentraut <peter.eisentraut@enterprisedb.com> wrote:
How about we just hardcode "openssl" here instead? We could build that array dynamically, of course, but maybe we leave that until we actually have a need?
At least for 16 keeping it hardcoded is an entirely safe bet so +1 for leaving
additional complexity for when needed.We already have the 'ssl_library' variable. Can't we use that instead
of hardcoding 'openssl'? e.g:summary(
{
'ssl': ssl.found() ? [ssl, '(@0@)'.format(ssl_library)] : ssl,
},
section: 'External libraries',
list_sep: ', ',
)And it will output:
ssl : YES 3.0.8, (openssl)I don't think that using 'ssl_library' will increase the complexity.
Then we might as well use ssl_library as the key, like:
{
...
'selinux': selinux,
ssl_library: ssl,
'systemd': systemd,
...
}
Hi,
On Thu, 9 Mar 2023 at 17:18, Peter Eisentraut
<peter.eisentraut@enterprisedb.com> wrote:
On 09.03.23 15:12, Nazir Bilal Yavuz wrote:
Hi,
On Thu, 9 Mar 2023 at 16:54, Daniel Gustafsson <daniel@yesql.se> wrote:
On 9 Mar 2023, at 14:45, Peter Eisentraut <peter.eisentraut@enterprisedb.com> wrote:
How about we just hardcode "openssl" here instead? We could build that array dynamically, of course, but maybe we leave that until we actually have a need?
At least for 16 keeping it hardcoded is an entirely safe bet so +1 for leaving
additional complexity for when needed.We already have the 'ssl_library' variable. Can't we use that instead
of hardcoding 'openssl'? e.g:summary(
{
'ssl': ssl.found() ? [ssl, '(@0@)'.format(ssl_library)] : ssl,
},
section: 'External libraries',
list_sep: ', ',
)And it will output:
ssl : YES 3.0.8, (openssl)I don't think that using 'ssl_library' will increase the complexity.
Then we might as well use ssl_library as the key, like:
{
...
'selinux': selinux,
ssl_library: ssl,
'systemd': systemd,
...
}
There will be a problem if ssl is not found. It will output 'none: NO'
because 'ssl_library' is initialized as 'none' for now. We can
initialize 'ssl_library' as 'ssl' but I am not sure that is a good
idea.
Regards,
Nazir Bilal Yavuz
Microsoft
On 09.03.23 14:54, Daniel Gustafsson wrote:
On 9 Mar 2023, at 14:45, Peter Eisentraut <peter.eisentraut@enterprisedb.com> wrote:
How about we just hardcode "openssl" here instead? We could build that array dynamically, of course, but maybe we leave that until we actually have a need?
At least for 16 keeping it hardcoded is an entirely safe bet so +1 for leaving
additional complexity for when needed.
I have committed it like this.
I didn't like the other variants, because they would cause the openssl
line to stick out for purely implementation reasons (e.g., we don't have
a line "compression: YES (lz4)". If we get support for another ssl
library, we can easily reconsider this.
On Mon, Mar 13, 2023 at 07:27:18AM +0100, Peter Eisentraut wrote:
I have committed it like this.
I noticed that after 6a30027, if you don't have the OpenSSL headers
installed, 'meson setup' will fail:
meson.build:1195:4: ERROR: C header 'openssl/ssl.h' not found
Shouldn't "auto" cause Postgres to be built without OpenSSL if the required
headers are not present?
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
Hi,
On Mon, 13 Mar 2023 at 21:04, Nathan Bossart <nathandbossart@gmail.com> wrote:
On Mon, Mar 13, 2023 at 07:27:18AM +0100, Peter Eisentraut wrote:
I have committed it like this.
I noticed that after 6a30027, if you don't have the OpenSSL headers
installed, 'meson setup' will fail:meson.build:1195:4: ERROR: C header 'openssl/ssl.h' not found
Shouldn't "auto" cause Postgres to be built without OpenSSL if the required
headers are not present?
Yes, I tested again and it is working as expected on my end. It
shouldn't fail like that unless the 'ssl' option is set to 'openssl'.
Is it possible that it has been set to 'openssl' without you noticing?
Regards,
Nazir Bilal Yavuz
Microsoft
On Mon, Mar 13, 2023 at 09:57:22PM +0300, Nazir Bilal Yavuz wrote:
On Mon, 13 Mar 2023 at 21:04, Nathan Bossart <nathandbossart@gmail.com> wrote:
Shouldn't "auto" cause Postgres to be built without OpenSSL if the required
headers are not present?Yes, I tested again and it is working as expected on my end. It
shouldn't fail like that unless the 'ssl' option is set to 'openssl'.
Is it possible that it has been set to 'openssl' without you noticing?
I do not believe so. For the test in question, here are the build options
reported in meson-log.txt:
Build Options: -Dlz4=enabled -Dplperl=enabled -Dplpython=enabled -Dpltcl=enabled -Dlibxml=enabled -Duuid=ossp -Dlibxslt=enabled -Ddebug=true -Dcassert=true -Dtap_tests=enabled -Dwerror=True
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
Hi,
On 2023-03-13 11:04:32 -0700, Nathan Bossart wrote:
On Mon, Mar 13, 2023 at 07:27:18AM +0100, Peter Eisentraut wrote:
I have committed it like this.
I noticed that after 6a30027, if you don't have the OpenSSL headers
installed, 'meson setup' will fail:meson.build:1195:4: ERROR: C header 'openssl/ssl.h' not found
Shouldn't "auto" cause Postgres to be built without OpenSSL if the required
headers are not present?
Yea. I found another thing: When dependency() found something, but the headers
weren't present, ssl_int wouldn't exist.
Maybe something like the attached?
Greetings,
Andres Freund
Attachments:
v1-0001-meson-fix-openssl-detection-issues-in-6a30027.patchtext/x-diff; charset=us-asciiDownload
From b8bd0200667bac16674e40e769a9fb4bc4f54306 Mon Sep 17 00:00:00 2001
From: Andres Freund <andres@anarazel.de>
Date: Mon, 13 Mar 2023 13:11:37 -0700
Subject: [PATCH v1] meson: fix openssl detection issues in 6a30027
Reported-by: Nathan Bossart <nathandbossart@gmail.com>
Discussion: https://postgr.es/m/20230313180432.GA246741@nathanxps13
---
meson.build | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/meson.build b/meson.build
index 8208815c96c..2ebdf914c1b 100644
--- a/meson.build
+++ b/meson.build
@@ -1189,23 +1189,29 @@ if sslopt in ['auto', 'openssl']
# via pkg-config et al
ssl = dependency('openssl', required: false)
+ # only meson >= 0.57 supports declare_dependency() in cc.has_function(), so
+ # we pass cc.find_library() results if necessary
+ ssl_int = []
# via library + headers
if not ssl.found()
ssl_lib = cc.find_library('ssl',
dirs: test_lib_d,
header_include_directories: postgres_inc,
- has_headers: ['openssl/ssl.h', 'openssl/err.h'])
+ has_headers: ['openssl/ssl.h', 'openssl/err.h'],
+ required: openssl_required)
crypto_lib = cc.find_library('crypto',
dirs: test_lib_d,
- header_include_directories: postgres_inc)
- ssl_int = [ssl_lib, crypto_lib]
-
- ssl = declare_dependency(dependencies: ssl_int,
- include_directories: postgres_inc)
+ required: openssl_required)
+ if ssl_lib.found() and crypto_lib.found()
+ ssl_int = [ssl_lib, crypto_lib]
+ ssl = declare_dependency(dependencies: ssl_int, include_directories: postgres_inc)
+ endif
elif cc.has_header('openssl/ssl.h', args: test_c_args, dependencies: ssl, required: openssl_required) and \
cc.has_header('openssl/err.h', args: test_c_args, dependencies: ssl, required: openssl_required)
ssl_int = [ssl]
+ else
+ ssl = not_found_dep
endif
if ssl.found()
--
2.38.0
Hi,
On 2023-03-13 21:57:22 +0300, Nazir Bilal Yavuz wrote:
On Mon, 13 Mar 2023 at 21:04, Nathan Bossart <nathandbossart@gmail.com> wrote:
On Mon, Mar 13, 2023 at 07:27:18AM +0100, Peter Eisentraut wrote:
I have committed it like this.
I noticed that after 6a30027, if you don't have the OpenSSL headers
installed, 'meson setup' will fail:meson.build:1195:4: ERROR: C header 'openssl/ssl.h' not found
Shouldn't "auto" cause Postgres to be built without OpenSSL if the required
headers are not present?Yes, I tested again and it is working as expected on my end. It
shouldn't fail like that unless the 'ssl' option is set to 'openssl'.
Is it possible that it has been set to 'openssl' without you noticing?
It worked for the dependency() path, but not the cc.find_library() path. See
the patch I just sent.
Greetings,
Andres Freund
On Mon, Mar 13, 2023 at 01:13:31PM -0700, Andres Freund wrote:
On 2023-03-13 11:04:32 -0700, Nathan Bossart wrote:
I noticed that after 6a30027, if you don't have the OpenSSL headers
installed, 'meson setup' will fail:meson.build:1195:4: ERROR: C header 'openssl/ssl.h' not found
Shouldn't "auto" cause Postgres to be built without OpenSSL if the required
headers are not present?Yea. I found another thing: When dependency() found something, but the headers
weren't present, ssl_int wouldn't exist.Maybe something like the attached?
ssl_lib = cc.find_library('ssl', dirs: test_lib_d, header_include_directories: postgres_inc, - has_headers: ['openssl/ssl.h', 'openssl/err.h']) + has_headers: ['openssl/ssl.h', 'openssl/err.h'], + required: openssl_required) crypto_lib = cc.find_library('crypto', dirs: test_lib_d, - header_include_directories: postgres_inc) - ssl_int = [ssl_lib, crypto_lib] - - ssl = declare_dependency(dependencies: ssl_int, - include_directories: postgres_inc) + required: openssl_required) + if ssl_lib.found() and crypto_lib.found() + ssl_int = [ssl_lib, crypto_lib] + ssl = declare_dependency(dependencies: ssl_int, include_directories: postgres_inc) + endif
I was just about to post a patch to set "required" like you have for
ssl_lib and crypto_lib. It seemed to work alright without the 'if
ssl_lib.found() and crypto_lib.found()' line, but I haven't worked with
these meson files very much, so what you have is probably better form.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
Hi,
On Mon, 13 Mar 2023 at 23:14, Andres Freund <andres@anarazel.de> wrote:
Hi,
On 2023-03-13 21:57:22 +0300, Nazir Bilal Yavuz wrote:
On Mon, 13 Mar 2023 at 21:04, Nathan Bossart <nathandbossart@gmail.com> wrote:
On Mon, Mar 13, 2023 at 07:27:18AM +0100, Peter Eisentraut wrote:
I have committed it like this.
I noticed that after 6a30027, if you don't have the OpenSSL headers
installed, 'meson setup' will fail:meson.build:1195:4: ERROR: C header 'openssl/ssl.h' not found
Shouldn't "auto" cause Postgres to be built without OpenSSL if the required
headers are not present?Yes, I tested again and it is working as expected on my end. It
shouldn't fail like that unless the 'ssl' option is set to 'openssl'.
Is it possible that it has been set to 'openssl' without you noticing?It worked for the dependency() path, but not the cc.find_library() path. See
the patch I just sent.
Thanks for the patch, I understand the problem now and your patch fixes this.
Regards,
Nazir Bilal Yavuz
Microsoft
Hi,
On 2023-03-13 23:46:41 +0300, Nazir Bilal Yavuz wrote:
Thanks for the patch, I understand the problem now and your patch fixes this.
Pushed the patch.
Greetings,
Andres Freund
On Mon, Mar 13, 2023 at 02:45:29PM -0700, Andres Freund wrote:
Pushed the patch.
Thanks for the prompt fix.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
Hi,
On Wed, 22 Feb 2023 at 12:14, Peter Eisentraut
<peter.eisentraut@enterprisedb.com> wrote:
On 21.02.23 17:32, Nazir Bilal Yavuz wrote:
I like the second approach, with a 'uuid' feature option. As you wrote
earlier, adding an 'auto' choice to a combo option doesn't work fully like a
real feature option.But we can make it behave exactly like one, by checking the auto_features
option.Yes, we can set it like `uuidopt = get_option('auto_features')`.
However, if someone wants to set 'auto_features' to 'disabled' but
'uuid' to 'enabled'(to find at least one working uuid library); this
won't be possible. We can add 'enabled', 'disabled and 'auto' choices
to 'uuid' combo option to make all behaviours possible but adding
'uuid' feature option and 'uuid_library' combo option seems better to
me.I think the uuid side of this is making this way too complicated. I'm
content leaving this as a manual option for now.There is much more value in making the ssl option work automatically.
So I would welcome a patch that just makes -Dssl=auto work smoothly,
perhaps using the "trick" that Andres described.
I tried to implement what we did for ssl to uuid as well, do you have
any comments?
Regards,
Nazir Bilal Yavuz
Microsoft
Attachments:
v1-0001-meson-Make-auto-the-default-of-the-uuid-option.patchtext/x-diff; charset=US-ASCII; name=v1-0001-meson-Make-auto-the-default-of-the-uuid-option.patchDownload
From 0e92050c15ca6e9ebeddaafad229eee53fc9e7b0 Mon Sep 17 00:00:00 2001
From: Nazir Bilal Yavuz <byavuz81@gmail.com>
Date: Tue, 14 Mar 2023 16:38:02 +0300
Subject: [PATCH v1] meson: Make auto the default of the uuid option
---
.cirrus.yml | 5 +--
meson.build | 80 ++++++++++++++++++++++++++-------------
meson_options.txt | 4 +-
src/makefiles/meson.build | 2 +-
4 files changed, 59 insertions(+), 32 deletions(-)
diff --git a/.cirrus.yml b/.cirrus.yml
index 505c50f3285..1ccb14c6340 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -181,7 +181,7 @@ task:
su postgres <<-EOF
meson setup \
--buildtype=debug \
- -Dcassert=true -Duuid=bsd -Dtcl_version=tcl86 -Ddtrace=auto \
+ -Dcassert=true -Dtcl_version=tcl86 -Ddtrace=auto \
-DPG_TEST_EXTRA="$PG_TEST_EXTRA" \
-Dextra_lib_dirs=/usr/local/lib -Dextra_include_dirs=/usr/local/include/ \
build
@@ -243,7 +243,6 @@ LINUX_CONFIGURE_FEATURES: &LINUX_CONFIGURE_FEATURES >-
LINUX_MESON_FEATURES: &LINUX_MESON_FEATURES >-
-Dllvm=enabled
- -Duuid=e2fs
task:
@@ -496,7 +495,7 @@ task:
-Dextra_include_dirs=${brewpath}/include \
-Dextra_lib_dirs=${brewpath}/lib \
-Dcassert=true \
- -Duuid=e2fs -Ddtrace=auto \
+ -Ddtrace=auto \
-Dsegsize_blocks=6 \
-DPG_TEST_EXTRA="$PG_TEST_EXTRA" \
build
diff --git a/meson.build b/meson.build
index 2ebdf914c1b..7955ae42d03 100644
--- a/meson.build
+++ b/meson.build
@@ -1282,34 +1282,61 @@ endif
###############################################################
uuidopt = get_option('uuid')
+uuid_library = 'none'
+uuid = not_found_dep
+if uuidopt == 'auto' and auto_features.disabled()
+ uuidopt = 'none'
+endif
+
if uuidopt != 'none'
- uuidname = uuidopt.to_upper()
- if uuidopt == 'e2fs'
- uuid = dependency('uuid', required: true)
- uuidfunc = 'uuid_generate'
- uuidheader = 'uuid/uuid.h'
- elif uuidopt == 'bsd'
- # libc should have uuid function
- uuid = declare_dependency()
- uuidfunc = 'uuid_to_string'
- uuidheader = 'uuid.h'
- elif uuidopt == 'ossp'
- uuid = dependency('ossp-uuid', required: true)
- uuidfunc = 'uuid_export'
- uuidheader = 'uuid.h'
- else
- error('huh')
- endif
- if not cc.has_header_symbol(uuidheader, uuidfunc, args: test_c_args, dependencies: uuid)
- error('uuid library @0@ missing required function @1@'.format(uuidopt, uuidfunc))
- endif
- cdata.set('HAVE_@0@'.format(uuidheader.underscorify().to_upper()), 1)
+ uuids = {
+ 'e2fs': {
+ 'uuiddep': 'uuid',
+ 'uuidfunc': 'uuid_generate',
+ 'uuidheader': 'uuid/uuid.h',
+ },
+ 'bsd': {
+ 'uuiddep': '',
+ 'uuidfunc': 'uuid_to_string',
+ 'uuidheader': 'uuid.h',
+ },
+ 'ossp': {
+ 'uuiddep': 'ossp-uuid',
+ 'uuidfunc': 'uuid_export',
+ 'uuidheader': 'uuid.h',
+ }
+ }
- cdata.set('HAVE_UUID_@0@'.format(uuidname), 1,
- description: 'Define to 1 if you have @0@ UUID support.'.format(uuidname))
-else
- uuid = not_found_dep
+ foreach uuidname, uuid_values : uuids
+ if uuidopt != 'auto' and uuidname != uuidopt
+ continue
+ endif
+ uuid_required = (uuidname == uuidopt)
+ uuiddep = uuid_values['uuiddep']
+ uuidheader = uuid_values['uuidheader']
+ uuidfunc = uuid_values['uuidfunc']
+
+ # libc should have uuid function
+ uuid = uuiddep != '' ? dependency(uuiddep, required: uuid_required) : \
+ declare_dependency()
+
+ if uuid.found() and cc.has_header_symbol(uuidheader, uuidfunc, args: test_c_args,
+ dependencies: uuid, required: uuid_required)
+ uuidname_upper = uuidname.to_upper()
+ uuid_library = uuidname
+ cdata.set('HAVE_@0@'.format(uuidheader.underscorify().to_upper()), 1)
+ cdata.set('HAVE_UUID_@0@'.format(uuidname_upper), 1,
+ description: 'Define to 1 if you have @0@ UUID support.'.format(uuidname_upper))
+ break
+ else
+ uuid = not_found_dep
+ endif
+ endforeach
+endif
+
+if uuidopt == 'auto' and auto_features.enabled() and not uuid.found()
+ error('no UUID library found')
endif
@@ -3302,11 +3329,12 @@ if meson.version().version_compare('>=0.57')
'readline': readline,
'selinux': selinux,
'systemd': systemd,
- 'uuid': uuid,
+ 'uuid': uuid.found() ? [uuid, '(@0@)'.format(uuid_library)] : uuid,
'zlib': zlib,
'zstd': zstd,
},
section: 'External libraries',
+ list_sep: ', ',
)
endif
diff --git a/meson_options.txt b/meson_options.txt
index 4402dd4299d..3ffe6fb758d 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -137,8 +137,8 @@ option('ssl', type : 'combo', choices : ['auto', 'none', 'openssl'],
option('systemd', type : 'feature', value: 'auto',
description: 'build with systemd support')
-option('uuid', type : 'combo', choices : ['none', 'bsd', 'e2fs', 'ossp'],
- value : 'none',
+option('uuid', type : 'combo', choices : ['auto', 'none', 'bsd', 'e2fs', 'ossp'],
+ value : 'auto',
description: 'build contrib/uuid-ossp using LIB')
option('zlib', type : 'feature', value: 'auto',
diff --git a/src/makefiles/meson.build b/src/makefiles/meson.build
index 7635771c5ae..25e2a54a6bb 100644
--- a/src/makefiles/meson.build
+++ b/src/makefiles/meson.build
@@ -67,7 +67,7 @@ pgxs_kv = {
# want the chosen option, rather than the library
'with_ssl' : ssl_library,
- 'with_uuid': uuidopt,
+ 'with_uuid': uuid_library,
'default_port': get_option('pgport'),
'with_system_tzdata': get_option('system_tzdata'),
--
2.39.2
On 14.03.23 15:07, Nazir Bilal Yavuz wrote:
I think the uuid side of this is making this way too complicated. I'm
content leaving this as a manual option for now.There is much more value in making the ssl option work automatically.
So I would welcome a patch that just makes -Dssl=auto work smoothly,
perhaps using the "trick" that Andres described.I tried to implement what we did for ssl to uuid as well, do you have
any comments?
For the uuid option, we have three different choices. What should be
the search order and why?
Hi,
On Wed, 15 Mar 2023 at 11:12, Peter Eisentraut
<peter.eisentraut@enterprisedb.com> wrote:
On 14.03.23 15:07, Nazir Bilal Yavuz wrote:
I think the uuid side of this is making this way too complicated. I'm
content leaving this as a manual option for now.There is much more value in making the ssl option work automatically.
So I would welcome a patch that just makes -Dssl=auto work smoothly,
perhaps using the "trick" that Andres described.I tried to implement what we did for ssl to uuid as well, do you have
any comments?For the uuid option, we have three different choices. What should be
the search order and why?
Docs [1]https://www.postgresql.org/docs/current/uuid-ossp.html say that: OSSP uuid library is not well maintained, and is
becoming increasingly difficult to port to newer platforms; so we can
put 'uuid-ossp' to the end. Between 'uuid-e2fs' and 'uuid-bsd', I
believe 'uuid-e2fs' is used more often than 'uuid-bsd'.
Hence, they can be ordered as 'uuid-e2fs', 'uuid-bsd', 'uuid-ossp'.
Does that make sense?
Regards,
Nazir Bilal Yavuz
Microsoft
On 22.03.23 11:16, Nazir Bilal Yavuz wrote:
Hi,
On Wed, 15 Mar 2023 at 11:12, Peter Eisentraut
<peter.eisentraut@enterprisedb.com> wrote:On 14.03.23 15:07, Nazir Bilal Yavuz wrote:
I think the uuid side of this is making this way too complicated. I'm
content leaving this as a manual option for now.There is much more value in making the ssl option work automatically.
So I would welcome a patch that just makes -Dssl=auto work smoothly,
perhaps using the "trick" that Andres described.I tried to implement what we did for ssl to uuid as well, do you have
any comments?For the uuid option, we have three different choices. What should be
the search order and why?Docs [1] say that: OSSP uuid library is not well maintained, and is
becoming increasingly difficult to port to newer platforms; so we can
put 'uuid-ossp' to the end. Between 'uuid-e2fs' and 'uuid-bsd', I
believe 'uuid-e2fs' is used more often than 'uuid-bsd'.
Hence, they can be ordered as 'uuid-e2fs', 'uuid-bsd', 'uuid-ossp'.Does that make sense?
I think that's fair.
Hi,
I was looking at older threads and found that. There were failures
when rebasing v1-uuid patch to master so I updated it and also added
documentation. I attached the v2 of the patch. I am sending v2 patch
to this thread but should I create a new thread for uuid patch?
Regards,
Nazir Bilal Yavuz
Microsoft
Attachments:
v2-0001-meson-Make-auto-the-default-of-the-uuid-option.patchtext/x-diff; charset=US-ASCII; name=v2-0001-meson-Make-auto-the-default-of-the-uuid-option.patchDownload
From 83f3222f83b5bb508fc7ce7140731f742e84f166 Mon Sep 17 00:00:00 2001
From: Nazir Bilal Yavuz <byavuz81@gmail.com>
Date: Thu, 17 Aug 2023 13:08:32 +0300
Subject: [PATCH v2] meson: Make auto the default of the uuid option
The 'uuid' option is of type 'combo', but we add a choice 'auto' that
simulates the behavior of a feature option. This way, uuid is used
automatically by default if present, but we retain the ability to
potentially select another uuid library.
uuid search order is 'e2fs', 'bsd', 'ossp'. Because, docs [1] states
that OSSP uuid library is not well maintained, and is becoming
increasingly difficult to port to newer platforms; so we can put 'ossp'
to the end. Between 'e2fs' and 'bsd', 'e2fs' is used more often
than 'bsd'. Hence, they can be ordered as 'e2fs', 'bsd', 'ossp'.
[1] https://www.postgresql.org/docs/current/uuid-ossp.html
---
.cirrus.yml | 5 +-
doc/src/sgml/installation.sgml | 19 +++++++-
meson.build | 87 ++++++++++++++++++++++++----------
meson_options.txt | 4 +-
src/makefiles/meson.build | 2 +-
5 files changed, 83 insertions(+), 34 deletions(-)
diff --git a/.cirrus.yml b/.cirrus.yml
index 314ae2d804b..c39f06c893b 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -181,7 +181,7 @@ task:
su postgres <<-EOF
meson setup \
--buildtype=debug \
- -Dcassert=true -Duuid=bsd -Dtcl_version=tcl86 -Ddtrace=auto \
+ -Dcassert=true -Dtcl_version=tcl86 -Ddtrace=auto \
-DPG_TEST_EXTRA="$PG_TEST_EXTRA" \
-Dextra_lib_dirs=/usr/local/lib -Dextra_include_dirs=/usr/local/include/ \
build
@@ -243,7 +243,6 @@ LINUX_CONFIGURE_FEATURES: &LINUX_CONFIGURE_FEATURES >-
LINUX_MESON_FEATURES: &LINUX_MESON_FEATURES >-
-Dllvm=enabled
- -Duuid=e2fs
task:
@@ -509,7 +508,7 @@ task:
-Dextra_include_dirs=${brewpath}/include \
-Dextra_lib_dirs=${brewpath}/lib \
-Dcassert=true \
- -Duuid=e2fs -Ddtrace=auto \
+ -Ddtrace=auto \
-DPG_TEST_EXTRA="$PG_TEST_EXTRA" \
build
diff --git a/doc/src/sgml/installation.sgml b/doc/src/sgml/installation.sgml
index ac8eee47c66..59722356475 100644
--- a/doc/src/sgml/installation.sgml
+++ b/doc/src/sgml/installation.sgml
@@ -2574,7 +2574,7 @@ ninja install
</varlistentry>
<varlistentry id="configure-with-uuid-meson">
- <term><option>-Duuid=<replaceable>LIBRARY</replaceable></option></term>
+ <term><option>-Duuid={ auto | none | <replaceable>LIBRARY</replaceable> }</option></term>
<listitem>
<para>
Build the <xref linkend="uuid-ossp"/> module
@@ -2585,7 +2585,22 @@ ninja install
<itemizedlist>
<listitem>
<para>
- <option>none</option> to not build the uuid module. This is the default.
+ <option>auto</option> to select UUID library module automatically between
+ <option>e2fs, bsd, ossp</option>. This is the default.
+ </para>
+ <para>
+ Search order is <option>e2fs, bsd, ossp</option>. Because,
+ <option>ossp</option> UUID library is not well maintained; and is
+ becoming increasingly difficult to port to newer platforms. So,
+ <option>ossp</option> will be last option. Between
+ <option>e2fs</option> and <option>bsd</option>, <option>e2fs</option> is
+ used more often than <option>bsd</option>. Hence, they ordered as
+ <option>e2fs, bsd, ossp</option>.
+ </para>
+ </listitem>
+ <listitem>
+ <para>
+ <option>none</option> to not build the uuid module.
</para>
</listitem>
<listitem>
diff --git a/meson.build b/meson.build
index f5ec442f9a9..3f0067e4754 100644
--- a/meson.build
+++ b/meson.build
@@ -1333,36 +1333,71 @@ endif
###############################################################
uuidopt = get_option('uuid')
+uuid_library = 'none'
+uuid = not_found_dep
+if uuidopt == 'auto' and auto_features.disabled()
+ uuidopt = 'none'
+endif
+
if uuidopt != 'none'
- uuidname = uuidopt.to_upper()
- if uuidopt == 'e2fs'
- uuid = dependency('uuid', required: true)
- uuidfunc = 'uuid_generate'
- uuidheader = 'uuid/uuid.h'
- elif uuidopt == 'bsd'
- # libc should have uuid function
- uuid = declare_dependency()
- uuidfunc = 'uuid_to_string'
- uuidheader = 'uuid.h'
- elif uuidopt == 'ossp'
- uuid = dependency('ossp-uuid', required: true)
- uuidfunc = 'uuid_export'
- uuidheader = 'uuid.h'
- else
- error('unknown uuid build option value: @0@'.format(uuidopt))
- endif
- if not cc.has_header_symbol(uuidheader, uuidfunc, args: test_c_args, dependencies: uuid)
- error('uuid library @0@ missing required function @1@'.format(uuidopt, uuidfunc))
- endif
- cdata.set('HAVE_@0@'.format(uuidheader.underscorify().to_upper()), 1)
+ # uuid search order is 'e2fs', 'bsd', 'ossp'. Because, docs [1] states
+ # that OSSP uuid library is not well maintained, and is becoming
+ # increasingly difficult to port to newer platforms; so we can put 'ossp'
+ # to the end. Between 'e2fs' and 'bsd', 'e2fs' is used more often
+ # than 'bsd'. Hence, they can be ordered as 'e2fs', 'bsd', 'ossp'.
+ # [1] https://www.postgresql.org/docs/current/uuid-ossp.html
+ uuids = {
+ 'e2fs': {
+ 'uuiddep': 'uuid',
+ 'uuidfunc': 'uuid_generate',
+ 'uuidheader': 'uuid/uuid.h',
+ },
+ 'bsd': {
+ 'uuiddep': 'bsd',
+ 'uuidfunc': 'uuid_to_string',
+ 'uuidheader': 'uuid.h',
+ },
+ 'ossp': {
+ 'uuiddep': 'ossp-uuid',
+ 'uuidfunc': 'uuid_export',
+ 'uuidheader': 'uuid.h',
+ }
+ }
- cdata.set('HAVE_UUID_@0@'.format(uuidname), 1,
- description: 'Define to 1 if you have @0@ UUID support.'.format(uuidname))
-else
- uuid = not_found_dep
+ foreach uuidname, uuid_values : uuids
+ if uuidopt != 'auto' and uuidname != uuidopt
+ continue
+ endif
+ uuid_required = (uuidname == uuidopt)
+ uuiddep = uuid_values['uuiddep']
+ uuidheader = uuid_values['uuidheader']
+ uuidfunc = uuid_values['uuidfunc']
+
+ # libc should have uuid function
+ if uuiddep == 'bsd'
+ uuid = declare_dependency()
+ else
+ uuid = dependency(uuiddep, required: uuid_required)
+ endif
+
+ if uuid.found() and cc.has_header_symbol(uuidheader, uuidfunc, args: test_c_args,
+ dependencies: uuid, required: uuid_required)
+ uuidname_upper = uuidname.to_upper()
+ uuid_library = uuidname
+ cdata.set('HAVE_@0@'.format(uuidheader.underscorify().to_upper()), 1)
+ cdata.set('HAVE_UUID_@0@'.format(uuidname_upper), 1,
+ description: 'Define to 1 if you have @0@ UUID support.'.format(uuidname_upper))
+ break
+ else
+ uuid = not_found_dep
+ endif
+ endforeach
endif
+if uuidopt == 'auto' and auto_features.enabled() and not uuid.found()
+ error('no UUID library found')
+endif
###############################################################
@@ -3393,7 +3428,7 @@ if meson.version().version_compare('>=0.57')
'readline': readline,
'selinux': selinux,
'systemd': systemd,
- 'uuid': uuid,
+ 'uuid': uuid.found() ? [uuid, '(@0@)'.format(uuid_library)] : uuid,
'zlib': zlib,
'zstd': zstd,
},
diff --git a/meson_options.txt b/meson_options.txt
index d2f95cfec36..bef942d82e8 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -146,8 +146,8 @@ option('ssl', type: 'combo', choices: ['auto', 'none', 'openssl'],
option('systemd', type: 'feature', value: 'auto',
description: 'systemd support')
-option('uuid', type: 'combo', choices: ['none', 'bsd', 'e2fs', 'ossp'],
- value: 'none',
+option('uuid', type: 'combo', choices: ['auto', 'none', 'bsd', 'e2fs', 'ossp'],
+ value: 'auto',
description: 'Use LIB for contrib/uuid-ossp support')
option('zlib', type: 'feature', value: 'auto',
diff --git a/src/makefiles/meson.build b/src/makefiles/meson.build
index be946f7b38b..d9037a8b4a3 100644
--- a/src/makefiles/meson.build
+++ b/src/makefiles/meson.build
@@ -66,7 +66,7 @@ pgxs_kv = {
# want the chosen option, rather than the library
'with_ssl' : ssl_library,
- 'with_uuid': uuidopt,
+ 'with_uuid': uuid_library,
'default_port': get_option('pgport'),
'with_system_tzdata': get_option('system_tzdata'),
--
2.40.1