From 2334f5ffcbe599cfa6e39c51a1123d8e54146377 Mon Sep 17 00:00:00 2001 From: Tom Bereknyei Date: Fri, 23 Aug 2024 23:52:21 -0400 Subject: [PATCH] fetchTree: Return a path instead of a store path Co-authored-by: Eelco Dolstra Co-authored-by: Robert Hensing fixup: remove FlakeCache Make path values lazy 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. SourceAccessor: insert colon after prefix 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() This showPath is getting a little too ad hoc, but it works for now. Fix nix flake init eval for path value WIP fix baseNameOf (needs test maybe) https://github.com/NixOS/nix/pull/10252#issuecomment-2275035429 fix findFile assertion failure A string is only allowed to create one path component; containing no slashes. tests: nix:derivation-internal.nix renders with a scheme now fixup: Re-enable import .drv code --- src/libcmd/common-eval-args.cc | 11 +- src/libexpr/eval-settings.hh | 3 +- src/libexpr/eval.cc | 66 +++++++--- src/libexpr/eval.hh | 9 +- src/libexpr/nixexpr.cc | 2 +- src/libexpr/nixexpr.hh | 11 +- src/libexpr/parser.y | 33 +++-- src/libexpr/primops.cc | 35 ++++-- src/libexpr/primops/fetchTree.cc | 58 +++++++-- src/libfetchers/fetchers.cc | 26 +++- src/libflake/flake/flake.cc | 113 ++++++------------ src/libflake/flake/flake.hh | 2 +- src/libflake/flake/flakeref.cc | 6 +- src/libflake/flake/flakeref.hh | 2 +- src/libutil/posix-source-accessor.cc | 5 + src/libutil/posix-source-accessor.hh | 2 + src/libutil/source-accessor.cc | 8 +- src/libutil/source-accessor.hh | 9 ++ src/nix/flake.cc | 21 ++-- tests/functional/flakes/flakes.sh | 1 - .../lang/eval-fail-derivation-name.err.exp | 6 +- 21 files changed, 270 insertions(+), 159 deletions(-) diff --git a/src/libcmd/common-eval-args.cc b/src/libcmd/common-eval-args.cc index ccbf957d97e..06ec1fa45f9 100644 --- a/src/libcmd/common-eval-args.cc +++ b/src/libcmd/common-eval-args.cc @@ -34,8 +34,7 @@ EvalSettings evalSettings { // FIXME `parseFlakeRef` should take a `std::string_view`. auto flakeRef = parseFlakeRef(fetchSettings, std::string { rest }, {}, true, false); debug("fetching flake search path element '%s''", rest); - auto storePath = flakeRef.resolve(store).fetchTree(store).first; - return store->toRealPath(storePath); + return flakeRef.resolve(store).lazyFetch(store).first; }, }, }, @@ -176,15 +175,15 @@ SourcePath lookupFileArg(EvalState & state, std::string_view s, const Path * bas state.store, state.fetchSettings, EvalSettings::resolvePseudoUrl(s)); - auto storePath = fetchToStore(*state.store, SourcePath(accessor), FetchMode::Copy); - return state.rootPath(CanonPath(state.store->toRealPath(storePath))); + state.registerAccessor(accessor); + return SourcePath(accessor); } else if (hasPrefix(s, "flake:")) { experimentalFeatureSettings.require(Xp::Flakes); auto flakeRef = parseFlakeRef(fetchSettings, std::string(s.substr(6)), {}, true, false); - auto storePath = flakeRef.resolve(state.store).fetchTree(state.store).first; - return state.rootPath(CanonPath(state.store->toRealPath(storePath))); + auto accessor = flakeRef.resolve(state.store).lazyFetch(state.store).first; + return SourcePath(accessor); } else if (s.size() > 2 && s.at(0) == '<' && s.at(s.size() - 1) == '>') { diff --git a/src/libexpr/eval-settings.hh b/src/libexpr/eval-settings.hh index 115e3ee501b..b17d56e558c 100644 --- a/src/libexpr/eval-settings.hh +++ b/src/libexpr/eval-settings.hh @@ -7,6 +7,7 @@ namespace nix { class Store; +struct SourcePath; struct EvalSettings : Config { @@ -22,7 +23,7 @@ struct EvalSettings : Config * @todo Return (`std::optional` of) `SourceAccssor` or something * more structured instead of mere `std::string`? */ - using LookupPathHook = std::optional(ref store, std::string_view); + using LookupPathHook = std::optional(ref store, std::string_view); /** * Map from "scheme" to a `LookupPathHook`. diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index f17753415fc..3938c38854b 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -283,6 +283,7 @@ EvalState::EvalState( { corepkgsFS->setPathDisplay(""); internalFS->setPathDisplay("«nix-internal»", ""); + rootFS->setPathDisplay("/", ""); countCalls = getEnv("NIX_COUNT_CALLS").value_or("0") != "0"; @@ -842,6 +843,11 @@ void Value::mkPath(const SourcePath & path) mkPath(&*path.accessor, makeImmutableString(path.path.abs())); } +void EvalState::registerAccessor(const ref accessor) +{ + sourceAccessors.push_back(accessor); +} + inline Value * EvalState::lookupVar(Env * env, const ExprVar & var, bool noEval) { @@ -1937,10 +1943,22 @@ void EvalState::concatLists(Value & v, size_t nrLists, Value * const * lists, co v.mkList(list); } +// FIXME limit recursion +Value * resolveOutPath(EvalState & state, Value * v, const PosIdx pos) +{ + state.forceValue(*v, pos); + if (v->type() != nAttrs) + return v; + auto found = v->attrs()->find(state.sOutPath); + if (found != v->attrs()->end()) + return resolveOutPath(state, found->value, pos); + return v; +} void ExprConcatStrings::eval(EvalState & state, Env & env, Value & v) { NixStringContext context; + std::shared_ptr accessor; std::vector s; size_t sSize = 0; NixInt n{0}; @@ -1974,8 +1992,9 @@ void ExprConcatStrings::eval(EvalState & state, Env & env, Value & v) Value * vTmpP = values.data(); for (auto & [i_pos, i] : *es) { - Value & vTmp = *vTmpP++; - i->eval(state, env, vTmp); + Value & vTmp0 = *vTmpP++; + i->eval(state, env, vTmp0); + Value & vTmp = *resolveOutPath(state, &vTmp0, i_pos); /* If the first element is a path, then the result will also be a path, we don't copy anything (yet - that's done later, @@ -1983,6 +2002,9 @@ void ExprConcatStrings::eval(EvalState & state, Env & env, Value & v) and none of the strings are allowed to have contexts. */ if (first) { firstType = vTmp.type(); + if (firstType == nPath) { + accessor = vTmp.path().accessor; + } } if (firstType == nInt) { @@ -2029,7 +2051,7 @@ void ExprConcatStrings::eval(EvalState & state, Env & env, Value & v) else if (firstType == nPath) { if (!context.empty()) state.error("a string that refers to a store path cannot be appended to a path").atPos(pos).withFrame(env, *this).debugThrow(); - v.mkPath(state.rootPath(CanonPath(canonPath(str())))); + v.mkPath({ref(accessor), CanonPath(str())}); } else v.mkStringMove(c_str(), context); } @@ -2279,6 +2301,8 @@ BackedStringView EvalState::coerceToString( v.payload.path.path : copyToStore ? store->printStorePath(copyPathToStore(context, v.path())) + : v.path().accessor->toStringReturnsStorePath() + ? store->printStorePath(copyPathToStore(context, SourcePath(v.path().accessor, CanonPath::root))) + v.path().path.absOrEmpty() : std::string(v.path().path.abs()); } @@ -2391,10 +2415,14 @@ SourcePath EvalState::coerceToPath(const PosIdx pos, Value & v, NixStringContext if (v.type() == nPath) return v.path(); - /* Similarly, handle __toString where the result may be a path + /* Similarly, handle outPath and __toString where the result may be a path value. */ if (v.type() == nAttrs) { - auto i = v.attrs()->find(sToString); + auto i = v.attrs()->find(sOutPath); + if (i != v.attrs()->end()) { + return coerceToPath(pos, *i->value, context, errorCtx); + } + i = v.attrs()->find(sToString); if (i != v.attrs()->end()) { Value v1; callFunction(*i->value, v, v1, pos); @@ -3017,12 +3045,14 @@ SourcePath EvalState::findFile(const LookupPath & lookupPath, const std::string_ if (!suffixOpt) continue; auto suffix = *suffixOpt; - auto rOpt = resolveLookupPathPath(i.path); + auto rOpt = resolveLookupPathPath( + i.path, + true); if (!rOpt) continue; auto r = *rOpt; - Path res = suffix == "" ? r : concatStrings(r, "/", suffix); - if (pathExists(res)) return rootPath(CanonPath(canonPath(res))); + auto res = r / CanonPath(suffix); + if (res.pathExists()) return res; } if (hasPrefix(path, "nix/")) @@ -3037,26 +3067,30 @@ SourcePath EvalState::findFile(const LookupPath & lookupPath, const std::string_ } -std::optional EvalState::resolveLookupPathPath(const LookupPath::Path & value0, bool initAccessControl) +std::optional EvalState::resolveLookupPathPath(const LookupPath::Path & value0, bool initAccessControl) { auto & value = value0.s; auto i = lookupPathResolved.find(value); if (i != lookupPathResolved.end()) return i->second; - auto finish = [&](std::string res) { + auto finish = [&](SourcePath res) { debug("resolved search path element '%s' to '%s'", value, res); lookupPathResolved.emplace(value, res); return res; }; + std::optional res; + if (EvalSettings::isPseudoUrl(value)) { try { auto accessor = fetchers::downloadTarball( store, fetchSettings, EvalSettings::resolvePseudoUrl(value)); + // Traditional search path lookups use the absolute path space for + // historical consistency. auto storePath = fetchToStore(*store, SourcePath(accessor), FetchMode::Copy); - return finish(store->toRealPath(storePath)); + res.emplace(rootPath(CanonPath(store->toRealPath(storePath)))); } catch (Error & e) { logWarning({ .msg = HintFmt("Nix search path entry '%1%' cannot be downloaded, ignoring", value) @@ -3075,22 +3109,22 @@ std::optional EvalState::resolveLookupPathPath(const LookupPath::Pa } { - auto path = absPath(value); + auto path = rootPath(value); /* Allow access to paths in the search path. */ if (initAccessControl) { - allowPath(path); - if (store->isInStore(path)) { + allowPath(path.path.abs()); + if (store->isInStore(path.path.abs())) { try { StorePathSet closure; - store->computeFSClosure(store->toStorePath(path).first, closure); + store->computeFSClosure(store->toStorePath(path.path.abs()).first, closure); for (auto & p : closure) allowPath(p); } catch (InvalidPath &) { } } } - if (pathExists(path)) + if (path.pathExists()) return finish(std::move(path)); else { logWarning({ diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh index f7ed6be83af..e999d9d1097 100644 --- a/src/libexpr/eval.hh +++ b/src/libexpr/eval.hh @@ -260,6 +260,9 @@ public: const SourcePath callFlakeInternal; + /* A collection of InputAccessors, just to keep them alive. */ + std::list> sourceAccessors; + /** * Store used to materialise .drv files. */ @@ -342,7 +345,7 @@ private: LookupPath lookupPath; - std::map> lookupPathResolved; + std::map> lookupPathResolved; /** * Cache used by prim_match(). @@ -384,6 +387,8 @@ public: */ SourcePath rootPath(PathView path); + void registerAccessor(ref accessor); + /** * Allow access to a path. */ @@ -449,7 +454,7 @@ public: * * If it is not found, return `std::nullopt` */ - std::optional resolveLookupPathPath( + std::optional resolveLookupPathPath( const LookupPath::Path & elem, bool initAccessControl = false); diff --git a/src/libexpr/nixexpr.cc b/src/libexpr/nixexpr.cc index 063ff07537b..8d48f08a51f 100644 --- a/src/libexpr/nixexpr.cc +++ b/src/libexpr/nixexpr.cc @@ -45,7 +45,7 @@ void ExprString::show(const SymbolTable & symbols, std::ostream & str) const void ExprPath::show(const SymbolTable & symbols, std::ostream & str) const { - str << s; + str << path; } void ExprVar::show(const SymbolTable & symbols, std::ostream & str) const diff --git a/src/libexpr/nixexpr.hh b/src/libexpr/nixexpr.hh index bdf4e214a59..5e8c1d798ea 100644 --- a/src/libexpr/nixexpr.hh +++ b/src/libexpr/nixexpr.hh @@ -135,12 +135,15 @@ struct ExprString : Expr struct ExprPath : Expr { - ref accessor; - std::string s; + const SourcePath path; Value v; - ExprPath(ref accessor, std::string s) : accessor(accessor), s(std::move(s)) + ExprPath(SourcePath && path) + : path(path) { - v.mkPath(&*accessor, this->s.c_str()); + v.mkPath( + &*path.accessor, + // TODO: GC_STRDUP + strdup(path.path.abs().c_str())); } Value * maybeThunk(EvalState & state, Env & env) override; COMMON_METHODS diff --git a/src/libexpr/parser.y b/src/libexpr/parser.y index 944c7b1af31..7172e42a1d4 100644 --- a/src/libexpr/parser.y +++ b/src/libexpr/parser.y @@ -136,6 +136,10 @@ static Expr * makeCall(PosIdx pos, Expr * fn, Expr * arg) { std::vector> * inheritAttrs; std::vector> * string_parts; std::vector>> * ind_string_parts; + struct { + nix::Expr * e; + bool appendSlash; + } pathStart; } %type start expr expr_function expr_if expr_op @@ -149,7 +153,8 @@ static Expr * makeCall(PosIdx pos, Expr * fn, Expr * arg) { %type attrs %type string_parts_interpolated %type ind_string_parts -%type path_start string_parts string_attr +%type path_start +%type string_parts string_attr %type attr %token ID %token STR IND_STR @@ -304,9 +309,11 @@ expr_simple $$ = state->stripIndentation(CUR_POS, std::move(*$2)); delete $2; } - | path_start PATH_END + | path_start PATH_END { $$ = $1.e; } | path_start string_parts_interpolated PATH_END { - $2->insert($2->begin(), {state->at(@1), $1}); + if ($1.appendSlash) + $2->insert($2->begin(), {noPos, new ExprString("/")}); + $2->insert($2->begin(), {state->at(@1), $1.e}); $$ = new ExprConcatStrings(CUR_POS, false, $2); } | SPATH { @@ -359,11 +366,17 @@ string_parts_interpolated path_start : PATH { - Path path(absPath(std::string_view{$1.p, $1.l}, state->basePath.path.abs())); - /* add back in the trailing '/' to the first segment */ - if ($1.p[$1.l-1] == '/' && $1.l > 1) - path += "/"; - $$ = new ExprPath(ref(state->rootFS), std::move(path)); + std::string_view path({$1.p, $1.l}); + $$ = { + .e = new ExprPath( + /* Absolute paths are always interpreted relative to the + root filesystem accessor, rather than the accessor of the + current Nix expression. */ + hasPrefix(path, "/") + ? SourcePath{state->rootFS, CanonPath(path)} + : SourcePath{state->basePath.accessor, CanonPath(path, state->basePath.path)}), + .appendSlash = hasSuffix(path, "/") + }; } | HPATH { if (state->settings.pureEval) { @@ -372,8 +385,8 @@ path_start std::string_view($1.p, $1.l) ); } - Path path(getHome() + std::string($1.p + 1, $1.l - 1)); - $$ = new ExprPath(ref(state->rootFS), std::move(path)); + CanonPath path(getHome() + std::string($1.p + 1, $1.l - 1)); + $$ = {.e = new ExprPath(SourcePath{state->rootFS, std::move(path)}), .appendSlash = true}; } ; diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 84aa6bac91b..a2e79265b51 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -257,7 +257,6 @@ static void import(EvalState & state, const PosIdx pos, Value & vPath, Value * v auto path = realisePath(state, pos, vPath, std::nullopt); auto path2 = path.path.abs(); - // FIXME auto isValidDerivationInStore = [&]() -> std::optional { if (!state.store->isStorePath(path2)) return std::nullopt; @@ -267,14 +266,27 @@ static void import(EvalState & state, const PosIdx pos, Value & vPath, Value * v return storePath; }; - if (auto storePath = isValidDerivationInStore()) { - derivationToValue(state, pos, path, *storePath, v); - } - else if (vScope) { - scopedImport(state, pos, path, vScope, v); - } - else { - state.evalFile(path, v); + Env * env = &state.allocEnv(vScope->attrs()->size()); + env->up = &state.baseEnv; + + auto staticEnv = std::make_shared(nullptr, state.staticBaseEnv.get(), vScope->attrs()->size()); + + unsigned int displ = 0; + for (auto & attr : *vScope->attrs()) { + staticEnv->vars.emplace_back(attr.name, displ); + env->values[displ++] = attr.value; + } + + state.forceFunction(**state.vImportedDrvToDerivation, pos, "while evaluating imported-drv-to-derivation.nix.gen.hh"); + v.mkApp(*state.vImportedDrvToDerivation, w); + state.forceAttrs(v, pos, "while calling imported-drv-to-derivation.nix.gen.hh"); + } else { + if (!vScope) + state.evalFile(path, v); + else { + state.forceAttrs(*vScope, pos, "while evaluating the first argument passed to builtins.scopedImport"); + + e->eval(state, *env, v); } } @@ -1731,7 +1743,10 @@ static std::string_view legacyBaseNameOf(std::string_view path) static void prim_baseNameOf(EvalState & state, const PosIdx pos, Value * * args, Value & v) { NixStringContext context; - v.mkString(legacyBaseNameOf(*state.coerceToString(pos, *args[0], context, + if (v.type() == nPath) + v.mkString(baseNameOf(v.path().path.abs()), context); + else + v.mkString(legacyBaseNameOf(*state.coerceToString(pos, *args[0], context, "while evaluating the first argument passed to builtins.baseNameOf", false, false)), context); } diff --git a/src/libexpr/primops/fetchTree.cc b/src/libexpr/primops/fetchTree.cc index d2266e2bc57..d18064860f6 100644 --- a/src/libexpr/primops/fetchTree.cc +++ b/src/libexpr/primops/fetchTree.cc @@ -21,21 +21,20 @@ namespace nix { void emitTreeAttrs( EvalState & state, - const StorePath & storePath, const fetchers::Input & input, Value & v, + std::function setOutPath, bool emptyRevFallback, bool forceDirty) { auto attrs = state.buildBindings(100); - state.mkStorePathString(storePath, attrs.alloc(state.sOutPath)); + setOutPath(attrs.alloc(state.sOutPath)); // FIXME: support arbitrary input attributes. - auto narHash = input.getNarHash(); - assert(narHash); - attrs.alloc("narHash").mkString(narHash->to_string(HashFormat::SRI, true)); + if (auto narHash = input.getNarHash()) + attrs.alloc("narHash").mkString(narHash->to_string(HashFormat::SRI, true)); if (input.getType() == "git") attrs.alloc("submodules").mkBool( @@ -74,10 +73,28 @@ void emitTreeAttrs( v.mkAttrs(attrs); } +void emitTreeAttrs( + EvalState & state, + const SourcePath & storePath, + const fetchers::Input & input, + Value & v, + bool emptyRevFallback, + bool forceDirty) +{ + emitTreeAttrs(state, input, v, + [&](Value & vOutPath) { + state.registerAccessor(storePath.accessor); + vOutPath.mkPath(storePath); + }, + emptyRevFallback, + forceDirty); +} + struct FetchTreeParams { bool emptyRevFallback = false; bool allowNameArgument = false; bool isFetchGit = false; + bool returnPath = true; // whether to return a SourcePath instead of a StorePath }; static void fetchTree( @@ -114,7 +131,9 @@ static void fetchTree( for (auto & attr : *args[0]->attrs()) { if (attr.name == state.sType) continue; + state.forceValue(*attr.value, attr.pos); + if (attr.value->type() == nPath || attr.value->type() == nString) { auto s = state.coerceToString(attr.pos, *attr.value, context, "", false, false).toOwned(); attrs.emplace(state.symbols[attr.name], @@ -195,11 +214,31 @@ static void fetchTree( state.checkURI(input.toURLString()); - auto [storePath, input2] = input.fetchToStore(state.store); + if (params.returnPath) { + // Clang16+: change to `auto [accessor, input2] =` + auto pair = input.getAccessor(state.store); + auto & accessor = pair.first; + auto & input2 = pair.second; + + emitTreeAttrs(state, input2, v, + [&](Value & vOutPath) { + state.registerAccessor(accessor); + vOutPath.mkPath(SourcePath { accessor, CanonPath::root }); + }, + params.emptyRevFallback, false); + } else { + auto pair = input.fetchToStore(state.store); + auto & storePath = pair.first; + auto & input2 = pair.second; - state.allowPath(storePath); + state.allowPath(storePath); - emitTreeAttrs(state, storePath, input2, v, params.emptyRevFallback, false); + emitTreeAttrs(state, input2, v, + [&](Value & vOutPath) { + state.mkStorePathString(storePath, vOutPath); + }, + params.emptyRevFallback, false); + } } static void prim_fetchTree(EvalState & state, const PosIdx pos, Value * * args, Value & v) @@ -616,7 +655,8 @@ static void prim_fetchGit(EvalState & state, const PosIdx pos, Value * * args, V FetchTreeParams { .emptyRevFallback = true, .allowNameArgument = true, - .isFetchGit = true + .isFetchGit = true, + .returnPath = false, }); } diff --git a/src/libfetchers/fetchers.cc b/src/libfetchers/fetchers.cc index b07e8cb6ee7..cae270b2dc3 100644 --- a/src/libfetchers/fetchers.cc +++ b/src/libfetchers/fetchers.cc @@ -167,17 +167,39 @@ bool Input::contains(const Input & other) const std::pair Input::fetchToStore(ref store) const { +// TODO: lazy-trees gets rid of this. Why? +#if 0 if (!scheme) throw Error("cannot fetch unsupported input '%s'", attrsToJSON(toAttrs())); + /* The tree may already be in the Nix store, or it could be + substituted (which is often faster than fetching from the + original source). So check that. */ + if (getNarHash()) { + try { + auto storePath = computeStorePath(*store); + + store->ensurePath(storePath); + + debug("using substituted/cached input '%s' in '%s'", + to_string(), store->printStorePath(storePath)); + + return {std::move(storePath), *this}; + } catch (Error & e) { + debug("substitution of input '%s' failed: %s", to_string(), e.what()); + } + } +#endif + auto [storePath, input] = [&]() -> std::pair { try { auto [accessor, final] = getAccessorUnchecked(store); auto storePath = nix::fetchToStore(*store, SourcePath(accessor), FetchMode::Copy, final.getName()); - auto narHash = store->queryPathInfo(storePath)->narHash; - final.attrs.insert_or_assign("narHash", narHash.to_string(HashFormat::SRI, true)); + // TODO: do we really want to throw this out? + // auto narHash = store->queryPathInfo(storePath)->narHash; + // final.attrs.insert_or_assign("narHash", narHash.to_string(HashFormat::SRI, true)); scheme->checkLocks(*this, final); diff --git a/src/libflake/flake/flake.cc b/src/libflake/flake/flake.cc index d18e01464fd..c50ed96183d 100644 --- a/src/libflake/flake/flake.cc +++ b/src/libflake/flake/flake.cc @@ -21,62 +21,17 @@ using namespace flake; namespace flake { -typedef std::pair FetchedFlake; -typedef std::vector> FlakeCache; - -static std::optional lookupInFlakeCache( - const FlakeCache & flakeCache, - const FlakeRef & flakeRef) -{ - // FIXME: inefficient. - for (auto & i : flakeCache) { - if (flakeRef == i.first) { - debug("mapping '%s' to previously seen input '%s' -> '%s", - flakeRef, i.first, i.second.second); - return i.second; - } - } - - return std::nullopt; -} - -static std::tuple fetchOrSubstituteTree( +static FlakeRef maybeResolve( EvalState & state, const FlakeRef & originalRef, - bool allowLookup, - FlakeCache & flakeCache) + bool useRegistries) { - auto fetched = lookupInFlakeCache(flakeCache, originalRef); - FlakeRef resolvedRef = originalRef; - - if (!fetched) { if (originalRef.input.isDirect()) { - fetched.emplace(originalRef.fetchTree(state.store)); - } else { - if (allowLookup) { - resolvedRef = originalRef.resolve(state.store); - auto fetchedResolved = lookupInFlakeCache(flakeCache, originalRef); - if (!fetchedResolved) fetchedResolved.emplace(resolvedRef.fetchTree(state.store)); - flakeCache.push_back({resolvedRef, *fetchedResolved}); - fetched.emplace(*fetchedResolved); - } - else { + if (!useRegistries) throw Error("'%s' is an indirect flake reference, but registry lookups are not allowed", originalRef); - } - } - flakeCache.push_back({originalRef, *fetched}); - } - - auto [storePath, lockedRef] = *fetched; - - debug("got tree '%s' from '%s'", - state.store->printStorePath(storePath), lockedRef); - - state.allowPath(storePath); - - assert(!originalRef.input.getNarHash() || storePath == originalRef.input.computeStorePath(*state.store)); - - return {std::move(storePath), resolvedRef, lockedRef}; + return originalRef.resolve(state.store); + } else + return originalRef; } static void forceTrivialValue(EvalState & state, Value & value, const PosIdx pos) @@ -318,24 +273,19 @@ static Flake getFlake( EvalState & state, const FlakeRef & originalRef, bool allowLookup, - FlakeCache & flakeCache, InputPath lockRootPath) { - auto [storePath, resolvedRef, lockedRef] = fetchOrSubstituteTree( - state, originalRef, allowLookup, flakeCache); + auto resolvedRef = maybeResolve(state, originalRef, allowLookup); + auto [accessor, lockedRef] = resolvedRef.lazyFetch(state.store); - return readFlake(state, originalRef, resolvedRef, lockedRef, state.rootPath(state.store->toRealPath(storePath)), lockRootPath); -} + state.registerAccessor(accessor); -Flake getFlake(EvalState & state, const FlakeRef & originalRef, bool allowLookup, FlakeCache & flakeCache) -{ - return getFlake(state, originalRef, allowLookup, flakeCache, {}); + return readFlake(state, originalRef, resolvedRef, lockedRef, SourcePath {accessor, CanonPath::root}, lockRootPath); } Flake getFlake(EvalState & state, const FlakeRef & originalRef, bool allowLookup) { - FlakeCache flakeCache; - return getFlake(state, originalRef, allowLookup, flakeCache); + return getFlake(state, originalRef, allowLookup, {}); } static LockFile readLockFile( @@ -357,11 +307,9 @@ LockedFlake lockFlake( { experimentalFeatureSettings.require(Xp::Flakes); - FlakeCache flakeCache; - auto useRegistries = lockFlags.useRegistries.value_or(settings.useRegistries); - auto flake = getFlake(state, topRef, useRegistries, flakeCache); + auto flake = getFlake(state, topRef, useRegistries); if (lockFlags.applyNixConfig) { flake.config.apply(settings); @@ -555,7 +503,7 @@ LockedFlake lockFlake( } if (mustRefetch) { - auto inputFlake = getFlake(state, oldLock->lockedRef, false, flakeCache, inputPath); + auto inputFlake = getFlake(state, oldLock->lockedRef, false, inputPath); nodePaths.emplace(childNode, inputFlake.path.parent()); computeLocks(inputFlake.inputs, childNode, inputPath, oldLock, lockRootPath, parentPath, false); } else { @@ -588,7 +536,7 @@ LockedFlake lockFlake( if (localRef.input.getType() == "path") localPath = absPath(*input.ref->input.getSourcePath(), parentPath); - auto inputFlake = getFlake(state, localRef, useRegistries, flakeCache, inputPath); + auto inputFlake = getFlake(state, localRef, useRegistries, inputPath); auto childNode = make_ref(inputFlake.lockedRef, ref); @@ -617,12 +565,17 @@ LockedFlake lockFlake( } else { - auto [storePath, resolvedRef, lockedRef] = fetchOrSubstituteTree( - state, *input.ref, useRegistries, flakeCache); + auto [path, lockedRef] = [&]() -> std::tuple + { + auto resolvedRef = maybeResolve(state, *input.ref, useRegistries); + auto [accessor, lockedRef] = resolvedRef.lazyFetch(state.store); + state.registerAccessor(accessor); + return {SourcePath(accessor), lockedRef}; + }(); auto childNode = make_ref(lockedRef, ref, false); - nodePaths.emplace(childNode, state.rootPath(state.store->toRealPath(storePath))); + nodePaths.emplace(childNode, path); node->inputs.insert_or_assign(id, childNode); } @@ -725,8 +678,7 @@ LockedFlake lockFlake( repo, so we should re-read it. FIXME: we could also just clear the 'rev' field... */ auto prevLockedRef = flake.lockedRef; - FlakeCache dummyCache; - flake = getFlake(state, topRef, useRegistries, dummyCache); + flake = getFlake(state, topRef, useRegistries); if (lockFlags.commitLockFile && flake.lockedRef.input.getRev() && @@ -785,11 +737,24 @@ void callFlake(EvalState & state, auto lockedNode = node.dynamic_pointer_cast(); - auto [storePath, subdir] = sourcePathToStorePath(state.store, sourcePath); + /* + // TODO: why is this commented out? + // FIXME: This is a hack to support chroot stores. Remove this + // once we can pass a sourcePath rather than a storePath to + // call-flake.nix. + // auto path = sourcePath.path.abs(); + // if (auto store = state.store.dynamic_pointer_cast()) { + // auto realStoreDir = store->getRealStoreDir(); + // if (isInDir(path, realStoreDir)) + // path = store->storeDir + path.substr(realStoreDir.size()); + // } + // + // auto [storePath, subdir] = state.store->toStorePath(path); + */ emitTreeAttrs( state, - storePath, + SourcePath(sourcePath.accessor), lockedNode ? lockedNode->lockedRef.input : lockedFlake.flake.lockedRef.input, vSourceInfo, false, @@ -800,7 +765,7 @@ void callFlake(EvalState & state, override .alloc(state.symbols.create("dir")) - .mkString(CanonPath(subdir).rel()); + .mkString(sourcePath.path.rel()); overrides.alloc(state.symbols.create(key->second)).mkAttrs(override); } diff --git a/src/libflake/flake/flake.hh b/src/libflake/flake/flake.hh index 496e186736c..d75c4cecec7 100644 --- a/src/libflake/flake/flake.hh +++ b/src/libflake/flake/flake.hh @@ -228,7 +228,7 @@ std::pair sourcePathToStorePath( void emitTreeAttrs( EvalState & state, - const StorePath & storePath, + const SourcePath & storePath, const fetchers::Input & input, Value & v, bool emptyRevFallback = false, diff --git a/src/libflake/flake/flakeref.cc b/src/libflake/flake/flakeref.cc index 01fe747f9c0..17cfc5f5980 100644 --- a/src/libflake/flake/flakeref.cc +++ b/src/libflake/flake/flakeref.cc @@ -287,10 +287,10 @@ FlakeRef FlakeRef::fromAttrs( fetchers::maybeGetStrAttr(attrs, "dir").value_or("")); } -std::pair FlakeRef::fetchTree(ref store) const +std::pair, FlakeRef> FlakeRef::lazyFetch(ref store) const { - auto [storePath, lockedInput] = input.fetchToStore(store); - return {std::move(storePath), FlakeRef(std::move(lockedInput), subdir)}; + auto [accessor, lockedInput] = input.getAccessor(store); + return {accessor, FlakeRef(std::move(lockedInput), subdir)}; } std::tuple parseFlakeRefWithFragmentAndExtendedOutputsSpec( diff --git a/src/libflake/flake/flakeref.hh b/src/libflake/flake/flakeref.hh index 1064538a72a..551f36da106 100644 --- a/src/libflake/flake/flakeref.hh +++ b/src/libflake/flake/flakeref.hh @@ -63,7 +63,7 @@ struct FlakeRef const fetchers::Settings & fetchSettings, const fetchers::Attrs & attrs); - std::pair fetchTree(ref store) const; + std::pair, FlakeRef> lazyFetch(ref store) const; }; std::ostream & operator << (std::ostream & str, const FlakeRef & flakeRef); diff --git a/src/libutil/posix-source-accessor.cc b/src/libutil/posix-source-accessor.cc index f26f74d5846..c88b4a29929 100644 --- a/src/libutil/posix-source-accessor.cc +++ b/src/libutil/posix-source-accessor.cc @@ -199,4 +199,9 @@ ref makeFSSourceAccessor(std::filesystem::path root) { return make_ref(std::move(root)); } + +bool PosixSourceAccessor::toStringReturnsStorePath() const { + return false; +} + } diff --git a/src/libutil/posix-source-accessor.hh b/src/libutil/posix-source-accessor.hh index 40f60bb54b8..f80425276e1 100644 --- a/src/libutil/posix-source-accessor.hh +++ b/src/libutil/posix-source-accessor.hh @@ -57,6 +57,8 @@ struct PosixSourceAccessor : virtual SourceAccessor */ static SourcePath createAtRoot(const std::filesystem::path & path); + virtual bool toStringReturnsStorePath() const override; + private: /** diff --git a/src/libutil/source-accessor.cc b/src/libutil/source-accessor.cc index e797951c76e..422b3f3eb7c 100644 --- a/src/libutil/source-accessor.cc +++ b/src/libutil/source-accessor.cc @@ -64,7 +64,9 @@ void SourceAccessor::setPathDisplay(std::string displayPrefix, std::string displ std::string SourceAccessor::showPath(const CanonPath & path) { - return displayPrefix + path.abs() + displaySuffix; + return displayPrefix + + ((displayPrefix.empty() || displayPrefix.ends_with("/")) ? "" : ":") + path.rel() + + displaySuffix; } CanonPath SourceAccessor::resolveSymlinks( @@ -105,4 +107,8 @@ CanonPath SourceAccessor::resolveSymlinks( return res; } +bool SourceAccessor::toStringReturnsStorePath() const { + return true; +} + } diff --git a/src/libutil/source-accessor.hh b/src/libutil/source-accessor.hh index b16960d4a4c..79cec5e995f 100644 --- a/src/libutil/source-accessor.hh +++ b/src/libutil/source-accessor.hh @@ -161,6 +161,15 @@ struct SourceAccessor : std::enable_shared_from_this virtual std::string showPath(const CanonPath & path); + /** + * System paths: `toString /foo/bar = "/foo/bar"` + * Virtual paths: fetched to the store + * + * In both cases, the returned string functionally identifies the path, + * and can still be read. + */ + virtual bool toStringReturnsStorePath() const; + /** * Resolve any symlinks in `path` according to the given * resolution mode. diff --git a/src/nix/flake.cc b/src/nix/flake.cc index 640a80aed78..6295c9ee3ea 100644 --- a/src/nix/flake.cc +++ b/src/nix/flake.cc @@ -18,6 +18,7 @@ #include "markdown.hh" #include "users.hh" #include "terminal.hh" +#include "value/context.hh" #include #include @@ -214,7 +215,7 @@ struct CmdFlakeMetadata : FlakeCommand, MixJSON auto & flake = lockedFlake.flake; // Currently, all flakes are in the Nix store via the rootFS accessor. - auto storePath = store->printStorePath(sourcePathToStorePath(store, flake.path).first); + auto storePath = store->printStorePath(store->toStorePath(flake.path.path.abs()).first); if (json) { nlohmann::json j; @@ -236,7 +237,6 @@ struct CmdFlakeMetadata : FlakeCommand, MixJSON j["revCount"] = *revCount; if (auto lastModified = flake.lockedRef.input.getLastModified()) j["lastModified"] = *lastModified; - j["path"] = storePath; j["locks"] = lockedFlake.lockFile.toJSON().first; if (auto fingerprint = lockedFlake.getFingerprint(store)) j["fingerprint"] = fingerprint->to_string(HashFormat::Base16, false); @@ -253,9 +253,6 @@ struct CmdFlakeMetadata : FlakeCommand, MixJSON logger->cout( ANSI_BOLD "Description:" ANSI_NORMAL " %s", *flake.description); - logger->cout( - ANSI_BOLD "Path:" ANSI_NORMAL " %s", - storePath); if (auto rev = flake.lockedRef.input.getRev()) logger->cout( ANSI_BOLD "Revision:" ANSI_NORMAL " %s", @@ -893,7 +890,9 @@ struct CmdFlakeInitCommon : virtual Args, EvalCommand auto cursor = installable.getCursor(*evalState); auto templateDirAttr = cursor->getAttr("path"); - auto templateDir = templateDirAttr->getString(); + auto & v = templateDirAttr->forceValue(); + NixStringContext ctx; + auto templateDir = evalState->coerceToString(noPos, v, ctx, "while casting the template value to a path", false, true).toOwned(); if (!store->isInStore(templateDir)) evalState->error( @@ -1525,21 +1524,15 @@ struct CmdFlakePrefetch : FlakeCommand, MixJSON { auto originalRef = getFlakeRef(); auto resolvedRef = originalRef.resolve(store); - auto [storePath, lockedRef] = resolvedRef.fetchTree(store); - auto hash = store->queryPathInfo(storePath)->narHash; + auto [accessor, lockedRef] = resolvedRef.lazyFetch(store); if (json) { auto res = nlohmann::json::object(); - res["storePath"] = store->printStorePath(storePath); - res["hash"] = hash.to_string(HashFormat::SRI, true); res["original"] = fetchers::attrsToJSON(resolvedRef.toAttrs()); res["locked"] = fetchers::attrsToJSON(lockedRef.toAttrs()); logger->cout(res.dump()); } else { - notice("Downloaded '%s' to '%s' (hash '%s').", - lockedRef.to_string(), - store->printStorePath(storePath), - hash.to_string(HashFormat::SRI, true)); + notice("Fetched '%s'.", lockedRef.to_string()); } } }; diff --git a/tests/functional/flakes/flakes.sh b/tests/functional/flakes/flakes.sh index aa4cb1e1855..6684857cd45 100755 --- a/tests/functional/flakes/flakes.sh +++ b/tests/functional/flakes/flakes.sh @@ -195,7 +195,6 @@ nix flake metadata "$flake1Dir" | grepQuiet 'URL:.*flake1.*' # Test 'nix flake metadata --json'. json=$(nix flake metadata flake1 --json | jq .) [[ $(echo "$json" | jq -r .description) = 'Bla bla' ]] -[[ -d $(echo "$json" | jq -r .path) ]] [[ $(echo "$json" | jq -r .lastModified) = $(git -C "$flake1Dir" log -n1 --format=%ct) ]] hash1=$(echo "$json" | jq -r .revision) [[ -n $(echo "$json" | jq -r .fingerprint) ]] diff --git a/tests/functional/lang/eval-fail-derivation-name.err.exp b/tests/functional/lang/eval-fail-derivation-name.err.exp index 0ef98674d81..127370b5f8f 100644 --- a/tests/functional/lang/eval-fail-derivation-name.err.exp +++ b/tests/functional/lang/eval-fail-derivation-name.err.exp @@ -1,20 +1,20 @@ error: … while evaluating the attribute 'outPath' - at ::: + at ::: | value = commonAttrs // { | outPath = builtins.getAttr outputName strict; | ^ | drvPath = strict.drvPath; … while calling the 'getAttr' builtin - at ::: + at ::: | value = commonAttrs // { | outPath = builtins.getAttr outputName strict; | ^ | drvPath = strict.drvPath; … while calling the 'derivationStrict' builtin - at ::: + at ::: | | strict = derivationStrict drvAttrs; | ^