-
-
Notifications
You must be signed in to change notification settings - Fork 14.6k
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
python3.pkgs.buildPython*: allow overriding of the stdenv #173411
Conversation
I wish the compiler would go into |
I wonder how well this works with sysconfigdata. |
Actually it is maybe better to pass in |
Try it! If it seems to work we can merge this. If this solution turns out not to be ideal we can always follow-up with another solution. |
Following up on this, @SomeoneSerge @FRidh any ideas on whether this would break (or otherwise worsen) cross-compilation, or whether that's easily tested? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not entirely sure what "breaking" cross would mean in this context.
I rebased this PR on 12bdeb0 and then ran some cross builds of the Coverage Python package, which includes a C extension (if that's the correct terminology). (Numpy was too big for some quick tests. It requires a cross GFortran, which is not cached.) This is on an x86_64-linux.
Cross without overriding stdenv
pkgsCross.aarch64-multiplatform.python3Packages.coverage
:
$ file -b /nix/store/k5cysvrkgfcpif713a6xa2rx1d5pwyi1-python3.10-coverage-7.2.1-aarch64-unknown-linux-gnu/lib/python3.10/site-packages/coverage/tracer.cpython-310-aarch64-linux-gnu.so
ELF 64-bit LSB shared object, ARM aarch64, version 1 (SYSV), dynamically linked, not stripped
Cross overriding stdenv to use Clang
pkgsCross.aarch64-multiplatform.python3Packages.coverage.overridePythonAttrs (_: { stdenv = pkgsCross.aarch64-multiplatform.clangStdenv; })
:
$ file -b /nix/store/4n6rzmdsps2587sip62wnfjchaaxw15g-python3.10-coverage-7.2.1-aarch64-unknown-linux-gnu/lib/python3.10/site-packages/coverage/tracer.cpython-310-aarch64-linux-gnu.so
ELF 64-bit LSB shared object, ARM aarch64, version 1 (SYSV), dynamically linked, not stripped
Cross overriding with a non-cross stdenv
This is the only scenario in which you could claim cross is "broken."
By explicitly passing a non-cross stdenv, which is then indiscriminately used to compile C bits, the resulting objects are indeed non-cross-compiled.
I do not agree this should be considered broken, but, it might catch someone by surprise at first.
pkgsCross.aarch64-multiplatform.python3Packages.coverage.overridePythonAttrs (_: { stdenv = clangStdenv; })
:
$ file -b /nix/store/m2pm702mnmjakhqhdp60cn0p49p43rdl-python3.10-coverage-7.2.1/lib/python3.10/site-packages/coverage/tracer.cpython-310-aarch64-linux-gnu.so
ELF 64-bit LSB shared object, x86-64, version 1 (SYSV), dynamically linked, not stripped
P.S.: Fairly eager to get this merged because it allows us to fix pybind11 when building with Clang 16. I'm not sure how often master is merged to staging during a release window so might this require targeting at staging instead?
Is indeed what I wanted to have verified. Thanks for checking. |
With this change it is possible to pass in `stdenv` directly to `buildPython*` or override it using e.g. ``` numpy.overridePythonAttrs(_: { stdenv = clangStdenv; }) ```
b8510c1
to
7a0d080
Compare
With this change it is possible to pass in
stdenv
directly tobuildPython*
or override it using e.g.Description of changes
Things done
sandbox = true
set innix.conf
? (See Nix manual)nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD"
. Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/
)nixos/doc/manual/md-to-db.sh
to update generated release notes