Skip to content

Fixing .override interface stability #404946

@pbsds

Description

@pbsds

Fixing override interface stability

TLDR: see the proposed "Option 3" for my personal favorite.

Note

I initially typed this up before looking for prior art, then I found #273534 which seems to have stalled aiming for perfection. Rather than post this there and risk it becoming noise or derailing progress over there I opted to open this new issue.

Currently the nixpkgs guidelines recommends preserving the .override interface.
Consider a bar by-name package which depends on a special version of foo, say foo_1_2_3: the current guidelines recommend the following pattern:

# pkgs/by-name/ba/bar/package.nix:
{
    foo
    ...
}: mkDerivation { ... foo ... }

# pkgs/top-level/all-packages.nix:
{
    ...
    bar = ../by-name/ba/bar/package.nix { foo = foo_1_2_3 };
    ...
}

In my opinion this pattern has at least two big issues:

  • Poor code-locality: domain knowledge about the bar package is spread wide and becomes less discoverable.
  • It directly refers to by-name paths in all-packages.nix. This makes future by-name refactoring like for example sharding pkgs/by-name/li/ into multiple longer prefixes (to account for all the lib* packages) a much larger undertaking.

I've observed more and more maintainers and committers (myself often included) disregard this guideline and just replace the package input with the special version:

# pkgs/by-name/ba/bar/package.nix:
{
    foo_1_2_3,
    ...,
}: mkDerivation { ... foo_1_2_3 ...; }

This breaks bar.override { foo = ...; } expressions in both nixpkgs and for downstream users.
It is also prone to cause extra review cycles depending on what a reviewer prefers or consider to be a blocker, making this issue prone to trigger multiple code-review antipatterns.

The problem stems from the fact that if an attribute exist in the scope, then callPackage will provide it as an input arguments even if that argument has a default value.
This also leaves us with this footgun:

# pkgs/by-name/ba/bar/package.nix:
{
    foo_1_2_3,
    foo ? foo_1_2_3, # default is not used
    ...,
}: mkDerivation { ... foo ...; } # ERROR: `foo` is `pkgs.foo`, not `pkgs.foo_1_2_3`

This risks becomes a silent error if the incorrect foo input don't result in a build failure but instead a runtime error.

Proposed options

Option 1: Don't consider .override interfaces stable.

This is the path of least resistance, and I've seen multiple contributors express this preference, partly due to #204303.
This option also avoids hammering down exactly what stability guarantees we even aim to provide.

To what degree do we actually consider using .override to be stable?
Should we support all enableX/disableX/withX flags indefinitely?
Should we try and accept (and ignore) all previously accepted package inputs?
When is it okay and not okay to change the interface?

Increasingly as package definitions that just .override some other package get migrated from all-packages.nix to by-name, we see more and more .override interfaces differ because by-name will via callPackage replace any pre-existing .override in the output with a new makeOverridable invocation.
An example if this is SDL2_mixer_2_0 whose .override interface does not match SDL2_mixer at all.

However going ahead with this option will make nixpkgs less reliable and powerful for downstream users.

Option 2: making callPackage smarter

We could extend callPackage to allow the callee to somehow inform callPackage what not to provide as initial arguments.

My dream implementation would look something like:

# pkgs/by-name/ba/bar/package.nix:
{
    lib,
    foo_1_2_3,
    foo ? lib.mkCallPackageDefault foo_1_2_3,
    ...,
}: mkDerivation { ... foo ...; }
# here ^ `foo` is `pkgs.foo_1_2_3` unless `bar` is explicitly overridden with `bar.override { foo = ...; }`

But this is to my knowledge not possible, since our only tool for the job builtins.functionArgs only informs us if a function argument has a default value but does not allow us to inspect what that default value is.
As a workaround I envision two solutions to extend callPackage:

A non-solution: Not setting arguments that have default values

While this would make

# pkgs/by-name/ba/bar/package.nix:
{
    foo_1_2_3,
    foo ? foo_1_2_3,
    ...,
}: mkDerivation { ... foo ...; }

"just work", it would also be a massive treewide breaking change.

