Merge pull request #3434 from Ericson2314/derivation-header-include-order

Revise division of labor in deserialization of derivations

7 files changed, 41 insertions, 48 deletions

diff --git a/perl/Makefile b/perl/Makefile
index 7ddb0cf69..259ed7dc3 100644
--- a/perl/Makefile
+++ b/perl/Makefile
@@ -1,6 +1,6 @@
 makefiles = local.mk
 
-GLOBAL_CXXFLAGS += -g -Wall
+GLOBAL_CXXFLAGS += -g -Wall -std=c++17
 
 -include Makefile.config
 
diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc
index dcc34407b..03fc4d1b3 100644
--- a/src/libexpr/primops.cc
+++ b/src/libexpr/primops.cc
@@ -101,7 +101,7 @@ static void prim_scopedImport(EvalState & state, const Pos & pos, Value * * args
     };
     if (auto optStorePath = isValidDerivationInStore()) {
         auto storePath = *optStorePath;
-        Derivation drv = readDerivation(*state.store, realPath, Derivation::nameFromPath(storePath));
+        Derivation drv = state.store->readDerivation(storePath);
         Value & w = *state.allocValue();
         state.mkAttrs(w, 3 + drv.outputs.size());
         Value * v2 = state.allocAttr(w, state.sDrvPath);
diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc
index 43bc61e55..d263cf0c5 100644
--- a/src/libstore/derivations.cc
+++ b/src/libstore/derivations.cc
@@ -185,7 +185,7 @@ static DerivationOutput parseDerivationOutput(const Store & store, std::istrings
 }
 
 
-static Derivation parseDerivation(const Store & store, std::string && s, std::string_view name)
+Derivation parseDerivation(const Store & store, std::string && s, std::string_view name)
 {
     Derivation drv;
     drv.name = name;
@@ -233,34 +233,6 @@ static Derivation parseDerivation(const Store & store, std::string && s, std::st
 }
 
 
-Derivation readDerivation(const Store & store, const Path & drvPath, std::string_view name)
-{
-    try {
-        return parseDerivation(store, readFile(drvPath), name);
-    } catch (FormatError & e) {
-        throw Error("error parsing derivation '%1%': %2%", drvPath, e.msg());
-    }
-}
-
-
-Derivation Store::derivationFromPath(const StorePath & drvPath)
-{
-    ensurePath(drvPath);
-    return readDerivation(drvPath);
-}
-
-
-Derivation Store::readDerivation(const StorePath & drvPath)
-{
-    auto accessor = getFSAccessor();
-    try {
-        return parseDerivation(*this, accessor->readFile(printStorePath(drvPath)), Derivation::nameFromPath(drvPath));
-    } catch (FormatError & e) {
-        throw Error("error parsing derivation '%s': %s", printStorePath(drvPath), e.msg());
-    }
-}
-
-
 static void printString(string & res, std::string_view s)
 {
     char buf[s.size() * 2 + 2];
diff --git a/src/libstore/derivations.hh b/src/libstore/derivations.hh
index 9656a32f2..573502c90 100644
--- a/src/libstore/derivations.hh
+++ b/src/libstore/derivations.hh
@@ -150,7 +150,7 @@ StorePath writeDerivation(Store & store,
     const Derivation & drv, RepairFlag repair = NoRepair);
 
 /* Read a derivation from a file. */
-Derivation readDerivation(const Store & store, const Path & drvPath, std::string_view name);
+Derivation parseDerivation(const Store & store, std::string && s, std::string_view name);
 
 // FIXME: remove
 bool isDerivation(const string & fileName);
diff --git a/src/libstore/local-store.hh b/src/libstore/local-store.hh
index 5af12c2b2..bb4ed9b19 100644
--- a/src/libstore/local-store.hh
+++ b/src/libstore/local-store.hh
@@ -23,9 +23,6 @@ namespace nix {
 const int nixSchemaVersion = 10;
 
 
-struct Derivation;
-
-
 struct OptimiseStats
 {
     unsigned long filesLinked = 0;
diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc
index b8ce71f3c..a41441079 100644
--- a/src/libstore/store-api.cc
+++ b/src/libstore/store-api.cc
@@ -1,11 +1,11 @@
 #include "crypto.hh"
+#include "fs-accessor.hh"
 #include "globals.hh"
 #include "store-api.hh"
 #include "util.hh"
 #include "nar-info-disk-cache.hh"
 #include "thread-pool.hh"
 #include "json.hh"
-#include "derivations.hh"
 #include "url.hh"
 #include "archive.hh"
 
@@ -983,6 +983,26 @@ Strings ValidPathInfo::shortRefs() const
 }
 
 
+Derivation Store::derivationFromPath(const StorePath & drvPath)
+{
+    ensurePath(drvPath);
+    return readDerivation(drvPath);
+}
+
+
+Derivation Store::readDerivation(const StorePath & drvPath)
+{
+    auto accessor = getFSAccessor();
+    try {
+        return parseDerivation(*this,
+            accessor->readFile(printStorePath(drvPath)),
+            Derivation::nameFromPath(drvPath));
+    } catch (FormatError & e) {
+        throw Error("error parsing derivation '%s': %s", printStorePath(drvPath), e.msg());
+    }
+}
+
+
 }
 
 
diff --git a/src/nix/repl.cc b/src/nix/repl.cc
index a74655200..a3e92b966 100644
--- a/src/nix/repl.cc
+++ b/src/nix/repl.cc
@@ -64,7 +64,7 @@ struct NixRepl
     void mainLoop(const std::vector<std::string> & files);
     StringSet completePrefix(string prefix);
     bool getLine(string & input, const std::string &prompt);
-    Path getDerivationPath(Value & v);
+    StorePath getDerivationPath(Value & v);
     bool processLine(string line);
     void loadFile(const Path & path);
     void initEnv();
@@ -375,13 +375,16 @@ bool isVarName(const string & s)
 }
 
 
-Path NixRepl::getDerivationPath(Value & v) {
+StorePath NixRepl::getDerivationPath(Value & v) {
     auto drvInfo = getDerivation(*state, v, false);
     if (!drvInfo)
         throw Error("expression does not evaluate to a derivation, so I can't build it");
-    Path drvPath = drvInfo->queryDrvPath();
-    if (drvPath == "" || !state->store->isValidPath(state->store->parseStorePath(drvPath)))
-        throw Error("expression did not evaluate to a valid derivation");
+    Path drvPathRaw = drvInfo->queryDrvPath();
+    if (drvPathRaw == "")
+        throw Error("expression did not evaluate to a valid derivation (no drv path)");
+    StorePath drvPath = state->store->parseStorePath(drvPathRaw);
+    if (!state->store->isValidPath(drvPath))
+        throw Error("expression did not evaluate to a valid derivation (invalid drv path)");
     return drvPath;
 }
 
@@ -474,29 +477,30 @@ bool NixRepl::processLine(string line)
         evalString("drv: (import <nixpkgs> {}).runCommand \"shell\" { buildInputs = [ drv ]; } \"\"", f);
         state->callFunction(f, v, result, Pos());
 
-        Path drvPath = getDerivationPath(result);
-        runProgram(settings.nixBinDir + "/nix-shell", Strings{drvPath});
+        StorePath drvPath = getDerivationPath(result);
+        runProgram(settings.nixBinDir + "/nix-shell", Strings{state->store->printStorePath(drvPath)});
     }
 
     else if (command == ":b" || command == ":i" || command == ":s") {
         Value v;
         evalString(arg, v);
-        Path drvPath = getDerivationPath(v);
+        StorePath drvPath = getDerivationPath(v);
+        Path drvPathRaw = state->store->printStorePath(drvPath);
 
         if (command == ":b") {
             /* We could do the build in this process using buildPaths(),
                but doing it in a child makes it easier to recover from
                problems / SIGINT. */
-            if (runProgram(settings.nixBinDir + "/nix", Strings{"build", "--no-link", drvPath}) == 0) {
-                auto drv = readDerivation(*state->store, drvPath, Derivation::nameFromPath(state->store->parseStorePath(drvPath)));
+            if (runProgram(settings.nixBinDir + "/nix", Strings{"build", "--no-link", drvPathRaw}) == 0) {
+                auto drv = state->store->readDerivation(drvPath);
                 std::cout << std::endl << "this derivation produced the following outputs:" << std::endl;
                 for (auto & i : drv.outputsAndPaths(*state->store))
                     std::cout << fmt("  %s -> %s\n", i.first, state->store->printStorePath(i.second.second));
             }
         } else if (command == ":i") {
-            runProgram(settings.nixBinDir + "/nix-env", Strings{"-i", drvPath});
+            runProgram(settings.nixBinDir + "/nix-env", Strings{"-i", drvPathRaw});
         } else {
-            runProgram(settings.nixBinDir + "/nix-shell", Strings{drvPath});
+            runProgram(settings.nixBinDir + "/nix-shell", Strings{drvPathRaw});
         }
     }