lib.extendMkDerivation: init

[ShamrockLee]

Description of changes

This PR introduces a unified approach to implement build helpers that support function-based attribute recursion and bring such support to existing build helpers.

The function-based attribute recursion support in stdenv.mkDerivation is introduced in #119942, and there are ongoing attempts to make other build helpers support such syntax (buildRustPackage refactor #194475, buildGoModule refactor #225051). The overlay-style overrideAttrs brought by the stdenv.mkDerivation change can be used to implement the functionality, which is adopted by the buildRustPackage refactor and previous version of buildGoModule refactor. The down side of such approach is that the whole set pattern matching the input arguments are degenerated into a single identifier, making it hard to see from the source code which attributes to the build helper accepts.

The new Nixpkgs Library function, lib.extendMkDerivation accepts a base build helper and the builder implementation -- an overlay in the form finalAttrs: inputAttrsPattern: resultAttrs.

The following is the definition of an example build helper, mkLocalDerivation:

{ lib
, python
}:

lib.extendMkDerivation (fpargs: ((fpargs { }).stdenv or python.stdenv).mkDerivation) (finalAttrs:

{ preferLocalBuild ? true
, allowSubstitute ? false
  # `stdenv` overriding
, stdenv ? python.stdenv
  # Some other input arguments
, ...
}@args:

removeAttrs [
  # attributes not to be passed into the builder
] args // {
  inherit preferLocalBuild allowSubstitute;
  # Some other build process
})

The (_: stdenv.mkDerivation) here is a function that specifies the base build helper to extend, from the unfixed input arguments. It is a constant function in most cases, but could be written otherwise to allow base build helper specification from the input arguments.

This PR also provide a variant function, lib.extendMkDerivationModified, that accept an extra parameter to modify the resulting derivation in case the attributes are not enough to include all the implementation.

Another plus to this approach is that we don't have to alter the indentation of an existing, working build helper to make it support recursive attributes. A subsequent PR #271387 enables the fixed-point arguments support in buildPythonPackage and buildPythonApplication with slight modification against mk-python-derivation.nix with no rebuilds.

This PR renames the following concepts to reduce ambigualty and avoid confusion:

• A function that produces a derivation is now said to be a "build helper" instead of a "builder", to distinguish from the builder input attribute of derivation or the remote builders of Nix.

• The use of a (finalAttrs: { })-style function instead of a plain attribute set as the input argument of another function and get the final state of an attribute from finalAttrs is now said to be "function-based attribute recursion" instead of "recursive attributes" to distinguish from the recursive set syntax (rec { }).

The path to existing documentation files are kept unchanged to avoid breaking other open PRs or staging changes.

Cc: Python maintainers: @FRidh @mweinelt @jonringer Author of the buildRustPackage refactor PR @amesgen Reviewer of the buildGoModule refactor PR @zowoq Author of the merged recursive stdenv.mkDerivation PR @roberth People who mention 119942 in Python application definition: @LunNova

Things done

• Built on platform(s)
  • [X] x86_64-linux
  • [ ] aarch64-linux
  • [ ] x86_64-darwin
  • [ ] aarch64-darwin
• [ ] For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
• [ ] 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
• [X] 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.05 Release Notes (or backporting 22.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
• [X] Fits CONTRIBUTING.md.

[ShamrockLee]

I should have checked more carefully.

There's a function called makeOverride that deprives the input builder of their recursive attributes support, and buildPythonPackage and buildPackageApplication happens to be makeOverrided. While the change keeps the current python packages unchanged, it still doesn't bring the recursive attributes support to buildPythonModule and buildPythonApplication unless we fix makeOverridable.

Updat: fixed, see below.

[ShamrockLee]

Fix lib.makeOverridable and makeOverridablePythonPackage. Now buildPython* accepts recursive attributes. Turn a Python application pyspread into the recursive attributes style.

[nixos-discourse]

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/proposal-move-values-from-derivation-attributes-to-function-arguments/20697/21

[nixos-discourse]

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/avoid-rec-expresions-in-nixpkgs/8293/18

[infinisil]

Recently the Packages Modules Working Group started investigating whether something like the module system could be used to evaluate packages. We're tracking all work in this repository, meeting weekly, but working mostly asynchronously. It would be great if you could join the Matrix room of the WG and chat with us, or even join the team yourself to work on such issues!

[ShamrockLee]

Thank you for taking time reviewing this!

I'm concerned about the increasing complexity, while the goal can be achieved through simplification instead: removing the python-specific level of overriding and turning it into an "overlay" on the mkDerivation arguments instead.

The idea to shift workflow-specific overlays sounds neat, and that could also be friendlier when packaging multi-language projects. Nevertheless, there are some issue on the way to the switch:

1. Current builders rely more or less on arguments they won't pass into mkDerivation attribute set. Developers would need to change the way they handle the arguments / attributes, either by passing them directly or through passthru. The impl input argument of the proposed function is essentially an overlay that allows passing not-to-pass-to-mkDerivation arguments through the previous set.

2. Current attempt to try to implement a builder that supports the (finalAttrs: { }) through an overlay passing to .overrideAttrs, such as #194475, discards the set pattern of input arguments ({ a, b ? "some default", ... }@args) completely and degenerate it into previousAttrs. It is a pity, since the set patterns itself serves as an interface of the builder, providing information about the expected input arguments to both and the Nix interpreter and people reading the Nix expression. The solution would be rather simple -- replace the previousAttrs with the set pattern, and then the remaining issue will be point 1.

Overall, the goal of the proposed function is to add (finalAttrs: { }) input support with minimum changes to the existing expressions, and raise the discussion about general approaches to bring such supports to builders.

[LunNova]

Just reviewing your change to input-remapper, I like how much this cleaned up its definition. :)

