Skip to content
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.

Sign up for GitHub

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

Jump to bottom

python: Propagate packageOverrides to pythonForBuild #105113

Merged
merged 1 commit into from Nov 30, 2020
Merged

python: Propagate packageOverrides to pythonForBuild #105113

merged 1 commit into from Nov 30, 2020

Conversation

adisbladis
Copy link
Member

@adisbladis adisbladis commented Nov 27, 2020 •
edited

When overriding versions of build tools injected via hooks by pythonForBuild
packageOverrides was not taken into account and 2 incompatible
versions of the same package (wheel in this case) ended up in the
closure, causing the builds to fail.

This was broken by b57c5d4.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@FRidh FRidh self-assigned this Nov 27, 2020
@FRidh
Copy link
Member

FRidh commented Nov 27, 2020

As follow-up of #104201 I think it would be good if we get rid of passing around pythonForBuild explicitly but instead have pythonPackagesBuildHost, for uniformity. As far as I can see that would solve this issue.

@FRidh FRidh mentioned this pull request Nov 27, 2020
Merged
10 tasks
FRidh added a commit to FRidh/nixpkgs that referenced this pull request Nov 28, 2020
@FRidh
Python: use pythonPackagesBuildHost instead of pythonForBuild
cce2fd5 
Follow-up to NixOS#104201, related to NixOS#105113.
@adisbladis
python: Propagate packageOverrides to pythonForBuild
49b8738 
When overriding versions of build tools injected via hooks
`packageOverrides` was not taken into account and 2 incompatible
versions of the same package (`wheel` in this case) ended up in the
closure, causing the builds to fail.
@adisbladis adisbladis merged commit a52850e into NixOS:master Nov 30, 2020
@FRidh
Copy link
Member

FRidh commented Nov 30, 2020

Have you tested again without this change on master?

@adisbladis
Copy link
Member Author

@FRidh Yes I have.

@FRidh
Copy link
Member

FRidh commented Nov 30, 2020

Alright. It would be nice to communicate that before merging this yourself.

.overrideScope' still led to the same issue?

@Ericson2314 seems like, while splicing is now fixed, overrides are not yet taken into account.

We'll need to make a test for this feature.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@FRidh FRidh Awaiting requested review from FRidh

@Ericson2314 Ericson2314 Awaiting requested review from Ericson2314

Assignees

@FRidh FRidh

Labels
6.topic: python 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@adisbladis @FRidh