lib.makeScope: allow overriding callPackage

[quantenzitrone]

this is probably needed for NixOS/nixpkgs-vet#206

Things done

• Built on platform:
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• Tested, as applicable:
  • NixOS tests in nixos/tests.
  • Package tests at passthru.tests.
  • Tests in lib/tests or pkgs/test for functions and "core" functionality.
• 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.

[quantenzitrone]

rebuilds are

{
  "rebuildsByKernel": {
    "darwin": [
      "nixpkgs-manual",
      "tests.lib-tests"
    ],
    "linux": [
      "extra-container",
      "nixos-container",
      "nixos-install-tools",
      "nixpkgs-manual",
      "tests.lib-tests",
      "tests.nixos-functions.nixos-test"
    ]
  }
}

[philiptaron]

Lgtm; @ShamrockLee or @hsjobeki what think you?

[ShamrockLee]

The diff seems reasonable.

I haven't got a chance to test it. Is it possible to guard this feature with some test cases?

Pinging Nixpkgs library experts: @infinisil @roberth @emilazy

[hsjobeki]

Lgtm; @ShamrockLee or @hsjobeki what think you?

Yes. A little bit out of my comfort zone here. I would ask robert or infinisil if they see problems here. Unfortunately didn't have enough time to dig into this area deep enough.

[philiptaron]

I pushed a commit with three tests for the new behavior:

1. The default callPackage still works when the scope function doesn't override it.
2. The scope function can provide its own callPackage and it takes effect.
3. The overridden callPackage persists through overrideScope.

I also verified that test 2 fails with the old makeScope, as the right-hand // clobbers the override, so you just get the default callPackage back. Thanks @ShamrockLee for flagging the need for tests.

[quantenzitrone]

do we need release notes for this?
because

lib.makeScope lib.callPackageWith (self: {
  inherit (self) callPackage;
})

now throws an infinite recursion error

i mean inheriting from self in a fixed point attrset is generally stupid but there was one case in nixpkgs (octave packages) so it might break stuff for people.

Also thanks for adding the tests.

[philiptaron]

i mean inheriting from self in a fixed point attrset is generally stupid but there was one case in nixpkgs (octave packages) so it might break stuff for people.

This is how I feel... but the release note seem so in the weeds that I hesitate to even draft it. I think I just want to merge this.

[9999years]

Hello this PR has made my makeScope invocation fail:

let
  pkgs = import ./. { };
in
pkgs.lib.makeScope pkgs.newScope (final: {
  inherit (final) callPackage newScope;
})

Here I am using this invocation in a project of mine: https://github.com/9999years/npingler/blob/570a77e2e7831ec42a84c29e9908b729e2fd91e7/lib/default.nix

I thought this was the 'normal' way to use makeScope, but unfortunately the docs are quite light. How should I fix this error?

I see similar patterns around GitHub:

https://github.com/michaelvanstraten/templates/blob/9c8c6389e26c2415c74f65d5348197413a89ea0e/templates/generic/lib/default.nix#L8

It seems like callPackage = self.newScope { }; is slightly more common, but the docs and tests don't have any relevant examples.

[quantenzitrone]

callPackage and newScope are automatically added to the resulting attrset
you don't need to inherit it
to shorten final.callPackage to callPackage you can do what https://github.com/michaelvanstraten/templates/blob/9c8c6389e26c2415c74f65d5348197413a89ea0e/templates/generic/lib/default.nix#L8 is doing and inherit it in a let in block

tldr

let
  pkgs = import ./. { };
in
pkgs.lib.makeScope pkgs.newScope (final: {
})