Move structured attrs handling into a separate class

This is primarily because Derivation::{can,will}BuildLocally() depends
on attributes like preferLocalBuild and requiredSystemFeatures, but it
can't handle them properly because it doesn't have access to the
structured attributes.

5 files changed, 145 insertions, 116 deletions

diff --git a/src/libstore/build.cc b/src/libstore/build.cc
index 69c3c2c1e041..eb7f106a20c3 100644
--- a/src/libstore/build.cc
+++ b/src/libstore/build.cc
@@ -11,6 +11,7 @@
 #include "compression.hh"
 #include "json.hh"
 #include "nar-info.hh"
+#include "parsed-derivations.hh"
 
 #include <algorithm>
 #include <iostream>
@@ -740,8 +741,7 @@ private:
     /* The derivation stored at drvPath. */
     std::unique_ptr<BasicDerivation> drv;
 
-    /* The contents of drv->env["__json"]. */
-    std::experimental::optional<nlohmann::json> structuredAttrs;
+    std::unique_ptr<ParsedDerivation> parsedDrv;
 
     /* The remainder is state held during the build. */
 
@@ -923,13 +923,6 @@ private:
     /* Fill in the environment for the builder. */
     void initEnv();
 
-    /* Get an attribute from drv->env or from drv->env["__json"]. */
-    std::experimental::optional<std::string> getAttr(const std::string & name);
-
-    bool getBoolAttr(const std::string & name, bool def = false);
-
-    std::experimental::optional<Strings> getStringsAttr(const std::string & name);
-
     /* Write a JSON file containing the derivation attributes. */
     void writeStructuredAttrs();
 
@@ -1149,15 +1142,7 @@ void DerivationGoal::haveDerivation()
         return;
     }
 
-    /* Parse the __json attribute, if any. */
-    auto jsonAttr = drv->env.find("__json");
-    if (jsonAttr != drv->env.end()) {
-        try {
-            structuredAttrs = nlohmann::json::parse(jsonAttr->second);
-        } catch (std::exception & e) {
-            throw Error("cannot process __json attribute of '%s': %s", drvPath, e.what());
-        }
-    }
+    parsedDrv = std::make_unique<ParsedDerivation>(drvPath, *drv);
 
     /* We are first going to try to create the invalid output paths
        through substitutes.  If that doesn't work, we'll build
@@ -1415,7 +1400,7 @@ void DerivationGoal::tryToBuild()
     /* Don't do a remote build if the derivation has the attribute
        `preferLocalBuild' set.  Also, check and repair modes are only
        supported for local builds. */
-    bool buildLocally = buildMode != bmNormal || drv->willBuildLocally();
+    bool buildLocally = buildMode != bmNormal || parsedDrv->willBuildLocally();
 
     auto started = [&]() {
         auto msg = fmt(
@@ -1664,7 +1649,7 @@ HookReply DerivationGoal::tryBuildHook()
         /* Tell the hook about system features (beyond the system type)
            required from the build machine.  (The hook could parse the
            drv file itself, but this is easier.) */
-        auto features = getStringsAttr("requiredSystemFeatures").value_or(Strings());
+        auto features = parsedDrv->getStringsAttr("requiredSystemFeatures").value_or(Strings());
 
         /* Send the request to the hook. */
         worker.hook->sink
@@ -1812,7 +1797,7 @@ static void preloadNSS() {
 void DerivationGoal::startBuilder()
 {
     /* Right platform? */
-    if (!drv->canBuildLocally()) {
+    if (!parsedDrv->canBuildLocally()) {
         throw Error(
             format("a '%1%' is required to build '%3%', but I am a '%2%'")
             % drv->platform % settings.thisSystem % drvPath);
@@ -1822,12 +1807,12 @@ void DerivationGoal::startBuilder()
         preloadNSS();
 
 #if __APPLE__
-    additionalSandboxProfile = getAttr("__sandboxProfile").value_or("");
+    additionalSandboxProfile = parsedDrv->getStringAttr("__sandboxProfile").value_or("");
 #endif
 
     /* Are we doing a chroot build? */
     {
-        auto noChroot = getBoolAttr("__noChroot");
+        auto noChroot = parsedDrv->getBoolAttr("__noChroot");
         if (settings.sandboxMode == smEnabled) {
             if (noChroot)
                 throw Error(format("derivation '%1%' has '__noChroot' set, "
@@ -1893,7 +1878,7 @@ void DerivationGoal::startBuilder()
     writeStructuredAttrs();
 
     /* Handle exportReferencesGraph(), if set. */
-    if (!structuredAttrs) {
+    if (!parsedDrv->getStructuredAttrs()) {
         /* The `exportReferencesGraph' feature allows the references graph
            to be passed to a builder.  This attribute should be a list of
            pairs [name1 path1 name2 path2 ...].  The references graph of
@@ -1958,7 +1943,7 @@ void DerivationGoal::startBuilder()
         PathSet allowedPaths = settings.allowedImpureHostPrefixes;
 
         /* This works like the above, except on a per-derivation level */
-        auto impurePaths = getStringsAttr("__impureHostDeps").value_or(Strings());
+        auto impurePaths = parsedDrv->getStringsAttr("__impureHostDeps").value_or(Strings());
 
         for (auto & i : impurePaths) {
             bool found = false;
@@ -2326,7 +2311,7 @@ void DerivationGoal::initEnv()
        passAsFile is ignored in structure mode because it's not
        needed (attributes are not passed through the environment, so
        there is no size constraint). */
-    if (!structuredAttrs) {
+    if (!parsedDrv->getStructuredAttrs()) {
 
         StringSet passAsFile = tokenizeString<StringSet>(get(drv->env, "passAsFile"));
         int fileNr = 0;
@@ -2373,7 +2358,7 @@ void DerivationGoal::initEnv()
        fixed-output derivations is by definition pure (since we
        already know the cryptographic hash of the output). */
     if (fixedOutput) {
-        for (auto & i : getStringsAttr("impureEnvVars").value_or(Strings()))
+        for (auto & i : parsedDrv->getStringsAttr("impureEnvVars").value_or(Strings()))
             env[i] = getEnv(i);
     }
 
@@ -2384,80 +2369,12 @@ void DerivationGoal::initEnv()
 }
 
 
-std::experimental::optional<std::string> DerivationGoal::getAttr(const std::string & name)
-{
-    if (structuredAttrs) {
-        auto i = structuredAttrs->find(name);
-        if (i == structuredAttrs->end())
-            return {};
-        else {
-            if (!i->is_string())
-                throw Error("attribute '%s' of derivation '%s' must be a string", name, drvPath);
-            return i->get<std::string>();
-        }
-    } else {
-        auto i = drv->env.find(name);
-        if (i == drv->env.end())
-            return {};
-        else
-            return i->second;
-    }
-}
-
-
-bool DerivationGoal::getBoolAttr(const std::string & name, bool def)
-{
-    if (structuredAttrs) {
-        auto i = structuredAttrs->find(name);
-        if (i == structuredAttrs->end())
-            return def;
-        else {
-            if (!i->is_boolean())
-                throw Error("attribute '%s' of derivation '%s' must be a Boolean", name, drvPath);
-            return i->get<bool>();
-        }
-    } else {
-        auto i = drv->env.find(name);
-        if (i == drv->env.end())
-            return def;
-        else
-            return i->second == "1";
-    }
-}
-
-
-std::experimental::optional<Strings> DerivationGoal::getStringsAttr(const std::string & name)
-{
-    if (structuredAttrs) {
-        auto i = structuredAttrs->find(name);
-        if (i == structuredAttrs->end())
-            return {};
-        else {
-            if (!i->is_array())
-                throw Error("attribute '%s' of derivation '%s' must be a list of strings", name, drvPath);
-            Strings res;
-            for (auto j = i->begin(); j != i->end(); ++j) {
-                if (!j->is_string())
-                    throw Error("attribute '%s' of derivation '%s' must be a list of strings", name, drvPath);
-                res.push_back(j->get<std::string>());
-            }
-            return res;
-        }
-    } else {
-        auto i = drv->env.find(name);
-        if (i == drv->env.end())
-            return {};
-        else
-            return tokenizeString<Strings>(i->second);
-    }
-}
-
-
 static std::regex shVarName("[A-Za-z_][A-Za-z0-9_]*");
 
 
 void DerivationGoal::writeStructuredAttrs()
 {
+    auto & structuredAttrs = parsedDrv->getStructuredAttrs();
     if (!structuredAttrs) return;
 
     auto json = *structuredAttrs;
@@ -2997,7 +2914,7 @@ void DerivationGoal::runChild()
 
             writeFile(sandboxFile, sandboxProfile);
 
-            bool allowLocalNetworking = getBoolAttr("__darwinAllowLocalNetworking");
+            bool allowLocalNetworking = parsedDrv->getBoolAttr("__darwinAllowLocalNetworking");
 
             /* The tmpDir in scope points at the temporary build directory for our derivation. Some packages try different mechanisms
                to find temporary directories, so we want to open up a broader place for them to dump their files, if needed. */
@@ -3283,7 +3200,7 @@ void DerivationGoal::registerOutputs()
 
         /* Enforce `allowedReferences' and friends. */
         auto checkRefs = [&](const string & attrName, bool allowed, bool recursive) {
-            auto value = getStringsAttr(attrName);
+            auto value = parsedDrv->getStringsAttr(attrName);
             if (!value) return;
 
             PathSet spec = parseReferenceSpecifiers(worker.store, *drv, *value);
diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc
index 1e187ec5e954..3961126fff9c 100644
--- a/src/libstore/derivations.cc
+++ b/src/libstore/derivations.cc
@@ -36,12 +36,6 @@ Path BasicDerivation::findOutput(const string & id) const
 }
 
 
-bool BasicDerivation::willBuildLocally() const
-{
-    return get(env, "preferLocalBuild") == "1" && canBuildLocally();
-}
-
-
 bool BasicDerivation::substitutesAllowed() const
 {
     return get(env, "allowSubstitutes", "1") == "1";
@@ -54,14 +48,6 @@ bool BasicDerivation::isBuiltin() const
 }
 
 
-bool BasicDerivation::canBuildLocally() const
-{
-    return platform == settings.thisSystem
-        || settings.extraPlatforms.get().count(platform) > 0
-        || isBuiltin();
-}
-
-
 Path writeDerivation(ref<Store> store,
     const Derivation & drv, const string & name, RepairFlag repair)
 {
diff --git a/src/libstore/derivations.hh b/src/libstore/derivations.hh
index 7b97730d3bf2..9753e796db5f 100644
--- a/src/libstore/derivations.hh
+++ b/src/libstore/derivations.hh
@@ -56,14 +56,10 @@ struct BasicDerivation
        the given derivation. */
     Path findOutput(const string & id) const;
 
-    bool willBuildLocally() const;
-
     bool substitutesAllowed() const;
 
     bool isBuiltin() const;
 
-    bool canBuildLocally() const;
-
     /* Return true iff this is a fixed-output derivation. */
     bool isFixedOutput() const;
 
diff --git a/src/libstore/parsed-derivations.cc b/src/libstore/parsed-derivations.cc
new file mode 100644
index 000000000000..0d7acf046afd
--- /dev/null
+++ b/src/libstore/parsed-derivations.cc
@@ -0,0 +1,97 @@
+#include "parsed-derivations.hh"
+
+namespace nix {
+
+ParsedDerivation::ParsedDerivation(const Path & drvPath, BasicDerivation & drv)
+    : drvPath(drvPath), drv(drv)
+{
+    /* Parse the __json attribute, if any. */
+    auto jsonAttr = drv.env.find("__json");
+    if (jsonAttr != drv.env.end()) {
+        try {
+            structuredAttrs = nlohmann::json::parse(jsonAttr->second);
+        } catch (std::exception & e) {
+            throw Error("cannot process __json attribute of '%s': %s", drvPath, e.what());
+        }
+    }
+}
+
+std::experimental::optional<std::string> ParsedDerivation::getStringAttr(const std::string & name) const
+{
+    if (structuredAttrs) {
+        auto i = structuredAttrs->find(name);
+        if (i == structuredAttrs->end())
+            return {};
+        else {
+            if (!i->is_string())
+                throw Error("attribute '%s' of derivation '%s' must be a string", name, drvPath);
+            return i->get<std::string>();
+        }
+    } else {
+        auto i = drv.env.find(name);
+        if (i == drv.env.end())
+            return {};
+        else
+            return i->second;
+    }
+}
+
+bool ParsedDerivation::getBoolAttr(const std::string & name, bool def) const
+{
+    if (structuredAttrs) {
+        auto i = structuredAttrs->find(name);
+        if (i == structuredAttrs->end())
+            return def;
+        else {
+            if (!i->is_boolean())
+                throw Error("attribute '%s' of derivation '%s' must be a Boolean", name, drvPath);
+            return i->get<bool>();
+        }
+    } else {
+        auto i = drv.env.find(name);
+        if (i == drv.env.end())
+            return def;
+        else
+            return i->second == "1";
+    }
+}
+
+std::experimental::optional<Strings> ParsedDerivation::getStringsAttr(const std::string & name) const
+{
+    if (structuredAttrs) {
+        auto i = structuredAttrs->find(name);
+        if (i == structuredAttrs->end())
+            return {};
+        else {
+            if (!i->is_array())
+                throw Error("attribute '%s' of derivation '%s' must be a list of strings", name, drvPath);
+            Strings res;
+            for (auto j = i->begin(); j != i->end(); ++j) {
+                if (!j->is_string())
+                    throw Error("attribute '%s' of derivation '%s' must be a list of strings", name, drvPath);
+                res.push_back(j->get<std::string>());
+            }
+            return res;
+        }
+    } else {
+        auto i = drv.env.find(name);
+        if (i == drv.env.end())
+            return {};
+        else
+            return tokenizeString<Strings>(i->second);
+    }
+}
+
+bool ParsedDerivation::canBuildLocally() const
+{
+    return drv.platform == settings.thisSystem
+        || settings.extraPlatforms.get().count(drv.platform) > 0
+        || drv.isBuiltin();
+}
+
+bool ParsedDerivation::willBuildLocally() const
+{
+    return getBoolAttr("preferLocalBuild") && canBuildLocally();
+}
+
+}
diff --git a/src/libstore/parsed-derivations.hh b/src/libstore/parsed-derivations.hh
new file mode 100644
index 000000000000..0c7dc32e1e04
--- /dev/null
+++ b/src/libstore/parsed-derivations.hh
@@ -0,0 +1,33 @@
+#include "derivations.hh"
+
+#include <nlohmann/json.hpp>
+
+namespace nix {
+
+class ParsedDerivation
+{
+    Path drvPath;
+    BasicDerivation & drv;
+    std::experimental::optional<nlohmann::json> structuredAttrs;
+
+public:
+
+    ParsedDerivation(const Path & drvPath, BasicDerivation & drv);
+
+    const std::experimental::optional<nlohmann::json> & getStructuredAttrs() const
+    {
+        return structuredAttrs;
+    }
+
+    std::experimental::optional<std::string> getStringAttr(const std::string & name) const;
+
+    bool getBoolAttr(const std::string & name, bool def = false) const;
+
+    std::experimental::optional<Strings> getStringsAttr(const std::string & name) const;
+
+    bool canBuildLocally() const;
+
+    bool willBuildLocally() const;
+};
+
+}