Skip to content

buildPython*: bring back buildPython*.override#366593

Merged
ShamrockLee merged 1 commit intoNixOS:masterfrom
ShamrockLee:build-python-function-override
Oct 22, 2025
Merged

buildPython*: bring back buildPython*.override#366593
ShamrockLee merged 1 commit intoNixOS:masterfrom
ShamrockLee:build-python-function-override

Conversation

@ShamrockLee
Copy link
Contributor

@ShamrockLee ShamrockLee commented Dec 19, 2024

Like packages, build helpers returned by callPackage have their own override that can produce a customized version of them with specific dependencies overridden. However, the current implementation of makeOverridablePythonPackage (the function that adds .overridePythonAttrs to the packages buildPython* returns) doesn't consider such <function>.override attribute, causing buildPython*.override to get lost.

This PR fixes makeOverridablePythonPackage and unshadows buildPython*.override, making it possible to override the dependencies of buildPython*. E.g., buildPythonPackage.override { unzip = unzip-custom; } returns a derived version of buildPythonPackage with the unzip package overridden with unzip-custom. Such overriding approaches can already be found in Nixpkgs. -- the implementation of fetchFromGitHub uses fetchzip.override { withUnzip = false; } to obtain a custom version of fetchzip without unzip-related dependencies inside its returning derivation.

This is a much simplified and less controversial version of #271762.

This PR causes no rebuild and is fully backward compatible.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 25.05 Release Notes (or backporting 24.11 and 25.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@github-actions github-actions bot added the 6.topic: python Python is a high-level, general-purpose programming language. label Dec 19, 2024
@ShamrockLee
Copy link
Contributor Author

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review


x86_64-linux

@ShamrockLee ShamrockLee marked this pull request as ready for review December 19, 2024 17:00
@nix-owners nix-owners bot requested review from mweinelt and natsukium December 19, 2024 17:01
@github-actions github-actions 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 Dec 19, 2024
@ShamrockLee ShamrockLee force-pushed the build-python-function-override branch from e7aa58a to c8b0f1d Compare January 22, 2025 18:15
@ShamrockLee
Copy link
Contributor Author

Rebased the feature branch on top of master.

@ShamrockLee ShamrockLee force-pushed the build-python-function-override branch from c8b0f1d to 442514d Compare February 16, 2025 22:00
@ShamrockLee
Copy link
Contributor Author

Bump onto master.

@ShamrockLee
Copy link
Contributor Author

Bump.

@ShamrockLee ShamrockLee added 1.severity: blocker This is preventing another PR or issue from being completed backport release-25.05 labels Jul 8, 2025
@nixpkgs-ci nixpkgs-ci bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Oct 2, 2025
Copy link
Contributor

@MattSturgeon MattSturgeon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is useful and will unblock #271762 (which itself is blocking fixed-point args in buildPythonPackage and buildPythonApplication).

I've tested locally in the repl that buildPythonPackage.override behaves as expected.

As this is such a trivial patch - and no one has complained in the 10 months it's been open - I plan to merge soon, assuming there is no further feedback. Or I support a self-merge by @ShamrockLee if they prefer.

@nixpkgs-ci nixpkgs-ci bot added 12.approvals: 2 This PR was reviewed and approved by two persons. and removed 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md labels Oct 22, 2025
Fix `makeOverridablePythonPackage` in python-package-base.nix
and unshadow `buildPython*.override`.

This makes it possible to override the dependencies of buildPython*.
E.g., `buildPythonPackage.override { unzip = unzip-custom; }`
returns a derived version of `buildPythonPackage` with
the `unzip` package overridden with `unzip-custom`.
@ShamrockLee ShamrockLee force-pushed the build-python-function-override branch from 0d01e8b to cce0f3f Compare October 22, 2025 19:08
@ShamrockLee
Copy link
Contributor Author

ShamrockLee commented Oct 22, 2025

The Linting GitHub action complains error: interrupted by the user. What happened?

Update: Cleared after rerunning the failed test and dependent tests.

@ShamrockLee ShamrockLee added this pull request to the merge queue Oct 22, 2025
Merged via the queue into NixOS:master with commit 7a4ad37 Oct 22, 2025
45 of 50 checks passed
@ShamrockLee ShamrockLee deleted the build-python-function-override branch October 22, 2025 19:23
@nixpkgs-ci
Copy link
Contributor

nixpkgs-ci bot commented Oct 22, 2025

Successfully created backport PR for release-25.05:

@github-actions github-actions bot added the 8.has: port to stable This PR already has a backport to the stable release. label Oct 22, 2025
@MattSturgeon
Copy link
Contributor

MattSturgeon commented Oct 22, 2025

Adding the backport label back; I think it's important that the python build helpers remain in sync across all supported releases. Otherwise, we can't backport changes to any package that depends on a buildPython* feature that hasn't itself been backported.

@sternenseemann
Copy link
Member

This change, together with another change only present on staging, causes evaluation of pkgsCross.ghcjs.buildPackages.jre to fail. This is quite important since it breaks evaluation of GHC in that package set which is the whole point of pkgsCross.ghcjs. The strictness introduced by the // appears to be the culprint. I've proposed a fix at #455362 (though I can't guarantee a similar issue won't crop up again).

@MattSturgeon
Copy link
Contributor

This change, together with another change only present on staging

Have you bisected the staging change?

@sternenseemann
Copy link
Member

No, otherwise I'd have noted it here. Others have claimed to be able to reproduce the issue on master as well, though I was not able to replicate that since I do not know a specific revision.

@alexfmpe
Copy link
Member

alexfmpe commented Oct 24, 2025

I bisected using nix-instantiate -A pkgsCross.ghcjs.haskell.packages.ghc912.hello or its negation as appropriate

I do not know a specific revision.

7a4ad371b14f245875a9364947c6ac4ea8384fa1 : this PR's merge commit introduces the issue on master
2fed9f37b5a2be02df46b246b95936f05ce6df2d: parent from master - does not have the issue
cce0f3f3006853f9182c893df2e3b04a262aece2: parent from the PR - has the issue

However, the very first commit of #271762, which this PR unblocked, fixes the eval issue again and was merged soon after.

This change, together with another change only present on staging,

I don't think staging is relevant ?

@ShamrockLee
Copy link
Contributor Author

Sorry for causing the trouble.

However, the very first commit of #271762, which this PR unblocked, fixes the eval issue again and was merged soon after.

The first commit of PR #271762 moves overridePythonAttrs out of the passthru argument of the (callPackage mk-python-derivation.nix { }) build helper and attach with attrset update instead.

I.e.,

Before:

nonoverridableBuildPythonPackage {
  passthru = {
    overridePythonAttrs = <the overrider>;
  };
}

After:

nonoverridableBuildPythonPackage {
  passthru = { };
}
// {
  overridePythonAttrs = <the overrider>;
}

@ShamrockLee
Copy link
Contributor Author

Could we construct a simplified reproducer for the evaluation error based on the merged commit of this PR? This would be a great opportunity to add an overriding test.

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

Labels

1.severity: blocker This is preventing another PR or issue from being completed 6.topic: python Python is a high-level, general-purpose programming language. 8.has: port to stable This PR already has a backport to the stable release. 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. 12.approvals: 2 This PR was reviewed and approved by two persons.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants