NixOS / nix

Nix, the purely functional package manager
https://nixos.org/
GNU Lesser General Public License v2.1
12.63k stars 1.51k forks source link

Restrict builtins.isStorePath #5973

Open edolstra opened 2 years ago

edolstra commented 2 years ago

The use of builtins.isStorePath can cause evaluation to diverge between Nix expressions in the Nix store versus those outside, see e.g. https://github.com/NixOS/nixpkgs/pull/153594#issuecomment-1020530593. Perhaps builtins.isStorePath should return false for flake source paths, even if they happen to be stored in the Nix store (which is arguably an implementation detail). In fact this will probably happen due to #3121 because we won't copy flakes to the store unless required.

roberth commented 2 years ago

A naive implementation of this will cause a lot of useless store- and nix-daemon I/O, because NixOS needs a copy of the nixpkgs repo contents to be in the store. Let's not rush this, as this problem is not a regression, but its solution could cause performance regressions. Perhaps for https://github.com/NixOS/nixpkgs/pull/153594, a good solution is to

dir:
  mkDerivation {
    src = builtins.path { name = source; path = pkgs.path + "/${dir}"; };
    buildPhase = "<filterIntoStore's logic as a script copying to $out>";
    __contentAddressed = true;
  }

This combination looks to be quite efficient. NixOS can cache its docs generation with few unnecessary rebuilds. When the Nixpkgs are necessarily in the store (a flake input for example), the docs generation works without hashing any nixpkgs subtree, without running any source filter and without copying any sources to the daemon. When the Nixpkgs files aren't in the store, because it is a #3121 local flake or a legacy build, only the relevant parts of the tree are copied and they're still filtered down to relevant files to avoid rebuilds using CA derivations.

tomberek commented 2 years ago

Related? https://github.com/NixOS/nix/issues/5868