From 7ccd9464077180f633e65c15906bdda707077e8c Mon Sep 17 00:00:00 2001
From: Eelco Dolstra <eelco.dolstra@logicblox.com>
Date: Thu, 20 Jun 2013 20:05:03 +0200
Subject: [PATCH] Don't set $preferLocalBuild and $requiredSystemFeatures in
 builders
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

With C++ std::map, doing a comparison like ‘map["foo"] == ...’ has the
side-effect of adding a mapping from "foo" to the empty string if
"foo" doesn't exist in the map.  So we ended up setting some
environment variables by accident.
---
 src/libstore/build.cc | 21 ++++++++++++++-------
 src/libstore/misc.hh  |  2 +-
 2 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/src/libstore/build.cc b/src/libstore/build.cc
index 6e0a2b97b..f71601a77 100644
--- a/src/libstore/build.cc
+++ b/src/libstore/build.cc
@@ -1263,6 +1263,13 @@ PathSet outputPaths(const DerivationOutputs & outputs)
 }
 
 
+static string get(const StringPairs & map, const string & key)
+{
+    StringPairs::const_iterator i = map.find(key);
+    return i == map.end() ? (string) "" : i->second;
+}
+
+
 static bool canBuildLocally(const string & platform)
 {
     return platform == settings.thisSystem
@@ -1273,9 +1280,9 @@ static bool canBuildLocally(const string & platform)
 }
 
 
-bool willBuildLocally(Derivation & drv)
+bool willBuildLocally(const Derivation & drv)
 {
-    return drv.env["preferLocalBuild"] == "1" && canBuildLocally(drv.platform);
+    return get(drv.env, "preferLocalBuild") == "1" && canBuildLocally(drv.platform);
 }
 
 
@@ -1610,7 +1617,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.) */
-    Strings features = tokenizeString<Strings>(drv.env["requiredSystemFeatures"]);
+    Strings features = tokenizeString<Strings>(get(drv.env, "requiredSystemFeatures"));
     foreach (Strings::iterator, i, features) checkStoreName(*i); /* !!! abuse */
 
     /* Send the request to the hook. */
@@ -1768,7 +1775,7 @@ void DerivationGoal::startBuilder()
        fixed-output derivations is by definition pure (since we
        already know the cryptographic hash of the output). */
     if (fixedOutput) {
-        Strings varNames = tokenizeString<Strings>(drv.env["impureEnvVars"]);
+        Strings varNames = tokenizeString<Strings>(get(drv.env, "impureEnvVars"));
         foreach (Strings::iterator, i, varNames) env[*i] = getEnv(*i);
     }
 
@@ -1779,7 +1786,7 @@ void DerivationGoal::startBuilder()
        temporary build directory.  The text files have the format used
        by `nix-store --register-validity'.  However, the deriver
        fields are left empty. */
-    string s = drv.env["exportReferencesGraph"];
+    string s = get(drv.env, "exportReferencesGraph");
     Strings ss = tokenizeString<Strings>(s);
     if (ss.size() % 2 != 0)
         throw BuildError(format("odd number of tokens in `exportReferencesGraph': `%1%'") % s);
@@ -1865,7 +1872,7 @@ void DerivationGoal::startBuilder()
     if (fixedOutput) useChroot = false;
 
     /* Hack to allow derivations to disable chroot builds. */
-    if (drv.env["__noChroot"] == "1") useChroot = false;
+    if (get(drv.env, "__noChroot") == "1") useChroot = false;
 
     if (useChroot) {
 #if CHROOT_ENABLED
@@ -2344,7 +2351,7 @@ void DerivationGoal::computeClosure()
            refer to), check that all references are in that list.  !!!
            allowedReferences should really be per-output. */
         if (drv.env.find("allowedReferences") != drv.env.end()) {
-            PathSet allowed = parseReferenceSpecifiers(drv, drv.env["allowedReferences"]);
+            PathSet allowed = parseReferenceSpecifiers(drv, get(drv.env, "allowedReferences"));
             foreach (PathSet::iterator, i, references)
                 if (allowed.find(*i) == allowed.end())
                     throw BuildError(format("output is not allowed to refer to path `%1%'") % *i);
diff --git a/src/libstore/misc.hh b/src/libstore/misc.hh
index 6f90ebe5e..144cb7f45 100644
--- a/src/libstore/misc.hh
+++ b/src/libstore/misc.hh
@@ -32,7 +32,7 @@ void queryMissing(StoreAPI & store, const PathSet & targets,
     PathSet & willBuild, PathSet & willSubstitute, PathSet & unknown,
     unsigned long long & downloadSize, unsigned long long & narSize);
 
-bool willBuildLocally(Derivation & drv);
+bool willBuildLocally(const Derivation & drv);
 
 
 }