python3.pkgs: fix splice through unsupported hosts by alyssais · Pull Request #212832 · NixOS/nixpkgs

alyssais · 2023-01-26T20:28:34Z

Description of changes

Previously, unless unsupported platforms were allowed, the following would fail to evaluate (from an "x86_64-linux" system):

pkgsCross.x86_64-freebsd.__splicedPackages.docutils.__spliced.buildHost

It shouldn't have, because the buildHost package ends up being for Linux. This broke evaluation of e.g. pkgsCross.x86_64-freebsd.libdrm, because it has docutils in nativeBuildInputs. mkDerivation would try to go through __spliced.buildHost on docutils to get to the Linux version, but the check in ensurePythonModules would kick in first, triggering the meta check because of the equality check in the implementation of hasPythonModule, which would fail because Python is not marked as supported on FreeBSD in Nixpkgs at the moment. Thus, even though they're not supposed to be, the meta checks would be triggered even though the only attribute being accessed on the unsupported derivation was __spliced.

We can fix this by using the same mechanism used to implement the meta checks themselves: lib.extendDerivation. Now, attempting to access drvPath or outPath on an attribute that fails the validity check will produce the same error as before, but other accesses will be allowed through, fixing splicing.

I've tested evaluation of packages that pass and fail the validity check, and confirmed that the behaviour is still correct.

While I was here, I also simplified the logic in the validity check.

Things done

It wasn't clear before, but it turns out that this is just four separate conditions, and an attribute is valid if any of the conditions is true, so we don't need a sea of parentheses to express it.

Previously, unless unsupported platforms were allowed, the following would fail to evaluate (from an "x86_64-linux" system): pkgsCross.x86_64-freebsd.__splicedPackages.docutils.__spliced.buildHost It shouldn't have, because the buildHost package ends up being for Linux. This broke evaluation of e.g. pkgsCross.x86_64-freebsd.libdrm, because it has docutils in nativeBuildInputs. mkDerivation would try to go through __spliced.buildHost on docutils to get to the Linux version, but the check in ensurePythonModules would kick in first, triggering the meta check because of the equality check in the implementation of hasPythonModule, which would fail because Python is not marked as supported on FreeBSD in Nixpkgs at the moment. Thus, even though they're not supposed to be, the meta checks would be triggered even though the only attribute being accessed on the unsupported derivation was __spliced. We can fix this by using the same mechanism used to implement the meta checks themselves: lib.extendDerivation. Now, attempting to access drvPath or outPath on an attribute that fails the validity check will produce the same error as before, but other accesses will be allowed through, fixing splicing. I've tested evaluation of packages that pass and fail the validity check, and confirmed that the behaviour is still correct.

ghost · 2023-01-28T07:25:56Z

but the check in ensurePythonModules would kick in first,

nixpkgs/pkgs/development/interpreters/python/default.nix

Lines 34 to 41 in de836e3

    
           ensurePythonModules = items: let 
        
             exceptions = [ 
        
               stdenv 
        
             ]; 
        
             providesSetupHook = lib.attrByPath [ "provides" "setupHook"] false; 
        
             valid = value: !((lib.isDerivation value) && !((pythonPackages.hasPythonModule value) || (providesSetupHook value))) || (lib.elem value exceptions); 
        
             func = name: value: if (valid value) then value else throw "${name} should use `buildPythonPackage` or `toPythonModule` if it is to be part of the Python packages set."; 
        
           in lib.mapAttrs func items;

triggering the meta check because of the equality check in the implementation of hasPythonModule

nixpkgs/pkgs/development/interpreters/python/python-packages-base.nix

Line 45 in 76be8d2

hasPythonModule = drv: drv?pythonModule && drv.pythonModule == python;

I would love to know why that equality check is there.

Also, testing meta-check-failing derivations for equality against each other shouldn't throw, should it?

the meta checks would be triggered even though the only attribute being accessed on the unsupported derivation was __spliced.

Yeah they should only be triggered upon access to (builtins.derivation drvAttrs).drvPath, since Nix equality among derivations is implemented by comparing .outPath.

ghost

LGTM

See also:

#213096

... which will prevent problems like this in the future.

FRidh · 2023-01-28T17:13:30Z

I would love to know why that equality check is there.

To avoid mixing Python packages of different interpreter versions.

Applications don't provide a module and so we stop recursing. We also stop recursing when we encounter a Python module but it is not for the same interpreter. The idea here is that, when we would use this with NIX_PYTHONPATH, we can have different Python versions in the same build not affecting one another.

FRidh · 2023-01-28T17:19:37Z

We can fix this by using the same mechanism used to implement the meta checks themselves: lib.extendDerivation. Now, attempting to access drvPath or outPath on an attribute that fails the validity check will produce the same error as before, but other accesses will be allowed through, fixing splicing.

Interesting trick. Could this also fix overriding a package that has disabled = true in pkgs/development/interpreters/python/mk-python-derivation.nix?

alyssais · 2023-01-28T17:31:31Z

Interesting trick. Could this also fix overriding a package that has disabled = true in pkgs/development/interpreters/python/mk-python-derivation.nix?

Yes, I think it could!

alyssais requested review from Ericson2314 and sternenseemann January 26, 2023 20:28

alyssais requested a review from FRidh as a code owner January 26, 2023 20:28

github-actions bot added the 6.topic: python Python is a high-level, general-purpose programming language. label Jan 26, 2023

alyssais added the 6.topic: cross-compilation Building packages on a different platform than they will be used on label Jan 26, 2023

alyssais changed the title ~~Python splicing~~ python3.pkgs: fix splice through unsupported hosts Jan 26, 2023

alyssais force-pushed the python-splicing branch from 7fa9326 to d3e9e24 Compare January 26, 2023 20:31

ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. labels Jan 26, 2023

alyssais force-pushed the python-splicing branch from d3e9e24 to 09aabe2 Compare January 26, 2023 21:12

alyssais added 2 commits January 26, 2023 21:12

python3.pkgs: simplify ensurePythonModules logic

9fb1491

It wasn't clear before, but it turns out that this is just four separate conditions, and an attribute is valid if any of the conditions is true, so we don't need a sea of parentheses to express it.

alyssais force-pushed the python-splicing branch from 09aabe2 to 47ac733 Compare January 26, 2023 21:12

ghost mentioned this pull request Jan 28, 2023

make-derivation.nix: allow equality-test of meta-check-failing derivations #213096

Closed

4 tasks

ghost approved these changes Jan 28, 2023

View reviewed changes

FRidh approved these changes Jan 28, 2023

View reviewed changes

alyssais merged commit b682fef into NixOS:master Jan 28, 2023

alyssais deleted the python-splicing branch January 28, 2023 21:09

alyssais mentioned this pull request Feb 2, 2023

buildPython*: allow overriding disabled packages #214189

Merged

13 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

python3.pkgs: fix splice through unsupported hosts #212832

python3.pkgs: fix splice through unsupported hosts #212832
alyssais merged 2 commits intoNixOS:masterfrom
alyssais:python-splicing

alyssais commented Jan 26, 2023

Uh oh!

ghost commented Jan 28, 2023 •

edited by ghost

Loading

Uh oh!

ghost left a comment •

edited by ghost

Loading

Uh oh!

FRidh commented Jan 28, 2023

Uh oh!

FRidh commented Jan 28, 2023

Uh oh!

alyssais commented Jan 28, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

alyssais commented Jan 26, 2023

Description of changes

Things done

Uh oh!

ghost commented Jan 28, 2023 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ghost left a comment • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

FRidh commented Jan 28, 2023

Uh oh!

FRidh commented Jan 28, 2023

Uh oh!

alyssais commented Jan 28, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ghost commented Jan 28, 2023 •

edited by ghost

Loading

ghost left a comment •

edited by ghost

Loading