python3Packages: fix Python overriding to make <pkg>.overridePythonAttrs work after <pkg>.overrideAttrs

[ShamrockLee]

Description of changes

This patch updates the <pkg>.overrideAttrs inside makeOverridablePythonPackage (the function that adds <pkg>.overridePythonAttrs). If applied, <pkgs>.overridePythonAttrs will work after applying <pkg>.overrideAttrs to override a Python package built with buildPythonPackage or buildPythonApplication.

This patch also adds corresponding test cases to tests.overriding.

Closes #267293.

Initial effort for #271387.

Cc: @FRidh

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:
  • NixOS test(s) (look inside nixos/tests)
  • and/or package tests
  • or, for functions and "core" functionality, tests in lib/tests or pkgs/test
  • made sure NixOS tests are linked to the relevant packages
• 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/)
• 23.11 Release Notes (or backporting 23.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.

[ShamrockLee]

@ofborg build tests.overriding

[SuperSandro2000]

Don't we need () here or are the redundant?

[ShamrockLee]

I don't quite get it.

result?overrideAttrs would be true only when result is an attribute. Maybe it would be more readable this way:

if builtins.isAttrs result then
  result // lib.optionalAttrs (result?overrideAttrs) {
    overrideAttrs =...;
  }
else if ...

If you are talking about the let-in construct above the if-then-else construct, it surely doesn't need parenthesis.

nix-repl> let in if 2 + 2 == 5 then "y" else "n"
"n"

[ShamrockLee]

Don't we need () here or are the redundant?

Or do you mean that set?attr has higher precedence over function application and the parenthesis around it is redundant?

[ShamrockLee]

I rebased the feature branch and resolved the merge conflict.