Skip to content

Commit 3fddd14

Browse files
committed
Only try to substitute input if fetching from its original location fails
Previously we always tried to substitute first (by calling `ensurePath()`). This wasn't a problem before lazy trees, because we always end up copying to the store anyway. But that's not the case with lazy trees. So frequently we ended up substituting an input that we had already fetched. This showed up as fetching source from https://cache.nixos.org for inputs that you could swear Nix had already fetched just before. This was especially a problem for Nixpkgs inputs, since many Nixpkgs revisions are in cache.nixos.org. Note that this could also be a problem without lazy trees, e.g. with a local input (like a Nixpkgs clone) that happens to be in the binary cache. So we now only try substitution as a last resort, if we cannot fetch the input normally.
1 parent 3ccbe84 commit 3fddd14

File tree

4 files changed

+77
-32
lines changed

4 files changed

+77
-32
lines changed

src/libfetchers/fetchers.cc

Lines changed: 44 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -327,47 +327,59 @@ std::pair<ref<SourceAccessor>, Input> Input::getAccessorUnchecked(ref<Store> sto
327327
if (!scheme)
328328
throw Error("cannot fetch unsupported input '%s'", attrsToJSON(toAttrs()));
329329

330-
/* The tree may already be in the Nix store, or it could be
331-
substituted (which is often faster than fetching from the
332-
original source). So check that. We only do this for final
333-
inputs, otherwise there is a risk that we don't return the
334-
same attributes (like `lastModified`) that the "real" fetcher
335-
would return.
336-
337-
FIXME: add a setting to disable this.
338-
FIXME: substituting may be slower than fetching normally,
339-
e.g. for fetchers like Git that are incremental!
340-
*/
341-
if (isFinal() && getNarHash()) {
342-
try {
343-
auto storePath = computeStorePath(*store);
344-
345-
store->ensurePath(storePath);
330+
std::optional<StorePath> storePath;
331+
if (isFinal() && getNarHash())
332+
storePath = computeStorePath(*store);
346333

347-
debug("using substituted/cached input '%s' in '%s'", to_string(), store->printStorePath(storePath));
334+
auto makeStoreAccessor = [&]() -> std::pair<ref<SourceAccessor>, Input> {
335+
auto accessor = make_ref<SubstitutedSourceAccessor>(makeStorePathAccessor(store, *storePath));
348336

349-
auto accessor = make_ref<SubstitutedSourceAccessor>(makeStorePathAccessor(store, storePath));
337+
accessor->fingerprint = getFingerprint(store);
350338

351-
accessor->fingerprint = getFingerprint(store);
339+
// FIXME: ideally we would use the `showPath()` of the
340+
// "real" accessor for this fetcher type.
341+
accessor->setPathDisplay("«" + to_string() + "»");
352342

353-
// FIXME: ideally we would use the `showPath()` of the
354-
// "real" accessor for this fetcher type.
355-
accessor->setPathDisplay("«" + to_string() + "»");
343+
return {accessor, *this};
344+
};
356345

357-
return {accessor, *this};
358-
} catch (Error & e) {
359-
debug("substitution of input '%s' failed: %s", to_string(), e.what());
360-
}
346+
/* If a tree with the expected hash is already in the Nix store,
347+
reuse it. We only do this for final inputs, since otherwise
348+
there is a risk that we don't return the same attributes (like
349+
`lastModified`) that the "real" fetcher would return. */
350+
if (storePath && store->isValidPath(*storePath)) {
351+
debug("using input '%s' in '%s'", to_string(), store->printStorePath(*storePath));
352+
return makeStoreAccessor();
361353
}
362354

363-
auto [accessor, result] = scheme->getAccessor(store, *this);
355+
try {
356+
auto [accessor, result] = scheme->getAccessor(store, *this);
364357

365-
if (!accessor->fingerprint)
366-
accessor->fingerprint = result.getFingerprint(store);
367-
else
368-
result.cachedFingerprint = accessor->fingerprint;
358+
if (!accessor->fingerprint)
359+
accessor->fingerprint = result.getFingerprint(store);
360+
else
361+
result.cachedFingerprint = accessor->fingerprint;
369362

370-
return {accessor, std::move(result)};
363+
return {accessor, std::move(result)};
364+
} catch (Error & e) {
365+
if (storePath) {
366+
// Fall back to substitution.
367+
try {
368+
store->ensurePath(*storePath);
369+
warn(
370+
"Successfully substituted input '%s' after failing to fetch it from its original location: %s",
371+
to_string(),
372+
e.info().msg);
373+
return makeStoreAccessor();
374+
}
375+
// Ignore any substitution error, rethrow the original error.
376+
catch (Error & e2) {
377+
debug("substitution of input '%s' failed: %s", to_string(), e2.info().msg);
378+
} catch (...) {
379+
}
380+
}
381+
throw;
382+
}
371383
}
372384

373385
Input Input::applyOverrides(std::optional<std::string> ref, std::optional<Hash> rev) const

src/nix/main.cc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ static void disableNet()
9090
{
9191
// FIXME: should check for command line overrides only.
9292
if (!settings.useSubstitutes.overridden)
93+
// FIXME: should not disable local substituters (like file:///).
9394
settings.useSubstitutes = false;
9495
if (!settings.tarballTtl.overridden)
9596
settings.tarballTtl = std::numeric_limits<unsigned int>::max();

tests/functional/flakes/meson.build

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ suites += {
3535
'old-lockfiles.sh',
3636
'trace-ifd.sh',
3737
'build-time-flake-inputs.sh',
38+
'substitution.sh',
3839
],
3940
'workdir' : meson.current_source_dir(),
4041
}
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
#! /usr/bin/env bash
2+
3+
# Test that inputs are substituted if they cannot be fetched from their original location.
4+
5+
source ./common.sh
6+
7+
if [[ $(nix config show lazy-trees) = true ]]; then
8+
exit 0
9+
fi
10+
11+
TODO_NixOS
12+
13+
createFlake1
14+
createFlake2
15+
16+
nix build --no-link "$flake2Dir#bar"
17+
18+
path1="$(nix flake metadata --json "$flake1Dir" | jq -r .path)"
19+
20+
# Building after an input disappeared should succeed, because it's still in the Nix store.
21+
mv "$flake1Dir" "$flake1Dir-tmp"
22+
nix build --no-link "$flake2Dir#bar" --no-eval-cache
23+
24+
# Check that Nix will fall back to fetching the input from a substituter.
25+
cache="file://$TEST_ROOT/binary-cache"
26+
nix copy --to "$cache" "$path1"
27+
clearStore
28+
nix build --no-link "$flake2Dir#bar" --no-eval-cache --substitute --substituters "$cache"
29+
30+
clearStore
31+
expectStderr 1 nix build --no-link "$flake2Dir#bar" --no-eval-cache | grepQuiet "The path.*does not exist"

0 commit comments

Comments
 (0)