xcbuild: move top level override into package.nix - #485704
Conversation
nixpkgs-ci
Bot
added
10.rebuild-darwin: 0
0dc22bf to
9487586
Compare
mdaniels5757
changed the title
nixpkgs-ci
Bot
added
the
12.approvals: 1
nixpkgs-ci
Bot
added
12.approvals: 2
| ninja, | ||
| stdenv, | ||
| # xcbuild is included in the SDK. Avoid an infinite recursion by using a bootstrap stdenv | ||
| stdenv' ? if stdenv.hostPlatform.isDarwin then darwin.bootstrapStdenv else stdenv, |
There was a problem hiding this comment.
stdenv' seems a bit riskier than other '-suffix params, but being able to override is still valuable.
I could envision someone deciding they want to add a pkgs.stdenv' at some point. Although I hope it is as unlikely as I think it is 😁
There was a problem hiding this comment.
@MattSturgeon This was added to address #453948 (comment), more info on this pattern at #474456 (comment).
I'm still curious about other people's opinion on it, if we should apply this pattern at more places and in which scenarios, currently these made sense to me:
- conditionally overriding
stdenvor other dependency that is hard to "patch" ->? - aliasing a package version (e.g.
ffmpeg = ffmpeg_7) ->let in - enabling/disabling features of a package (e.g.
codec2' = codec2.override { freedvSupport = true; }-> probably?but maybelet inis enough? - extending package sets (e.g.
beamPackages = beam27Packages.extend (self: super: { elixir = elixir_1_18; });-> probablylet inbut maybe??
stdenv'seems a bit riskier than other'-suffix params, but being able to override is still valuable.I could envision someone deciding they want to add a
pkgs.stdenv'at some point. Although I hope it is as unlikely as I think it is 😁
Thankfully ' at the top-level is rare, the only instance is a function rather than a package:
nix-repl> lib.filter (lib.hasInfix "'") (lib.attrNames pkgs)
[ "makeScopeWithSplicing'" ]If we want to be extra safe maybe this could be document not adding top-level packages with ' and/or add a nixpkgs-vet rule, but I don't think we would see new top level package's anyway.
There was a problem hiding this comment.
Yeah, that's essentially the same thought process I had. ' seems essentially impossible for package-specific names, and only slightly more likely for "hot" names like stdenv.
If we want to be extra safe maybe this could be document not adding top-level packages with
'and/or add a nixpkgs-vet rule, but I don't think we would see new top levelpackage's anyway.
I agree, adding top-level ' names to the package set would be a big smell. I would get behind formalising it as guidance if someone took the time to write up a PR
Not strictly necessary, though. Clearly it's gone without saying until now 😁
4 participants
This is a step towards #454525, which will help enable checking for additional by-name directories (e.g. Python) in nixpkgs-vet.
Things done