Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Restrict builtins.isStorePath #5973

Open
edolstra opened this issue Jan 24, 2022 · 2 comments · May be fixed by #6530
Open

Restrict builtins.isStorePath #5973

edolstra opened this issue Jan 24, 2022 · 2 comments · May be fixed by #6530

Comments

@edolstra
Copy link
Member

The use of builtins.isStorePath can cause evaluation to diverge between Nix expressions in the Nix store versus those outside, see e.g. NixOS/nixpkgs#153594 (comment). 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
Copy link
Member

roberth commented Jan 24, 2022

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 NixOS/nixpkgs#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
Copy link
Contributor

Related? #5868

@edolstra edolstra modified the milestones: nix-2.7, nix-2.8 Mar 3, 2022
@edolstra edolstra modified the milestones: nix-2.8, nix-2.9 Apr 14, 2022
@edolstra edolstra modified the milestones: nix-2.9, nix-2.10 May 27, 2022
@edolstra edolstra modified the milestones: nix-2.10, nix-2.11 Jul 11, 2022
@edolstra edolstra modified the milestones: nix-2.11, nix-2.12 Aug 25, 2022
@edolstra edolstra modified the milestones: nix-2.12, nix-2.13 Dec 6, 2022
@edolstra edolstra modified the milestones: nix-2.13, nix-2.14 Jan 17, 2023
@edolstra edolstra modified the milestones: nix-2.14, nix-2.15 Feb 28, 2023
@edolstra edolstra modified the milestones: nix-2.15, nix-2.16 May 26, 2023
@edolstra edolstra modified the milestones: nix-2.16, nix-2.17 May 31, 2023
@edolstra edolstra modified the milestones: nix-2.17, nix-2.18 Jul 24, 2023
@edolstra edolstra modified the milestones: nix-2.18, nix-2.19 Sep 20, 2023
@roberth roberth linked a pull request Nov 8, 2023 that will close this issue
9 tasks
@edolstra edolstra modified the milestones: nix-2.19, nix-2.20 Nov 20, 2023
@edolstra edolstra modified the milestones: nix-2.20, nix-2.21 Mar 4, 2024
@edolstra edolstra modified the milestones: nix-2.21, nix-2.22 Mar 11, 2024
@edolstra edolstra modified the milestones: nix-2.22, nix-2.23 Apr 23, 2024
@edolstra edolstra removed their assignment Apr 26, 2024
@edolstra edolstra removed this from the nix-2.23 milestone Jun 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants