Feedback-ID: i131946ab:Fastmail
Message-ID: <b695b531-eb98-87f1-45c8-13f2ed3973a8@enterprisedb.com>
Date: Wed, 1 Mar 2023 16:52:18 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0)
 Gecko/20100101 Thunderbird/102.8.0
Subject: Re: meson: Non-feature feature options
Content-Language: en-US
To: Nazir Bilal Yavuz <byavuz81@gmail.com>
Cc: Andres Freund <andres@anarazel.de>,
 pgsql-hackers <pgsql-hackers@postgresql.org>
References: <ad65ffd1-a9a7-fda1-59c6-f7dc763c3051@enterprisedb.com>
 <20230208162310.scllwgr5qp3m5tme@awork3.anarazel.de>
 <c649abc4-8ede-dba2-1a28-e6ee3cf35006@gmail.com>
 <000d58dd-6164-f695-fe12-38ac7e37543b@gmail.com>
 <1d1eb740-e2d6-dd92-ba35-985af38b30d5@enterprisedb.com>
 <20230220194249.lkirdwhosjq6zdkc@awork3.anarazel.de>
 <CAN55FZ2Bz5P_sd2+1KBwWa9v4vDByM2UX7rKfzQs7yj6buojsg@mail.gmail.com>
 <4e1fcc8d-77cf-082c-5dd8-9ab09bb7a05f@enterprisedb.com>
 <CAN55FZ3JkWJRgBOvaB0VDYg1F+v3QMg4vwVae1gpQ+mnsYc-3g@mail.gmail.com>
From: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
In-Reply-To: <CAN55FZ3JkWJRgBOvaB0VDYg1F+v3QMg4vwVae1gpQ+mnsYc-3g@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Archived-At: <https://www.postgresql.org/message-id/b695b531-eb98-87f1-45c8-13f2ed3973a8%40enterprisedb.com>
Precedence: bulk

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.