Skip to content

python3Minimal: remove dependency on pkg-config + add withMinimal flag#417581

Merged
mweinelt merged 2 commits intoNixOS:python-updatesfrom
DavHau:python
Aug 5, 2025
Merged

python3Minimal: remove dependency on pkg-config + add withMinimal flag#417581
mweinelt merged 2 commits intoNixOS:python-updatesfrom
DavHau:python

Conversation

@DavHau
Copy link
Member

@DavHau DavHau commented Jun 17, 2025

Best reviewed via individual commits.
The first commit is a pure refactoring commit which introduces a withMinimal flag for cpython.

The second commit removes the dependency on pkg-config if withMinimal is enabled.

This change is driven by the motivation to use python3 earlier in stdenv for hooks.

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/)
  • Nixpkgs 25.11 Release Notes (or backporting 24.11 and 25.05 Nixpkgs Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
  • NixOS 25.11 Release Notes (or backporting 24.11 and 25.05 NixOS Release notes)
    • (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, pkgs/README.md, maintainers/README.md and other contributing documentation in corresponding paths.

Add a 👍 reaction to pull requests you find important.

@nix-owners nix-owners bot requested review from mweinelt and natsukium June 17, 2025 15:23
@github-actions github-actions bot added 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 5001+ This PR causes many rebuilds on Linux and must target the staging branches. 6.topic: python Python is a high-level, general-purpose programming language. labels Jun 17, 2025
@DavHau DavHau force-pushed the python branch 2 times, most recently from e7ed88f to 0701532 Compare June 17, 2025 15:50
@github-actions github-actions bot added 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 5001+ This PR causes many rebuilds on Darwin and must target the staging branches. and removed 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. labels Jun 17, 2025
@DavHau
Copy link
Member Author

DavHau commented Jun 27, 2025

@mweinelt Does this change make sense?

@mweinelt
Copy link
Member

mweinelt commented Jun 27, 2025

Yes, I was just annoyed by the withMinimal name, but couldn't find a moment to pinpoint the reason.

It should be whenMinimal, so "make this decision when we are building minimal". Or minimalFlavor.

@emilazy
Copy link
Member

emilazy commented Jun 27, 2025

Perhaps it would be a good idea to avoid adding a bunch of independent withFoo flags here that are unlikely to see much use outside of the minimal variant? It might be possible to cut down on the existing set too, and just condition on the minimal flag directly.

(FWIW it looks like we have a few instances of enableMinimal and minimal, and one each of withMinimal and isMinimalBuild. whenMinimal looks like a typo to me personally.)

Comment on lines 212 to 215
Copy link
Member

@emilazy emilazy Jun 27, 2025

Choose a reason for hiding this comment

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

Have you checked python3Minimal still builds on Darwin with this change? (I can test it if not.)

Edit: Er, sorry, mentally inverted the conditional.

@mweinelt
Copy link
Member

mweinelt commented Jul 2, 2025

Need to make a decision soon to hit the next staging cycle.

@DavHau
Copy link
Member Author

DavHau commented Jul 8, 2025

Perhaps it would be a good idea to avoid adding a bunch of independent withFoo flags here that are unlikely to see much use

Are you sure? I think removing these flags would have the downside that overriding individual flags would not propagate as expected anymore.

For example, if I remove withLibxcrypt from the signature, the following statement further down in the expression

 LIBS = "${optionalString (!stdenv.hostPlatform.isDarwin && withLibxcrypt) "-lcrypt"}";

... would have to change to

 LIBS = "${optionalString (!stdenv.hostPlatform.isDarwin && libxcrypt != null && !withMinimal) "-lcrypt"}";

The consequence is that it would be harder to re-introduce libxcrypt via override.
Eg. python3Minimal.override {libxcrypt = pkgs.libxcryp} would break the build now, because -lcrypt is still not set.

Optimally the withMinimal flag would only be used as a meta attribute inside the function header to resolve to other attributes and not used elsewhere in the code.

Let me know if you still want me to remove these flags, and I'll do it.

Naming

How about naming it withMinimalFlavor? I think the fact that package function args are un-typed and un-documented is bad enough. If at least all the boolean flags have the same prefix I consider it a step forwards.

EDIT: Looking at ffmpeg which has several flavors, I think it makes sense to follow the naming there and call it withMinimalDeps

@mweinelt
Copy link
Member

mweinelt commented Jul 8, 2025

Looking at ffmpeg which has several flavors, I think it makes sense to follow the naming there and call it withMinimalDeps

Sounds good to me.

@emilazy
Copy link
Member

emilazy commented Jul 8, 2025

Are you sure? I think removing these flags would have the downside that overriding individual flags would not propagate as expected anymore.

For example, if I remove withLibxcrypt from the signature, the following statement further down in the expression

 LIBS = "${optionalString (!stdenv.hostPlatform.isDarwin && withLibxcrypt) "-lcrypt"}";

... would have to change to

 LIBS = "${optionalString (!stdenv.hostPlatform.isDarwin && libxcrypt != null && !withMinimal) "-lcrypt"}";

The consequence is that it would be harder to re-introduce libxcrypt via override. Eg. python3Minimal.override {libxcrypt = pkgs.libxcryp} would break the build now, because -lcrypt is still not set.

I’m not a Python maintainer, so take what I have to say with a grain of salt; it’s really up to @mweinelt :)

For me it’s about public API and what’s supported. Overriding dependencies manually to null or other things is clearly poking at internals somewhat and obviously might break; it’s something you do at your own risk. Offering an explicit feature flag is an invitation to use it and implies a maintenance and support burden that quickly becomes hard to satisfy because of the combinatorial explosion of flag combinations that can interact with each other to break things. So I personally prefer only adding feature flags when there is a strong justification for varying something independently and it feels like something that can and should be reliably supported going forward. (Relatedly, I dislike the fact that the callPackage/override interface conflates “implementation details” like how the package gets dependencies – even purely build‐time ones – and “exposed API” like feature flags. Even switching around the package set arguments you use to obtain the exact same package is technically a breaking change. I would prefer we had a cleaner separation between a user‐facing override API for intentionally exposed configuration, and something that handles monkey‐patching tasks like swapping out dependencies and overrideAttrs.)

It may be that there’s sufficient demand for “almost‐minimal Pythons” that having separate toggles makes sense. OTOH, in the Darwin bootstrap, we need more than python3Minimal for Meson and we just build a vanilla Python with all the features, and that works okay. So my personal inclination is to just support a binary toggle of minimal vs. vanilla, with the hope that if someone needs something in‐between they can send a PR and we can discuss the best options, and I wouldn’t have added the existing separate flags that go into the current state of python3Minimal. All that said, I have no standing here and fully defer to whatever you and @mweinelt think is best.

Thanks again for working on this. I’m eager to see less Bash. I wanted to use Python in #418819 to make it less slow but it wasn’t really an option since I needed to use it in the GCC package.

@nixpkgs-ci nixpkgs-ci bot added the 2.status: merge conflict This PR has merge conflicts with the target branch label Jul 24, 2025
@mweinelt
Copy link
Member

Can you migrate to withMinimalDeps?

DavHau added 2 commits August 5, 2025 04:26
simplifies the interface of building cpython minimally
Removes the build time dependency on pkg-config if python3 is build with the `withMinimal` flag enabled

This change is driven by the motivation to use python3 earlier in stdenv for hooks.
@mweinelt
Copy link
Member

mweinelt commented Aug 5, 2025

Wrt to what @emilazy pointed out … I don't think this is the venue to discuss the surface area of our public API.

@mweinelt mweinelt changed the base branch from staging to python-updates August 5, 2025 02:28
@nixpkgs-ci nixpkgs-ci bot closed this Aug 5, 2025
@nixpkgs-ci nixpkgs-ci bot reopened this Aug 5, 2025
@mweinelt mweinelt removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Aug 5, 2025
@mweinelt mweinelt merged commit 60c4a88 into NixOS:python-updates Aug 5, 2025
18 of 23 checks passed
@nixpkgs-ci nixpkgs-ci bot added 10.rebuild-linux-stdenv This PR causes stdenv to rebuild on Linux and must target a staging branch. 10.rebuild-darwin-stdenv This PR causes stdenv to rebuild on Darwin and must target a staging branch. labels Aug 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: python Python is a high-level, general-purpose programming language. 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 5001+ This PR causes many rebuilds on Darwin and must target the staging branches. 10.rebuild-darwin-stdenv This PR causes stdenv to rebuild on Darwin and must target a staging branch. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 5001+ This PR causes many rebuilds on Linux and must target the staging branches. 10.rebuild-linux-stdenv This PR causes stdenv to rebuild on Linux and must target a staging branch.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants