Fixed issue 65

lockfile updating

src/libexpr/primops/flake.cc

@@ -289,8 +289,6 @@ Flake getFlake(EvalState & state, const FlakeRef & flakeRef, bool impureIsAllowe
 
     FlakeRef resolvedRef = sourceInfo.resolvedRef;
 
-    resolvedRef = sourceInfo.resolvedRef; // `resolvedRef` is now immutable
-
     state.store->assertStorePath(sourceInfo.storePath);
 
     if (state.allowedPaths)
@@ -368,35 +366,59 @@ NonFlake getNonFlake(EvalState & state, const FlakeRef & flakeRef, FlakeAlias al
     return nonFlake;
 }
 
-/* Given a flake reference, recursively fetch it and its
+LockFile entryToLockFile(const LockFile::FlakeEntry & entry)
-   dependencies.
+{
-   FIXME: this should return a graph of flakes.
+    LockFile lockFile;
-*/
+    lockFile.flakeEntries = entry.flakeEntries;
-ResolvedFlake resolveFlake(EvalState & state, const FlakeRef & topRef,
+    lockFile.nonFlakeEntries = entry.nonFlakeEntries;
-    RegistryAccess registryAccess, bool isTopFlake)
+    return lockFile;
+}
+
+ResolvedFlake resolveFlakeFromLockFile(EvalState & state, const FlakeRef & flakeRef, RegistryAccess registryAccess,
+    LockFile lockFile, bool isTopFlake = false)
 {
     bool allowRegistries = registryAccess == AllowRegistry || (registryAccess == AllowRegistryAtTop && isTopFlake);
-    Flake flake = getFlake(state, topRef, allowRegistries);
+    Flake flake = getFlake(state, flakeRef, allowRegistries);
-    LockFile lockFile;
-
-    if (isTopFlake)
-        lockFile = readLockFile(flake.storePath + flake.resolvedRef.subdir + "/flake.lock"); // FIXME: symlink attack
 
     ResolvedFlake deps(flake);
 
-    for (auto & nonFlakeInfo : flake.nonFlakeRequires)
+    for (auto & nonFlakeInfo : flake.nonFlakeRequires) {
-        deps.nonFlakeDeps.push_back(getNonFlake(state, nonFlakeInfo.second, nonFlakeInfo.first));
+        FlakeRef ref = nonFlakeInfo.second;
+        auto i = lockFile.nonFlakeEntries.find(nonFlakeInfo.first);
+        if (i != lockFile.nonFlakeEntries.end()) ref = i->second.ref;
+        deps.nonFlakeDeps.push_back(getNonFlake(state, ref, nonFlakeInfo.first));
+    }
 
     for (auto newFlakeRef : flake.requires) {
+        FlakeRef ref = newFlakeRef;
+        LockFile newLockFile;
         auto i = lockFile.flakeEntries.find(newFlakeRef);
-        if (i != lockFile.flakeEntries.end()) newFlakeRef = i->second.ref;
+        if (i != lockFile.flakeEntries.end()) { // Propagate lockFile downwards if possible
-        // FIXME: propagate lockFile downwards
+            ref = i->second.ref;
-        deps.flakeDeps.push_back(resolveFlake(state, newFlakeRef, registryAccess, false));
+            newLockFile = entryToLockFile(i->second);
+        }
+        deps.flakeDeps.push_back(resolveFlakeFromLockFile(state, ref, registryAccess, newLockFile));
     }
 
     return deps;
 }
 