[ShamrockLee]

Rename the function to extendMkDerivation and extendMkDerivationModified, and name the finalAttrs builder feature "function-based attribute recursion".

Wonder if we should also change the corresponding sub-section title # Recursive attribute in mkDerivation in the Standard Environment chapter of the Nixpkgs Manual? Should we change the name of the anchor, too?

[ShamrockLee]

BTW, it turns out that I forgot to take care of overrideAttrs after applying the modifier, which is the probable cause of extendDerivation failing to disable Python modules for unsupported Python interpreters. Now the issue is fixed.

[ShamrockLee]

Rename the following concepts to reduce ambigualty and avoid confusion:

• A function that produces a derivation is now said to be a "build helper" instead of a "builder", to distinguish from the builder input attribute of derivation or the remote builders of Nix.

• The use of a (finalAttrs: { })-style function instead of a plain attribute set as the input argument of another function and get the final state of an attribute from finalAttrs is now said to be "function-based attribute recursion" instead of "recursive attributes" to distinguish from the recursive set syntax (rec { }).

Keep the path to existing documentation files unchanged to avoid breaking other open PRs or staging changes.

[ShamrockLee]

The packages doesn't evaluate after rebasing onto recent revision of the master branch, complaining that passthru.pythonDisabled is missing.

I suspect that someone uses overrideAttrs against python packages but accidentally discard passthru attributes from previousAttrs.

Update:

1. Tolerate the attribute pythonDisabled being missing with a lib.warn warning.
2. Find the package that causes the attribute missing (apache-airflow) and fix the overriding.

[ShamrockLee]

While digging into the overriding problem, it came to me that overrideAttrs doesn't work as expected for derivations build by buildPythonPackage in the current implementation (prior to this patch). It is not only because it doesn't cover the non-passing input arguments, but because the disabled logic, added through direct derivation modification, will get lost after overrideAttrs.

