Re: meson vs. llvm bitcode files - Mailing list pgsql-hackers

On 18.03.26 13:44, Nazir Bilal Yavuz wrote:
>> The comment that introduces postgresql-extension-warnings.pc says
>>
>> +# Extension modules should likely also use -fwrapv etc. But it it's a
>> bit odd
>> +# to expose it to a .pc file?
>>
>> but then -fwrapv ends up in postgresql-extension.pc anyway.  Not sure
>> what was intended here.
> 
> I asked Andres off-list and Andres said that we need to have these
> flags inside the .pc file but it is not very nice since these flags
> (-fwrapv for example) change the behavior. Maybe Andres could clarify
> this better.

Yes, it's probably right that extensions should build with the -f 
options that the server uses.  You probably need -fwrapv and 
-fno-strict-aliasing at least.  Then again, we don't know which compiler 
will consume the .pc file and whether it even supports these options in 
that particular spelling.

>> The Requires list in my case is for example
>>
>> Requires: krb5-gssapi, icu-uc, icu-i18n, ldap, libxml-2.0 >=  2.6.23,
>> liblz4, openssl, zlib, libzstd >= 1.4.0
>>
>> but I don't think these are actually required for building extensions
>> (unless a particular extension directly makes use of one of them, in
>> which case they should declare that on their own).
> 
> It seems that is how meson pkgconfig.generate() handles the
> dependencies, please see [1]:
> 
> ...
> * Dependencies provided by pkg-config are added into Requires: or
> Requires.private:. If a version was specified when declaring that
> dependency it will be written into the generated file too.
> ...

Sure, but that doesn't make it right. ;-)  It would be a quite a 
regression if extensions switched to using this .pc file (which we would 
want them to eventually), and then building an extension would require 
installing all these -dev packages.

>> If we are going to install these .pc files, we also need to build them
>> with with makefiles.  Alternatively, we could not install them for now
>> and just use them internally.
> 
> Unfortunately, these .pc files are always installed in meson build. I
> added a WIP patch (0007) for building .pc files with makefiles, I am
> not sure if I am following the correct way. I would appreciate any
> help on this.

Seems reasonable as a start.  (But there you have an empty Requires list.)

>> I don't know if it's possible to make meson use a different file than
>> meson.build, but if so, it might be better to keep these test
>> meson.build files together with their extensions, like
>> contrib/amcheck/meson-test.build.  Similar to how we have "PGXS" build
>> support in the makefiles.  Otherwise, I'm afraid this will get
>> annoying and error-prone if one has to remember to update other files
>> under src/test/ when adding for example a new .sql file to amcheck.
> 
> I don't think we can use something other than meson.build. I solved
> that by editing the test_meson_extension script, now meson-test.build
> files live under the actual contrib/${extension}/ directory and the
> test script moves them to the correct directory. I needed to use the
> get_option('meson_source_dir') hack to get paths of the source files.

That seems more manageable.  But I wonder whether it wouldn't be simpler 
to create a bespoke test module for testing this, rather than overlaying 
this onto production modules.  That way, in the future, it would be 
easier to add more tests and variants and play around with this more 
freely without having to interfere with the real modules.

>> v9-0003-meson-WIP-Add-docs-for-postgresql-extension.pc.patch
>>
>> Let's not rename existing ids.
>>
>> It seems to me that the .pc file can also be used without meson.
>> Let's take that into account a bit.  For example, the
>> id="extend-postgres-meson" could be id="extend-postgres-pkg-config" or
>> similar.
> 
> Sorry but I didn't understand how we can add a pkg-config
> documentation without renaming existing ids. 'Extension Building
> Infrastructure' is covered by <sect1 id="extend-pgxs">. I guess we
> would want to add pkg-config documentation under the extension
> building infrastructure, but it is something other than PGXS. So, it
> being under '<sect1 id="extend-pgxs">' doesn't sound correct to me.

I mean stuff like

-     <varlistentry id="extend-pgxs-modules">
+     <varlistentry id="extend-postgres-pgxs-modules">

seems unnecessary.  You can keep the id of the existing section as 
"extend-pgxs" and add a new one in parallel.  We don't need to have a 
perfect hierarchy of ids, especially if it means changing existing ones.


I think at this point it's clear that this patch set needs quite a bit 
more consideration, so let's move it to PG20.