Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Build nix-util with Meson #10855

Merged
merged 1 commit into from
Jun 12, 2024
Merged

Build nix-util with Meson #10855

merged 1 commit into from
Jun 12, 2024

Conversation

Ericson2314
Copy link
Member

@Ericson2314 Ericson2314 commented Jun 4, 2024

Motivation

The idea is two-fold:

  • Replace autotools with Meson

  • Build each library in its own derivation

The interaction of these two features is that Meson's "subprojects" feature (https://mesonbuild.com/Subprojects) allows us to have single dev shell for building all libraries still, while also building things separately. This allows us to break up the build without a huge productivity lost.

By doing the first two libraries, I hope to demonstrate how concerns are divided across the package divided, how configure checks (mostly) don't need to be duplicated, etc. Hopefully this this makes for an example that is easy enough to imitate so others can help out with this. That is in meson-libstore.

I tested the Linux native build, and NetBSD and Windows cross builds.

Also do some clean ups of the Flake in the process of supporting new jobs.

Context

Special thanks to everyone that has worked on a Meson port so far, @p01arst0rm and @Qyriad in particular.

Depends on #10839

Priorities and Process

Add 👍 to pull requests you find important.

The Nix maintainer team uses a GitHub project board to schedule and track reviews.

@Ericson2314 Ericson2314 requested a review from edolstra as a code owner June 4, 2024 15:49
@Ericson2314 Ericson2314 changed the base branch from subproject-move to master June 4, 2024 15:54
@github-actions github-actions bot added documentation contributor-experience Developer experience for Nix contributors with-tests Issues related to testing. PRs with tests have some priority c api Nix as a C library with a stable interface labels Jun 4, 2024
@Ericson2314 Ericson2314 force-pushed the meson-libutil branch 2 times, most recently from 70bb8ed to d6bad59 Compare June 4, 2024 16:41
@Ericson2314 Ericson2314 force-pushed the meson-libutil branch 2 times, most recently from c91404d to 4664e47 Compare June 4, 2024 17:06
Copy link

dpulls bot commented Jun 4, 2024

🎉 All dependencies have been resolved !

@fricklerhandwerk
Copy link
Contributor

This looks a actually tractable. The only question I have is whether there's a way around specifying every single source file. This sounds like guaranteed busywork.

@Ericson2314
Copy link
Member Author

Ericson2314 commented Jun 4, 2024

The only question I have is whether there's a way around specifying every single source file. This sounds like guaranteed busywork.

Nope, sorry https://thiblahute.github.io/jpakkane.github.io/FAQ.html#why-cant-i-specify-target-files-with-a-wildcard

Well there is https://thiblahute.github.io/jpakkane.github.io/FAQ.html#but-i-really-want-to-use-wildcards, but such a script is a portability hazard. So I rather not.

This level of verbosity is e.g. what bazel users put up with a lot, so I think it is OK for now. Thankfully both adding the missing file is easy, and forgetting a file quickly causes a failure.


Someday dynamic derivations Nix will be a backend instead of Ninja for Meson too, and the underlying issues thus overcome, and then we can use wildcards again :).

@L-as
Copy link
Member

L-as commented Jun 4, 2024

I assume they want to do the hard work in Ninja, and given that Meson is written in Python, it would be slow to generate all those Ninja rules in Python that way.
Since Ninja probably doesn't have dynamic derivations, they wouldn't be able to do it in Ninja.

Copy link
Contributor

@fricklerhandwerk fricklerhandwerk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In that case...

@L-as
Copy link
Member

L-as commented Jun 4, 2024

How will this interact with make compile_commands.json? (clang LSP still works for me in libutil after running it, but the logic should be handled by meson I assume?)

@Ericson2314
Copy link
Member Author

https://github.com/mesonbuild/meson/pull/4547/files/52071c6d4ee15c750f8a8620e038b78627db890e#r381455882 I am a bit confused about the status of the docs (@eli-schwartz?) but Meson has a bunch of stuff out of the box for this

https://mesonbuild.com/IDE-integration.html

@Ericson2314 Ericson2314 changed the title Build nix-util with Meson Build nix-util and nix-store with Meson Jun 4, 2024
@eli-schwartz
Copy link
Contributor

Yes, it might be a good idea to re-add mention of compile_commands.json to the docs.

Note that meson will always generate one at the end of meson setup without your intervention, on the theory that creating one is never harmful so why make people specify they want it.

@Ericson2314 Ericson2314 force-pushed the meson-libutil branch 2 times, most recently from 7f1a9cf to d63b491 Compare June 4, 2024 22:46
@Ericson2314
Copy link
Member Author

I'm happy to say that in the big dev shell, one can build the perl bindings too! No more forgetting to update them during development!

@Ericson2314
Copy link
Member Author

Thanks @eli-schwartz, those suggestions made it a lot better!

@Ericson2314 Ericson2314 force-pushed the meson-libutil branch 2 times, most recently from 8b61940 to 3215fa2 Compare June 6, 2024 00:11
@fricklerhandwerk
Copy link
Contributor

Tracking #10855

@fricklerhandwerk fricklerhandwerk enabled auto-merge (rebase) June 6, 2024 08:14
@edolstra
Copy link
Member

edolstra commented Jun 7, 2024

I'm not saying I'm against switching to Meson, but I'm not convinced that the approach in this PR is the way to go. It adds a significant amount of complexity to our build system: src/libstore/meson.build is 446 lines (compared to 104 lines for src/libstore/local.mk, and unlike the makefile it's very imperative), src/libstore/package.nix is 158 lines, src/libutil/meson.build is 287 lines, etc. It doesn't make me feel confident that switching to Meson will make Nix easier to maintain.

There is probably a use case for having libstore exposed as a separate derivation (though for most users, having a lib output should be enough), but libutil is strictly internal to the Nix project and is not intended as an external artifact.

)

sources = files(
'binary-cache-store.cc',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I much prefer the current approach where we don't explicitly enumerate every source file, i.e.

nix_SOURCES := \
  $(wildcard $(d)/*.cc) \

A list like this causes a lot of unnecessary cod churn and tends to be a source of merge conflicts.

Is that possible with Meson?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no, this is not possible by design. everything in meson is defined explicitly

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Meson has a FAQ entry on why this isn't natively supported:

https://mesonbuild.com/FAQ.html#why-cant-i-specify-target-files-with-a-wildcard

And another entry on how you can implement it yourself:

https://mesonbuild.com/FAQ.html#but-i-really-want-to-use-wildcards

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

code churn is a fair price to pay for a more robust build :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really long term, I hope Meson has a Nix backend, and the Nix backend (+ dynamic derivations) can help Meson support this without these trade-offs.

has_acl_support = cxx.has_header('sys/xattr.h') \
and cxx.has_function('llistxattr') \
and cxx.has_function('lremovexattr')
configdata.set('HAVE_ACL_SUPPORT', has_acl_support.to_int())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wouldn't mind getting rid of Autoconf-style checks like this and just assuming that if you're on (for example) Linux, then you must have ACL support. So we can just do

#if __linux__
#include <sys/xattr.h>
#endif

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

def a good idea, though the point of this PR is to change out the buildsystem pending future improvements

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This specific one is configure-style because one of the BSDs also supports this same interface.

@p01arst0rm
Copy link
Contributor

I'm not saying I'm against switching to Meson, but I'm not convinced that the approach in this PR is the way to go. It adds a significant amount of complexity to our build system: src/libstore/meson.build is 446 lines (compared to 104 lines for src/libstore/local.mk, and unlike the makefile it's very imperative), src/libstore/package.nix is 158 lines, src/libutil/meson.build is 287 lines, etc. It doesn't make me feel confident that switching to Meson will make Nix easier to maintain.

There is probably a use case for having libstore exposed as a separate derivation (though for most users, having a lib output should be enough), but libutil is strictly internal to the Nix project and is not intended as an external artifact.

iirc the reason this was the way we chose is because having seperate derivations helps improve testing. It also helps to keep the source files for each library in its own place; currrently docgen is a mess, and de-mangling it involves moving the gen files into the /proj/lib directory. seperating them out this way massively improves maintainability into the future :)

@p01arst0rm
Copy link
Contributor

we also are looking into adding testing structure for testing with/without optional dependencies so that can finally be fixed. making separate derivations makes this easier

];
};

outputs = [ "out" "dev" ];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We may consider pulling commonalities for the Nix libraries into a separate file, and use lib.extends to merge the functions overlay-style.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps, but at the same time, if there is much that needs to be duplicated between the packages, I consider that a Nixpkgs UX problem. E.g. propagatedBuildInputs should mean less need to duplicate deps.


project('nix-dev-shell', 'cpp',
version : run_command('cat', './.version', check : true).stdout().strip(),
subproject_dir : 'src',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see any other mention of nix-dev-shell. How do we use this in practice?
Maybe add a paragraph to the hacking guide?

flake.nix Outdated
Comment on lines 341 to 342
map (transformFlag "libutil") pkgs.nix-util.mesonFlags
++ map (transformFlag "libstore") pkgs.nix-store.mesonFlags
++ lib.optionals havePerl (map (transformFlag "perl") pkgs.nix-perl-bindings.mesonFlags)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thoughts on transformFlag:
I think mesonFlags should be an implementation detail.
Perhaps this is better implemented as a setup hook, or maybe this is just convenient. Not sure.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think eventually in Nixpkgs we should have structured Meson flags, and a nice shellForPackages thing that can combine a bunch of things with subprojects and prefix the meson flags of each package.

@roberth roberth disabled auto-merge June 9, 2024 10:00
@Ericson2314 Ericson2314 force-pushed the meson-libutil branch 3 times, most recently from 6327756 to 8ec5000 Compare June 9, 2024 11:00
fileset
stdenv
officialRelease
versionSuffix
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The version suffix is going to cause unnecessary rebuilds for each commit. I think officialRelease is enough "entropy" for the library builds.

@Ericson2314
Copy link
Member Author

@edolstra

I'm not saying I'm against switching to Meson, but I'm not convinced that the approach in this PR is the way to go.

and unlike the makefile it's very imperative

Yes, this is my least favorite thing about Meson, but I think it's worth all the benefits.

It adds a significant amount of complexity to our build system: src/libstore/meson.build is 446 lines (compared to 104 lines for src/libstore/local.mk), src/libstore/package.nix is 158 lines, src/libutil/meson.build is 287 lines, etc. It doesn't make me feel confident that switching to Meson will make Nix easier to maintain.

Comparing meson.build vs local.mk is not really correct, though, because meson.build contains configure.ac stuff too. When everything including removing mk is factored in, I think the line count diff will be quite positive.

There is probably a use case for having libstore exposed as a separate derivation (though for most users, having a lib output should be enough), but libutil is strictly internal to the Nix project and is not intended as an external artifact.

I am willing to reconsider these things, but (somewhat ironically) building things together with this approach would be more churn, since we would need to have e.g. src/libstore/libutil and src/libstore/libstore. I think the best approach is to err on the side of two many packages, and then reconsider what the boundaries should be after the old build system is removed.

@Ericson2314
Copy link
Member Author

We think the problem is the SQL preprocessing in libstore; I have reverted this PR to just libutil for now, and put the rest in meson-libstore.

@Ericson2314 Ericson2314 changed the title Build nix-util and nix-store with Meson Build nix-util with Meson Jun 12, 2024
The idea is two-fold:

- Replace autotools with Meson

- Build each library in its own derivation

The interaction of these two features is that Meson's "subprojects"
feature (https://mesonbuild.com/Subprojects) allows us to have single
dev shell for building all libraries still, while also building things
separately. This allows us to break up the build without a huge
productivity lost.

I tested the Linux native build, and NetBSD and Windows cross builds.

Also do some clean ups of the Flake in the process of supporting new
jobs.

Special thanks to everyone that has worked on a Meson port so far,
@p01arst0rm and @Qyriad in particular.

Co-Authored-By: p01arst0rm <[email protected]>
Co-Authored-By: Artemis Tosini <[email protected]>
Co-Authored-By: Artemis Tosini <[email protected]>
Co-Authored-By: Felix Uhl <[email protected]>
Co-Authored-By: Jade Lovelace <[email protected]>
Co-Authored-By: Lunaphied <[email protected]>
Co-Authored-By: Maximilian Bosch <[email protected]>
Co-Authored-By: Pierre Bourdon <[email protected]>
Co-Authored-By: Qyriad <[email protected]>
Co-Authored-By: Rebecca Turner <[email protected]>
Co-Authored-By: Winter <[email protected]>
Co-Authored-By: eldritch horrors <[email protected]>
Co-Authored-By: jade <[email protected]>
Co-Authored-By: julia <[email protected]>
Co-Authored-By: rebecca “wiggles” turner <[email protected]>
Co-Authored-By: wiggles dog <[email protected]>
Co-Authored-By: fricklerhandwerk <[email protected]>
Co-authored-By: Eli Schwartz <[email protected]>
Co-authored-by: Robert Hensing <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c api Nix as a C library with a stable interface contributor-experience Developer experience for Nix contributors documentation with-tests Issues related to testing. PRs with tests have some priority
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants