Check requiredSystemFeatures for local builds

For example, this prevents a "kvm" build on machines that don't have
KVM.

Fixes #2012.

5 files changed, 49 insertions, 15 deletions

diff --git a/src/libstore/build.cc b/src/libstore/build.cc
index eb7f106a20c3..0073b9b727ec 100644
--- a/src/libstore/build.cc
+++ b/src/libstore/build.cc
@@ -1646,18 +1646,13 @@ HookReply DerivationGoal::tryBuildHook()
 
     try {
 
-        /* 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 = parsedDrv->getStringsAttr("requiredSystemFeatures").value_or(Strings());
-
         /* Send the request to the hook. */
         worker.hook->sink
             << "try"
             << (worker.getNrLocalBuilds() < settings.maxBuildJobs ? 1 : 0)
             << drv->platform
             << drvPath
-            << features;
+            << parsedDrv->getRequiredSystemFeatures();
         worker.hook->sink.flush();
 
         /* Read the first line of input, which should be a word indicating
@@ -1797,11 +1792,13 @@ static void preloadNSS() {
 void DerivationGoal::startBuilder()
 {
     /* Right platform? */
-    if (!parsedDrv->canBuildLocally()) {
-        throw Error(
-            format("a '%1%' is required to build '%3%', but I am a '%2%'")
-            % drv->platform % settings.thisSystem % drvPath);
-    }
+    if (!parsedDrv->canBuildLocally())
+        throw Error("a '%s' with features {%s} is required to build '%s', but I am a '%s' with features {%s}",
+            drv->platform,
+            concatStringsSep(", ", parsedDrv->getRequiredSystemFeatures()),
+            drvPath,
+            settings.thisSystem,
+            concatStringsSep(", ", settings.systemFeatures));
 
     if (drv->isBuiltin())
         preloadNSS();
@@ -2625,7 +2622,7 @@ void DerivationGoal::runChild()
                 createDirs(chrootRootDir + "/dev/shm");
                 createDirs(chrootRootDir + "/dev/pts");
                 ss.push_back("/dev/full");
-                if (pathExists("/dev/kvm"))
+                if (settings.systemFeatures.get().count("kvm") && pathExists("/dev/kvm"))
                     ss.push_back("/dev/kvm");
                 ss.push_back("/dev/null");
                 ss.push_back("/dev/random");
diff --git a/src/libstore/globals.cc b/src/libstore/globals.cc
index d95db56726cb..a9c07b23a6f3 100644
--- a/src/libstore/globals.cc
+++ b/src/libstore/globals.cc
@@ -86,6 +86,21 @@ unsigned int Settings::getDefaultCores()
     return std::max(1U, std::thread::hardware_concurrency());
 }
 
+StringSet Settings::getDefaultSystemFeatures()
+{
+    /* For backwards compatibility, accept some "features" that are
+       used in Nixpkgs to route builds to certain machines but don't
+       actually require anything special on the machines. */
+    StringSet features{"nixos-test", "benchmark", "big-parallel"};
+
+    #if __linux__
+    if (access("/dev/kvm", R_OK | W_OK) == 0)
+        features.insert("kvm");
+    #endif
+
+    return features;
+}
+
 const string nixVersion = PACKAGE_VERSION;
 
 template<> void BaseSetting<SandboxMode>::set(const std::string & str)
diff --git a/src/libstore/globals.hh b/src/libstore/globals.hh
index f589078dbb98..cf4ae63cdc2f 100644
--- a/src/libstore/globals.hh
+++ b/src/libstore/globals.hh
@@ -32,6 +32,8 @@ class Settings : public Config {
 
     unsigned int getDefaultCores();
 
+    StringSet getDefaultSystemFeatures();
+
 public:
 
     Settings();
@@ -261,6 +263,10 @@ public:
         "These may be supported natively (e.g. armv7 on some aarch64 CPUs "
         "or using hacks like qemu-user."};
 
+    Setting<StringSet> systemFeatures{this, getDefaultSystemFeatures(),
+        "system-features",
+        "Optional features that this system implements (like \"kvm\")."};
+
     Setting<Strings> substituters{this,
         nixStore == "/nix/store" ? Strings{"https://cache.nixos.org/"} : Strings(),
         "substituters",
diff --git a/src/libstore/parsed-derivations.cc b/src/libstore/parsed-derivations.cc
index 0d7acf046afd..dc3286482736 100644
--- a/src/libstore/parsed-derivations.cc
+++ b/src/libstore/parsed-derivations.cc
@@ -82,11 +82,25 @@ std::experimental::optional<Strings> ParsedDerivation::getStringsAttr(const std:
     }
 }
 
+StringSet ParsedDerivation::getRequiredSystemFeatures() const
+{
+    StringSet res;
+    for (auto & i : getStringsAttr("requiredSystemFeatures").value_or(Strings()))
+        res.insert(i);
+    return res;
+}
+
 bool ParsedDerivation::canBuildLocally() const
 {
-    return drv.platform == settings.thisSystem
-        || settings.extraPlatforms.get().count(drv.platform) > 0
-        || drv.isBuiltin();
+    if (drv.platform != settings.thisSystem.get()
+        && !settings.extraPlatforms.get().count(drv.platform)
+        && !drv.isBuiltin())
+        return false;
+
+    for (auto & feature : getRequiredSystemFeatures())
+        if (!settings.systemFeatures.get().count(feature)) return false;
+
+    return true;
 }
 
 bool ParsedDerivation::willBuildLocally() const
diff --git a/src/libstore/parsed-derivations.hh b/src/libstore/parsed-derivations.hh
index 0c7dc32e1e04..0a82c146172b 100644
--- a/src/libstore/parsed-derivations.hh
+++ b/src/libstore/parsed-derivations.hh
@@ -25,6 +25,8 @@ public:
 
     std::experimental::optional<Strings> getStringsAttr(const std::string & name) const;
 
+    StringSet getRequiredSystemFeatures() const;
+
     bool canBuildLocally() const;
 
     bool willBuildLocally() const;