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

buildPython*: support fixed-point attributes #271387

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

Conversation

ShamrockLee
Copy link
Contributor

@ShamrockLee ShamrockLee commented Dec 1, 2023

Description of changes

This PR enables the fixed-point arguments support in buildPythonPackage and buildPythonApplication for Python 2 and 3 with slight modification against mk-python-derivation.nix with no rebuilds.

Prerequisites:

I will rebase this feature branch once the dependent PRs merge. The diff will be greatly shortened then.

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/)
  • 24.05 Release Notes (or backporting 23.05 and 23.11 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.

Priorities

Add a 👍 reaction to pull requests you find important.

@github-actions github-actions bot added 6.topic: python 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: documentation This PR adds or changes documentation 8.has: changelog 6.topic: lib The Nixpkgs function library labels Dec 1, 2023
@ShamrockLee ShamrockLee mentioned this pull request Dec 1, 2023
12 tasks
@ShamrockLee ShamrockLee changed the title buildPythonPackages, buildPythonApplication: support function-based attribute recursion buildPythonPackages, buildPythonApplication: support fixed-point attributes Dec 1, 2023
@ofborg ofborg bot requested a review from LunNova December 1, 2023 16:02
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 labels Dec 1, 2023
@LunNova
Copy link
Member

LunNova commented Dec 1, 2023

input-remapper part looks good, you'll probably get a merge conflict if #249777 gets merged first

@ShamrockLee
Copy link
Contributor Author

ShamrockLee commented Dec 1, 2023

input-remapper part looks good

Thank you for reviewing.

you'll probably get a merge conflict if #249777 gets merged first

That one is easy to resolve. No worries.

@SuperSandro2000
Copy link
Member

Python 2

Is this automatically done because the wrapper are shared or required extra codes changes? If it is the latter I am inclined to not add it to stronger underline the EOL status of python 2.

@@ -26,6 +26,23 @@
, eggInstallHook
}:

let
Copy link
Member

Choose a reason for hiding this comment

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

I am not sure about this but we could we just not add support for python 2 to move it closer to removal?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's still relevant because it is used by pypy2.pkgs.buildPython*. and that PyPy says that they will continue to support PyPy 2, as RPython (the language they use to implement the PyPy interpreter) is essentially a Python 2 subset.

Comment on lines 14 to 22
inherit (finalAttrs) version;
hash = "sha256-vbBu/dMXQf14F7qWvyHX5T8/AkjeZhaQt1eQ6Nidpsc=";
Copy link
Member

Choose a reason for hiding this comment

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

I don't like this pattern, when only changing version it silently makes the FOD none reproducible.

Copy link
Member

Choose a reason for hiding this comment

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

Looks like fetchpypi doesn't let you pass in anything that sends up in the source derivation's name

inherit url sha256 hash;
so can't fix it that way

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In xrootd.passthru.tests.test-xrdcp, I pass a version to the FOD that consists of <xrootd version from finalAttrs>-<xrootd drv hash from finalAttrs> to make it rebuilds everytime xrootd gets rebuild.

The version trick might also be suitable for source FODs. However, for vendoring FODs, it could be a waste of time and space to rebuild when the vendorHash remains the same.

The ultimate solution might be a mechanism to generate automatic passthru.tests for all the FOD passed into stdenv.mkDerivaiton. Automatically-generated passthru.tests test cases could also be a way to run ShellCheck against build commands (#21166).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In the case of fetchPypi, the outPath changes when version changes. The FOD package will be rebuild even when hash isn't changed.

@ShamrockLee ShamrockLee changed the title buildPythonPackages, buildPythonApplication: support fixed-point attributes buildPython*: support fixed-point attributes Dec 3, 2023
@ShamrockLee ShamrockLee force-pushed the python-fixed-point-args branch 3 times, most recently from eb79c07 to e942382 Compare December 4, 2023 01:00
@ShamrockLee ShamrockLee mentioned this pull request Dec 4, 2023
12 tasks
@ShamrockLee ShamrockLee force-pushed the python-fixed-point-args branch 2 times, most recently from 913d18b to 523affa Compare December 4, 2023 07:20
@t4ccer t4ccer mentioned this pull request Mar 18, 2024
13 tasks
@wegank wegank added the 2.status: merge conflict This PR has merge conflicts with the target branch label Mar 20, 2024
@wegank wegank added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jul 4, 2024
ShamrockLee and others added 12 commits January 23, 2025 00:05
Restructure the derivation to prepare for
- fixed-point arguments support
- overridePythonAttrs deprecation
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`.
Test with pip, which is less likely to fail.

Modularize the transforming function and testing function.

Co-authored-by: Sandro <[email protected]>
Make it possible to mix overridePythonAttrs and overrideAttrs, i.e.
((<pkg>.overrideAttrs (_: { foo = "a"; })).overridePythonAttrs (_: { })).foo now works
@ShamrockLee ShamrockLee force-pushed the python-fixed-point-args branch from 523affa to bfc715d Compare January 24, 2025 11:03
@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jan 24, 2025
@github-actions github-actions bot removed 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: changelog 6.topic: lib The Nixpkgs function library labels Jan 24, 2025
@ofborg ofborg bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Jan 24, 2025
@ShamrockLee ShamrockLee force-pushed the python-fixed-point-args branch from bfc715d to b91155b Compare January 24, 2025 11:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants