Source path error improvements by edolstra · Pull Request #12915 · NixOS/nix

edolstra · 2025-04-03T12:55:53Z

Motivation

Based on #12532 but heavily modified.

We now "mount" the source accessors of flake inputs on top of /nix/store (i.e. the MountedSourceAccessor named storeFS). This allows error messages similar to the lazy-trees branch, i.e. referring to the original source trees (like a Git repo) instead of the store path to which the input was copied.

For example, instead of

error: attribute 'foobar' missing
at /nix/store/ddzfiipzqlrh3gnprmqbadnsnrxsmc9i-source/machine/configuration.nix:209:7:
   208|
   209|       pkgs.foobar
      |       ^
   210|     ];

you now get

error: attribute 'foobar' missing
at /home/eelco/Misc/eelco-configurations/machine/configuration.nix:209:7:
   208|
   209|       pkgs.foobar
      |       ^
   210|     ];

Context

Add 👍 to pull requests you find important.

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

Mic92 · 2025-04-03T13:05:15Z

Cool. Could you write a changelog message for this? This is a major UX improvement.

src/libexpr/eval.cc

roberth · 2025-04-05T15:25:58Z

src/libexpr/eval.cc

+        // Backward compatibility hack: throw an exception if access
+        // to this path is not allowed.


Why is this a hack?

Yeah where is FilteringSourceAccessor even being used anymore? I only see it used with GitExportIgnoreSourceAccessor

roberth · 2025-04-05T15:26:07Z

src/libexpr/eval.cc

+            // Backward compatibility hack: throw an exception if access
+            // to this path is not allowed.


Why is this a hack?

roberth · 2025-04-05T15:29:34Z

src/libflake/flake/flake.cc

    auto storePath = fetchToStore(*state.store, accessor, FetchMode::Copy, input.getName());

-    state.allowPath(storePath);
+    state.allowPath(storePath); // FIXME: should just whitelist the entire virtual store


Not a bug:

Suggested change

state.allowPath(storePath); // FIXME: should just whitelist the entire virtual store

state.allowPath(storePath); // TODO: we could simply whitelist the entire virtual store, because all of that is created by the evaluator and its allowed external sources

roberth · 2025-04-05T15:51:34Z

In fact, we don't even need to copy the inputs to the store at all, so
this gets us very close to lazy trees. We just need to know the store
path so that requires hashing the entire input, which isn't lazy. But
the next step will be to use a virtual store path that gets rewritten
to the actual store path only when needed.

This could use some clarification so that others can follow.
We have multiple operations or optimizations that may be called "lazy":

lazy copying: copy as little to the store as possible
lazy evaluation and lazy locking: completely ignore an input if possible (already somewhat possible, and improving with ~~Make nix flake metadata|update|lock lazy #12432~~, Reuse input lock files #7730)
lazy outPath: use input metadata without fetching anything (e.g. (fetchTree foo).rev) probably already provided by call-flake.nix.
fetch as little as possible (we don't call this laziness, but someone might, and that'd be confusing)
incremental fetching: e.g. Git submodules and LFS only on access
lazy hashing: hash as little as possible, avoiding read operations from local cache

What you're suggesting as the next step is to provide lazy copying.
Lazy hashing and incremental fetching would be part of #11367 or #6530.

I hope I'm not adding to the confusion.

Ericson2314 · 2025-04-07T20:46:05Z

I suggest we do this in conjunction with #12531

Ericson2314 · 2025-04-09T21:39:29Z

I changed #12531 as discussed in the meeting, and then rebased this on top in https://github.com/Ericson2314/nix/tree/source-path-errors

This makes paths in error messages behave similar to lazy-trees, e.g. instead of store paths like error: attribute 'foobar' missing at /nix/store/ddzfiipzqlrh3gnprmqbadnsnrxsmc9i-source/machine/configuration.nix:209:7: 208| 209| pkgs.foobar | ^ 210| ]; you now get error: attribute 'foobar' missing at /home/eelco/Misc/eelco-configurations/machine/configuration.nix:209:7: 208| 209| pkgs.foobar | ^ 210| ]; In fact, we don't even need to copy the inputs to the store at all, so this gets us very close to lazy trees. We just need to know the store path so that requires hashing the entire input, which isn't lazy. But the next step will be to use a virtual store path that gets rewritten to the actual store path only when needed.

Mic92 · 2025-04-23T10:30:35Z

@edolstra I assume this was closed by accident?

xokdvium · 2025-10-24T23:05:49Z

This has been implemented in #14050 and released in 2.32.

This has already been implemented in 1e70955 as a side-effect of mounting the accessors in storeFS. Let's test this so it doesn't regress. (cherry-picked from #12915)

xokdvium · 2025-10-24T23:15:43Z

Cherry-picked the tests in #14352

nixos-discourse · 2025-10-24T23:34:10Z

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

https://discourse.nixos.org/t/nix-2-32-0-released/70528/9

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 Apr 3, 2025

This was referenced Apr 3, 2025

Source path error improvements DeterminateSystems/nix-src#20

Merged

Make rootFS's showPath() render the paths from the original accessors #12532

Closed

Mic92 reviewed Apr 3, 2025

View reviewed changes

src/libexpr/eval.cc Show resolved Hide resolved

edolstra force-pushed the source-path-errors branch from ffd65d7 to 8513598 Compare April 3, 2025 15:07

roberth reviewed Apr 5, 2025

View reviewed changes

Mic92 added this to Nix team Apr 5, 2025

github-project-automation bot moved this to To triage in Nix team Apr 5, 2025

roberth mentioned this pull request Apr 9, 2025

Don't copy nix store path to nix store #11229

Closed

edolstra added 4 commits April 10, 2025 10:58

Add FIXME

092f222

Fix missing file error messages from 'import'

f63dc71

Restore 'forbidden in restricted mode' errors

addec3c

edolstra force-pushed the source-path-errors branch from 8513598 to addec3c Compare April 10, 2025 09:00

Merge remote-tracking branch 'origin/master' into source-path-errors

6de45c1

edolstra closed this Apr 23, 2025

edolstra deleted the source-path-errors branch April 23, 2025 10:29

edolstra restored the source-path-errors branch April 23, 2025 10:29

Mic92 reopened this Apr 23, 2025

edolstra mentioned this pull request May 17, 2025

Lazy trees v2 #13225

Open

tomberek mentioned this pull request Jun 4, 2025

Add option to include untracked files in git and mercurial fetchers #9352

Open

xokdvium closed this Oct 24, 2025

xokdvium pushed a commit that referenced this pull request Oct 24, 2025

tests/functional: Add source-paths tests

1cd8458

This has already been implemented in 1e70955 as a side-effect of mounting the accessors in storeFS. Let's test this so it doesn't regress. (cherry-picked from #12915)

xokdvium mentioned this pull request Oct 24, 2025

tests/functional: Add source-paths tests #14352

Merged

		// Backward compatibility hack: throw an exception if access
		// to this path is not allowed.

	state.allowPath(storePath); // FIXME: should just whitelist the entire virtual store
	state.allowPath(storePath); // TODO: we could simply whitelist the entire virtual store, because all of that is created by the evaluator and its allowed external sources

Uh oh!

Comments

Conversation

edolstra commented Apr 3, 2025

Motivation

Context

Uh oh!

Mic92 commented Apr 3, 2025

Uh oh!

Uh oh!

roberth Apr 5, 2025

Choose a reason for hiding this comment

Uh oh!

Ericson2314 Apr 10, 2025

Choose a reason for hiding this comment

Uh oh!

roberth Apr 5, 2025

Choose a reason for hiding this comment

Uh oh!

roberth Apr 5, 2025

Choose a reason for hiding this comment

Uh oh!

roberth commented Apr 5, 2025

Uh oh!

Ericson2314 commented Apr 7, 2025

Uh oh!

Ericson2314 commented Apr 9, 2025

Uh oh!

Mic92 commented Apr 23, 2025

Uh oh!

xokdvium commented Oct 24, 2025

Uh oh!

xokdvium commented Oct 24, 2025

Uh oh!

nixos-discourse commented Oct 24, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants