Lazy paths #11367

roberth · 2024-08-25T08:27:27Z

(This is #10252, but as an upstream branch for easier collaboration)

Motivation

Improve performance, and make the fetchTree interface more capable while keeping it clean.

Description

This makes fetchTree return lazy InputAccessor-based SourcePaths instead of "cowardly" fetching them to the store and returning absolute "system" paths.
It stays close to existing path semantics, including support for readFile "${toString p}/..", which some expressions rely on.
It does not go as far as lazy-trees, but judging from the amount of change I could reuse, and how little of my own I had to add, lazy-trees will be a natural extension of this PR.

Done:

Packages and NixOS evaluate as usual
- no hash changes, based on my limited testing
Flake is not added to store unless e.g.
- "${flake.outPath}"
- uses module system (needs clever lazy source strings, or a change to the module system)
Note that the above is already an improvement over the status quo - always fetching to the store
iirc 0.1s reduction on nixpkgs#hello, and 1s reduction on nixosTests.simple

Conclusion so far:
Viable

TODO:

Check the TODO and FIXMEs
Update the test suite
- probably some intended behavior change, such as removal of narHash in some observable places
  - do we want to do that now? not sure
- possibly finds a bug
Check performance again
Cherry-pick the toString path behavior to lazy-trees
Run flake-regressions in full to assess the breakage of changing (fetchTree x).outPath
Run flake-regressions in full to assess the breakage of changing flake.outPath

Context

Priorities and Process

Add 👍 to pull requests you find important.

The Nix maintainer team uses a GitHub project board to schedule and track reviews.

Co-authored-by: Eelco Dolstra <[email protected]> Co-authored-by: Robert Hensing <[email protected]>

This fixes the double copy problem and improves performance for expressions that don't force the whole source to be added to the store. Rules for fast expressions: - Use path literals where possible - import ./foo.nix - Use + operator with slash in string - src = fetchTree foo + "/src"; - Use source filtering, lib.fileset - AVOID toString - If possible, AVOID interpolations ("${./.}") - If possible, move slashes into the interpolation to add less to the store - "${./src}/foo" -> "${./src/foo}" toString may be improved later as part of lazy-trees, so these recommendations are a snapshot. Path values are quite nice though.

This allows clever editors/IDEs to discern the path more easily for Ctrl+Click navigate to functionality, e.g. when building .?ref=HEAD

This showPath is getting a little too ad hoc, but it works for now.

#10252 (comment)

A string is only allowed to create one path component; containing no slashes.

tomberek · 2024-08-26T04:45:39Z

PosixAccessor calls assertNoSymlinks too often?

[ RUN      ] PrimOpTest.unsafeGetAttrPos
unknown file: Failure
C++ exception with description "error:
       … while calling the 'unsafeGetAttrPos' builtin
         at «string»:1:1:
            1| builtins.unsafeGetAttrPos "y" (import <nix/foo.nix>)
             | ^

       … while calling the 'import' builtin
         at «string»:1:32:
            1| builtins.unsafeGetAttrPos "y" (import <nix/foo.nix>)
             |                                ^

       (stack trace truncated; use '--show-trace' to show the full, detailed trace)

       error: path '/home/tom/.nix-defexpr/channels' is a symlink" thrown in the test body.

[  FAILED  ] PrimOpTest.unsafeGetAttrPos (0 ms)

findFile prefix/suffix separator is ":" or "/"

+(lang.sh:61) bash lang/eval-fail-derivation-name.postprocess lang/eval-fail-derivation-name
--- lang/eval-fail-derivation-name.err.exp      2024-08-25 17:27:55.317195410 -0400
+++ lang/eval-fail-derivation-name.err  2024-08-26 00:34:52.148460570 -0400
@@ -1,20 +1,20 @@
 error:
        … while evaluating the attribute 'outPath'
-         at <nix/derivation-internal.nix>:<number>:<number>:
+         at <nix:derivation-internal.nix>:<number>:<number>:
      <number>|       value = commonAttrs // {
      <number>|         outPath = builtins.getAttr outputName strict;
              |         ^
      <number>|         drvPath = strict.drvPath;

        … while calling the 'getAttr' builtin
-         at <nix/derivation-internal.nix>:<number>:<number>:
+         at <nix:derivation-internal.nix>:<number>:<number>:
      <number>|       value = commonAttrs // {
      <number>|         outPath = builtins.getAttr outputName strict;
              |                   ^
      <number>|         drvPath = strict.drvPath;

        … while calling the 'derivationStrict' builtin
-         at <nix/derivation-internal.nix>:<number>:<number>:
+         at <nix:derivation-internal.nix>:<number>:<number>:
      <number>|
      <number>|   strict = derivationStrict drvAttrs;
              |            ^

SourceAccessor is relative to root not to absolute path

++(toString-path.sh:8) nix eval --raw --impure --expr 'builtins.readFile (builtins.toString (builtins.fetchTree { type = "path"; path = "/tmp/nix-shell.yXAp0S/nix-test/toString-path/foo"; } + "/bar"))'
error:
       … while calling the 'readFile' builtin
         at «string»:1:1:
            1| builtins.readFile (builtins.toString (builtins.fetchTree { type = "path"; path = "/tmp/nix-shell.yXAp0S/nix-test/toString-path/foo"; } + "/bar"))
             | ^

       error: opening file '/bar': No such file or directory
+(toString-path.sh:8) [[ '' = bla ]]
++(toString-path.sh:8) onError
++(/home/tom/nix/lazy-paths/build/src/nix-functional-tests/common/functions.sh:237) set +x
toString-path.sh: test failed at:
  main in toString-path.sh:8
――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――

117/183 nix-functional-tests:main / read-only-store

Absolute path is used relative to root

++(toString-path.sh:8) nix eval --raw --impure --expr 'builtins.readFile (builtins.toString (builtins.fetchTree { type = "path"; path = "/tmp/nix-shell.yXAp0S/nix-test/toString-path/foo"; } + "/bar"))'
error:
       … while calling the 'readFile' builtin
         at «string»:1:1:
            1| builtins.readFile (builtins.toString (builtins.fetchTree { type = "path"; path = "/tmp/nix-shell.yXAp0S/nix-test/toString-path/foo"; } + "/bar"))
             | ^

       error: opening file '/bar': No such file or directory
+(toString-path.sh:8) [[ '' = bla ]]
++(toString-path.sh:8) onError
++(/home/tom/nix/lazy-paths/build/src/nix-functional-tests/common/functions.sh:237) set +x
toString-path.sh: test failed at:
  main in toString-path.sh:8
――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――

117/183 nix-functional-tests:main / read-only-store

tomberek · 2024-08-26T05:22:34Z

Unable to find input at specified path

+(follow-paths.sh:80) nix flake metadata /tmp/nix-shell.yXAp0S/nix-test/flakes/follow-paths/follows/flakeA
warning: Git tree '/tmp/nix-shell.yXAp0S/nix-test/flakes/follow-paths/follows/flakeA' is dirty
error:
       … while updating the lock file of flake 'git+file:///tmp/nix-shell.yXAp0S/nix-test/flakes/follow-paths/follows/flakeA'

       … while updating the flake input 'B'

       … while fetching the input 'path:./flakeB'

       error: path 'flakeB' does not exist
++(follow-paths.sh:80) onError
++(/home/tom/nix/lazy-paths/build/src/nix-functional-tests/common/functions.sh:237) set +x
follow-paths.sh: test failed at:
  main in follow-paths.sh:80

roberth · 2024-08-26T08:41:06Z

PosixAccessor calls assertNoSymlinks too often?

error: path '/home/tom/.nix-defexpr/channels' is a symlink" thrown in the test body.

This is probably the right behavior for a SourceAccessor, but we shouldn't use a SourceAccessor directly, or not its low level methods or not for this purpose.

nixos-discourse · 2024-08-28T19:04:57Z

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/2024-08-26-nix-team-meeting-minutes-172/51300/1

nixos-discourse · 2024-09-25T11:15:10Z

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/how-are-flake-dependencies-fetched/52638/8

tomberek · 2024-10-07T13:59:06Z

Next steps:

rebase
resolve remaining test failures
flake regression suite
long-term testing, rc? Or at beginning of release cycle.
announce in Discourse to get testers and review.

nixos-discourse · 2024-10-07T14:40:35Z

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/2024-10-07-nix-team-meeting-minutes-184/54046/1

nixos-discourse · 2024-10-28T23:09:11Z

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/nix-team-member-suggests-removing-flakes-data-suggest-it-isnt-a-good-idea-please-remove-the-experimental-flag-instead/54959/113

tomberek and others added 8 commits August 23, 2024 23:52

fetchTree: Return a path instead of a store path

50b00b0

Co-authored-by: Eelco Dolstra <[email protected]> Co-authored-by: Robert Hensing <[email protected]>

fixup: remove FlakeCache

b79c918

SourceAccessor: insert colon after prefix

5def18e

This allows clever editors/IDEs to discern the path more easily for Ctrl+Click navigate to functionality, e.g. when building .?ref=HEAD

Fix evalState::rootFS paths' to_string()

f3d2a16

This showPath is getting a little too ad hoc, but it works for now.

Fix nix flake init eval for path value

f13dc79

WIP fix baseNameOf (needs test maybe)

52c0ef2

#10252 (comment)

fix findFile assertion failure

1008847

A string is only allowed to create one path component; containing no slashes.

github-actions bot added new-cli Relating to the "nix" command with-tests Issues related to testing. PRs with tests have some priority fetching Networking with the outside (non-Nix) world, input locking labels Aug 25, 2024

This was referenced Aug 25, 2024

Lazy fetchTree outPath path values #10252

Closed

libgit2, GitRepo: Write (thin) packfiles #11330

Merged

tests: nix:derivation-internal.nix renders with a scheme now

68a5cfd

fixup: Re-enable import .drv code

359d124

roberth mentioned this pull request Sep 7, 2024

Fetch Git submodules lazily #11425

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Lazy paths #11367

Lazy paths #11367

roberth commented Aug 25, 2024 •

edited

Loading

tomberek commented Aug 26, 2024

tomberek commented Aug 26, 2024

roberth commented Aug 26, 2024

nixos-discourse commented Aug 28, 2024

nixos-discourse commented Sep 25, 2024

tomberek commented Oct 7, 2024

nixos-discourse commented Oct 7, 2024

nixos-discourse commented Oct 28, 2024

Lazy paths #11367

Are you sure you want to change the base?

Lazy paths #11367

Conversation

roberth commented Aug 25, 2024 • edited Loading

Motivation

Description

Context

Priorities and Process

tomberek commented Aug 26, 2024

PosixAccessor calls assertNoSymlinks too often?

findFile prefix/suffix separator is ":" or "/"

SourceAccessor is relative to root not to absolute path

Absolute path is used relative to root

tomberek commented Aug 26, 2024

Unable to find input at specified path

roberth commented Aug 26, 2024

nixos-discourse commented Aug 28, 2024

nixos-discourse commented Sep 25, 2024

tomberek commented Oct 7, 2024

nixos-discourse commented Oct 7, 2024

nixos-discourse commented Oct 28, 2024

roberth commented Aug 25, 2024 •

edited

Loading