It makes package definitions non-reusable across different scopes where some optional inputs are only available in some of the scopes.
This means the stdenv bootstrap for example would require a massive refactoring with likely high amounts of code duplication or hacky workarounds like { pkgs, baz ? pkgs.baz or null }: ....

Solution A: Extend callPackage to act on presence of magic arguments

We could make callPackage not provide certain inputs if the input has a matching __callPackage_ignore_* input.

# pkgs/by-name/ba/bar/package.nix:
{
    foo_1_2_3,
    foo ? foo_1_2_3,
    __callPackage_ignore_foo ? null,
    ...,
}: mkDerivation { ... foo ...; }

This change however affects the design space for NixOS/rfcs#169 and should be considered jointly.

Solution B: wrap package.nix expressions with a special type that callPackage acts on

This could look something like

# pkgs/by-name/ba/bar/package.nix:
{ lib }:
lib.callPackageIgnore [ "foo" ] (
    {
        lib,
        foo_1_2_3,
        foo ? foo_1_2_3,
        ...,
    }:
    mkDerivation { ... foo ... }
)

where callPackage then checks the result for some magic attribute set by lib.callPackageIgnore (like __callPackageIgnore = [ "foo" ]), and if present recursively applies a second callPackage.

However this design is prone to not play well with the formatter which puts all the git blame on you.
It also has a silent footgun where nixpkgs contributors and downstream users instead might set passthru.__callPackageIgnore inside the package instead of using lib.callPackageIgnore properly, which both increases the eval cost with multiple mkDerivation invocations, and is prone to cause infinite recursions.

(I also have great trouble in both picking a good intuitive name for this function and designing a interface for it that I like.)

Option 3: Extend the by-name interface

Rather than change callPackage we could extend the by-name interface and machinery.

Consider by-name checking for an additional optional override.nix file, whose output replaces the output of callPackage ./by-name/{shard}/{pname}/package.nix {};, and whose output is not wrapped with lib.makeOverrideable:

# pkgs/by-name/ba/bar/package.nix:
{
    foo,
    ...,
}: mkDerivation { ... foo ...; }

# pkgs/by-name/ba/bar/override.nix:
{
    foo_1_2_3,
}:
package:
package.override { foo = foo_1_2_3; }

This override.nix pattern could be made simpler using a more task-specific attrSet -> attrSet interface rather than attrSet -> nullOr overridableDrv -> any, but this is both way less powerful and already discussed in RFC140 as not a big enough of a value-add.

The attrSet -> nullOr overridableDrv -> any interface will allow us to migrate cases like the aforementioned SDL2_mixer_2_0 example to instead just define a override.nix file with no package.nix file (in which case the package input in override.nix would become null), fixing the SDL2_mixer/SDL2_mixer_2_0 override interface mismatch.
It would also enable switching to a different package set, resulting in the resulting package becoming simpler to override:

# pkgs/by-name/ba/bar/override.nix:
{ python3Packages }:
package: # if we never use/force `package` there should be no issues with its arguments not matching `pkgs` thanks to nix being lazy
python3Packages.callPackage ./package.nix { };

Since the override.nix inputs are provided by callPackage (sans makeOverrideable), we get get to override with correctly spliced packages and package sets, alleviating #204303.

(The override.nix name is actually a bit narrow in conveying its true power/affordance, perhaps apply.nix, package-apply.nix, or even just expr.nix is better?)


These options are not perfect, but they are actionable.
Personally I prefer option 3.
It would also allow us to eliminate all ../by-name/**/* references treewide, making by-name packages truly filename agnostic, which once achieved we may continue to enforce with CI.

We could in addition, since this option is orthogonal to option 3, also implement option 2A to support cases outside of by-name or RFC169.

Metadata

Metadata

Assignees

No one assigned

    Labels

    0.kind: enhancementAdd something new3.skill: sprintableA larger issue which is split into distinct actionable tasks6.topic: architectureRelating to code and API architecture of Nixpkgs6.topic: cross-compilationBuilding packages on a different platform than they will be used on6.topic: portabilityGeneral portability concerns, not specific to cross-compilation or a specific platform9.needs: community feedbackThis needs feedback from more community members.

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions