From 1d36d16086b6bdfe76d77de2d8d430754035a8f9 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 24 Feb 2022 21:03:41 +0100 Subject: [PATCH] Fix flakes --- src/libexpr/flake/flake.cc | 114 ++++++++++++++++-------------- src/libexpr/flake/flake.hh | 2 + src/libexpr/flake/flakeref.hh | 2 +- src/libexpr/flake/lockfile.cc | 13 ++-- src/libexpr/flake/lockfile.hh | 3 +- src/libexpr/parser.y | 2 +- src/libexpr/primops.cc | 6 +- src/libfetchers/input-accessor.hh | 6 ++ 8 files changed, 83 insertions(+), 65 deletions(-) diff --git a/src/libexpr/flake/flake.cc b/src/libexpr/flake/flake.cc index 7dffbc632..95548104d 100644 --- a/src/libexpr/flake/flake.cc +++ b/src/libexpr/flake/flake.cc @@ -89,12 +89,19 @@ static void expectType(EvalState & state, ValueType type, } static std::map parseFlakeInputs( - EvalState & state, Value * value, const Pos & pos, - const std::optional & baseDir, InputPath lockRootPath); + EvalState & state, + Value * value, + const Pos & pos, + const std::optional & baseDir, + const InputPath & lockRootPath); -static FlakeInput parseFlakeInput(EvalState & state, - const std::string & inputName, Value * value, const Pos & pos, - const std::optional & baseDir, InputPath lockRootPath) +static FlakeInput parseFlakeInput( + EvalState & state, + const std::string & inputName, + Value * value, + const Pos & pos, + const std::optional & baseDir, + const InputPath & lockRootPath) { expectType(state, nAttrs, *value, pos); @@ -168,8 +175,11 @@ static FlakeInput parseFlakeInput(EvalState & state, } static std::map parseFlakeInputs( - EvalState & state, Value * value, const Pos & pos, - const std::optional & baseDir, InputPath lockRootPath) + EvalState & state, + Value * value, + const Pos & pos, + const std::optional & baseDir, + const InputPath & lockRootPath) { std::map inputs; @@ -188,38 +198,31 @@ static std::map parseFlakeInputs( return inputs; } -static Flake getFlake( +static Flake readFlake( EvalState & state, - const FlakeRef & originalRef, - bool allowLookup, - FlakeCache & flakeCache, - InputPath lockRootPath) + const FlakeRef & lockedRef, + nix::ref accessor, + const InputPath & lockRootPath) { - auto [sourceInfo, resolvedRef, lockedRef] = fetchOrSubstituteTree( - state, originalRef, allowLookup, flakeCache); + auto flakeDir = canonPath("/" + lockedRef.subdir); + SourcePath flakePath{accessor, canonPath(flakeDir + "/flake.nix")}; - // Guard against symlink attacks. - auto flakeDir = canonPath(sourceInfo.actualPath + "/" + lockedRef.subdir, true); - auto flakeFile = canonPath(flakeDir + "/flake.nix", true); - if (!isInDir(flakeFile, sourceInfo.actualPath)) - throw Error("'flake.nix' file of flake '%s' escapes from '%s'", - lockedRef, state.store->printStorePath(sourceInfo.storePath)); - - Flake flake { - .originalRef = originalRef, - .resolvedRef = resolvedRef, - .lockedRef = lockedRef, - //.sourceInfo = std::make_shared(std::move(sourceInfo)) - }; - - if (!pathExists(flakeFile)) - throw Error("source tree referenced by '%s' does not contain a '%s/flake.nix' file", lockedRef, lockedRef.subdir); + if (!flakePath.pathExists()) + throw Error("source tree referenced by '%s' does not contain a file named '%s'", lockedRef, flakePath.path); Value vInfo; - // FIXME: use accessor - state.evalFile(state.rootPath(flakeFile), vInfo, true); // FIXME: symlink attack + state.evalFile(flakePath, vInfo, true); - expectType(state, nAttrs, vInfo, Pos(foFile, state.symbols.create(flakeFile), 0, 0)); + expectType(state, nAttrs, vInfo, Pos(foFile, state.symbols.create(state.packPath(flakePath)), 0, 0)); + + Flake flake { + // FIXME + .originalRef = lockedRef, + .resolvedRef = lockedRef, + .lockedRef = lockedRef, + .accessor = accessor, + .flakePath = dirOf(flakePath.path), + }; if (auto description = vInfo.attrs->get(state.sDescription)) { expectType(state, nString, *description->value, *description->pos); @@ -295,6 +298,19 @@ static Flake getFlake( return flake; } +static Flake getFlake( + EvalState & state, + const FlakeRef & originalRef, + bool allowLookup, + FlakeCache & flakeCache, + const InputPath & lockRootPath) +{ + // FIXME: resolve + auto [accessor, input] = originalRef.input.lazyFetch(state.store); + + return readFlake(state, originalRef, accessor, lockRootPath); +} + Flake getFlake(EvalState & state, const FlakeRef & originalRef, bool allowLookup, FlakeCache & flakeCache) { return getFlake(state, originalRef, allowLookup, flakeCache, {}); @@ -306,6 +322,14 @@ Flake getFlake(EvalState & state, const FlakeRef & originalRef, bool allowLookup return getFlake(state, originalRef, allowLookup, flakeCache); } +static LockFile readLockFile(const Flake & flake) +{ + SourcePath lockFilePath{flake.accessor, canonPath(flake.flakePath + "/flake.lock")}; + return lockFilePath.pathExists() + ? LockFile(lockFilePath.readFile(), fmt("%s", lockFilePath)) + : LockFile(); +} + /* Compute an in-memory lock file for the specified top-level flake, and optionally write it to file, if the flake is writable. */ LockedFlake lockFlake( @@ -319,19 +343,16 @@ LockedFlake lockFlake( auto useRegistries = lockFlags.useRegistries.value_or(fetchSettings.useRegistries); - auto flake = getFlake(state, topRef, useRegistries, flakeCache); + auto flake = getFlake(state, topRef, useRegistries, flakeCache, {}); if (lockFlags.applyNixConfig) { flake.config.apply(); state.store->setOptions(); } - #if 0 try { - // FIXME: symlink attack - auto oldLockFile = LockFile::read( - flake.sourceInfo->actualPath + "/" + flake.lockedRef.subdir + "/flake.lock"); + auto oldLockFile = readLockFile(flake); debug("old lock file: %s", oldLockFile); @@ -545,8 +566,7 @@ LockedFlake lockFlake( inputFlake.inputs, childNode, inputPath, oldLock ? std::dynamic_pointer_cast(oldLock) - : LockFile::read( - inputFlake.sourceInfo->actualPath + "/" + inputFlake.lockedRef.subdir + "/flake.lock").root, + : readLockFile(inputFlake).root, oldLock ? lockRootPath : inputPath, localPath, false); } @@ -565,12 +585,9 @@ LockedFlake lockFlake( } }; - // Bring in the current ref for relative path resolution if we have it - auto parentPath = canonPath(flake.sourceInfo->actualPath + "/" + flake.lockedRef.subdir, true); - computeLocks( flake.inputs, newLockFile.root, {}, - lockFlags.recreateLockFile ? nullptr : oldLockFile.root, {}, parentPath, false); + lockFlags.recreateLockFile ? nullptr : oldLockFile.root, {}, flake.flakePath, false); for (auto & i : lockFlags.inputOverrides) if (!overridesUsed.count(i.first)) @@ -670,9 +687,6 @@ LockedFlake lockFlake( e.addTrace({}, "while updating the lock file of flake '%s'", flake.lockedRef.to_string()); throw; } - #endif - - throw UnimplementedError("lockFlake"); } void callFlake(EvalState & state, @@ -687,17 +701,13 @@ void callFlake(EvalState & state, vLocks->mkString(lockedFlake.lockFile.to_string()); - #if 0 emitTreeAttrs( state, - //*lockedFlake.flake.sourceInfo, + {lockedFlake.flake.accessor, lockedFlake.flake.flakePath}, lockedFlake.flake.lockedRef.input, *vRootSrc, false, lockedFlake.flake.forceDirty); - #endif - - throw UnimplementedError("callFlake"); vRootSubdir->mkString(lockedFlake.flake.lockedRef.subdir); diff --git a/src/libexpr/flake/flake.hh b/src/libexpr/flake/flake.hh index 068d5c6af..04a0099f5 100644 --- a/src/libexpr/flake/flake.hh +++ b/src/libexpr/flake/flake.hh @@ -61,6 +61,8 @@ struct Flake FlakeRef originalRef; // the original flake specification (by the user) FlakeRef resolvedRef; // registry references and caching resolved to the specific underlying flake FlakeRef lockedRef; // the specific local store result of invoking the fetcher + ref accessor; + Path flakePath; bool forceDirty = false; // pretend that 'lockedRef' is dirty std::optional description; FlakeInputs inputs; diff --git a/src/libexpr/flake/flakeref.hh b/src/libexpr/flake/flakeref.hh index 1fddfd9a0..81caca55b 100644 --- a/src/libexpr/flake/flakeref.hh +++ b/src/libexpr/flake/flakeref.hh @@ -34,7 +34,7 @@ typedef std::string FlakeId; struct FlakeRef { - /* fetcher-specific representation of the input, sufficient to + /* Fetcher-specific representation of the input, sufficient to perform the fetch operation. */ fetchers::Input input; diff --git a/src/libexpr/flake/lockfile.cc b/src/libexpr/flake/lockfile.cc index 60b52d578..a34cfde97 100644 --- a/src/libexpr/flake/lockfile.cc +++ b/src/libexpr/flake/lockfile.cc @@ -66,7 +66,7 @@ std::shared_ptr LockFile::findInput(const InputPath & path) return pos; } -LockFile::LockFile(const nlohmann::json & json, const Path & path) +LockFile::LockFile(const nlohmann::json & json, std::string_view path) { auto version = json.value("version", 0); if (version < 5 || version > 7) @@ -116,6 +116,11 @@ LockFile::LockFile(const nlohmann::json & json, const Path & path) // a bit since we don't need to worry about cycles. } +LockFile::LockFile(std::string_view contents, std::string_view path) + : LockFile(nlohmann::json::parse(contents), path) +{ +} + nlohmann::json LockFile::toJSON() const { nlohmann::json nodes; @@ -183,12 +188,6 @@ std::string LockFile::to_string() const return toJSON().dump(2); } -LockFile LockFile::read(const Path & path) -{ - if (!pathExists(path)) return LockFile(); - return LockFile(nlohmann::json::parse(readFile(path)), path); -} - std::ostream & operator <<(std::ostream & stream, const LockFile & lockFile) { stream << lockFile.toJSON().dump(2); diff --git a/src/libexpr/flake/lockfile.hh b/src/libexpr/flake/lockfile.hh index 96f1edc76..d51bdb86d 100644 --- a/src/libexpr/flake/lockfile.hh +++ b/src/libexpr/flake/lockfile.hh @@ -50,7 +50,8 @@ struct LockFile std::shared_ptr root = std::make_shared(); LockFile() {}; - LockFile(const nlohmann::json & json, const Path & path); + LockFile(const nlohmann::json & json, std::string_view path); + LockFile(std::string_view contents, std::string_view path); nlohmann::json toJSON() const; diff --git a/src/libexpr/parser.y b/src/libexpr/parser.y index bf3240dab..3258327a0 100644 --- a/src/libexpr/parser.y +++ b/src/libexpr/parser.y @@ -716,7 +716,7 @@ Expr * EvalState::parseExprFromFile(const SourcePath & path) Expr * EvalState::parseExprFromFile(const SourcePath & path, StaticEnv & staticEnv) { auto packed = packPath(path); - auto buffer = path.accessor->readFile(path.path); + auto buffer = path.readFile(); // readFile hopefully have left some extra space for terminators buffer.append("\0\0", 2); return parse(buffer.data(), buffer.size(), foFile, packed, dirOf(packed), staticEnv); diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index c15051d4a..85d2e64e5 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -1394,7 +1394,7 @@ static void prim_pathExists(EvalState & state, const Pos & pos, Value * * args, auto path = realisePath(state, pos, *args[0], { .checkForPureEval = false }); try { - v.mkBool(path.accessor->pathExists(path.path)); + v.mkBool(path.pathExists()); } catch (SysError & e) { /* Don't give away info from errors while canonicalising ‘path’ in restricted mode. */ @@ -1459,7 +1459,7 @@ static RegisterPrimOp primop_dirOf({ static void prim_readFile(EvalState & state, const Pos & pos, Value * * args, Value & v) { auto path = realisePath(state, pos, *args[0]); - auto s = path.accessor->readFile(path.path); + auto s = path.readFile(); if (s.find((char) 0) != std::string::npos) throw Error("the contents of the file '%1%' cannot be represented as a Nix string", path); auto refs = @@ -1547,7 +1547,7 @@ static void prim_hashFile(EvalState & state, const Pos & pos, Value * * args, Va auto path = realisePath(state, pos, *args[1]); // FIXME: state.toRealPath(path, context) - v.mkString(hashString(*ht, path.accessor->readFile(path.path)).to_string(Base16, false)); + v.mkString(hashString(*ht, path.readFile()).to_string(Base16, false)); } static RegisterPrimOp primop_hashFile({ diff --git a/src/libfetchers/input-accessor.hh b/src/libfetchers/input-accessor.hh index 872fcd3cd..b8735f6ca 100644 --- a/src/libfetchers/input-accessor.hh +++ b/src/libfetchers/input-accessor.hh @@ -67,6 +67,12 @@ struct SourcePath Path path; std::string_view baseName() const; + + std::string readFile() const + { return accessor->readFile(path); } + + bool pathExists() const + { return accessor->pathExists(path); } }; std::ostream & operator << (std::ostream & str, const SourcePath & path);