-
-
Notifications
You must be signed in to change notification settings - Fork 14.4k
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
ninja_1_11: init at 1.11.1 #354367
ninja_1_11: init at 1.11.1 #354367
Conversation
pkgs/by-name/ni/ninja/package.nix
Outdated
hash = "sha256-LvV/Fi2ARXBkfyA1paCRmLUwCh/rTyz+tGMg2/qEepI="; | ||
}; | ||
|
||
"1.12" = fetchFromGitHub { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"1.12" = fetchFromGitHub { | |
"latest" = fetchFromGitHub { |
this one should be the latest version, we're not trying to pin it to any specific version.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair enough; pushed. Hopefully this is gone by the time 1.13 comes around either way.
382907c
to
5f0b2a5
Compare
De jure Ninja maintainers don’t seem to be actively involved; cc @JohnRTitor @vcunat @NickCao @AndersonTorres who worked on this derivation since 2023. |
@@ -11,18 +11,35 @@ | |||
, buildPackages | |||
, buildDocs ? true | |||
, nix-update-script | |||
, ninjaRelease ? "latest" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
, ninjaRelease ? "latest" | |
, use_1_11 ? false |
nit. a bool rather than a string would eliminate the error case of an invalid string and make the below assert unnecessary
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about this?
pkgs/by-name/ni/ninja/package.nix
Outdated
rev = "v1.12.1"; | ||
hash = "sha256-RT5u+TDvWxG5EVQEYj931EZyrHUSAqK73OKDAascAwA="; | ||
}; | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
}; | |
} or (throw "Unsupported Ninja release."); |
pkgs/by-name/ni/ninja/package.nix
Outdated
|
||
assert lib.assertMsg (releases ? ${ninjaRelease}) "Unsupported Ninja release: ${ninjaRelease}"; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
assert lib.assertMsg (releases ? ${ninjaRelease}) "Unsupported Ninja release: ${ninjaRelease}"; |
Swift 5.8 doesn’t reliably build with Ninja 1.12, and apparently neither do old versions of ROCm, due to a build scheduling change breaking builds with undeclared dependencies. Upstream Swift have pinned 1.11 for now. Reintroducing it isn’t a good long‐term solution, but seems like the easiest way to avoid shipping 24.11 with a broken Swift on Darwin. The patch for 32‐bit systems has not been restored, as Swift doesn’t support them anyway.
5f0b2a5
to
05ccba8
Compare
Implemented a variant of @azuwis’s suggestion. I prefer not using a boolean here and it seems pretty compact now. |
Going to take the thumbs up as the closest thing we’ll get to Ninja maintainer approval here :) |
Result of 1 package built:
|
I want to study that patch from in memoriam @amjoseph that enables multiple versions. IIRC it populates passthru.versions with the various derivations. |
This change unfortunately breaks $ nix-prefetch -I nixpkgs=. ninja
error:
… while evaluating attribute 'log'
… while calling the 'head' builtin
at /home/n/git/nixpkgs/lib/attrsets.nix:1574:11:
1573| || pred here (elemAt values 1) (head values) then
1574| head values
| ^
1575| else
… while evaluating the attribute 'name'
at /home/n/git/nixpkgs/pkgs/stdenv/generic/make-derivation.nix:336:7:
335| // (optionalAttrs (attrs ? name || (attrs ? pname && attrs ? version)) {
336| name =
| ^
337| let
(stack trace truncated; use '--show-trace' to show the full, detailed trace)
error: attribute 'rev' missing
at /home/n/git/nixpkgs/pkgs/by-name/ni/ninja/package.nix:19:34:
18| pname = "ninja";
19| version = lib.removePrefix "v" finalAttrs.src.rev;
| ^
20| |
I suspect |
I think the issue here is that the version depends on the rev, unlike most of the nixpkgs where it does not. And the tools were written in assumption to that. |
It would be easy to |
The changes introduced with NixOS#354367 lead currently to an evaluation error when prefetching vendored go dependencies with `nix-prefetch`. Although this is probably a bug in `nix-prefetch`, I prefer to use `nurl` instead as it does the same thing, seems better maintained and doesn't block k3s updates even longer.
The changes introduced with NixOS#354367 lead currently to an evaluation error when prefetching vendored go dependencies with `nix-prefetch`. Although this is probably a bug in `nix-prefetch`, I prefer to use `nurl` instead as it does the same thing, seems better maintained and doesn't block k3s updates even longer.
The changes introduced with NixOS#354367 lead currently to an evaluation error when prefetching vendored go dependencies with `nix-prefetch`. Although this is probably a bug in `nix-prefetch`, I prefer to use `nurl` instead as it does the same thing, seems better maintained and doesn't block k3s updates even longer.
Swift 5.8 doesn’t reliably build with Ninja 1.12, and apparently neither do old versions of ROCm, due to a build scheduling change breaking builds with undeclared dependencies. Upstream Swift have pinned 1.11 for now.
Reintroducing it isn’t a good long‐term solution, but seems like the easiest way to avoid shipping 24.11 with a broken Swift on Darwin. The patch for 32‐bit systems has not been restored, as Swift doesn’t support them anyway.
cc @azuwis @JohnRTitor. I think this is a better approach than
overrideAttrs
. I confirmed thatnix-update
can still update the package in this form.Things done
nix.conf
? (See Nix manual)sandbox = relaxed
sandbox = true
nix-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.