Re: Building with meson on NixOS/nixpkgs

To build on NixOS/nixpkgs I came up with a few small patches to
meson.build. All of this works fine with Autoconf/Make already.

Hi,

Thank you for the patches!

On Sat, 16 Mar 2024 at 14:48, Wolfgang Walther <walther(at)technowledgy(dot)de> wrote:
>
> To build on NixOS/nixpkgs I came up with a few small patches to
> meson.build. All of this works fine with Autoconf/Make already.

I do not have NixOS but I confirm that patches cleanly apply to master
and do pass CI. I have a small feedback:

0001 & 0002: Adding code comments to explain why they have fallback
could be nice.
0003: Looks good to me.

--
Regards,
Nazir Bilal Yavuz
Microsoft

On 2024-Mar-16, Wolfgang Walther wrote:

> The upstream name for the ossp-uuid package / pkg-config file is "uuid". Many
> distributions change this to be "ossp-uuid" to not conflict with e2fsprogs.

I can confirm that this is true for Debian, at least; the packaging
rules have this in override_dh_install:

install -D -m 644 debian/tmp/usr/lib/$(DEB_HOST_MULTIARCH)/pkgconfig/uuid.pc \
debian/libossp-uuid-dev/usr/lib/pkgconfig/ossp-uuid.pc

which matches the fact that Engelschall's official repository has the
file named simply uuid.pc:
https://github.com/rse/uuid/blob/master/uuid.pc.in

--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/

Nazir Bilal Yavuz:
> 0001 & 0002: Adding code comments to explain why they have fallback
> could be nice.
> 0003: Looks good to me.

Added some comments in the attached.

Best,

Wolfgang

Wolfgang Walther:
> To build on NixOS/nixpkgs I came up with a few small patches to
> meson.build. All of this works fine with Autoconf/Make already.

In v3, I added another small patch for meson, this one about proper
handling of -Dlibedit_preferred when used together with -Dreadline=enabled.

Best,

Wolfgang

Hi,

From your prior reply:

On Thu, 21 Mar 2024 at 23:44, Wolfgang Walther <walther(at)technowledgy(dot)de> wrote:
>
> Nazir Bilal Yavuz:
> > 0001 & 0002: Adding code comments to explain why they have fallback
> > could be nice.
> > 0003: Looks good to me.
>
> Added some comments in the attached.

Comments look good, thanks.

On Fri, 29 Mar 2024 at 21:48, <walther(at)technowledgy(dot)de> wrote:
>
> In v3, I added another small patch for meson, this one about proper
> handling of -Dlibedit_preferred when used together with -Dreadline=enabled.

You are right. I confirm the bug and your proposed patch fixes this.

--
Regards,
Nazir Bilal Yavuz
Microsoft

On 29.03.24 19:47, walther(at)technowledgy(dot)de wrote:
> - uuid = dependency('ossp-uuid', required: true)
> + # upstream is called "uuid", but many distros change this to
"ossp-uuid"
> + uuid = dependency('ossp-uuid', 'uuid', required: true)

How would this behave if you have only uuid.pc from e2fsprogs installed
but choose -Duuid=ossp? Then it would pick up uuid.pc here, but fail to
compile later?

Peter Eisentraut:
> On 29.03.24 19:47, walther(at)technowledgy(dot)de wrote:
> > -    uuid = dependency('ossp-uuid', required: true)
> > +    # upstream is called "uuid", but many distros change this to
> "ossp-uuid"
> > +    uuid = dependency('ossp-uuid', 'uuid', required: true)
>
> How would this behave if you have only uuid.pc from e2fsprogs installed
> but choose -Duuid=ossp?  Then it would pick up uuid.pc here, but fail to
> compile later?

It would still fail the meson setup step, because for e2fs we have:

uuidfunc = 'uuid_generate'
uuidheader = 'uuid/uuid.h'

while for ossp we have:

uuidfunc = 'uuid_export'
uuidheader = 'uuid.h'

and later we do:

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

Best,

Wolfgang

Heikki asked me to take a look at this patchset for the commitfest.
Looks good to me.

Heikki, just be careful rebasing the first patch. You need to make sure
the newly set `required: false` gets carried forward.

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

On 26/07/2024 23:01, Tristan Partin wrote:
> Heikki asked me to take a look at this patchset for the commitfest.
> Looks good to me.
>
> Heikki, just be careful rebasing the first patch. You need to make sure
> the newly set `required: false` gets carried forward.

