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

Port Nix-Perl to Meson #10378

Merged
merged 3 commits into from
Apr 25, 2024
Merged

Port Nix-Perl to Meson #10378

merged 3 commits into from
Apr 25, 2024

Conversation

p01arst0rm
Copy link
Contributor

Motivation

The current buildsystem impacts Nix portability, and obstructs development of Nix on non GNU/Linux systems. Replacing the buildsystem with meson fixes this, along with adding better access to developer tools (i.e. valgrind).

Context

#3160
NixOS/rfcs#132
#9342
NixOS/nixpkgs#26850

@Ericson2314 Ericson2314 added the build-problem Nix fails to compile or test; also improvements to build process label Apr 1, 2024
perl/default.nix Outdated Show resolved Hide resolved
perl/default.nix Outdated Show resolved Hide resolved
perl/.yath.rc Outdated Show resolved Hide resolved
perl/default.nix Outdated Show resolved Hide resolved
@Ericson2314
Copy link
Member

I think ] and even ) on their own line with trailing commas for the last list item / argument would be preferred.

@p01arst0rm
Copy link
Contributor Author

I think ] and even ) on their own line with trailing commas for the last list item / argument would be preferred.

the style isnt standard across the codebase, there doesnt seem to be anything to suggest which should be used.

@p01arst0rm p01arst0rm force-pushed the nix-perl-port branch 2 times, most recently from e939c25 to e4483f5 Compare April 1, 2024 23:07
Copy link

@dcbaker dcbaker left a comment

Choose a reason for hiding this comment

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

There's an inconsistant use of spaces on both sides of :, I'm partial to the space on each side style, but probably should just pick one or the other and stick with it.

perl/meson.build Outdated Show resolved Hide resolved
perl/meson.build Outdated Show resolved Hide resolved
perl/meson_options.txt Outdated Show resolved Hide resolved
perl/meson_options.txt Outdated Show resolved Hide resolved
perl/lib/Nix/meson.build Outdated Show resolved Hide resolved
perl/meson_options.txt Outdated Show resolved Hide resolved
@p01arst0rm p01arst0rm force-pushed the nix-perl-port branch 2 times, most recently from 63ea7ac to 7d9ae33 Compare April 1, 2024 23:38
@Ericson2314
Copy link
Member

the style isnt standard across the codebase, there doesnt seem to be anything to suggest which should be used.

For C++ we put the ) on the same line, but for { ... } initializer lists we always put the } on its own line. That would be the equivalent of [ ... ].

perl/meson_options.txt Outdated Show resolved Hide resolved
perl/meson_options.txt Outdated Show resolved Hide resolved
perl/default.nix Outdated Show resolved Hide resolved
@roberth roberth changed the title Ported Nix-Perl to Meson Port Nix-Perl to Meson Apr 2, 2024
@roberth
Copy link
Member

roberth commented Apr 2, 2024

I love to see progress on this.

Just a small point: could you use the imperative mood instead of past tense in commit messages?
Nix, Nixpkgs and Git itself use it that way.

perl/t/meson.build Outdated Show resolved Hide resolved
@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/2023-04-03-nix-team-meeting-135/42962/1

@roberth
Copy link
Member

roberth commented Apr 23, 2024

@Ericson2314 is this stuck on style alone?

@Ericson2314
Copy link
Member

@roberth Yes, if that is even a blocker.

@p01arst0rm
Copy link
Contributor Author

@Ericson2314 is this stuck on style alone?

style can be fixed trivially, waiting on consensus of whether to merge before investing more time :)

@roberth
Copy link
Member

roberth commented Apr 23, 2024

👍 from me.
@thufschmitt has approved of it too.

Based on the notes, while Eelco wasn't convinced, he didn't outright reject it, so I think we should try it. After all, the point of doing a part of the project first is that we can evaluate meson in practice.

@p01arst0rm
Copy link
Contributor Author

sounds good, ill fix up the style issues and get the pr corrected.

@Ericson2314
Copy link
Member

Yes and now that mesonbuild/meson#13055 is merged, I think all style matters can squarely follow upstream recommendations, rather than me winging it :)

Copy link
Member

@Ericson2314 Ericson2314 left a comment

Choose a reason for hiding this comment

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

I corrected a few small things, and this now has my final approval

For most purposes, the stock `ninja test` should be fine, but this
allows for doing other things with the `yath` during development.
@Ericson2314 Ericson2314 merged commit 28043fe into NixOS:master Apr 25, 2024
9 checks passed
@Ericson2314 Ericson2314 mentioned this pull request Apr 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build-problem Nix fails to compile or test; also improvements to build process
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants