Building with meson on NixOS/nixpkgs

Hi,

Thank you for the patches!

On Sat, 16 Mar 2024 at 14:48, Wolfgang Walther <walther@technowledgy.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@technowledgy.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@technowledgy.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@technowledgy.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@technowledgy.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.linnakangas@iki.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.linnakangas@iki.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: /messages/by-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@anarazel.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@pgadmin.org>
Reviewed-by: Tristan Partin <tristan@partin.io>
Discussion: /messages/by-id/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.linnakangas@iki.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.linnakangas@iki.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: /messages/by-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@anarazel.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@pgadmin.org>
Reviewed-by: Tristan Partin <tristan@partin.io>
Discussion: /messages/by-id/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.linnakangas@iki.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@anarazel.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.linnakangas@iki.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