From 60834492aea935b8043cdc8ccbc1270edebbc20a Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 16 Apr 2019 15:02:02 +0200 Subject: [PATCH] Update lock files from InstallableFlake::toValue() This ensures that the lock file is updated *before* evaluating it, and that it gets updated for any nix command, not just 'nix build'. Also, while computing the lock file, allow arbitrary registry lookups, not just at top-level. Also, improve some error messages slightly. --- src/libexpr/primops/flake.cc | 42 +++++++++++++++++++----------------- src/libexpr/primops/flake.hh | 8 +++---- src/nix/build.cc | 14 ------------ src/nix/command.hh | 7 ++---- src/nix/flake.cc | 2 +- src/nix/installables.cc | 20 +++++++++-------- 6 files changed, 40 insertions(+), 53 deletions(-) diff --git a/src/libexpr/primops/flake.cc b/src/libexpr/primops/flake.cc index 3d11d9ec4..37dadd474 100644 --- a/src/libexpr/primops/flake.cc +++ b/src/libexpr/primops/flake.cc @@ -159,6 +159,9 @@ static FlakeRef lookupFlake(EvalState & state, const FlakeRef & flakeRef, const std::vector> & registries, std::vector pastSearches = {}) { + if (registries.empty() && !flakeRef.isDirect()) + throw Error("indirect flake reference '%s' is not allowed", flakeRef.to_string()); + for (std::shared_ptr registry : registries) { auto i = registry->entries.find(flakeRef); if (i != registry->entries.end()) { @@ -178,8 +181,10 @@ static FlakeRef lookupFlake(EvalState & state, const FlakeRef & flakeRef, return lookupFlake(state, newRef, registries, pastSearches); } } + if (!flakeRef.isDirect()) - throw Error("indirect flake URI '%s' is the result of a lookup", flakeRef.to_string()); + throw Error("could not resolve flake reference '%s'", flakeRef.to_string()); + return flakeRef; } @@ -192,7 +197,8 @@ struct FlakeSourceInfo static FlakeSourceInfo fetchFlake(EvalState & state, const FlakeRef flakeRef, bool impureIsAllowed = false) { - FlakeRef fRef = lookupFlake(state, flakeRef, state.getFlakeRegistries()); + FlakeRef fRef = lookupFlake(state, flakeRef, + impureIsAllowed ? state.getFlakeRegistries() : std::vector>()); // This only downloads only one revision of the repo, not the entire history. if (auto refData = std::get_if(&fRef.data)) { @@ -349,16 +355,18 @@ NonFlake getNonFlake(EvalState & state, const FlakeRef & flakeRef, FlakeAlias al dependencies. FIXME: this should return a graph of flakes. */ -Dependencies resolveFlake(EvalState & state, const FlakeRef & topRef, bool impureTopRef, bool isTopFlake) +Dependencies resolveFlake(EvalState & state, const FlakeRef & topRef, + RegistryAccess registryAccess, bool isTopFlake) { - Flake flake = getFlake(state, topRef, isTopFlake && impureTopRef); + Flake flake = getFlake(state, topRef, + registryAccess == AllowRegistry || (registryAccess == AllowRegistryAtTop && isTopFlake)); Dependencies deps(flake); for (auto & nonFlakeInfo : flake.nonFlakeRequires) deps.nonFlakeDeps.push_back(getNonFlake(state, nonFlakeInfo.second, nonFlakeInfo.first)); for (auto & newFlakeRef : flake.requires) - deps.flakeDeps.push_back(resolveFlake(state, newFlakeRef, false)); + deps.flakeDeps.push_back(resolveFlake(state, newFlakeRef, registryAccess, false)); return deps; } @@ -376,9 +384,9 @@ LockFile::FlakeEntry dependenciesToFlakeEntry(const Dependencies & deps) return entry; } -LockFile getLockFile(EvalState & evalState, FlakeRef & flakeRef) +static LockFile makeLockFile(EvalState & evalState, FlakeRef & flakeRef) { - Dependencies deps = resolveFlake(evalState, flakeRef, true); + Dependencies deps = resolveFlake(evalState, flakeRef, AllowRegistry); LockFile::FlakeEntry entry = dependenciesToFlakeEntry(deps); LockFile lockFile; lockFile.flakeEntries = entry.flakeEntries; @@ -388,16 +396,9 @@ LockFile getLockFile(EvalState & evalState, FlakeRef & flakeRef) void updateLockFile(EvalState & state, const Path & path) { - // 'path' is the path to the local flake repo. - FlakeRef flakeRef = FlakeRef("file://" + path); - if (std::get_if(&flakeRef.data)) { - LockFile lockFile = getLockFile(state, flakeRef); - writeLockFile(lockFile, path + "/flake.lock"); - } else if (std::get_if(&flakeRef.data)) { - throw UsageError("you can only update local flakes, not flakes on GitHub"); - } else { - throw UsageError("you can only update local flakes, not flakes through their FlakeAlias"); - } + FlakeRef flakeRef = FlakeRef("file://" + path); // FIXME: ugly + auto lockFile = makeLockFile(state, flakeRef); + writeLockFile(lockFile, path + "/flake.lock"); } void callFlake(EvalState & state, const Dependencies & flake, Value & v) @@ -436,15 +437,16 @@ void callFlake(EvalState & state, const Dependencies & flake, Value & v) // Return the `provides` of the top flake, while assigning to `v` the provides // of the dependencies as well. -void makeFlakeValue(EvalState & state, const FlakeRef & flakeRef, bool impureTopRef, Value & v) +void makeFlakeValue(EvalState & state, const FlakeRef & flakeRef, RegistryAccess registryAccess, Value & v) { - callFlake(state, resolveFlake(state, flakeRef, impureTopRef), v); + callFlake(state, resolveFlake(state, flakeRef, registryAccess), v); } // This function is exposed to be used in nix files. static void prim_getFlake(EvalState & state, const Pos & pos, Value * * args, Value & v) { - makeFlakeValue(state, state.forceStringNoCtx(*args[0], pos), false, v); + makeFlakeValue(state, state.forceStringNoCtx(*args[0], pos), + evalSettings.pureEval ? DisallowRegistry : AllowRegistryAtTop, v); } static RegisterPrimOp r2("getFlake", 1, prim_getFlake); diff --git a/src/libexpr/primops/flake.hh b/src/libexpr/primops/flake.hh index 347dd2077..655d87f03 100644 --- a/src/libexpr/primops/flake.hh +++ b/src/libexpr/primops/flake.hh @@ -29,7 +29,9 @@ struct LockFile Path getUserRegistryPath(); -void makeFlakeValue(EvalState & state, const FlakeRef & flakeRef, bool impureTopRef, Value & v); +enum RegistryAccess { DisallowRegistry, AllowRegistry, AllowRegistryAtTop }; + +void makeFlakeValue(EvalState & state, const FlakeRef & flakeRef, RegistryAccess registryAccess, Value & v); std::shared_ptr readRegistry(const Path &); @@ -73,9 +75,7 @@ struct Dependencies Dependencies(const Flake & flake) : flake(flake) {} }; -Dependencies resolveFlake(EvalState &, const FlakeRef &, bool impureTopRef, bool isTopFlake = true); - -FlakeRegistry updateLockFile(EvalState &, const Flake &); +Dependencies resolveFlake(EvalState &, const FlakeRef &, RegistryAccess registryAccess, bool isTopFlake = true); void updateLockFile(EvalState &, const Path & path); diff --git a/src/nix/build.cc b/src/nix/build.cc index 9ef07dcdb..d6a6a8071 100644 --- a/src/nix/build.cc +++ b/src/nix/build.cc @@ -11,8 +11,6 @@ struct CmdBuild : MixDryRun, InstallablesCommand { Path outLink = "result"; - bool update = true; - CmdBuild() { mkFlag() @@ -26,11 +24,6 @@ struct CmdBuild : MixDryRun, InstallablesCommand .longName("no-link") .description("do not create a symlink to the build result") .set(&outLink, Path("")); - - mkFlag() - .longName("no-update") - .description("don't update the lock file") - .set(&update, false); } std::string name() override @@ -77,13 +70,6 @@ struct CmdBuild : MixDryRun, InstallablesCommand store2->addPermRoot(output.second, absPath(symlink), true); } } - - if (update) - for (auto installable : installables) { - auto flakeUri = installable->installableToFlakeUri(); - if (flakeUri) - updateLockFile(*evalState, *flakeUri); - } } }; diff --git a/src/nix/command.hh b/src/nix/command.hh index 5d0c0c82c..a52fbb9ba 100644 --- a/src/nix/command.hh +++ b/src/nix/command.hh @@ -66,11 +66,6 @@ struct Installable Buildable toBuildable(); - virtual std::optional installableToFlakeUri() - { - return std::nullopt; - } - virtual Value * toValue(EvalState & state) { throw Error("argument '%s' cannot be evaluated", what()); @@ -81,6 +76,8 @@ struct SourceExprCommand : virtual Args, StoreCommand, MixEvalArgs { std::optional file; + bool updateLockFile = true; + SourceExprCommand(); ref getEvalState(); diff --git a/src/nix/flake.cc b/src/nix/flake.cc index 2079b1c27..1e03669c3 100644 --- a/src/nix/flake.cc +++ b/src/nix/flake.cc @@ -80,7 +80,7 @@ struct CmdFlakeDeps : FlakeCommand, MixJSON, StoreCommand, MixEvalArgs FlakeRef flakeRef(flakeUri); - Dependencies deps = resolveFlake(*evalState, flakeRef, true); + Dependencies deps = resolveFlake(*evalState, flakeRef, AllowRegistryAtTop); std::queue todo; todo.push(deps); diff --git a/src/nix/installables.cc b/src/nix/installables.cc index 963321336..9d87c70c3 100644 --- a/src/nix/installables.cc +++ b/src/nix/installables.cc @@ -21,6 +21,11 @@ SourceExprCommand::SourceExprCommand() .label("file") .description("evaluate a set of attributes from FILE (deprecated)") .dest(&file); + + mkFlag() + .longName("no-update") + .description("don't create/update flake lock files") + .set(&updateLockFile, false); } ref SourceExprCommand::getEvalState() @@ -147,8 +152,13 @@ struct InstallableFlake : InstallableValue Value * toValue(EvalState & state) override { + auto path = std::get_if(&flakeRef.data); + if (cmd.updateLockFile && path) { + updateLockFile(state, path->path); + } + auto vFlake = state.allocValue(); - makeFlakeValue(state, flakeRef, true, *vFlake); + makeFlakeValue(state, flakeRef, AllowRegistryAtTop, *vFlake); auto vProvides = (*vFlake->attrs->get(state.symbols.create("provides")))->value; @@ -169,14 +179,6 @@ struct InstallableFlake : InstallableValue state.forceValue(*v); return v; } - - std::optional installableToFlakeUri() override - { - if (std::get_if(&flakeRef.data)) - return flakeRef.to_string(); - else - return std::nullopt; - } }; // FIXME: extend