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

zig: 0.11 -> 0.12 #306077

Merged
Merged

Conversation

iFreilicht
Copy link
Contributor

@iFreilicht iFreilicht commented Apr 22, 2024

This upgrades the default version of zig to zig_0_12, which builds reproducibly on darwin, and marks all older versions as broken on darwin.

Fixes #299091.

Also pins all packages compatible with zig 0.12 to that version.
I tried to upgrade packages currently pinning 0.11 as well, but only a
few worked.

Supersedes #304369

Description of changes

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.05 Release Notes (or backporting 23.05 and 23.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@iFreilicht iFreilicht mentioned this pull request Apr 22, 2024
13 tasks
@ofborg ofborg bot added the 6.topic: darwin Running or building packages on Darwin label Apr 22, 2024
@wegank
Copy link
Member

wegank commented Apr 22, 2024

Technically, the PR is not about fixing builds, but bumping default Zig version to 0.12 (which should be in a separate PR), and marking builds that succeed on Hydra as broken (which I don't really agree with).

The reason why builds are broken locally could, unfortunately, be obscure on macOS. I'm aware that having Xcode Command Line Tools, or even XQuartz installed, can break some builds. Instead of marking them as broken, I'd suggest seeing if the build works in a fresh macOS and Nix installation on a virtual machine.

@iFreilicht
Copy link
Contributor Author

Technically, the PR is not about fixing builds, but bumping default Zig version to 0.12 (which should be in a separate PR), and marking builds that succeed on Hydra as broken (which I don't really agree with).

Yeah I see what you're saying. By "fix" I meant fix non-reproducibility, as a non-reproducible build can't be relied upon to not break on an input update, but I guess it's not necessarily broken per se.

The reason why builds are broken locally could, unfortunately, be obscure on macOS. I'm aware that having Xcode Command Line Tools, or even XQuartz installed, can break some builds.

Even with sandboxing? That's annoying.

Instead of marking them as broken, I'd suggest seeing if the build works in a fresh macOS and Nix installation on a virtual machine.

Will do.

@iFreilicht
Copy link
Contributor Author

@wegank The build does not succeed, even in a fresh macOS VM:

# Enabled sandbox, nix-command and flakes, then relaunched nix-daemon
$ nix build --rebuild github:NixOS/nixpkgs/0c97ced70e0b92d46e5e53e239fec5201f8b0811#zig
error: builder for '/nix/store/93wiqvqj0xm7bw8difdzdixjxb0rx26y-zig-0.11.0.drv' failed with exit code 2;
       last 25 log lines:
       >    t15.f1 = zig_addo_u64(&t15.f0, t3, t2, UINT8_C(64));
       >                          ^~~~~~~
       > /tmp/nix-build-zig-0.11.0.drv-1/source/stage1/zig.h:562:43: note: passing argument to parameter 'res' here
       > static inline bool zig_addo_u64(uint64_t *res, uint64_t lhs, uint64_t rhs, uint8_t bits) {
       >                                           ^
       > /tmp/nix-build-zig-0.11.0.drv-1/source/build/zig2.c:2892813:23: warning: incompatible pointer types passing 'uintptr_t *' (aka 'unsigned long *') to parameter of type 'uint64_t *' (aka 'unsigned long long *') [-Wincompatible-pointer-types]
       >  t1.f1 = zig_subo_u64(&t1.f0, a0, t0, UINT8_C(64));
       >                       ^~~~~~
       > /tmp/nix-build-zig-0.11.0.drv-1/source/stage1/zig.h:670:43: note: passing argument to parameter 'res' here
       > static inline bool zig_subo_u64(uint64_t *res, uint64_t lhs, uint64_t rhs, uint8_t bits) {
       >                                           ^
       > /tmp/nix-build-zig-0.11.0.drv-1/source/build/zig2.c:2892819:23: warning: incompatible pointer types passing 'uintptr_t *' (aka 'unsigned long *') to parameter of type 'uint64_t *' (aka 'unsigned long long *') [-Wincompatible-pointer-types]
       >  t5.f1 = zig_subo_u64(&t5.f0, t4, t0, UINT8_C(64));
       >                       ^~~~~~
       > /tmp/nix-build-zig-0.11.0.drv-1/source/stage1/zig.h:670:43: note: passing argument to parameter 'res' here
       > static inline bool zig_subo_u64(uint64_t *res, uint64_t lhs, uint64_t rhs, uint8_t bits) {
       >                                           ^
       > 44 warnings generated.
       > [ 94%] Linking CXX executable zig2
       > [ 94%] Built target zig2
       > [100%] Building stage3
       > error: unable to create compilation: DarwinSdkNotFound
       > make[2]: *** [CMakeFiles/stage3.dir/build.make:73: stage3/bin/zig] Error 1
       > make[1]: *** [CMakeFiles/Makefile2:196: CMakeFiles/stage3.dir/all] Error 2
       > make: *** [Makefile:136: all] Error 2
       For full logs, run 'nix log /nix/store/93wiqvqj0xm7bw8difdzdixjxb0rx26y-zig-0.11.0.drv'.
$ nix-shell -p nix-info --run "nix-info -m"
these 4 paths will be fetched (1.14 MiB download, 8.59 MiB unpacked):
  /nix/store/bqpqw4d01m24irxbkwy94x9x7nfy6q5y-DarwinTools-1
  /nix/store/sn6b0ydc6y7ixrgaa5gqfkxqlmqipvj6-bash-interactive-5.2p26
  /nix/store/w16qjzzi4jl5cd0iy05c5lf4ffil1282-nix-info
  /nix/store/njnn57q1i3w40av980fnkllkh9bl5q0k-readline-8.2p10
copying path '/nix/store/bqpqw4d01m24irxbkwy94x9x7nfy6q5y-DarwinTools-1' from 'https://cache.nixos.org'...
copying path '/nix/store/njnn57q1i3w40av980fnkllkh9bl5q0k-readline-8.2p10' from 'https://cache.nixos.org'...
copying path '/nix/store/sn6b0ydc6y7ixrgaa5gqfkxqlmqipvj6-bash-interactive-5.2p26' from 'https://cache.nixos.org'...
copying path '/nix/store/w16qjzzi4jl5cd0iy05c5lf4ffil1282-nix-info' from 'https://cache.nixos.org'...
 - system: `"aarch64-darwin"`
 - host os: `Darwin 23.4.0, macOS 14.4.1`
 - multi-user?: `yes`
 - sandbox: `yes`
 - version: `nix-env (Nix) 2.21.1`
 - channels(root): `"nixpkgs"`
 - nixpkgs: `/nix/var/nix/profiles/per-user/root/channels/nixpkgs`

My strong suspicion is that it does succeed on older versions of macOS, but I have no way of testing this hypothesis.

However, I agree with your general assessment, this is a version upgrade.

Additionally, I tried to upgrade the versions of all the packages depending on zig_0_11, and most failed to build, so clearly 0.12 is not supported widely yet.

Sooo, I guess we just leave all the zig stuff as is and hope there's no input update until the release that breaks it?

@wegank
Copy link
Member

wegank commented Apr 23, 2024

My strong suspicion is that it does succeed on older versions of macOS

Oh, that's sad.

Sooo, I guess we just leave all the zig stuff as is and hope there's no input update until the release that breaks it?

You can still proceed with the version upgrade, just pin the failing packages to Zig 0.11. You can check #248153 to see how this was done.

@AndersonTorres
Copy link
Member

AndersonTorres commented Apr 24, 2024

We decided a long time ago to pin all references to Zig in packages, since it is not stable enough.

In other words, always use zig = zig_0_12; when possible.

@iFreilicht
Copy link
Contributor Author

@AndersonTorres Yes, I noticed. There are a few packages that reference zig directly (none of those broke), but most already specify zig_0_11 in their inputs.

I'll add the ones that the upgrade worked for plus the review suggestions next week Wednesday.

@Mic92 Mic92 modified the milestone: 24.05 Apr 26, 2024
@iFreilicht iFreilicht force-pushed the fix-zig-build-mark-broken-on-darwin branch from 687617c to e6c4f9f Compare May 1, 2024 17:57
@iFreilicht iFreilicht force-pushed the fix-zig-build-mark-broken-on-darwin branch from e6c4f9f to 8148ea5 Compare May 1, 2024 20:01
@iFreilicht iFreilicht changed the title zig: fix build on darwin zig: 0.11 -> 0.12 May 1, 2024
@iFreilicht iFreilicht force-pushed the fix-zig-build-mark-broken-on-darwin branch from 8148ea5 to 968e2c9 Compare May 1, 2024 20:05
@iFreilicht
Copy link
Contributor Author

So I've gone through and tried to upgrade as many packages as possible, most failed, which is not a problem, but one blocks this PR; zls, the zig language server.

Upstream updated it 5 days ago to be compatible with zig 0.12, and a PR for updating it exists already: #308248

That has to be merged before this PR, otherwise something like nix shell nixpkgs#{zig,zls} would install incompatible versions of zig and its language server, which I deem unacceptable.

@AndersonTorres
Copy link
Member

We should add some passthru.tests for ZLS.

@iFreilicht iFreilicht force-pushed the fix-zig-build-mark-broken-on-darwin branch from 968e2c9 to 3a69789 Compare May 1, 2024 21:24
@iFreilicht iFreilicht mentioned this pull request May 1, 2024
13 tasks
@iFreilicht
Copy link
Contributor Author

@AndersonTorres I agree that would be best, though I'm unsure if I can land it in this PR. I played around a bit, and I guess one thing to do would be something like this:

  passthru.tests = {
    equal-zig-version = ''
      test "$(${zls.meta.mainProgram} --version)" = "$(${zig.meta.mainProgram} version)"
  };

But that is only a test inside of zls. If the "global" zig is changed, this will still succeed, as the zig input to zls is pinned in all-packages.

I thought about adding a tests.nix file that is called with callPackage, as that would "inject" the correct version of zig AFAIU, but then the zig input would basically not be overridable at all anymore.

@AndersonTorres
Copy link
Member

AndersonTorres commented May 1, 2024

The idea is not to test versions, but the softwares themselves. Something like we do with live555+vlc.

Let's postpone this to a future PR.

Copy link
Contributor

@nbraud nbraud left a comment

Choose a reason for hiding this comment

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

I just merged the zls update — #308248 — and this looks good to go.

@wegank
Copy link
Member

wegank commented May 2, 2024

@ofborg build backlight-auto

@wegank wegank force-pushed the fix-zig-build-mark-broken-on-darwin branch 2 times, most recently from 08f405e to 12c39e8 Compare May 2, 2024 17:34
This upgrades the default version of zig to zig_0_12, which builds
reproducibly on darwin.

Fixes NixOS#299091.

Also upgrades all packages compatible with zig 0.12 to that version.
I tried to upgrade packages currently pinning 0.11 as well, but only a
few worked.
@wegank wegank force-pushed the fix-zig-build-mark-broken-on-darwin branch from 12c39e8 to a38793e Compare May 2, 2024 17:35
@wegank
Copy link
Member

wegank commented May 2, 2024

Result of nixpkgs-review pr 306077 run on x86_64-linux 1

7 packages built:
  • cargo-lambda
  • cargo-zigbuild
  • clipbuzz
  • zig
  • zig.doc
  • zig_0_11
  • zig_0_11.doc

@nbraud nbraud merged commit 4c70474 into NixOS:master May 2, 2024
7 of 8 checks passed
@iFreilicht iFreilicht deleted the fix-zig-build-mark-broken-on-darwin branch May 2, 2024 18:43
pull bot pushed a commit to auxolotl/nixpkgs that referenced this pull request May 2, 2024
This upgrades the default version of zig to zig_0_12, which builds
reproducibly on darwin.

Fixes NixOS#299091.

Also upgrades all packages compatible with zig 0.12 to that version.
I tried to upgrade packages currently pinning 0.11 as well, but only a
few worked.

Co-authored-by: Weijia Wang (wegank) <contact@weijia.wang>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Build failure: zig_0_11
6 participants