stage: use fix' to allow deep overriding of `pkgs` from within Nixpkgs #43828

FRidh · 2018-07-19T20:47:33Z

Motivation for this change

Test the effect on performance (memory)

Things done

Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
Built on platform(s)
- NixOS
- macOS
- other Linux distributions
Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
Tested execution of all binary files (usually in ./result/bin/)
Determined the impact on package closure size (by running nix path-info -S before and after)
Fits CONTRIBUTING.md.

FRidh · 2018-07-19T20:49:51Z

Motivation for this change is the overriding required for Sage (#43755)
Earlier version of this change (5aa92b6c94b002b602299f55f48ad0e2f6b390ae)

FRidh · 2018-07-19T20:54:11Z

Statistics:

$ NIX_SHOW_STATS=1 nix-env -f . -qaP --meta --json --drv-path --show-trace >/dev/null

$ diff before after
2c2
<   time elapsed: 6.3265
---
>   time elapsed: 6.12604
12,14c12,14
<   sets allocated: 1240804 (393330248 bytes)
<   right-biased unions: 544820
<   values copied in right-biased unions: 12181948
---
>   sets allocated: 1240832 (396424696 bytes)
>   right-biased unions: 544834
>   values copied in right-biased unions: 12310798
18c18
<   number of thunks avoided: 5513756
---
>   number of thunks avoided: 5513770
22c22
<   total allocations: 734351336 bytes
---
>   total allocations: 737445784 bytes
24c24
<   total Boehm heap allocations: 909242384 bytes
---
>   total Boehm heap allocations: 912338480 bytes

Ericson2314 · 2018-07-19T22:32:01Z

Don't you really want makeExtensible? This is what @nbp removed at one point. I'll find commit in a bit.

nbp

Using packageOverride in Nixpkgs should have been banned before, and identically for re-importing Nixpkgs within Nixpkgs evaluation.

You should not need a re-evaluation of Nixpkgs for doing this kind of operations. What you are looking for is what the bootstrap mechanism should have been instead of being an additional fix-point, which is a duplicated and isolated sub-set of overriden packages within Nixpkgs fix-point.

You will have to add an overrideWith attribute to makeOverridable, which redo the argument intersection logic of callPackageWith.

self: super:

{
  sageDeps = (super.sageDeps or {}) // {
    foo = super.foo.overrideWith self.sageDeps;
    # take foo from self.sageDeps.foo instead of self.foo
    bar = super.bar.overrideWith self.sageDeps;
  };
}

nbp · 2018-07-20T00:33:46Z

I will also note that this overrideWith logic can be used to replace all inner fix-points for pythonPackages, emacsPackages, and so on ...

self: super:

{
  pythonPackagesRaw = {
    foo = callPackages import ./foo.nix {};
    bar = callPackages import ./bar.nix {};
    baz = callPackages import ./baz.nix {};
  };

  python27Packages =
    let scope = self.python27Packages // { python = self.python27; }; in
    lib.mapAttrs (n: v: v.overrideWith scope)) pythonPackagesRaw;
  python35Packages =
    let scope = self.python35Packages // { python = self.python35; }; in
    lib.mapAttrs (n: v: v.overrideWith scope)) pythonPackagesRaw;
}

FRidh · 2018-07-20T06:58:44Z

Thank you @nbp. Let's see if I understand this.

The following expression represents all-packages.nix. Instead of using self: pkgs: we use here self: super as we commonly use when talking about fix-point. In this example, the package set with Sage dependencies sageDepsis added directly in all-packages.nix. We list packages already defined in all-packages.nix but we override them using the expressions in this sageDeps set.

self: super:

{
  sageDeps = (super.sageDeps or {}) // {
    foo = super.foo.overrideWith self.sageDeps;
    # take foo from self.sageDeps.foo instead of self.foo
    bar = super.bar.overrideWith self.sageDeps;
  };
}

Because having to write overrideWith self.sageDeps for every package is quite verbose, you show in your next example how to use mapAttrs. We could use that in the first example:

self: super:

{
  sageDeps = let scope = self.sageDeps; in lib.mapAttrs (n: v: v.overrideWith scope) 
  ((super.sageDeps or {}) // {
    foo = super.foo;
    # take foo from self.sageDeps.foo instead of self.foo
    bar = super.bar;
  });
}

Now, I want to call a package. Typically I would use callPackage for this. In your second example you use callPackages import. What is that for?

Example

I would very much like to see an example representing three files:

all-packages.nix
package-set.nix which is called by all-packages.nix
package.nix which is called by package-set.nix

Let's see how we could do that.

`all-packages.nix`

self: pkgs:

