Skip to content

Commit 1d13049

Browse files
edolstraEricson2314
authored andcommitted
Mount inputs on storeFS to restore fetchToStore() caching
fetchToStore() caching was broken because it uses the fingerprint of the accessor, but now that the accessor (typically storeFS) is a composite (like MountedSourceAccessor or AllowListSourceAccessor), there was no fingerprint anymore. So fetchToStore now uses the new getFingerprint() method to get the specific fingerprint for the subpath.
1 parent ec6d5c7 commit 1d13049

File tree

8 files changed

+70
-51
lines changed

8 files changed

+70
-51
lines changed

src/libexpr/eval.cc

Lines changed: 11 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -227,24 +227,17 @@ EvalState::EvalState(
227227
{CanonPath(store->storeDir), store->getFSAccessor(settings.pureEval)},
228228
}))
229229
, rootFS([&] {
230-
auto accessor = [&]() -> decltype(rootFS) {
231-
/* In pure eval mode, we provide a filesystem that only
232-
contains the Nix store. */
233-
if (settings.pureEval)
234-
return storeFS;
235-
236-
/* If we have a chroot store and pure eval is not enabled,
237-
use a union accessor to make the chroot store available
238-
at its logical location while still having the underlying
239-
directory available. This is necessary for instance if
240-
we're evaluating a file from the physical /nix/store
241-
while using a chroot store. */
242-
auto realStoreDir = dirOf(store->toRealPath(StorePath::dummy));
243-
if (store->storeDir != realStoreDir)
244-
return makeUnionSourceAccessor({getFSSourceAccessor(), storeFS});
245-
246-
return getFSSourceAccessor();
247-
}();
230+
/* In pure eval mode, we provide a filesystem that only
231+
contains the Nix store.
232+
233+
Otherwise, use a union accessor to make the augmented store
234+
available at its logical location while still having the
235+
underlying directory available. This is necessary for
236+
instance if we're evaluating a file from the physical
237+
/nix/store while using a chroot store, and also for lazy
238+
mounted fetchTree. */
239+
auto accessor = settings.pureEval ? storeFS.cast<SourceAccessor>()
240+
: makeUnionSourceAccessor({getFSSourceAccessor(), storeFS});
248241

249242
/* Apply access control if needed. */
250243
if (settings.restrictEval || settings.pureEval)

src/libexpr/include/nix/expr/eval.hh

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ class Store;
4242
namespace fetchers {
4343
struct Settings;
4444
struct InputCache;
45+
struct Input;
4546
} // namespace fetchers
4647
struct EvalSettings;
4748
class EvalState;
@@ -514,6 +515,11 @@ public:
514515

515516
void checkURI(const std::string & uri);
516517

518+
/**
519+
* Mount an input on the Nix store.
520+
*/
521+
StorePath mountInput(fetchers::Input & input, const fetchers::Input & originalInput, ref<SourceAccessor> accessor);
522+
517523
/**
518524
* Parse a Nix expression from the specified file.
519525
*/

src/libexpr/paths.cc

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
#include "nix/store/store-api.hh"
22
#include "nix/expr/eval.hh"
3+
#include "nix/util/mounted-source-accessor.hh"
4+
#include "nix/fetchers/fetch-to-store.hh"
35

46
namespace nix {
57

@@ -18,4 +20,27 @@ SourcePath EvalState::storePath(const StorePath & path)
1820
return {rootFS, CanonPath{store->printStorePath(path)}};
1921
}
2022

23+
StorePath
24+
EvalState::mountInput(fetchers::Input & input, const fetchers::Input & originalInput, ref<SourceAccessor> accessor)
25+
{
26+
auto storePath = fetchToStore(fetchSettings, *store, accessor, FetchMode::Copy, input.getName());
27+
28+
allowPath(storePath); // FIXME: should just whitelist the entire virtual store
29+
30+
storeFS->mount(CanonPath(store->printStorePath(storePath)), accessor);
31+
32+
auto narHash = store->queryPathInfo(storePath)->narHash;
33+
input.attrs.insert_or_assign("narHash", narHash.to_string(HashFormat::SRI, true));
34+
35+
if (originalInput.getNarHash() && narHash != *originalInput.getNarHash())
36+
throw Error(
37+
(unsigned int) 102,
38+
"NAR hash mismatch in input '%s', expected '%s' but got '%s'",
39+
originalInput.to_string(),
40+
narHash.to_string(HashFormat::SRI, true),
41+
originalInput.getNarHash()->to_string(HashFormat::SRI, true));
42+
43+
return storePath;
44+
}
45+
2146
} // namespace nix

src/libexpr/primops/fetchTree.cc

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
#include "nix/util/url.hh"
1111
#include "nix/expr/value-to-json.hh"
1212
#include "nix/fetchers/fetch-to-store.hh"
13+
#include "nix/fetchers/input-cache.hh"
1314

1415
#include <nlohmann/json.hpp>
1516

@@ -218,11 +219,11 @@ static void fetchTree(
218219
throw Error("input '%s' is not allowed to use the '__final' attribute", input.to_string());
219220
}
220221

221-
auto [storePath, input2] = input.fetchToStore(state.store);
222+
auto cachedInput = state.inputCache->getAccessor(state.store, input, fetchers::UseRegistries::No);
222223

223-
state.allowPath(storePath);
224+
auto storePath = state.mountInput(cachedInput.lockedInput, input, cachedInput.accessor);
224225

225-
emitTreeAttrs(state, storePath, input2, v, params.emptyRevFallback, false);
226+
emitTreeAttrs(state, storePath, cachedInput.lockedInput, v, params.emptyRevFallback, false);
226227
}
227228

228229
static void prim_fetchTree(EvalState & state, const PosIdx pos, Value ** args, Value & v)

src/libfetchers/fetch-to-store.cc

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,14 +27,19 @@ StorePath fetchToStore(
2727

2828
std::optional<fetchers::Cache::Key> cacheKey;
2929

30-
if (!filter && path.accessor->fingerprint) {
31-
cacheKey = makeFetchToStoreCacheKey(std::string{name}, *path.accessor->fingerprint, method, path.path.abs());
30+
auto [subpath, fingerprint] = filter ? std::pair<CanonPath, std::optional<std::string>>{path.path, std::nullopt}
31+
: path.accessor->getFingerprint(path.path);
32+
33+
if (fingerprint) {
34+
cacheKey = makeFetchToStoreCacheKey(std::string{name}, *fingerprint, method, subpath.abs());
3235
if (auto res = settings.getCache()->lookupStorePath(*cacheKey, store)) {
3336
debug("store path cache hit for '%s'", path);
3437
return res->storePath;
3538
}
36-
} else
39+
} else {
40+
// FIXME: could still provide in-memory caching keyed on `SourcePath`.
3741
debug("source path '%s' is uncacheable", path);
42+
}
3843

3944
Activity act(
4045
*logger,

src/libfetchers/fetchers.cc

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -356,8 +356,10 @@ std::pair<ref<SourceAccessor>, Input> Input::getAccessorUnchecked(ref<Store> sto
356356

357357
auto [accessor, result] = scheme->getAccessor(store, *this);
358358

359-
assert(!accessor->fingerprint);
360-
accessor->fingerprint = result.getFingerprint(store);
359+
if (!accessor->fingerprint)
360+
accessor->fingerprint = result.getFingerprint(store);
361+
else
362+
result.cachedFingerprint = accessor->fingerprint;
361363

362364
return {accessor, std::move(result)};
363365
}

src/libflake/flake.cc

Lines changed: 11 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -24,21 +24,6 @@ using namespace flake;
2424

2525
namespace flake {
2626

27-
static StorePath copyInputToStore(
28-
EvalState & state, fetchers::Input & input, const fetchers::Input & originalInput, ref<SourceAccessor> accessor)
29-
{
30-
auto storePath = fetchToStore(*input.settings, *state.store, accessor, FetchMode::Copy, input.getName());
31-
32-
state.allowPath(storePath);
33-
34-
auto narHash = state.store->queryPathInfo(storePath)->narHash;
35-
input.attrs.insert_or_assign("narHash", narHash.to_string(HashFormat::SRI, true));
36-
37-
assert(!originalInput.getNarHash() || storePath == originalInput.computeStorePath(*state.store));
38-
39-
return storePath;
40-
}
41-
4227
static void forceTrivialValue(EvalState & state, Value & value, const PosIdx pos)
4328
{
4429
if (value.isThunk() && value.isTrivial())
@@ -360,11 +345,14 @@ static Flake getFlake(
360345
lockedRef = FlakeRef(std::move(cachedInput2.lockedInput), newLockedRef.subdir);
361346
}
362347

363-
// Copy the tree to the store.
364-
auto storePath = copyInputToStore(state, lockedRef.input, originalRef.input, cachedInput.accessor);
365-
366348
// Re-parse flake.nix from the store.
367-
return readFlake(state, originalRef, resolvedRef, lockedRef, state.storePath(storePath), lockRootAttrPath);
349+
return readFlake(
350+
state,
351+
originalRef,
352+
resolvedRef,
353+
lockedRef,
354+
state.storePath(state.mountInput(lockedRef.input, originalRef.input, cachedInput.accessor)),
355+
lockRootAttrPath);
368356
}
369357

370358
Flake getFlake(EvalState & state, const FlakeRef & originalRef, fetchers::UseRegistries useRegistries)
@@ -721,11 +709,10 @@ lockFlake(const Settings & settings, EvalState & state, const FlakeRef & topRef,
721709

722710
auto lockedRef = FlakeRef(std::move(cachedInput.lockedInput), input.ref->subdir);
723711

724-
// FIXME: allow input to be lazy.
725-
auto storePath = copyInputToStore(
726-
state, lockedRef.input, input.ref->input, cachedInput.accessor);
727-
728-
return {state.storePath(storePath), lockedRef};
712+
return {
713+
state.storePath(
714+
state.mountInput(lockedRef.input, input.ref->input, cachedInput.accessor)),
715+
lockedRef};
729716
}
730717
}();
731718

tests/functional/lang/eval-fail-hashfile-missing.err.exp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,4 +10,4 @@ error:
1010

1111
… while calling the 'hashFile' builtin
1212

13-
error: opening file '/pwd/lang/this-file-is-definitely-not-there-7392097': No such file or directory
13+
error: path '/pwd/lang/this-file-is-definitely-not-there-7392097' does not exist

0 commit comments

Comments
 (0)