pkgs: introduce `package-list.nix` and `trivialPackages` stage #50643

oxij · 2018-11-18T23:18:02Z

The idea here is to move as much of all-packages.nix to package-list.nix as humanly possible.

Pros:

It improves metrics because import package-list.nix gets cached between stages. Meanwhile, doing the same with import all-packages.nix produces a negative impact because all-packages.nix is complicated and caching it adds pressure on GC. Meanwhile, package-list.nix is just a list of paths, which is cheap.
It allows for easy experimentation with alternatives to callPackage as package-list.nix is, by definition, a set of trivial packages. (This was the initial motivation for this change.)
It's homogeneous with NixOS' module-list.nix.
Eventually package-list.nix can be completely autogenerated.

Cons:

Migration to package-list.nix does introduce some code churn.

This also adds maintainers/scripts/all-packages-to-package-list.sh script that does the (very conservative, currently) conversion automagically.

Take you time merging and testing this, conflicts are easy to fix: just rebase dropping the last commit and rerun the script.

Yes, this is a massive noop change. :)

/cc @matthewbauer @Ericson2314 @vcunat @edolstra

oxij · 2018-11-18T23:23:10Z

As a side note, to convert all of all-packages.nix to this format the policy on the naming of package arguments needs to be reconsidered. With this you usually won't want to have { python = python3; } and similar.

The idea here is to move as much of `all-packages.nix` to `package-list.nix` as humanly possible. Pros: - It improves metrics because `import package-list.nix` gets cached between stages. Meanwhile, doing the same with `import all-packages.nix` produces a negative impact because `all-packages.nix` is complicated and caching it adds pressure on GC. Meanwhile, `package-list.nix` is just a list of paths, which is cheap. - It allows for easy experimentation with alternatives to `callPackage` as `package-list.nix` is, by definition, a set of trivial packages. (This was the initial motivation for this change.) - It's homogeneous with NixOS' `module-list.nix`. - Eventually `package-list.nix` can be completely autogenerated. Cons: - Migration to `package-list.nix` does introduce some code churn. This also adds `maintainers/scripts/all-packages-to-package-list.sh` script that does the (very conservative, currently) conversion automagically.

Hint: if this conflicts with your change you can revert this commit, rebase your change on top, run the script yourself, commit the result, squash. It's magic.

oxij · 2018-11-22T10:07:35Z

Ping. I see thumbs down, but can I have an explanation why? I see no reason not to proceed in this way except for the churn. And the churn won't be too much of an issue if we migrate in tiny installments (i.e. if we agree to merge the infra from the first commit and start adding new packages there).

Concerning package arguments, I think the better policy to doing what the current policy does is switching to using the following method:

Now:

in package:
  { stdenv, package }: ...

in all-packages:
  xxx = callPackage ... { package = package_version; };

After:

in package:
  { stdenv, package_version, package ? package_version }: ...

in all-packages:
  xxx = callPackage ... { };

oxij · 2018-11-22T10:15:34Z

I'm not even against making an RFC for this change, but I want to hear out some thoughts first.

Disclaimer: Yes, I want this because it greatly simplifies the proper implementation of #12877, but even if you don't want use-flags functionality in Nixpkgs I see no reason not to implement this particular change.

7c6f434c · 2018-11-22T10:38:39Z

Re: re-importing: not sure if it would be better than mapping callPackage over the list in all-packages.nix — I guess some packages will still be in all-packages.nix till the next Nix language redesign, so it would be too fragile to be used.

Future plans of modularising Nixpkgs seems to be an argument in favour of doing version selection in-package. Treewide version pinning changes might become more complicated, though.

Unlike all-packages.nix, the list of trivial packages is almost sure to end up in a sorted state. all-packages.nix is quasi-sorted in an order that I fail to fully understand and that is also sometimes unintentionally violated and that is also sometimes intentionally violated. Not sure if we consider the current intentional violations of sorting order valuable.

oxij · 2018-11-22T11:20:43Z

Re: re-importing ...

Please elaborate, I don't understand what you are trying to convey in that paragraph. "mapping callPackage over the list in all-packages.nix" what list? all-packages.nix is not a list.

