zig: commonify & bootstrap #331011
zig: commonify & bootstrap #331011
Conversation
RossComputerGuy
force-pushed
the
feat/zig-bootstrap
branch
from
92ca0ed to
511678d
Compare
ofborg
bot
added
11.by: package-maintainer
pkgs/by-name/me/meson/package.nix
Outdated
| hash = "sha256-dDOmSRBKl/gs7I3kmLXIyQk3zsOdlaYov72pPSel4+I="; | ||
| }) | ||
| ]; | ||
| ./linker-support-zig-cc.patch; |
There was a problem hiding this comment.
Why are we patching this in Nixpkgs at all instead of fixing it properly upstream? My read of the upstream PR was that upstream was responsive and interested, and that there was just the small issue of getting the test suite fixed left to do, and upstream had even provided a hint for how to do that.
There was a problem hiding this comment.
Because there's already an upstream PR. This is just a vendored copy after reading the feedback left after #318034 was merged. I don't see anything about a test suite fix and I'm not familiar with how meson works.
There was a problem hiding this comment.
I understand that this is just vendoring the patch, but I'm not thrilled that it was added in the first place — the original author of the upstream PR doesn't seem to be continuing with it, which means that we have to keep this patch around (with the implied maintenance burden on Nixpkgs' Meson maintainers) until somebody else picks it up. We can add patches to Nixpkgs when they're urgent, when it's not feasible to upstream them, or when it's doing something Nixpkgs-specific, but allowing patches like this, where none of those things are true, means that, for non-maintainers, there's never any incentive to upstream anything, resulting in packages getting more and more complicated until they collapse under their own weight. For that reason, I think this patch needs to be sorted out upstream, and that we should not keep it around in Nixpkgs in the meantime. The path to getting it upstream is clear, and doesn't sound like it would be a lot of work.
I don't see anything about a test suite fix
There was a problem hiding this comment.
Yeah, @tomberek and I talked yesterday about this patch. I only added it so meson projects would at least configure in pkgsZig. I understand the patch isn't exactly stable but there's not really any other way around the error which occurs without this patch. The problem looks like tests in meson fail because Zig doesn't support a flag which means we likely would have to implement the -rpath-link flag.
There was a problem hiding this comment.
there's not really any other way around the error which occurs without this patch.
Well, there is, and it's to help this get fixed upstream, which as far as I can see means just extending an existing check to also check for Zig.
There was a problem hiding this comment.
Alright, I've made a patch mesonbuild/meson#13493
There was a problem hiding this comment.
Great, thanks. For testing on NixOS I just use a nix-shell with python3 and whatever else the test suite needs.
There was a problem hiding this comment.
Yeah it looks like I'll need to pull in the CC wrapper for the tests to perform correctly.
ofborg
bot
added
10.rebuild-darwin-stdenv
RossComputerGuy
force-pushed
the
feat/zig-bootstrap
branch
2 times, most recently
from
c19a377 to
ace18eb
Compare
|
The CI is considered not correct on unused arguments in this case, as noted in #331085. |
RossComputerGuy
force-pushed
the
feat/zig-bootstrap
branch
7 times, most recently
from
a0c11c2 to
5af85b0
Compare
|
|
RossComputerGuy
force-pushed
the
feat/zig-bootstrap
branch
from
5af85b0 to
6f3ab6e
Compare
RossComputerGuy
force-pushed
the
feat/zig-bootstrap
branch
from
5e38670 to
72aa943
Compare
| '' | ||
| mkdir -p $out/bin | ||
| for tool in ar objcopy; do | ||
| makeWrapper "$zig/bin/zig" "$out/bin/${targetPrefix}-$tool" \ |
There was a problem hiding this comment.
Shouldn't it be ${targetPrefix}$tool (without the -)?
There was a problem hiding this comment.
Yes, that was a typo.
RossComputerGuy
force-pushed
the
feat/zig-bootstrap
branch
from
72aa943 to
0a2ad4b
Compare
ofborg
bot
added
10.rebuild-darwin: 11-100
10.rebuild-linux: 11-100
and removed
2.status: merge conflict
| } | ||
| '' | ||
| mkdir -p $out/bin | ||
| for tool in cc c++; do |
There was a problem hiding this comment.
| for tool in cc c++; do | |
| for tool in cc c++ ld.lld; do |
| mv $out/bin/c++ $out/bin/clang++ | ||
| mv $out/bin/cc $out/bin/clang |
There was a problem hiding this comment.
| mv $out/bin/c++ $out/bin/clang++ | |
| mv $out/bin/cc $out/bin/clang | |
| mv $out/bin/cc $out/bin/clang | |
| mv $out/bin/c++ $out/bin/clang++ | |
| mv $out/bin/ld.lld $out/bin/ld |
| } | ||
| '' | ||
| mkdir -p $out/bin | ||
| for tool in ar objcopy; do |
There was a problem hiding this comment.
| for tool in ar objcopy; do | |
| for tool in ar objcopy ranlib; do |
Those are the only 3 bintools available as zig subcommands though, so some packages fail to build with zigStdenv because they rely on other tools like strip.
RossComputerGuy
force-pushed
the
feat/zig-bootstrap
branch
from
0a2ad4b to
4a3852d
Compare
|
|
I've figured out that Zig 0.9 and 0.10 have the |
|
we can soon remove 0.9 benbusby/colorstorm#15 |
|
zig 0.10 is also unused after #365116 |
RossComputerGuy
force-pushed
the
feat/zig-bootstrap
branch
from
4a3852d to
56506fd
Compare
RossComputerGuy
force-pushed
the
feat/zig-bootstrap
branch
from
56506fd to
db214ed
Compare
RossComputerGuy
force-pushed
the
feat/zig-bootstrap
branch
from
db214ed to
356a033
Compare
Description of changes
This is the first step towards my goal of being able to bootstrap LLVM in Nix with Zig. A couple things this PR does is commonify Zig to make handling future updates simpler, I've been meaning to do this for a while. This PR also implements the bootstrap derivations for Zig.
Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.