Subject: Re: meson: Non-feature feature options

From: Peter Eisentraut <[email-masked]>
Date: Wed, 22 Feb 2023 10:14:05 +0100

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.

---

From: Nazir Bilal Yavuz <[email-masked]>
Date: Tue, 14 Mar 2023 17:07:15 +0300

Hi,

> 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
From 0e92050c15ca6e9ebeddaafad229eee53fc9e7b0 Mon Sep 17 00:00:00 2001
From: Nazir Bilal Yavuz <[email-masked]>
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

---

From: Peter Eisentraut <[email-masked]>
Date: Wed, 15 Mar 2023 09:12:27 +0100

> I tried to implement what we did for ssl to uuid as well, do you have

For the uuid option, we have three different choices.  What should be 
the search order and why?

---

From: Nazir Bilal Yavuz <[email-masked]>
Date: Wed, 22 Mar 2023 13:16:54 +0300

Hi,

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

Regards,
Nazir Bilal Yavuz
Microsoft

[1] https://www.postgresql.org/docs/current/uuid-ossp.html

---

From: Peter Eisentraut <[email-masked]>
Date: Wed, 22 Mar 2023 16:22:33 +0100

> Hence, they can be ordered as 'uuid-e2fs', 'uuid-bsd', 'uuid-ossp'.

I think that's fair.

---

From: Nazir Bilal Yavuz <[email-masked]>
Date: Thu, 17 Aug 2023 14:18:58 +0300

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
From 83f3222f83b5bb508fc7ce7140731f742e84f166 Mon Sep 17 00:00:00 2001
From: Nazir Bilal Yavuz <[email-masked]>
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(-)

index 314ae2d804b..c39f06c893b 100644

 task:
@@ -509,7 +508,7 @@ task:
 
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
 ###############################################################
 
-    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',
+    }
+  }
 
+    if uuiddep == 'bsd'
+      uuid = declare_dependency()
+      uuid = dependency(uuiddep, required: uuid_required)

 ###############################################################
@@ -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 = {

2.40.1