Treewide version pinning changes might become more complicated, though.

I don't see why, you can pin in all-packages.nix as before and then just grep around. If anything, it doesn't change much in that regard as many packages depend on particular versions already, so you have to grep anyway when you want to pin something.

quasi-sorted in an order that I fail to fully understand and that is also sometimes unintentionally violated and that is also sometimes intentionally violated

Oh, yes, that bugs me too.

7c6f434c · 2018-11-22T11:51:11Z

re: re-importing:

I expect that all-packages.nix will stay in some form even if the current PR is adopted, so a full simplification won't happen.

I am not sure that the extra caching will be noticeable — I would expect that looking up any attribute name in the resulting attrset should end up with the same outcome as evaluation of the current all-packages.nix; the mapAttrs will need to be evaluated enough to have the list of names.

Re: pinning: replacing a single argument name in a package is a simpler operation that doing something about two related ones.

edolstra · 2018-11-22T12:26:12Z

This was tried before (ece61b7) but was reverted. In short, it's confusing if some packages are in all-packages.nix and some aren't. And if you need to override an argument, you now need to modify two files. Saving a few lines didn't really justify this.

oxij · 2018-11-22T12:26:42Z

I expect that `all-packages.nix` will stay in some form even if the current PR is adopted, so a full simplification won't happen.

For now, that's correct. Ideally, though, `all-packages.nix` should just become the "default version selector overlay". But that needs considerably more work than I managed to put in till now (I do have a branch that moves all option defaults from `all-packages.nix` to the package expressions already, though, but I'm not happy about it yet) as `all-packages.nix` has all kinds fun stuff ATM.

I am not sure that the extra caching will be noticeable — I would expect that looking up any attribute name in the resulting attrset should end up with the same outcome as evaluation of the current `all-packages.nix`; the `mapAttrs` will need to be evaluated enough to have the list of names.

Right, but that's exactly what I don't do here. The ``` packageList = lib.listToAttrs (map (path: lib.nameValuePair (baseNameOf (toString path)) path) (import ../package-list.nix)); ``` bit that gets cached doesn't `map` with `callPackage`. That's the whole point. `packageList` is an attrset of paths, which is very cheap. Meanwhile, `import all-packages.nix` produces an attrset of code thunks, which, as you rightfully note, is not cheap. The effect of this is quite noticeable already (I still need to measure carefully, but I think it was about 10% in -qa time last time I checked), even before most of `all-packages.nix` is converted. (And that whole `map list into attrset` construction could be made into a builtin to speed up the thing even more as it wastes allocations by constantly recycling `nameValuePair`s.)

Re: pinning: replacing a single argument name in a package is a simpler operation that doing something about two related ones.

Well, strictly speaking, there are two places now too: one for specifying the global default version, and one for overriding in `callPackage` block for each packages that wants a different one. No real change here. So, ideally, if a package wants a particular version it would specify it with the above default attrset trick instead of the `callPackage` block like now, if it does not, it simply receives a default from the "default version selector overlay" as usual.

oxij · 2018-11-22T12:36:58Z

@edolstra

In short, it's confusing if some packages are in `all-packages.nix` and some aren't.

That's a fair point for the transition period, but it stops being an issue when `all-packages.nix` becomes "default version selector overlay" as noted above.

And if you need to override an argument, you now need to modify two files. Saving a few lines didn't really justify this.

No, you don't, you just override the default with a default value in the package expression itself, see above.

7c6f434c · 2018-11-22T12:37:18Z

The effect of this is quite noticeable already (I still need to measure carefully, but I think it was about 10% in -qa time last time I checked), even before most of `all-packages.nix` is converted.

Is there an import that doesn't get accessed at all? Because I don't see how an access wouldn't create an attrset of thunks.

oxij · 2018-11-22T13:03:27Z

Is there an import that doesn't get accessed at all? Because I don't see how an access wouldn't create an attrset of thunks.