Committed and backpatched to v16 and v17. Thanks for the good
explanations in the commit messages, Walther!

--
Heikki Linnakangas
Neon (https://neon.tech)

Hi,

commit 4d8de281b5834c8f5e0be6ae21e884e69dffd4ce
Author: Heikki Linnakangas <heikki(dot)linnakangas(at)iki(dot)fi>
Date: 2024-07-27 13:53:11 +0300

Fallback to clang in PATH with meson

Some distributions put clang into a different path than the llvm
binary path.

For example, this is the case on NixOS / nixpkgs, which failed to find
clang with meson before this patch.

I think this is a bad change unfortunately - this way clang and llvm version
can mismatch. Yes, we've done it that way for autoconf, but back then LLVM
broke compatibility far less often.

commit a00fae9d43e5adabc56e64a4df6d332062666501
Author: Heikki Linnakangas <heikki(dot)linnakangas(at)iki(dot)fi>
Date: 2024-07-27 13:53:08 +0300

Fallback to uuid for ossp-uuid with meson

The upstream name for the ossp-uuid package / pkg-config file is
"uuid". Many distributions change this to be "ossp-uuid" to not
conflict with e2fsprogs.

This lookup fails on distributions which don't change this name, for
example NixOS / nixpkgs. Both "ossp-uuid" and "uuid" are also checked
in configure.ac.

Author: Wolfgang Walther
Reviewed-by: Nazir Bilal Yavuz, Alvaro Herrera, Peter Eisentraut
Reviewed-by: Tristan Partin
Discussion: https://www.postgresql.org/message-id/ca8f37e1-a2c3-40e2-91f6-59c3d3652ad4@technowledgy.de
Backpatch: 16-, where meson support was added

I think this is a redundant change with

commit 2416fdb3ee30bdd2810408f93f14d47bff840fea
Author: Andres Freund <andres(at)anarazel(dot)de>
Date: 2024-07-20 13:51:08 -0700

meson: Add support for detecting ossp-uuid without pkg-config

This is necessary as ossp-uuid on windows installs neither a pkg-config nor a
cmake dependency information. Nor is there another supported uuid
implementation available on windows.

Reported-by: Dave Page <dpage(at)pgadmin(dot)org>
Reviewed-by: Tristan Partin <tristan(at)partin(dot)io>
Discussion: https://postgr.es/m/20240709065101.xhc74r3mdg2lmn4w@awork3.anarazel.de
Backpatch: 16-, where meson support was added

Greetings,

Andres Freund

On Fri Aug 9, 2024 at 11:14 AM CDT, Andres Freund wrote:
> Hi,
>
>
> commit 4d8de281b5834c8f5e0be6ae21e884e69dffd4ce
> Author: Heikki Linnakangas <heikki(dot)linnakangas(at)iki(dot)fi>
> Date: 2024-07-27 13:53:11 +0300
>
> Fallback to clang in PATH with meson
>
> Some distributions put clang into a different path than the llvm
> binary path.
>
> For example, this is the case on NixOS / nixpkgs, which failed to find
> clang with meson before this patch.
>
>
> I think this is a bad change unfortunately - this way clang and llvm version
> can mismatch. Yes, we've done it that way for autoconf, but back then LLVM
> broke compatibility far less often.

See the attached patch on how we could make this situation better.

> commit a00fae9d43e5adabc56e64a4df6d332062666501
> Author: Heikki Linnakangas <heikki(dot)linnakangas(at)iki(dot)fi>
> Date: 2024-07-27 13:53:08 +0300
>
> Fallback to uuid for ossp-uuid with meson
>
> The upstream name for the ossp-uuid package / pkg-config file is
> "uuid". Many distributions change this to be "ossp-uuid" to not
> conflict with e2fsprogs.
>
> This lookup fails on distributions which don't change this name, for
> example NixOS / nixpkgs. Both "ossp-uuid" and "uuid" are also checked
> in configure.ac.
>
> Author: Wolfgang Walther
> Reviewed-by: Nazir Bilal Yavuz, Alvaro Herrera, Peter Eisentraut
> Reviewed-by: Tristan Partin
> Discussion: https://www.postgresql.org/message-id/ca8f37e1-a2c3-40e2-91f6-59c3d3652ad4@technowledgy.de
> Backpatch: 16-, where meson support was added
>
> I think this is a redundant change with
>
> commit 2416fdb3ee30bdd2810408f93f14d47bff840fea
> Author: Andres Freund <andres(at)anarazel(dot)de>
> Date: 2024-07-20 13:51:08 -0700
>
> meson: Add support for detecting ossp-uuid without pkg-config
>
> This is necessary as ossp-uuid on windows installs neither a pkg-config nor a
> cmake dependency information. Nor is there another supported uuid
> implementation available on windows.
>
> Reported-by: Dave Page <dpage(at)pgadmin(dot)org>
> Reviewed-by: Tristan Partin <tristan(at)partin(dot)io>
> Discussion: https://postgr.es/m/20240709065101.xhc74r3mdg2lmn4w@awork3.anarazel.de
> Backpatch: 16-, where meson support was added

I'm not sure I would call them redundant. It's cheaper (and better) to
do a pkg-config lookup than it is to do the various checks in your
patch. I think the two patches are complementary. Yours services Windows
plus anywhere else that doesn't have a pkg-config file, while Wolfgang's
services distros that install the pkg-config with a different name.

--
Tristan Partin
https://tristan.partin.io

Tristan Partin:
> On Fri Aug 9, 2024 at 11:14 AM CDT, Andres Freund wrote:
> [..]
>> commit a00fae9d43e5adabc56e64a4df6d332062666501
>> Author: Heikki Linnakangas <heikki(dot)linnakangas(at)iki(dot)fi>
>> Date:   2024-07-27 13:53:08 +0300
>>
>>     Fallback to uuid for ossp-uuid with meson
>> [..]
>>
>> I think this is a redundant change with
>>
>> commit 2416fdb3ee30bdd2810408f93f14d47bff840fea
>> Author: Andres Freund <andres(at)anarazel(dot)de>
>> Date:   2024-07-20 13:51:08 -0700
>>
>>     meson: Add support for detecting ossp-uuid without pkg-config
>> [..]
>
> I'm not sure I would call them redundant. It's cheaper (and better) to
> do a pkg-config lookup than it is to do the various checks in your
> patch. I think the two patches are complementary. Yours services Windows
> plus anywhere else that doesn't have a pkg-config file, while Wolfgang's
> services distros that install the pkg-config with a different name.

Agreed.

There is also a small difference in output for meson: When uuid is
queried via pkg-config, meson also detects the version, so I get this
output:

External libraries
[..]
uuid : YES 1.6.2

Without pkg-config:

External libraries
[..]
uuid : YES

Best,

Wolfgang

Tristan Partin:
> On Fri Aug 9, 2024 at 11:14 AM CDT, Andres Freund wrote:
>> commit 4d8de281b5834c8f5e0be6ae21e884e69dffd4ce
>> Author: Heikki Linnakangas <heikki(dot)linnakangas(at)iki(dot)fi>
>> Date:   2024-07-27 13:53:11 +0300
>>
>>     Fallback to clang in PATH with meson
>> [..]
>>
>> I think this is a bad change unfortunately - this way clang and llvm
>> version
>> can mismatch. Yes, we've done it that way for autoconf, but back then
>> LLVM
>> broke compatibility far less often.
>
> See the attached patch on how we could make this situation better.

Works great.

With the correct clang on path:

Program clang found: YES 18.1.8 18.1.8
(/nix/store/mr1y1rxkx59dr2bci2akmw2zkbbpmc15-clang-wrapper-18.1.8/bin/clang)

With a mismatching version on path:

Program
/nix/store/x4gwwwlw2ylv0d9vjmkx3dmlcb7gingd-llvm-18.1.8/bin/clang clang
found: NO found 16.0.6 but need: '18.1.8'
(/nix/store/r85xsa9z0s04n0y21xhrii47bh74g2a8-clang-wrapper-16.0.6/bin/clang)

Yes, the match is exact, also fails with a newer version:

Program
/nix/store/x4gwwwlw2ylv0d9vjmkx3dmlcb7gingd-llvm-18.1.8/bin/clang clang
found: NO found 19.1.0 but need: '18.1.8'
(/nix/store/rjsfx6sxjpkgd4f9hl9apm0n8dk7jd9w-clang-wrapper-19.1.0-rc2/bin/clang)

+1 for this patch.

Best,

Wolfgang