Defining variables with default values in package arguments can lead to problems

[Aleksanaa]

Problem

To make overriding more feasible, people often write something like

{
  sth,
  doCheck? true,
  patches? [],
  foo? callPackage ./foo.nix,
}:

{ ... }

But this is actually dangerous. We don't know when a top-level package named "doCheck", "patches" or "foo" will appear. While the first two can easily be found on ofBorg failures, the third might be quietly passed, causing more problems (and it even looks unrelated!).

Reproduce

# default.nix
{ aaa? "1" }:

aaa

This should be "1", right?

=> nix eval --impure --expr 'with import <nixpkgs> {}; callPackage ./default.nix {}' 
«derivation /nix/store/l36n95rwc21rn1f8sld428j3sarv3ssd-aaa-1.1.1.drv»

Well it's not.

Solution

My idea is that we set some "reserved attribute prefixes", such as names starting with "var-", and make sure that they cannot be used for top-level package names. We may also need to take the same approach for other sets that define callPackage.

[nim65s]

Prefixing those variables with pname could also be an option. For example, in package zblarg, one could probably use zblargDoCheck, zblargPatches, zblargWithPython and zblargAaa with a minimal risk of a new unrelated package being named that.

It would also be more explicit, as zblargWithPython and fooWithPython would be different, while var-withPython and var-withPython in packages zblarg and foo wouldn't be.

And I think this is a common enough practice in various language and build systems already, so this wouldn't be too hard to adopt here.

[TomaSajt]

An example for when I've encountered this unexpected default populating logic was when taking in an src value (it turns out it's also the name of a VCS package)

[reckenrode]

Does NixOS/rfcs#169 at least partially address this issue?

[jtojnar]

I would say that if you need to pass doCheck, src, patches or other mkDerivation arguments, you are not calling a package but a package builder/factory, so you should not use callPackage. And if your goal is to be overridable, those can be overridden nicely with overrideAttrs (assuming finalAttrs pattern) without having to stuff them into package expression arguments.

And if you are doing that trying to follow DRY or something, that is just more mess. Especially since using callPackage inside callPackage will break override anyway #311780 (comment)

The boolean/enum feature parameters are also ugly but at least they typically follow consistent naming pattern that is unlikely to conflict.

Lastly, as for internal packages, it might be fine if they are not overridable. They are often temporary or tied to the main derivation closely enough that the need to override them should be rare.

Though if we really want to, …

we could create a private scope and call the package from within – something like the example below. Then it would be just possible to use override like with any other package mainProgram.override (old: { internalLib = old.internalLib.overrideAttrs …; })

pkgs/top-level/all-packages.nix

# …
my-program = import pkgs/applications/my-program { inherit pkgs lib; };
# …

pkgs/applications/my-program/default.nix

{ pkgs, lib }:

let
  privateScope = lib.makeScope pkgs.newScope (self:
    let
      inherit (self) callPackage;
    in {
      internalLib = callPackage ./foo {};
      # depends on internalLib
      mainProgram = callPackage ./prog {};
    }
  );

in
privateScope.mainProgram

[bendlas]

I would say that if you need to pass doCheck, src, patches or other mkDerivation arguments, you are not calling a package but a package builder/factory, so you should not use callPackage.

So if you're doing a factory, but you still need a couple of inputs, that you'd rather not pass through, what would you recommend instead of auto-wiring with callPackage? Passing pkgs and selecting the dependencies yourself?

Is there room for a callPackage variant, that doesn't inject arguments, that have defaults?

Arguably in hindsight, wouldn't that have been a better behavior for callPackage in the first place?