Check requiredSystemFeatures for local builds

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

Fixes #2012.

8 files changed, 79 insertions, 16 deletions

diff --git a/doc/manual/command-ref/conf-file.xml b/doc/manual/command-ref/conf-file.xml
index fd09883be402..e9947ebc673f 100644
--- a/doc/manual/command-ref/conf-file.xml
+++ b/doc/manual/command-ref/conf-file.xml
@@ -757,6 +757,33 @@ password <replaceable>my-password</replaceable>
   </varlistentry>
 
 
+  <varlistentry xml:id="conf-system-features"><term><literal>system-features</literal></term>
+
+    <listitem><para>A set of system “features” supported by this
+    machine, e.g. <literal>kvm</literal>. Derivations can express a
+    dependency on such features through the derivation attribute
+    <varname>requiredSystemFeatures</varname>. For example, the
+    attribute
+
+<programlisting>
+requiredSystemFeatures = [ "kvm" ];
+</programlisting>
+
+    ensures that the derivation can only be built on a machine with
+    the <literal>kvm</literal> feature.</para>
+
+    <para>This setting by default includes <literal>kvm</literal> if
+    <filename>/dev/kvm</filename> is accessible, and the
+    pseudo-features <literal>nixos-test</literal>,
+    <literal>benchmark</literal> and <literal>big-parallel</literal>
+    that are used in Nixpkgs to route builds to specific
+    machines.</para>
+
+    </listitem>
+
+  </varlistentry>
+
+
   <varlistentry xml:id="conf-timeout"><term><literal>timeout</literal></term>
 
     <listitem>
diff --git a/doc/manual/release-notes/release-notes.xml b/doc/manual/release-notes/release-notes.xml
index ff4085cb792d..e8ff586fa43f 100644
--- a/doc/manual/release-notes/release-notes.xml
+++ b/doc/manual/release-notes/release-notes.xml
@@ -12,6 +12,7 @@
 </partintro>
 -->
 
+<xi:include href="rl-2.2.xml" />
 <xi:include href="rl-2.1.xml" />
 <xi:include href="rl-2.0.xml" />
 <xi:include href="rl-1.11.10.xml" />
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;
diff --git a/tests/build-remote.sh b/tests/build-remote.sh
index 9bca0f4a3856..ddd68f327a15 100644
--- a/tests/build-remote.sh
+++ b/tests/build-remote.sh
@@ -11,7 +11,8 @@ rm -rf $TEST_ROOT/store0 $TEST_ROOT/store1
 
 nix build -f build-hook.nix -o $TEST_ROOT/result --max-jobs 0 \
   --sandbox-paths /nix/store --sandbox-build-dir /build-tmp \
-  --builders "$TEST_ROOT/store0; $TEST_ROOT/store1 - - 1 1 foo"
+  --builders "$TEST_ROOT/store0; $TEST_ROOT/store1 - - 1 1 foo" \
+  --system-features foo
 
 outPath=$TEST_ROOT/result