`pkgs/top-level/default.nix`, basically: ``` allPackages = newArgs: import ./stage.nix ({ inherit lib nixpkgsFun; } // newArgs); boot = import ../stdenv/booter.nix { inherit lib allPackages; }; in boot stages ``` `pkgs/stdenv/booter.nix` then evaluates `allPackages` a bunch of times to bootstrap `stdenv`, each eval of that imports `stage.nix` and then evaluates the result as a function on that `... // newArgs` argument. Here goes the important bit. The body of the function defined in `stage.nix` imports `all-packages.nix`. Since `allPackages` gets evaluated multiple times, `stage.nix` function gets evaluated multiple times, which re-imports `all-packages.nix` multiple times (which is easy to check by adding a `trace` there). But if you move that import of `all-packages.nix` from `stage.nix` to `default.nix` (kinda like I did with `packageList`) you will actually loose performance because nix will have to keep two copies of it in memory while applying arguments evaluating the package set: one for the initial cached version it can't GC because it's cached, and another one for the evaluation of the function `all-packages.nix` contains. With `packageList` there's no such issue, it's not a function, it's an attrset of paths. And converting larger chunks of `all-packages.nix` to `package-list.nix` will produce larger difference in performance. I do need to measure more stuff, though, like I have a feeling that adding some tactical `deepSeq` there might produce an even better result.

danbst · 2019-01-24T13:47:54Z

I've made a test which showed:

proposed approach runs a bit faster than original
I don't observe "metrics improved"

Also, I've added a third option, indexed all-package.nix. It is like

self: super: {
  inherit (import ./all-packages.nix self super)
    package1
    package2
    package3
  ;
}

and measures how better would things be if "callPackage" string is offloaded until real need.

So, the result for 40000 packages:

original runtime is 0.168197s, oxij's list is 0.144745s, index version is 0.08515s
original has 96 bytes per package, oxij's list has 416, index has 96
original has 1,7M on disk, oxij's has 927K, index has 498K

The test script:

#!/usr/bin/env bash

NUM_PACKAGES="${1:-100}"

mkdir -p packages