{
  packageSet = let scope = packageSet; in lib.mapAttrs (n: v: v.overrideWith scope) 
  ((pkgs.packageSet or {}) // callPackage package-set.nix { });
}

`package-set.nix`

Note we need to be able to access items in this set as well.

{ pkgs }:

{
  # Override package explicitly with given parameters
  foo = pkgs.foo.override { ... };
  # Call our package
  bar = pkgs.callPackage package.nix { };
  # Explicitly include a version of an existing package that uses
  # our overridden package set
  foobar = pkgs.foobar;
}

`package.nix`

{ ...
}:

stdenv.mkDerivation {
  ...
}

@nbp could you check this example and improve in-place?

FRidh · 2018-07-20T07:25:16Z

There are of course different sets to consider:

those sets that offer only new packages (pythonPackages, haskellPackages, gnome3), and not pkgs
those that modify the main package set (sagePackages)

Not sure where to put libsForQt5.

FRidh · 2018-07-21T10:02:58Z

Closing because this won't go in.

timokau · 2018-07-21T10:26:51Z

What about the alternative?

nbp · 2018-07-21T10:39:10Z

Now, I want to call a package. Typically I would use callPackage for this. In your second example you use callPackages import. What is that for?

This is an example short-cut for a new package definition.

For the example you are looking for, putting things in a different file involve some extra naming, and you should not just give one argument to the package-set.nix file because then you confuse the nixpkgs fix-point from the rec-like fix-point. For the following example, I will name them differently but follow the same order as expected by all-packages.nix.

(side-note: Why is callPackage taken from the nixpkgs fixpoint ... sounds like somebody did not listen to my talk about the things that should be avoided.)

all-packages.nix

# There is no super argument to all-packages.nix, which is a shame as this is not a proper overlay.
rec_: fixPkgs: # today named `self: pkgs:`

{
  packageSet =
    # This does not include (super.packageSet or {}) because we have no `super`.
    # The last fixPkgs, should be the `super` attribute of all-packages.nix for 
    let raw = import ./package-set.nix fixPkgs rec_ fixPkgs; in
    # overrideWith fixPkgs.___ such that overlay modification of packageSet.foo
    # are visible by packageSet.foobar.
    lib.mapAttrs (n: v: v.overrideWith fixPkgs.packageSet) raw;
}

package-set.nix

# We need this extra argument because instead of making an overlay as I suggested in the previous
# example, we are making it part of all-packages.nix, which implies that all packages added in
# all-packages.nix are not (supposed to be) available from the super attribute. Also, it comes from
# the nixpkgs fix-point today, using `override` or `overrideWith` might lead to an infinite loop later.
self: allPkgs: super:

let
  inherit (super) callPackage;
in

{
  # Override package explicitly with given parameters
  foo = allPkgs.foo.override { enableToto = true; };
  # Call our package
  bar = callPackage package.nix { };
  # Expect foo and bar as inputs. With the mapAttrs from all-packages.nix, the foo and bar
  # would be overwritten with the foo and bar from this set.
  foobar = allPkgs.foobar;
}

timokau · 2018-07-23T15:51:54Z

@FRidh can you re-open this? And are you still interested in helping me with this? I don't quite understand the underlying issues and the advantages of @nbp's version yet, but if I have to I'll try to work my way through it.

stage: use fix' to allow deep overriding of pkgs from within Nixpkgs

d898fe3

FRidh added the 2.status: work-in-progress label Jul 19, 2018

FRidh requested a review from nbp July 19, 2018 20:47

FRidh requested a review from Ericson2314 as a code owner July 19, 2018 20:47

GrahamcOfBorg added 10.rebuild-darwin: 0 10.rebuild-linux: 0 labels Jul 19, 2018

nbp suggested changes Jul 20, 2018

View reviewed changes

FRidh closed this Jul 21, 2018

nbp mentioned this pull request Jul 29, 2018

Add pkgs.overrideWithScope #44196

Closed

FRidh mentioned this pull request Sep 17, 2019

Enhancing lib.makeScope #68967

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

stage: use fix' to allow deep overriding of `pkgs` from within Nixpkgs #43828

stage: use fix' to allow deep overriding of `pkgs` from within Nixpkgs #43828

FRidh commented Jul 19, 2018 •

edited

FRidh commented Jul 19, 2018

FRidh commented Jul 19, 2018

Ericson2314 commented Jul 19, 2018

nbp left a comment

nbp commented Jul 20, 2018

FRidh commented Jul 20, 2018 •

edited

FRidh commented Jul 20, 2018

FRidh commented Jul 21, 2018

timokau commented Jul 21, 2018

nbp commented Jul 21, 2018 •

edited

timokau commented Jul 23, 2018

stage: use fix' to allow deep overriding of pkgs from within Nixpkgs #43828

stage: use fix' to allow deep overriding of pkgs from within Nixpkgs #43828

Conversation

FRidh commented Jul 19, 2018 • edited

Motivation for this change

Things done

FRidh commented Jul 19, 2018

FRidh commented Jul 19, 2018

Ericson2314 commented Jul 19, 2018

nbp left a comment

Choose a reason for hiding this comment

nbp commented Jul 20, 2018

FRidh commented Jul 20, 2018 • edited

Example

all-packages.nix

package-set.nix

package.nix

FRidh commented Jul 20, 2018

FRidh commented Jul 21, 2018

timokau commented Jul 21, 2018

nbp commented Jul 21, 2018 • edited

all-packages.nix

package-set.nix

timokau commented Jul 23, 2018

stage: use fix' to allow deep overriding of `pkgs` from within Nixpkgs #43828

stage: use fix' to allow deep overriding of `pkgs` from within Nixpkgs #43828

FRidh commented Jul 19, 2018 •

edited

FRidh commented Jul 20, 2018 •

edited

`all-packages.nix`

`package-set.nix`

`package.nix`

nbp commented Jul 21, 2018 •

edited