nix-repl> (pkgs.python3Packages.buildPythonPackage { pname =
"foo"; version = "0.1.0"; src = pkgs.emptyFile; disabled = tr
ue; }).overrideAttrs (_: { })es.buildPythonPackage { pname = «derivation /nix/store/hlkhgm4njsxp8k46qq3arywk8861ba9z-python3.10-foo-0.1.0.drv»

nix-repl> pkgs.python3Packages.buildPythonPackage { pname = "foo"; version = "0.1.0"; src = pkgs.emptyFile; disabled = true; }
error:
       … in the condition of the assert statement

         at /home/shamrock/Projects/NixOS/nixpkgs_optparse/lib/customisation.nix:222:17:

          221|     in commonAttrs // {
          222|       drvPath = assert condition; drv.drvPath;
             |                 ^
          223|       outPath = assert condition; drv.outPath;

       … in the right operand of the IMPL (->) operator

         at /home/shamrock/Projects/NixOS/nixpkgs_optparse/pkgs/development/interpreters/python/mk-python-derivation.nix:256:13:

          255| in lib.extendDerivation
          256|   (disabled -> throw "${name} not supported for interpreter ${python.executable}")
             |             ^
          257|   passthru

       (stack trace truncated; use '--show-trace' to show the full trace)

       error: foo-0.1.0 not supported for interpreter python3.10
«derivation
nix-repl> (pkgs.python3Packages.buildPythonPackage { pname = "foo"; version = "0.1.0"; src = pkgs.emptyFile; disabled = true; }).overrideAttrs (_: { })
«derivation /nix/store/hlkhgm4njsxp8k46qq3arywk8861ba9z-python3.10-foo-0.1.0.drv»

The effects of toPythonModule survive through overrideAttrs as it is implemented through overrideAttrs, and is applied prior to the "disabled logic".

[roberth]

Not 100% sure I follow, but iirc we've made it possible to override overrideAttrs via passthru, if you really really have to. I hope that's not what you need. It probably isn't what you need, and you probably shouldn't use it because it's more surprising, but now you know.

The effects of toPythonModule survive through overrideAttrs as it is implemented through overrideAttrs, and is applied prior to the "disabled logic".

This is a puzzling sentence. Maybe it's because I don't really have the patience today, but it wouldn't need to be said if overridePythonAttrs were just overrideAttrs, so I do still think that overridePythonAttrs should be removed at some point, just so that everyone's head doesn't explode when they have to deal with python overriding.

[ShamrockLee]

• Overriding up to mkDerivation is tested in pkgs/test/overriding.nix. This function is not language specific, so it could be tested there too.

Thank you @roberth for telling me where to write the tests!

It came to me that that .override after .overridePythonAttrs is already broken before this PR. @FRidh Is this a known issue?

Here's the tests I added:

https://github.com/NixOS/nixpkgs/compare/master...ShamrockLee:nixpkgs:test-overriding-python-commutative?expand=1

[ShamrockLee]

It came to me that that .override after .overridePythonAttrs is already broken before this PR. @FRidh Is this a known issue?

Ah, I know why. The override attribute is added by makeOverrideae called by callPackage, which is applied after the builder (stdenv.mkDerivation or language-specific builders). .overrideAttrs works fine after .override just because makeOverrideable takes special care of ovelrideAttrs.

We don't expect makeOverrideable to take care of these builder-specific overriding attributes. It would be great to gradually deprecate these builder-specific overriding attributes, and pass special arguments through passthru instead. Hope that these attributes could become an alias to overrideAttrs someday.

[ShamrockLee]

Progress:

• Split the documentation part into a new PR #262648 for further discussion.
• Add lib.applyToOverridable as a general approach to modify an "overridable" (attribute sets that comes with attributes that enable overriding, including derivations produced by stdenv.mkDerivation) and all its overriding results. Cover it with unit tests in pkgs/test/overriding.nix.
• Define lib.extendMkDerivationModified with lib.extendMkDerivation and lib.applyToOverridable.

[ShamrockLee]

Does this PR need a release note entry?

[ShamrockLee]

Rebase and update the documentation as #262301 have been merged.

[ShamrockLee]

nixpkgs-review reports that this PR removes the package python310Packages.camelot, but that packages evaluates and builds successfully on my laptop.

What's wrong?

[ShamrockLee]

@ofborg build python310Packages.camelot

[ShamrockLee]

I accidentally rebased and pushed 4 previous commits.

The problem is now fixed, sorry for the noise.

[ShamrockLee]

Rebase to resolve merge conflict with the master branch.

[nixos-discourse]

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/2023-10-30-documentation-team-meeting-notes-90/34936/1

[ShamrockLee]

this PR still seems to big, it adds 4 new functions to lib.customisation, each of which supposedly with its own use case. I'd like to see this split into the smallest possible change that makes sense on its own.

I could drop the currently-unused lib.applyToOverridableWithCustomName to (slightly) ease the workload.

lib.applyToOverridable was originally inlined inside the lib.extendMkDerivationModified implementation. I abstracted out the logic and make it a separated function to increase the granularity of test cases. Now lib.extendMkDerivationModified is simply a composition of lib.extendMkDerivation and lib.applyToOverridable, i.e.

extendMkDerivationModified = modify: mkDerivationBase: impl: rattr:
  lib.applyToOverridable modify (lib.extendMkDerivation mkDerivationBase impl rattr);

The main factor that makes this PR so big is that it

1. Introduces lib.extendMkDerivation and lib.extendMkDerivationModified, which involves
   • Defining lib.applyToOverridable
   • Defining lib.extendMkDerivation
   • Defining lib.extendMkDerivationModified as their composition.
2. Makes buildPython* support fixed-points argumnets, which involves
   • Making lib.makeOverridable supports fixed-point arguments.
   • Making makeOverridablePythonAttrs in python-packages-base.nix supports fixed-point arguments.
   • Fixing makeOverridablePythonAttrs to make it play well with overrideAttrs to prepare for the (possible) gradual deprecation of overridePythonAttrs.
   • Making buildPython* support fixed-point arguments with lib.extendMkDerivationModified.
   • Converting two python packages pyspread and input-remapper to fixed-point argument style to demonstrate and and test the new feature.

The purpose to include the buildPython* changes in this PR is to demonstrate the usage of lib.extendMkDerivation[Modified], otherwise they would have become another PR.

[ShamrockLee]

Split the Python part into #267296 and #271387. Now this one only focuses on lib.extendMkDerivation[Modified], and is much shorter than it used to.

[ShamrockLee]

@ofborg build tests.overriding

[ShamrockLee]

I shortly changed the argument mkDerivationBase to a function getMkDerivationBase yesterday, trying to preserve a buildPython* functionality that takes stdenv as one of its input arguments to customize its stdenv.mkDerivation (see 7a0d08029866bef6dcc39d11d68ea6c52e12643d).

I reverted back to mkDerivationBase afterward because a usable value of getMkDerivationBase would be ugly and the specification would not be overridable, which contradicts the goal of lib.extendMkDerivation.

The stdenv specification should be done like (buildPythonPackage.override { inherit stdenv; }} { ... }, the way other build helpers would do.

The .override attribute of buildPython* gets lost because of problems inside python-package-base.nix, which is probably why stdenv is passed in such a strange way. #271762 brings back buildPython*.override and enable the (buildPythonPackage.override { inherit stdenv; }} { ... } way of custom stdenv specification.

[ShamrockLee]

Rebase onto the master branch and try to polish the sentences of the rease-note entry.

[ShamrockLee]

Rebas onto the master branch and update the Nixpkgs manual section about fixed-point arguments of build helpers.

[ShamrockLee]

@fricklerhandwerk Could you help me take a look at the documentation?