echo 'self: super: with self; {' > all-packages.nix
echo 'self: super: { inherit (import ./all-packages.nix self super)' > all-packages-index.nix
echo 'self: super:
let nameValuePair = n: v: { name = n; value = v; };
in builtins.listToAttrs
    (map (path: nameValuePair (baseNameOf (toString path)) path) [' > packages-list.nix
for p in $(seq 1 $NUM_PACKAGES); do
  echo "package$p = callPackage ./packages/$p;" >> all-packages.nix
  echo "package$p" >> all-packages-index.nix
  echo "./packages/package$p" >> packages-list.nix
done
echo '}' >> all-packages.nix
echo ';}' >> all-packages-index.nix
echo '])' >> packages-list.nix

# twice to reduce caching interfernece
for file in all-packages.nix all-packages.nix \
            all-packages-index.nix all-packages-index.nix \
            packages-list.nix packages-list.nix; do
  NIX_SHOW_STATS=1 NIX_COUNT_CALLS=1 \
    nix-instantiate --eval -E "builtins.attrNames (import ./$file {} {})" \
    2> result.$file 1>/dev/null
done

oxij · 2019-01-25T11:45:49Z

Yeah, before I suspended this (temporary) I, too, decided to go with a simple attrset and only offload the `callPackage`. My motivation for the list route was conceptual simplicity, but going the list route is just too hard with all the weird discrepancies between attr and file names the current `./all-packages.nix` has, which adds to the complexity of the mapping function, which quickly reduces any efficiency gains to nil.

- I don't observe "metrics improved"

No changes for `nix-env -qa` metrics? On either variant?

danbst · 2019-01-25T12:00:29Z

No changes for nix-env -qa metrics? On either variant?

no, in my simple test. I hope it tests the pure extract of your idea.

mmahut · 2019-08-18T08:23:24Z

Are there any updates on this pull request, please?

stale · 2020-06-01T12:03:37Z

Thank you for your contributions.
This has been automatically marked as stale because it has had no activity for 180 days.
If this is still important to you, we ask that you leave a comment below. Your comment can be as simple as "still important to me". This lets people see that at least one person still cares about this. Someone will have to do this at most twice a year if there is no other activity.
Here are suggestions that might help resolve this more quickly:

Search for maintainers and people that previously touched the
related code and @ mention them in a comment.
Ask on the NixOS Discourse. 3. Ask on the #nixos channel on
irc.freenode.net.

stale · 2021-06-05T00:56:30Z

I marked this as stale due to inactivity. → More info

Artturin · 2023-01-16T22:34:36Z

a similar thing https://github.com/nixpkgs-architecture/simple-package-paths

oxij requested review from Ericson2314, matthewbauer and nbp as code owners November 18, 2018 23:18

oxij added 2 commits November 18, 2018 23:39

oxij force-pushed the tree/package-list branch from ed488ce to 1970134 Compare November 18, 2018 23:42

GrahamcOfBorg added 10.rebuild-darwin: 0 10.rebuild-linux: 1-10 labels Nov 18, 2018

oxij mentioned this pull request Jan 18, 2019

some recurseIntoAttrs hackery #45841

Closed

1 task

oxij mentioned this pull request Feb 3, 2019

all-packages.nix: move defaults to package files #55129

Merged

This was referenced Feb 20, 2019

[Demo, RFC] Gentoo-like use-flags #56110

Closed

Typed nixpkgs.config with Gentoo-like use-flags #56227

Closed

stale bot added the 2.status: stale label Jun 1, 2020

ryantm added the 2.status: merge conflict label Oct 3, 2020

ryantm added 2.status: merge conflict and removed 2.status: merge conflict labels Oct 3, 2020

stale bot removed the 2.status: stale label Oct 3, 2020

ryantm added 2.status: merge conflict and removed 2.status: merge conflict labels Oct 3, 2020

stale bot added the 2.status: stale label Jun 5, 2021

Artturin closed this Jan 16, 2023

This was referenced Jul 18, 2023

[RFC 0109] Nixpkgs Generated Code Policy NixOS/rfcs#109

Closed

[RFC 140] Simple package paths, part 1b: Enabling the directory structure #237439

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

pkgs: introduce `package-list.nix` and `trivialPackages` stage #50643

pkgs: introduce `package-list.nix` and `trivialPackages` stage #50643

oxij commented Nov 18, 2018

oxij commented Nov 18, 2018

oxij commented Nov 22, 2018

oxij commented Nov 22, 2018

7c6f434c commented Nov 22, 2018

oxij commented Nov 22, 2018

7c6f434c commented Nov 22, 2018

edolstra commented Nov 22, 2018

oxij commented Nov 22, 2018 via email

oxij commented Nov 22, 2018 via email

7c6f434c commented Nov 22, 2018 via email

oxij commented Nov 22, 2018 via email

danbst commented Jan 24, 2019

oxij commented Jan 25, 2019 via email

danbst commented Jan 25, 2019

mmahut commented Aug 18, 2019

stale bot commented Jun 1, 2020

stale bot commented Jun 5, 2021

Artturin commented Jan 16, 2023

pkgs: introduce package-list.nix and trivialPackages stage #50643

pkgs: introduce package-list.nix and trivialPackages stage #50643

Conversation

oxij commented Nov 18, 2018

oxij commented Nov 18, 2018

oxij commented Nov 22, 2018

oxij commented Nov 22, 2018

7c6f434c commented Nov 22, 2018

oxij commented Nov 22, 2018

7c6f434c commented Nov 22, 2018

edolstra commented Nov 22, 2018

oxij commented Nov 22, 2018 via email

oxij commented Nov 22, 2018 via email

7c6f434c commented Nov 22, 2018 via email

oxij commented Nov 22, 2018 via email

danbst commented Jan 24, 2019

oxij commented Jan 25, 2019 via email

danbst commented Jan 25, 2019

mmahut commented Aug 18, 2019

stale bot commented Jun 1, 2020

stale bot commented Jun 5, 2021

Artturin commented Jan 16, 2023

pkgs: introduce `package-list.nix` and `trivialPackages` stage #50643

pkgs: introduce `package-list.nix` and `trivialPackages` stage #50643