Explicitly model all settings and fail on unrecognized ones

Previously, the Settings class allowed other code to query for string
properties, which led to a proliferation of code all over the place making
up new options without any sort of central registry of valid options. This
commit pulls all those options back into the central Settings class and
removes the public get() methods, to discourage future abuses like that.

Furthermore, because we know the full set of options ahead of time, we
now fail loudly if someone enters an unrecognized option, thus preventing
subtle typos. With some template fun, we could probably also dump the full
set of options (with documentation, defaults, etc.) to the command line,
but I'm not doing that yet here.

1 files changed, 11 insertions, 32 deletions

diff --git a/src/libstore/build.cc b/src/libstore/build.cc
index 1ce23135fc..4a7e1a62b5 100644
--- a/src/libstore/build.cc
+++ b/src/libstore/build.cc
@@ -34,13 +34,6 @@
 #include <pwd.h>
 #include <grp.h>
 
-/* chroot-like behavior from Apple's sandbox */
-#if __APPLE__
-    #define DEFAULT_ALLOWED_IMPURE_PREFIXES "/System/Library /usr/lib /dev /bin/sh"
-#else
-    #define DEFAULT_ALLOWED_IMPURE_PREFIXES ""
-#endif
-
 /* Includes required for chroot support. */
 #if __linux__
 #include <sys/socket.h>
@@ -1279,7 +1272,7 @@ void DerivationGoal::inputsRealised()
 
     /* Don't repeat fixed-output derivations since they're already
        verified by their output hash.*/
-    nrRounds = fixedOutput ? 1 : settings.get("build-repeat", 0) + 1;
+    nrRounds = fixedOutput ? 1 : settings.buildRepeat + 1;
 
     /* Okay, try to build.  Note that here we don't wait for a build
        slot to become available, since we don't need one if there is a
@@ -1685,9 +1678,7 @@ void DerivationGoal::startBuilder()
 
     /* Are we doing a chroot build? */
     {
-        string x = settings.get("build-use-sandbox",
-            /* deprecated alias */
-            settings.get("build-use-chroot", string("false")));
+        string x = settings.useSandbox;
         if (x != "true" && x != "false" && x != "relaxed")
             throw Error("option ‘build-use-sandbox’ must be set to one of ‘true’, ‘false’ or ‘relaxed’");
         if (x == "true") {
@@ -1744,21 +1735,10 @@ void DerivationGoal::startBuilder()
 
     if (useChroot) {
 
-        string defaultChrootDirs;
-#if __linux__
-        if (worker.store.isInStore(BASH_PATH))
-            defaultChrootDirs = "/bin/sh=" BASH_PATH;
-#endif
-
         /* Allow a user-configurable set of directories from the
            host file system. */
-        PathSet dirs = tokenizeString<StringSet>(
-            settings.get("build-sandbox-paths",
-                /* deprecated alias with lower priority */
-                settings.get("build-chroot-dirs", defaultChrootDirs)));
-        PathSet dirs2 = tokenizeString<StringSet>(
-            settings.get("build-extra-chroot-dirs",
-                settings.get("build-extra-sandbox-paths", string(""))));
+        PathSet dirs = settings.sandboxPaths;
+        PathSet dirs2 = settings.extraSandboxPaths;
         dirs.insert(dirs2.begin(), dirs2.end());
 
         dirsInChroot.clear();
@@ -1790,8 +1770,7 @@ void DerivationGoal::startBuilder()
         for (auto & i : closure)
             dirsInChroot[i] = i;
 
-        string allowed = settings.get("allowed-impure-host-deps", string(DEFAULT_ALLOWED_IMPURE_PREFIXES));
-        PathSet allowedPaths = tokenizeString<StringSet>(allowed);
+        PathSet allowedPaths = settings.allowedImpureHostPrefixes;
 
         /* This works like the above, except on a per-derivation level */
         Strings impurePaths = tokenizeString<Strings>(get(drv->env, "__impureHostDeps"));
@@ -1811,7 +1790,7 @@ void DerivationGoal::startBuilder()
                 }
             }
             if (!found)
-                throw Error(format("derivation ‘%1%’ requested impure path ‘%2%’, but it was not in allowed-impure-host-deps (‘%3%’)") % drvPath % i % allowed);
+                throw Error(format("derivation ‘%1%’ requested impure path ‘%2%’, but it was not in allowed-impure-host-deps") % drvPath % i);
 
             dirsInChroot[i] = i;
         }
@@ -2433,7 +2412,7 @@ void DerivationGoal::runChild()
             /* Mount a new tmpfs on /dev/shm to ensure that whatever
                the builder puts in /dev/shm is cleaned up automatically. */
             if (pathExists("/dev/shm") && mount("none", (chrootRootDir + "/dev/shm").c_str(), "tmpfs", 0,
-                    fmt("size=%s", settings.get("sandbox-dev-shm-size", std::string("50%"))).c_str()) == -1)
+                    fmt("size=%s", settings.sandboxShmSize).c_str()) == -1)
                 throw SysError("mounting /dev/shm");
 
 #if 0
@@ -2596,7 +2575,7 @@ void DerivationGoal::runChild()
             sandboxProfile += "(version 1)\n";
 
             /* Violations will go to the syslog if you set this. Unfortunately the destination does not appear to be configurable */
-            if (settings.get("darwin-log-sandbox-violations", false)) {
+            if (settings.darwinLogSandboxViolations) {
                 sandboxProfile += "(deny default)\n";
             } else {
                 sandboxProfile += "(deny default (with no-log))\n";
@@ -2743,7 +2722,7 @@ void DerivationGoal::registerOutputs()
     InodesSeen inodesSeen;
 
     Path checkSuffix = ".check";
-    bool runDiffHook = settings.get("run-diff-hook", false);
+    bool runDiffHook = settings.runDiffHook;
     bool keepPreviousRound = settings.keepFailed || runDiffHook;
 
     /* Check whether the output paths were created, and grep each
@@ -2981,7 +2960,7 @@ void DerivationGoal::registerOutputs()
                     ? fmt("output ‘%1%’ of ‘%2%’ differs from ‘%3%’ from previous round", i->path, drvPath, prev)
                     : fmt("output ‘%1%’ of ‘%2%’ differs from previous round", i->path, drvPath);
 
-                auto diffHook = settings.get("diff-hook", std::string(""));
+                auto diffHook = settings.diffHook;
                 if (prevExists && diffHook != "" && runDiffHook) {
                     try {
                         auto diff = runProgram(diffHook, true, {prev, i->path});
@@ -2992,7 +2971,7 @@ void DerivationGoal::registerOutputs()
                     }
                 }
 
-                if (settings.get("enforce-determinism", true))
+                if (settings.enforceDeterminism)
                     throw NotDeterministic(msg);
 
                 printError(msg);