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.

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 packageOverrides improvements #87388

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Python packageOverrides improvements #87388

wants to merge 2 commits into from

Conversation

alyssais
Copy link
Member

@alyssais alyssais commented May 9, 2020

Between these two changes, complex uses of packageOverrides become a lot more tolerable (even if they’re still not pleasant). Explanations in commit messages.

@alyssais
python3.pkgs: apply packageOverrides recursively
4899a45 
This makes it so that if I apply a patch to Django using
packageOverrides, it's applied to python3.pkgs.python.pkgs.django
(and python3.pkgs.python.pkgs.python.pkgs.django) as well as
python3.pkgs.django.
@alyssais
python3: expose packageOverrides in passthru
a1c0d1d 
This allows (manually) composing packageOverrides, like so:

    python3.override {
      packageOverrides =
        lib.composeExtensions python3.packageOverrides (final: super: { ... });
    }

This is still not great, but without this, you can't use
packageOverrides twice at all, because the second use just clobbers
the first with no way to make sure the first override is applied as
well.
@FRidh
Copy link
Member

FRidh commented May 9, 2020

For python3.pkgs: apply packageOverrides recursively, this is why it is typically also needed to pass self = python; if I recall correctly.

Regarding python3: expose packageOverrides in passthru , see also #67422. Another idea was to offer the contents of python-packages.nix as a top-level "overlay" so what you want to do here could apply also to the other Python sets.

@alyssais
Copy link
Member Author

alyssais commented May 9, 2020 via email

@FRidh
Copy link
Member

FRidh commented May 9, 2020

Does my change then remove the need to do that?

I don't think so. Consider you override both the interpreter build, and a package in the package set. In that case you really still need to pass self = python; to make it use that Python. Thus, your change should not be needed then.

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

Lines 20 to 23 in a68a408

pythonPackages = callPackage ../../../top-level/python-packages.nix {
python = self;
overrides = packageOverrides;
};

This is why I regret introducing the passthru to Python instead of just adding everything to pythonPackages like it's done for Haskell. It's ugly but way more practical.

Exposing packageOverrides is a nice simple change that makes multiple
overrides possible, if not pretty. It doesn't stop us doing it better
later, but I don't have the capacity to do that now.

I've said before this probably should go via an RFC but frankly I don't see that happening. This change is simple and very useful so IMO its good.

@FRidh
Copy link
Member

FRidh commented Jul 2, 2020

Alternative at #91850.

@stale
Copy link

stale bot commented Dec 29, 2020

I marked this as stale due to inactivity. → More info

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Dec 29, 2020
@RaitoBezarius
Copy link
Member

Still very interested into this PR, composing packageOverrides is still not obvious to me without the passthru solution.

@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jan 8, 2021
@fricklerhandwerk
Copy link
Contributor

This is a super valuable quick fix for the problem of not being able to compose Python overrides.
@FRidh @jonringer Anything in the way of merging this except for the conflict? @alyssais Could you please rebase?

@jonringer
Copy link
Contributor

personally, I'm of the opinion that we should use the lib.makeScope helper.

cc @FRidh

@FRidh FRidh added this to the 21.05 milestone May 22, 2021
@FRidh FRidh self-assigned this May 22, 2021
@stale
Copy link

stale bot commented Jan 9, 2022

I marked this as stale due to inactivity. → More info

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jan 9, 2022
@Artturin Artturin modified the milestones: 21.05, 23.05 Dec 31, 2022
@stale stale bot removed 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md labels Dec 31, 2022
@RaitoBezarius
Copy link
Member

Hello again, I would like to call on a decision for this.

AFAIK, composing packageOverrides is still a hard thing, see #223268 for latest issue with it.

If the makeScope helper is the way to go, could we document it and opt to close this PR (and document the idea of not going for this solution or explain why this is not necessarily a good thing)?

@fricklerhandwerk
Copy link
Contributor

@RaitoBezarius strongly in favor of documenting the result. @NixOS/documentation-team can assist with making it nice. Who else should we involve in making the technical decision? @DavHau opinions?

@RaitoBezarius RaitoBezarius modified the milestones: 23.05, 23.11 May 31, 2023
@lf- lf- added the significant Novel ideas, large API changes, notable refactorings, issues with RFC potential, etc. label Mar 9, 2024
@wegank wegank added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Mar 19, 2024
@wegank wegank marked this pull request as draft March 20, 2024 15:40
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 FRidh is a code owner

@jonringer jonringer Awaiting requested review from jonringer

Assignees

@FRidh FRidh

Labels
2.status: merge conflict 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md 6.topic: python 10.rebuild-darwin: 0 10.rebuild-linux: 0 significant Novel ideas, large API changes, notable refactorings, issues with RFC potential, etc.
Projects
None yet
Milestone
23.11
Development

Successfully merging this pull request may close these issues.

None yet
9 participants
@alyssais @FRidh @RaitoBezarius @fricklerhandwerk @jonringer @lf- @SuperSandro2000 @wegank @Artturin