-
-
Notifications
You must be signed in to change notification settings - Fork 13.7k
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
make-derivation.nix: lib.warn if drv.__spliced missing #263082
Closed
Closed
Changes from all commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
596a57d
targetPackages_bintools: give it a top-level entry
61f0912
stdenv: lib.warn if drv.__splice is missing when expected
23bbc79
stdenv: allow unspliced hostTarget dependencies (for now)
6cae19e
stdenv: add list-of-shame
849df93
gcc: use stdenv.cc.bintools instead of targetPackages_bintools in bui…
a42fd7a
isl: fix splicing failure
1b1cd5d
rustc-wasm32: give it a custom pname for stdenv list-of-shame
6a5f5a9
cracklib: do not use buildPackages for self-reference (fails to splice)
e178e10
stdenv: alphabetize list-of-shame
8891c0a
stdenv: improve diagnostics for unspliced-dependency check
ec58a9e
Revert "gcc: use stdenv.cc.bintools instead of targetPackages_bintool…
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,67 @@ | ||
let | ||
|
||
# | ||
# This is a list of pnames of packages that have failed to splice. | ||
# Every one of these is an unfixed bug. Please fix them instead | ||
# of taking shameful behavior, you scoundrel! | ||
# | ||
list-of-shame = | ||
builtins.listToAttrs | ||
(builtins.map | ||
(name: { inherit name; value = true; }) | ||
[ | ||
# shame on you, python | ||
"pypa-build-hook.sh" | ||
"pypa-install-hook" | ||
"pytest-check-hook" | ||
"python-catch-conflicts-hook" | ||
"python-imports-check-hook.sh" | ||
"python-namespaces-hook.sh" | ||
"python-output-dist-hook" | ||
"python-remove-bin-bytecode-hook" | ||
"python-remove-tests-dir-hook" | ||
"python-remove-tests-dir-hook" | ||
"python3-3.11.5-env" | ||
"setuptools-check-hook" | ||
"setuptools-setup-hook" | ||
"unittest-check-hook" | ||
"wrap-python-hook" | ||
"wrap-python-hook" | ||
"wrap-python-hook" | ||
|
||
# shame on you, llvm! | ||
"llvm" | ||
"compiler-rt" | ||
"libcxx" | ||
"libcxxabi" | ||
Comment on lines
+32
to
+36
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the llvm sets use the normal I'm little by little working on making the sets better #253671 Nice list name. |
||
"libunwind" | ||
|
||
# shame on you, embedded ARM! | ||
"arm-none-eabi-stage-final-gcc-wrapper" | ||
"arm-none-eabi-binutils-wrapper" | ||
|
||
# wasm doesn't care about shamefulness, so I won't bother... | ||
"rustc-wasm32" | ||
|
||
# windows | ||
# has overrides which discard splicing | ||
# improvement: convert makeScope to makeScopeWithSplicing' | ||
# may not fix it but it'll improve the set | ||
# https://github.com/NixOS/nixpkgs/blob/53aa767c849b159cdb8c59dce4a5a44f167fc31b/pkgs/os-specific/windows/default.nix#L20 | ||
"mcfgthreads" | ||
This conversation was marked as resolved.
Show resolved
Hide resolved
|
||
"i686-w64-mingw32-stage-final-gcc-wrapper" | ||
"x86_64-w64-mingw32-stage-final-gcc-wrapper" | ||
|
||
# other shameful derivations | ||
"git-minimal" | ||
"busybox" | ||
"avr-stage-final-gcc-wrapper" | ||
"boost-build" | ||
"cmake-boot" | ||
"perl" # only for libxcrypt; we should fix this | ||
]); | ||
in | ||
|
||
{ lib, config }: | ||
|
||
stdenv: | ||
|
@@ -283,32 +347,89 @@ else let | |
references = nativeBuildInputs ++ buildInputs | ||
++ propagatedNativeBuildInputs ++ propagatedBuildInputs; | ||
|
||
getSpliced = type: dep: | ||
dep.__spliced or | ||
(let approximate-pname = dep.pname or dep.name; in | ||
assert | ||
(dep?stdenv && | ||
dep.stdenv.buildPlatform != dep.stdenv.targetPlatform && | ||
|
||
# I can't test on Darwin, so let's just ignore it for now. | ||
!dep.stdenv.buildPlatform.isDarwin && | ||
!dep.stdenv.hostPlatform.isDarwin && | ||
!dep.stdenv.targetPlatform.isDarwin && | ||
|
||
!(list-of-shame ? ${approximate-pname}) | ||
) | ||
|
||
# Hi there! If you're reading this, it's probably | ||
# because the line below caused your PR to fail CI. | ||
# Don't panic! Most likely, what happened is that you | ||
# used one of the following as a dependency: | ||
# | ||
# - `buildPackages.something` | ||
# - `targetPackages.something` | ||
# - `pkgs{Build,Host,Target}{Build,Host,Target}.something` | ||
# | ||
# You can't use those as dependencies (i.e. in a | ||
# `buildInputs`, `nativeBuildInputs`, or | ||
# `deps{Build,Host,Target}{Build,Host,Target}` | ||
# attribute). The reason is a bit obscure, but the fix | ||
# is easy: just use `something` instead! The explicit | ||
# packagesets (the three bullet points above) are mainly | ||
# for when you need to reference a package with string | ||
# interpolation (e.g. "cat blah | ${buildPackages.jq}"). | ||
# For dependencies, you control which packageset is used | ||
# by *which attribute you put the dependency in* -- if | ||
# you put it in `depsBuildHost`, it will get pulled from | ||
# `pkgsBuildHost`. | ||
# | ||
# Please take a moment to try to fix your PR. If you | ||
# can't get it fixed, ping @amjoseph-nixpkgs who can | ||
# help you fix it. If this is a crisis situation and | ||
# the future of humanity depends on your PR passing CI | ||
# pronto, you can mute the warning by adding your | ||
# package's `pname` to the `list-of-shame` at the top of | ||
# this file. But please don't do that. | ||
# | ||
-> lib.warn | ||
''derivation ${attrs.pname or "!!no pname!!"}: | ||
unspliced ${type} dependency ${approximate-pname} | ||
build=${dep.stdenv.buildPlatform.config} | ||
host=${dep.stdenv.hostPlatform.config} | ||
target=${dep.stdenv.targetPlatform.config} | ||
For advice on fixing this, read the comment above the lib.warn that produced this message. | ||
'' true; | ||
{}); | ||
|
||
dependencies = map (map chooseDevOutputs) [ | ||
[ | ||
(map (drv: drv.__spliced.buildBuild or drv) (checkDependencyList "depsBuildBuild" depsBuildBuild)) | ||
(map (drv: drv.__spliced.buildHost or drv) (checkDependencyList "nativeBuildInputs" nativeBuildInputs')) | ||
(map (drv: drv.__spliced.buildTarget or drv) (checkDependencyList "depsBuildTarget" depsBuildTarget)) | ||
(map (drv: (getSpliced "build-build" drv).buildBuild or drv) (checkDependencyList "depsBuildBuild" depsBuildBuild)) | ||
(map (drv: (getSpliced "build-host" drv).buildHost or drv) (checkDependencyList "nativeBuildInputs" nativeBuildInputs')) | ||
(map (drv: (getSpliced "build-target" drv).buildTarget or drv) (checkDependencyList "depsBuildTarget" depsBuildTarget)) | ||
] | ||
[ | ||
(map (drv: drv.__spliced.hostHost or drv) (checkDependencyList "depsHostHost" depsHostHost)) | ||
(map (drv: (getSpliced "host-host" drv).hostHost or drv) (checkDependencyList "depsHostHost" depsHostHost)) | ||
(map (drv: drv.__spliced.hostTarget or drv) (checkDependencyList "buildInputs" buildInputs')) | ||
#(map (drv: (getSpliced "host-target" drv).hostTarget or drv) (checkDependencyList "buildInputs" buildInputs')) | ||
] | ||
[ | ||
(map (drv: drv.__spliced.targetTarget or drv) (checkDependencyList "depsTargetTarget" depsTargetTarget)) | ||
(map (drv: (getSpliced "target-target" drv).targetTarget or drv) (checkDependencyList "depsTargetTarget" depsTargetTarget)) | ||
] | ||
]; | ||
propagatedDependencies = map (map chooseDevOutputs) [ | ||
[ | ||
(map (drv: drv.__spliced.buildBuild or drv) (checkDependencyList "depsBuildBuildPropagated" depsBuildBuildPropagated)) | ||
(map (drv: drv.__spliced.buildHost or drv) (checkDependencyList "propagatedNativeBuildInputs" propagatedNativeBuildInputs)) | ||
(map (drv: drv.__spliced.buildTarget or drv) (checkDependencyList "depsBuildTargetPropagated" depsBuildTargetPropagated)) | ||
(map (drv: (getSpliced "build-build propagated" drv).buildBuild or drv) (checkDependencyList "depsBuildBuildPropagated" depsBuildBuildPropagated)) | ||
(map (drv: (getSpliced "build-host propagated" drv).buildHost or drv) (checkDependencyList "propagatedNativeBuildInputs" propagatedNativeBuildInputs)) | ||
(map (drv: (getSpliced "build-target propagated" drv).buildTarget or drv) (checkDependencyList "depsBuildTargetPropagated" depsBuildTargetPropagated)) | ||
] | ||
[ | ||
(map (drv: drv.__spliced.hostHost or drv) (checkDependencyList "depsHostHostPropagated" depsHostHostPropagated)) | ||
(map (drv: (getSpliced "host-host propagated" drv).hostHost or drv) (checkDependencyList "depsHostHostPropagated" depsHostHostPropagated)) | ||
(map (drv: drv.__spliced.hostTarget or drv) (checkDependencyList "propagatedBuildInputs" propagatedBuildInputs)) | ||
#(map (drv: (getSpliced "host-target propagated" drv).hostTarget or drv) (checkDependencyList "propagatedBuildInputs" propagatedBuildInputs)) | ||
] | ||
[ | ||
(map (drv: drv.__spliced.targetTarget or drv) (checkDependencyList "depsTargetTargetPropagated" depsTargetTargetPropagated)) | ||
(map (drv: (getSpliced "target-target propagated" drv).targetTarget or drv) (checkDependencyList "depsTargetTargetPropagated" depsTargetTargetPropagated)) | ||
] | ||
]; | ||
|
||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
I am trying to get more fit in cross: Shouldn't this use canExecute?
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.
If this is what you mean, then yes, that would be an improvement (let's put it in a separate PR though -- one thing at a time).
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.
This goes against your comment below about using
buildPackages
innativeBuildInputs
.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.
Indeed! Nice catch. If this PR were merged CI would have caught that.
The patch above should, in addition to the changes it makes, also delete the
buildPackages.
beforecracklib
.