python: unsupported platforms check and buildPythonApplication support #128
Conversation
There was a problem hiding this comment.
Thanks for the contribution. cc'ing @SuperSandro2000 as this is based on his suggestion.
overlays have to be added to the git index in order for their checks to be discovered and applied which seems like a nixpkgs-hammering limitation that ought to be removed or documented.
This is Nix flakes limitation so docs are probably the only solution.
|
|
||
| checkDerivation = drvArgs: drv: | ||
| let | ||
| python = lib.lists.last (builtins.filter (p: p ? pythonVersion) drv.propagatedBuildInputs); |
There was a problem hiding this comment.
I am not very confident about this. Maybe add a comment pointing to the source code so that we can easily update this when it breaks: https://github.com/NixOS/nixpkgs/blob/dbe1337750f324137ca2ba78f20a3be00e16481a/pkgs/development/interpreters/python/mk-python-derivation.nix#L155
There was a problem hiding this comment.
Hmm, I had not seen the comments added by NixOS/nixpkgs#179318 which does seem to draw into question the long-term inclusion of python as an input.
| sources = [ | ||
| "buildPythonPackage" | ||
| sources = if drv ? pythonModule | ||
| then [ "buildPythonPackage" |
There was a problem hiding this comment.
What is the goal of including this in the error message?
There was a problem hiding this comment.
I'm not convinced the inclusion provides any value but I followed the existing pattern. Here's an example error message as it currently stands:
$ nix run ../nixpkgs-hammering hindsight
warning: Git tree '/home/user/nixpkgs-hammering' is dirty
error: build log of '/nix/store/z36zmlak3h7x5kxcg4mr3fm7pj6w0zbf-hindsight-2021.01.16' is not available
error: build log of '/nix/store/z36zmlak3h7x5kxcg4mr3fm7pj6w0zbf-hindsight-2021.01.16' is not available
When evaluating attribute ‘hindsight’:
warning: python-inconsistent-interpreters
Between buildPythonApplication and propagatedBuildInputs, this derivation seems
to be simultaneously trying to use packages from multiple different Python package sets.
Mixing package sets like this is not supported.
Detected Python packages:
- python3-3.8.7
- python-2.7.18
See: https://github.com/jtojnar/nixpkgs-hammering/blob/master/explanations/python-inconsistent-interpreters.mdShall I remove "buildPythonPackage" and "buildPythonApplication" from these messages?
| @@ -0,0 +1,26 @@ | |||
| final: prev: | |||
There was a problem hiding this comment.
In the future, we will probably want the same rule for other languages like Go and Rust. I wonder if we should name this rule e.g. language-builder-unsupported-platforms to allow easily extending it in the future.
There was a problem hiding this comment.
Maybe but it seems like it could get messy trying to get the default platform of every builder in one overlay. Perhaps it would make more sense to abstract the shared logic into a checkDefaultPlatformsForBuilder method in lib?
This effects python-inconsistent-interpreters because we can no longer assume there to be a drv.pythonModule attribute.
ryneeverett
force-pushed
the
python-unsupported-platforms
branch
from
f26c376
to
323fff3
Compare
For context, I first typed the substance of this out over a year ago when I was looking for automated ways to avoid some of the review comments I received from Sandro in NixOS/nixpkgs#110620. As I recall, the reason I got stuck and forgot about it is that overlays have to be added to the git index in order for their checks to be discovered and applied which seems like a nixpkgs-hammering limitation that ought to be removed or documented.