+/* Given a flake reference, recursively fetch it and its dependencies.
+   FIXME: this should return a graph of flakes.
+*/
+ResolvedFlake resolveFlake(EvalState & state, const FlakeRef & topRef, RegistryAccess registryAccess,
+    bool recreateLockFile)
+{
+    bool allowRegistries = registryAccess == AllowRegistry || registryAccess == AllowRegistryAtTop;
+    Flake flake = getFlake(state, topRef, allowRegistries);
+    LockFile lockFile;
+
+    if (!recreateLockFile) // If recreateLockFile, start with an empty lockfile
+        lockFile = readLockFile(flake.storePath + "/flake.lock"); // FIXME: symlink attack
+
+    return resolveFlakeFromLockFile(state, topRef, registryAccess, lockFile, true);
+}
+
 LockFile::FlakeEntry dependenciesToFlakeEntry(const ResolvedFlake & resolvedFlake)
 {
     LockFile::FlakeEntry entry(resolvedFlake.flake.resolvedRef, resolvedFlake.flake.hash);
@@ -410,31 +432,25 @@ LockFile::FlakeEntry dependenciesToFlakeEntry(const ResolvedFlake & resolvedFlak
     return entry;
 }
 
-static LockFile makeLockFile(EvalState & evalState, FlakeRef & flakeRef)
+static LockFile makeLockFile(EvalState & evalState, FlakeRef & flakeRef, bool recreateLockFile)
 {
-    ResolvedFlake resFlake = resolveFlake(evalState, flakeRef, AllowRegistry);
+    ResolvedFlake resFlake = resolveFlake(evalState, flakeRef, AllowRegistry, recreateLockFile);
-    LockFile::FlakeEntry entry = dependenciesToFlakeEntry(resFlake);
+    return entryToLockFile(dependenciesToFlakeEntry(resFlake));
-    LockFile lockFile;
-    lockFile.flakeEntries = entry.flakeEntries;
-    lockFile.nonFlakeEntries = entry.nonFlakeEntries;
-    return lockFile;
 }
 
-void updateLockFile(EvalState & state, const FlakeUri & flakeUri)
+void updateLockFile(EvalState & state, const FlakeUri & uri, bool recreateLockFile)
 {
-    // FIXME: We are writing the lockfile to the store here! Very bad practice!
+    FlakeRef flakeRef = FlakeRef(uri);
-    FlakeRef flakeRef = FlakeRef(flakeUri);
+    auto lockFile = makeLockFile(state, flakeRef, recreateLockFile);
     if (auto refData = std::get_if<FlakeRef::IsPath>(&flakeRef.data)) {
-        auto lockFile = makeLockFile(state, flakeRef);
+        writeLockFile(lockFile, refData->path + (flakeRef.subdir == "" ? "" : "/" + flakeRef.subdir) + "/flake.lock");
-        writeLockFile(lockFile, refData->path + "/" + flakeRef.subdir + "/flake.lock");
 
         // Hack: Make sure that flake.lock is visible to Git. Otherwise,
         // exportGit will fail to copy it to the Nix store.
-        runProgram("git", true,
+        runProgram("git", true, { "-C", refData->path, "add",
-            { "-C", refData->path, "add", "--intent-to-add",
+                  (flakeRef.subdir == "" ? "" : flakeRef.subdir + "/") + "flake.lock" });
-              (flakeRef.subdir == "" ? "" : flakeRef.subdir + "/") + "flake.lock" });
     } else
-        throw Error("flakeUri %s can't be updated because it is not a path", flakeUri);
+        throw Error("flakeUri %s can't be updated because it is not a path", uri);
 }
 
 void callFlake(EvalState & state, const ResolvedFlake & resFlake, Value & v)
@@ -485,16 +501,17 @@ void callFlake(EvalState & state, const ResolvedFlake & resFlake, 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, RegistryAccess registryAccess, Value & v)
+void makeFlakeValue(EvalState & state, const FlakeRef & flakeRef, RegistryAccess registryAccess, Value & v, bool recreateLockFile)
 {
-    callFlake(state, resolveFlake(state, flakeRef, registryAccess), v);
+    callFlake(state, resolveFlake(state, flakeRef, registryAccess, recreateLockFile), 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),
-        evalSettings.pureEval ? DisallowRegistry : AllowRegistryAtTop, v);
+        evalSettings.pureEval ? DisallowRegistry : AllowRegistryAtTop, v, false);
+    // `recreateLockFile == false` because this is the evaluation stage, which should be pure, and hence not recreate lockfiles.
 }
 
 static RegisterPrimOp r2("getFlake", 1, prim_getFlake);

src/libexpr/primops/flake.hh

@@ -45,7 +45,7 @@ Path getUserRegistryPath();
 
 enum RegistryAccess { DisallowRegistry, AllowRegistry, AllowRegistryAtTop };
 
-void makeFlakeValue(EvalState & state, const FlakeRef & flakeRef, RegistryAccess registryAccess, Value & v);
+void makeFlakeValue(EvalState & state, const FlakeRef & flakeRef, RegistryAccess registryAccess, Value & v, bool recreateLockFile);
 
 std::shared_ptr<FlakeRegistry> readRegistry(const Path &);
 
@@ -103,9 +103,9 @@ struct ResolvedFlake
     ResolvedFlake(const Flake & flake) : flake(flake) {}
 };
 
-ResolvedFlake resolveFlake(EvalState &, const FlakeRef &, RegistryAccess registryAccess, bool isTopFlake = true);
+ResolvedFlake resolveFlake(EvalState &, const FlakeRef &, RegistryAccess, bool recreateLockFile);
 
-void updateLockFile(EvalState &, const FlakeUri &);
+void updateLockFile(EvalState &, const FlakeUri &, bool recreateLockFile);
 
 void gitCloneFlake (std::string flakeUri, EvalState &, Registries, Path);
 }

src/nix/command.hh

@@ -76,10 +76,10 @@ struct SourceExprCommand : virtual Args, StoreCommand, MixEvalArgs
 {
     std::optional<Path> file;
 
-    bool updateLockFile = true;
-
     SourceExprCommand();
 
+    bool recreateLockFile = false;
+
     ref<EvalState> getEvalState();
 
     std::vector<std::shared_ptr<Installable>> parseInstallables(

src/nix/flake.cc

@@ -114,7 +114,8 @@ struct CmdFlakeDeps : FlakeCommand, MixJSON, StoreCommand, MixEvalArgs
 
         FlakeRef flakeRef(flakeUri);
 
-        ResolvedFlake resFlake = resolveFlake(*evalState, flakeRef, AllowRegistryAtTop);
+        bool recreateLockFile = false;
+        ResolvedFlake resFlake = resolveFlake(*evalState, flakeRef, AllowRegistryAtTop, recreateLockFile);
 
         std::queue<ResolvedFlake> todo;
         todo.push(resFlake);
@@ -132,7 +133,7 @@ struct CmdFlakeDeps : FlakeCommand, MixJSON, StoreCommand, MixEvalArgs
     }
 };
 
-struct CmdFlakeUpdate : StoreCommand, GitRepoCommand, MixEvalArgs
+struct CmdFlakeUpdate : StoreCommand, FlakeCommand, MixEvalArgs
 {
     std::string name() override
     {
@@ -148,8 +149,8 @@ struct CmdFlakeUpdate : StoreCommand, GitRepoCommand, MixEvalArgs
     {
         auto evalState = std::make_shared<EvalState>(searchPath, store);
 
-        if (gitPath == "") gitPath = absPath(".");
+        bool recreateLockFile = true;
-        updateLockFile(*evalState, gitPath);
+        updateLockFile(*evalState, flakeUri, recreateLockFile);
     }
 };
 

src/nix/installables.cc

@@ -23,9 +23,9 @@ SourceExprCommand::SourceExprCommand()
         .dest(&file);
 
     mkFlag()
-        .longName("no-update")
+        .longName("recreate-lock-file")
-        .description("don't create/update flake lock files")
+        .description("recreate lock file from scratch")
-        .set(&updateLockFile, false);
+        .set(&recreateLockFile, true);
 }
 
 ref<EvalState> SourceExprCommand::getEvalState()
@@ -157,13 +157,11 @@ struct InstallableFlake : InstallableValue
 
     Value * toValue(EvalState & state) override
     {
-        auto path = std::get_if<FlakeRef::IsPath>(&flakeRef.data);
-        if (cmd.updateLockFile && path) {
-            updateLockFile(state, path->path);
-        }
-
         auto vFlake = state.allocValue();
-        makeFlakeValue(state, flakeRef, AllowRegistryAtTop, *vFlake);
+        if (std::get_if<FlakeRef::IsPath>(&flakeRef.data))
+            updateLockFile(state, flakeRef.to_string(), cmd.recreateLockFile);
+
+        makeFlakeValue(state, flakeRef, AllowRegistryAtTop, *vFlake, cmd.recreateLockFile);
 
         auto vProvides = (*vFlake->attrs->get(state.symbols.create("provides")))->value;
 

tests/flakes.sh

@@ -9,11 +9,11 @@ clearStore
 
 registry=$TEST_ROOT/registry.json
 
-flake1=$TEST_ROOT/flake1
+flake1Dir=$TEST_ROOT/flake1
-flake2=$TEST_ROOT/flake2
+flake2Dir=$TEST_ROOT/flake2
-flake3=$TEST_ROOT/flake3
+flake3Dir=$TEST_ROOT/flake3
 
-for repo in $flake1 $flake2 $flake3; do
+for repo in $flake1Dir $flake2Dir $flake3Dir; do
     rm -rf $repo
     mkdir $repo
     git -C $repo init
@@ -21,7 +21,7 @@ for repo in $flake1 $flake2 $flake3; do
     git -C $repo config user.name "Foobar"
 done
 
-cat > $flake1/flake.nix <<EOF
+cat > $flake1Dir/flake.nix <<EOF
 {
   name = "flake1";
 
@@ -36,11 +36,11 @@ cat > $flake1/flake.nix <<EOF
 }
 EOF
 
-cp ./simple.nix ./simple.builder.sh ./config.nix $flake1/
+cp ./simple.nix ./simple.builder.sh ./config.nix $flake1Dir/
-git -C $flake1 add flake.nix simple.nix simple.builder.sh config.nix
+git -C $flake1Dir add flake.nix simple.nix simple.builder.sh config.nix
-git -C $flake1 commit -m 'Initial'
+git -C $flake1Dir commit -m 'Initial'
 
-cat > $flake2/flake.nix <<EOF
+cat > $flake2Dir/flake.nix <<EOF
 {
   name = "flake2";
 
@@ -56,8 +56,8 @@ cat > $flake2/flake.nix <<EOF
 }
 EOF
 
-git -C $flake2 add flake.nix
+git -C $flake2Dir add flake.nix
-git -C $flake2 commit -m 'Initial'
+git -C $flake2Dir commit -m 'Initial'
 
 cat > $flake3/flake.nix <<EOF
 {
@@ -82,13 +82,13 @@ cat > $registry <<EOF
 {
     "flakes": {
         "flake1": {
-            "uri": "file://$flake1"
+            "uri": "file://$flake1Dir"
         },
         "flake2": {
-            "uri": "file://$flake2"
+            "uri": "file://$flake2Dir"
         },
         "flake3": {
-            "uri": "file://$flake3"
+            "uri": "file://$flake3Dir"
         },
         "nixpkgs": {
             "uri": "flake1"
@@ -121,13 +121,12 @@ nix build -o $TEST_ROOT/result --flake-registry $registry flake1:
 (! nix build -o $TEST_ROOT/result --flake-registry $registry flake2:bar)
 
 # But should succeed in impure mode.
-# FIXME: this currently fails.
+nix build -o $TEST_ROOT/result --flake-registry $registry flake2:bar --impure
-#nix build -o $TEST_ROOT/result --flake-registry $registry flake2:bar --impure
 
 # Test automatic lock file generation.
-nix build -o $TEST_ROOT/result --flake-registry $registry $flake2:bar
+nix build -o $TEST_ROOT/result --flake-registry $registry $flake2Dir:bar
-[[ -e $flake2/flake.lock ]]
+[[ -e $flake2Dir/flake.lock ]]
-git -C $flake2 commit flake.lock -m 'Add flake.lock'
+git -C $flake2Dir commit flake.lock -m 'Add flake.lock'
 
 # Rerunning the build should not change the lockfile.
 #nix build -o $TEST_ROOT/result --flake-registry $registry $flake2:bar
@@ -137,7 +136,7 @@ git -C $flake2 commit flake.lock -m 'Add flake.lock'
 nix build -o $TEST_ROOT/result --flake-registry $registry flake2:bar
 
 # Or without a registry.
-nix build -o $TEST_ROOT/result file://$flake2:bar
+nix build -o $TEST_ROOT/result file://$flake2Dir:bar
 
 # Test whether indirect dependencies work.
 #nix build -o $TEST_ROOT/result --flake-registry $registry $flake3:xyzzy