Skip to content

tests.overriding: buildPythonPackage: test stdenv customisation via buildPythonPackage.override#455462

Merged
MattSturgeon merged 3 commits intoNixOS:masterfrom
ShamrockLee:python-function-override-test
Dec 10, 2025
Merged

tests.overriding: buildPythonPackage: test stdenv customisation via buildPythonPackage.override#455462
MattSturgeon merged 3 commits intoNixOS:masterfrom
ShamrockLee:python-function-override-test

Conversation

@ShamrockLee
Copy link
Contributor

@ShamrockLee ShamrockLee commented Oct 25, 2025

This PR adds overriding tests for PR #271762

Things done

  • Built on platform:
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • Tested, as applicable:
  • Ran nixpkgs-review on this PR. See nixpkgs-review usage.
  • Tested basic functionality of all binary files, usually in ./result/bin/.
  • Nixpkgs Release Notes
    • Package update: when the change is major or breaking.
  • NixOS Release Notes
    • Module addition: when adding a new NixOS module.
    • Module update: when the change is significant.
  • Fits CONTRIBUTING.md, pkgs/README.md, maintainers/README.md and other READMEs.

Add a 👍 reaction to pull requests you find important.

@ShamrockLee ShamrockLee changed the title buildPythonPackage tests.overriding: buildPythonPackage: test stdenv customisation via buildPythonPackage.override Oct 25, 2025
@ShamrockLee ShamrockLee marked this pull request as ready for review October 25, 2025 06:04
@nixpkgs-ci nixpkgs-ci bot added 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. 9.needs: reviewer This PR currently has no reviewers requested and needs attention. labels Oct 25, 2025
@nixpkgs-ci nixpkgs-ci bot removed the 9.needs: reviewer This PR currently has no reviewers requested and needs attention. label Oct 25, 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.

Thanks for adding tests!

It'd be good if we also had tests covering the deprecated way of overriding stdenv (to avoid it breaking prematurely).

Now that you've added tests, are we ready to backport (minus release notes)?

@nixpkgs-ci nixpkgs-ci bot added the 12.approvals: 1 This PR was reviewed and approved by one person. label Oct 26, 2025
@MattSturgeon
Copy link
Contributor

MattSturgeon commented Oct 26, 2025

Although I've approved, I would ideally like to see these changes:

  1. Use kebab-case names to for test cases
  2. Simplify the test-case exprs
  3. Add coverage for deprecated methods of overriding stdenv
diff --git a/pkgs/test/overriding.nix b/pkgs/test/overriding.nix
index a5acddde3fef..0dc85634eae4 100644
--- a/pkgs/test/overriding.nix
+++ b/pkgs/test/overriding.nix
@@ -300,14 +300,18 @@ let
         {
           buildPythonPackage,
           emptyDirectory,
+          extraAttrs,
         }:
-        buildPythonPackage {
-          pname = "package-stub";
-          version = "0.1.0";
-          pyproject = true;
-          src = emptyDirectory;
-        }
-      ) { };
+        buildPythonPackage (
+          {
+            pname = "package-stub";
+            version = "0.1.0";
+            pyproject = true;
+            src = emptyDirectory;
+          }
+          // extraAttrs
+        )
+      ) { extraAttrs = { }; };
 
       package-stub-gcc = package-stub.override (previousArgs: {
         buildPythonPackage = previousArgs.buildPythonPackage.override {
@@ -332,16 +336,36 @@ let
         });
     in
     {
-      buildPythonPackageOverrideStdenvGCC = {
-        expr = package-stub-gcc.stdenv.cc.cc.pname == pkgs.gccStdenv.cc.cc.pname;
+      buildPythonPackage-override-gccStdenv = {
+        expr = package-stub-gcc.stdenv == pkgs.gccStdenv;
         expected = true;
       };
-      buildPythonPackageOverrideStdenvClang = {
-        expr = package-stub-clang.stdenv.cc.cc.pname == pkgs.clangStdenv.cc.cc.pname;
+      buildPythonPackage-override-clangStdenv = {
+        expr = package-stub-clang.stdenv == pkgs.clangStdenv;
         expected = true;
       };
-      buildPythonPackageOverrideStdenvLibCXX = {
-        expr = package-stub-libcxx.stdenv.cc.libcxx.pname == pkgs.libcxxStdenv.cc.libcxx.pname;
+      buildPythonPackage-override-libcxxStdenv = {
+        expr = package-stub-libcxx.stdenv == pkgs.libcxxStdenv;
+        expected = true;
+      };
+      buildPythonPackage-override-clangStdenv-deprecated = {
+        expr =
+          let
+            p = package-stub.override {
+              extraAttrs.stdenv = pkgs.clangStdenv;
+            };
+          in
+          p.stdenv == pkgs.clangStdenv;
+        expected = true;
+      };
+      overridePythonAttrs-override-clangStdenv-deprecated = {
+        expr =
+          let
+            p = package-stub.overridePythonAttrs {
+              stdenv = pkgs.clangStdenv;
+            };
+          in
+          p.stdenv == pkgs.clangStdenv;
         expected = true;
       };
 

I also tried adding this test case:

overridePythonAttrs-override-clangStdenv-deprecated-nested = {
  expr =
    let
      p = package-stub-gcc.overridePythonAttrs {
        stdenv = pkgs.clangStdenv;
      };
    in
    p.stdenv == pkgs.clangStdenv;
  expected = true;
};

However, it fails because of:

Intentionally drop the effect of overrideStdenvCompat when calling buildPython*.override.

In a separate PR, we'll need to find a better way to handle that scenario.

@ShamrockLee ShamrockLee force-pushed the python-function-override-test branch 3 times, most recently from 34f3742 to c994993 Compare December 10, 2025 19:39
@ShamrockLee ShamrockLee force-pushed the python-function-override-test branch from c994993 to 7ac8eda Compare December 10, 2025 19:42
@ShamrockLee
Copy link
Contributor Author

Although I've approved, I would ideally like to see these changes:

1. Use kebab-case names to for test cases

2. Simplify the test-case `expr`s

3. Add coverage for deprecated methods of overriding `stdenv`

Addressed. Thanks for your suggestions!

@ShamrockLee ShamrockLee force-pushed the python-function-override-test branch from 7ac8eda to 06711e4 Compare December 10, 2025 19:55
@MattSturgeon MattSturgeon added this pull request to the merge queue Dec 10, 2025
Merged via the queue into NixOS:master with commit 1947952 Dec 10, 2025
27 of 30 checks passed
@ShamrockLee ShamrockLee added the backport release-25.11 Backport PR automatically label Dec 12, 2025
@nixpkgs-ci

This comment has been minimized.

@ShamrockLee ShamrockLee removed the backport release-25.11 Backport PR automatically label Dec 12, 2025
@ShamrockLee
Copy link
Contributor Author

ShamrockLee commented Dec 12, 2025

PR #470091 that backports PR #267296 needs to go first before backporting this PR.

@ShamrockLee ShamrockLee added the backport release-25.11 Backport PR automatically label Dec 12, 2025
@nixpkgs-ci
Copy link
Contributor

nixpkgs-ci bot commented Dec 12, 2025

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

Labels

8.has: port to stable This PR already has a backport to the stable release. 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. 12.approvals: 1 This PR was reviewed and approved by one person. backport release-25.11 Backport PR automatically

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants