tree-wide: use top-level cctools#328077
Conversation
reckenrode
requested review from
K900,
NickCao,
Profpatsch,
RaghavSood,
SuperSandro2000,
ajs124,
corngood,
jbedo,
lukegb,
mmahut,
mweinelt,
natsukium,
ttuegel and
winterqt
as code owners
github-actions
bot
added
6.topic: python
paparodeo
left a comment
paparodeo
left a comment
There was a problem hiding this comment.
LGTM. looks pretty mechanical, didn't notice any surprises.
|
Thanks! Once ofborg confirms evaluation, I’ll merge. |
ofborg
bot
added
8.has: clean-up
| inherit (pkgs) stdenv lib python2 runCommand writeTextFile writeShellScript; | ||
| inherit pkgs nodejs; | ||
| libtool = if pkgs.stdenv.isDarwin then pkgs.darwin.cctools else null; | ||
| libtool = if pkgs.stdenv.isDarwin then pkgs.cctools or pkgs.darwin.cctools else null; |
There was a problem hiding this comment.
Do we really need the or here?
There was a problem hiding this comment.
good spot -- pkgs.darwin.cctools is now an alias so these should all be removed, i think. there are a number of them.
There was a problem hiding this comment.
The files with this pattern were generated by node2nix. They were updated by hand, but I will be submitting a patch upstream to make it generate this output (assuming node2nix wants to support older nixpkgs).
The intent with these particular changes was to avoid unnecessary diffs assuming these files will be refreshed at some time in the future with the updated node2nix.
There was a problem hiding this comment.
I think you need to apply the patch to node2nix inside nodePackages otherwise we quickly run into problems here.
There was a problem hiding this comment.
Also don't we trigger the alias everytime unless people have aliases turned off locally?
There was a problem hiding this comment.
I think you need to apply the patch to node2nix inside nodePackages otherwise we quickly run into problems here.
I can patch the in-tree copy once I have submitted it upstream. I don’t want this PR to cause any rebuilds, so that would be done in a separate PR. (Update: This is done. See #328077 (comment).)
There was a problem hiding this comment.
Also don't we trigger the alias everytime unless people have aliases turned off locally?
Aren’t they disabled in the ofborg checks, meaning eval will fail unless the aliases aren’t used?
There was a problem hiding this comment.
Upstream PR for node2nix: svanderburg/node2nix#334
nixpkgs PR for node2nix: #328167
There was a problem hiding this comment.
Yeah, ofborg won't have them.
Derp, i forgot that package aliases don't trigger warnings, only options do.
cctools was updated and migrated to the `by-name` hierarchy in nixpkgs, which moves it to the top-level. It is also being added to `darwin-aliases.nix`, which will make the old name unavailable for use in nixpkgs. This change preferentially uses the new name while falling back to the old one for out-of-tree users. Relevant nixpkgs PRs: - NixOS/nixpkgs#307880 - NixOS/nixpkgs#328077
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: |
cctools was updated and migrated to the `by-name` hierarchy in nixpkgs, which moves it to the top-level. It is also being added to `darwin-aliases.nix`, which will make the old name unavailable for use in nixpkgs. This change preferentially uses the new name while falling back to the old one for out-of-tree users. Relevant nixpkgs PRs: - NixOS/nixpkgs#307880 - NixOS/nixpkgs#328077
4 participants
Description of changes
This PR is the first follow-up to #307880. It finishes moving darwin.cctools to the top-level by adding darwin.cctools and darwin.cctools-port to darwin-aliases.nix.
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.