Merge remote-tracking branch 'origin/master' into flakes

16 files changed, 661 insertions, 148 deletions

diff --git a/src/libstore/build.cc b/src/libstore/build.cc
index bf259e0b6..0de2f5bd2 100644
--- a/src/libstore/build.cc
+++ b/src/libstore/build.cc
@@ -14,6 +14,7 @@
 #include "nar-info.hh"
 #include "parsed-derivations.hh"
 #include "machines.hh"
+#include "daemon.hh"
 
 #include <algorithm>
 #include <iostream>
@@ -34,6 +35,7 @@
 #include <sys/select.h>
 #include <sys/resource.h>
 #include <sys/socket.h>
+#include <sys/un.h>
 #include <fcntl.h>
 #include <netdb.h>
 #include <unistd.h>
@@ -766,9 +768,6 @@ private:
        immediate input paths). */
     PathSet inputPaths;
 
-    /* Referenceable paths (i.e., input and output paths). */
-    PathSet allPaths;
-
     /* Outputs that are already valid.  If we're repairing, these are
        the outputs that are valid *and* not corrupt. */
     PathSet validPaths;
@@ -806,9 +805,13 @@ private:
     /* Pipe for the builder's standard output/error. */
     Pipe builderOut;
 
-    /* Pipe for synchronising updates to the builder user namespace. */
+    /* Pipe for synchronising updates to the builder namespaces. */
     Pipe userNamespaceSync;
 
+    /* The mount namespace of the builder, used to add additional
+       paths to the sandbox as a result of recursive Nix calls. */
+    AutoCloseFD sandboxMountNamespace;
+
     /* The build hook. */
     std::unique_ptr<HookInstance> hook;
 
@@ -887,6 +890,29 @@ private:
     /* The remote machine on which we're building. */
     std::string machineName;
 
+    /* The recursive Nix daemon socket. */
+    AutoCloseFD daemonSocket;
+
+    /* The daemon main thread. */
+    std::thread daemonThread;
+
+    /* The daemon worker threads. */
+    std::vector<std::thread> daemonWorkerThreads;
+
+    /* Paths that were added via recursive Nix calls. */
+    PathSet addedPaths;
+
+    /* Recursive Nix calls are only allowed to build or realize paths
+       in the original input closure or added via a recursive Nix call
+       (so e.g. you can't do 'nix-store -r /nix/store/<bla>' where
+       /nix/store/<bla> is some arbitrary path in a binary cache). */
+    bool isAllowed(const Path & path)
+    {
+        return inputPaths.count(path) || addedPaths.count(path);
+    }
+
+    friend class RestrictedStore;
+
 public:
     DerivationGoal(const Path & drvPath, const StringSet & wantedOutputs,
         Worker & worker, BuildMode buildMode = bmNormal);
@@ -940,9 +966,20 @@ private:
     /* Fill in the environment for the builder. */
     void initEnv();
 
+    /* Setup tmp dir location. */
+    void initTmpDir();
+
     /* Write a JSON file containing the derivation attributes. */
     void writeStructuredAttrs();
 
+    void startDaemon();
+
+    void stopDaemon();
+
+    /* Add 'path' to the set of paths that may be referenced by the
+       outputs, and make it appear in the sandbox. */
+    void addDependency(const Path & path);
+
     /* Make a file owned by the builder. */
     void chownToBuilder(const Path & path);
 
@@ -1044,6 +1081,7 @@ DerivationGoal::~DerivationGoal()
     /* Careful: we should never ever throw an exception from a
        destructor. */
     try { killChild(); } catch (...) { ignoreException(); }
+    try { stopDaemon(); } catch (...) { ignoreException(); }
     try { deleteTmpDir(false); } catch (...) { ignoreException(); }
     try { closeLogFile(); } catch (...) { ignoreException(); }
 }
@@ -1333,12 +1371,6 @@ void DerivationGoal::inputsRealised()
     /* Gather information necessary for computing the closure and/or
        running the build hook. */
 
-    /* The outputs are referenceable paths. */
-    for (auto & i : drv->outputs) {
-        debug(format("building path '%1%'") % i.second.path);
-        allPaths.insert(i.second.path);
-    }
-
     /* Determine the full set of input paths. */
 
     /* First, the input derivations. */
@@ -1363,8 +1395,6 @@ void DerivationGoal::inputsRealised()
 
     debug(format("added input paths %1%") % showPaths(inputPaths));
 
-    allPaths.insert(inputPaths.begin(), inputPaths.end());
-
     /* Is this a fixed-output derivation? */
     fixedOutput = drv->isFixedOutput();
 
@@ -1516,7 +1546,7 @@ void replaceValidPath(const Path & storePath, const Path tmpPath)
 }
 
 
-MakeError(NotDeterministic, BuildError)
+MakeError(NotDeterministic, BuildError);
 
 
 void DerivationGoal::buildDone()
@@ -1528,6 +1558,8 @@ void DerivationGoal::buildDone()
        uid and then messing around with our output. */
     Finally releaseBuildUser([&]() { buildUser.reset(); });
 
+    sandboxMountNamespace = -1;
+
     /* Since we got an EOF on the logger pipe, the builder is presumed
        to have terminated.  In fact, the builder could also have
        simply have closed its end of the pipe, so just to be sure,
@@ -1559,6 +1591,9 @@ void DerivationGoal::buildDone()
        root. */
     if (buildUser) buildUser->kill();
 
+    /* Terminate the recursive Nix daemon. */
+    stopDaemon();
+
     bool diskFull = false;
 
     try {
@@ -1957,13 +1992,6 @@ void DerivationGoal::startBuilder()
     auto drvName = storePathToName(drvPath);
     tmpDir = createTempDir("", "nix-build-" + drvName, false, false, 0700);
 
-    /* In a sandbox, for determinism, always use the same temporary
-       directory. */
-#if __linux__
-    tmpDirInSandbox = useChroot ? settings.sandboxBuildDir : tmpDir;
-#else
-    tmpDirInSandbox = tmpDir;
-#endif
     chownToBuilder(tmpDir);
 
     /* Substitute output placeholders with the actual output paths. */
@@ -2218,6 +2246,11 @@ void DerivationGoal::startBuilder()
         }
     }
 
+    /* Fire up a Nix daemon to process recursive Nix calls from the
+       builder. */
+    if (parsedDrv->getRequiredSystemFeatures().count("recursive-nix"))
+        startDaemon();
+
     /* Run the builder. */
     printMsg(lvlChatty, format("executing builder '%1%'") % drv->builder);
 
@@ -2367,7 +2400,7 @@ void DerivationGoal::startBuilder()
         int res = helper.wait();
         if (res != 0 && settings.sandboxFallback) {
             useChroot = false;
-            tmpDirInSandbox = tmpDir;
+            initTmpDir();
             goto fallback;
         } else if (res != 0)
             throw Error("unable to start build process");
@@ -2392,6 +2425,12 @@ void DerivationGoal::startBuilder()
         writeFile("/proc/" + std::to_string(pid) + "/gid_map",
             (format("%d %d 1") % sandboxGid % hostGid).str());
 
+        /* Save the mount namespace of the child. We have to do this
+           *before* the child does a chroot. */
+        sandboxMountNamespace = open(fmt("/proc/%d/ns/mnt", (pid_t) pid).c_str(), O_RDONLY);
+        if (sandboxMountNamespace.get() == -1)
+            throw SysError("getting sandbox mount namespace");
+
         /* Signal the builder that we've updated its user namespace. */
         writeFull(userNamespaceSync.writeSide.get(), "1");
         userNamespaceSync.writeSide = -1;
@@ -2423,31 +2462,14 @@ void DerivationGoal::startBuilder()
 }
 
 
-void DerivationGoal::initEnv()
-{
-    env.clear();
-
-    /* Most shells initialise PATH to some default (/bin:/usr/bin:...) when
-       PATH is not set.  We don't want this, so we fill it in with some dummy
-       value. */
-    env["PATH"] = "/path-not-set";
-
-    /* Set HOME to a non-existing path to prevent certain programs from using
-       /etc/passwd (or NIS, or whatever) to locate the home directory (for
-       example, wget looks for ~/.wgetrc).  I.e., these tools use /etc/passwd
-       if HOME is not set, but they will just assume that the settings file
-       they are looking for does not exist if HOME is set but points to some
-       non-existing path. */
-    env["HOME"] = homeDir;
-
-    /* Tell the builder where the Nix store is.  Usually they
-       shouldn't care, but this is useful for purity checking (e.g.,
-       the compiler or linker might only want to accept paths to files
-       in the store or in the build directory). */
-    env["NIX_STORE"] = worker.store.storeDir;
-
-    /* The maximum number of cores to utilize for parallel building. */
-    env["NIX_BUILD_CORES"] = (format("%d") % settings.buildCores).str();
+void DerivationGoal::initTmpDir() {
+    /* In a sandbox, for determinism, always use the same temporary
+       directory. */
+#if __linux__
+    tmpDirInSandbox = useChroot ? settings.sandboxBuildDir : tmpDir;
+#else
+    tmpDirInSandbox = tmpDir;
+#endif
 
     /* In non-structured mode, add all bindings specified in the
        derivation via the environment, except those listed in the
@@ -2486,6 +2508,35 @@ void DerivationGoal::initEnv()
        inode of the current directory doesn't appear in .. (because
        getdents returns the inode of the mount point). */
     env["PWD"] = tmpDirInSandbox;
+}
+
+void DerivationGoal::initEnv()
+{
+    env.clear();
+
+    /* Most shells initialise PATH to some default (/bin:/usr/bin:...) when
+       PATH is not set.  We don't want this, so we fill it in with some dummy
+       value. */
+    env["PATH"] = "/path-not-set";
+
+    /* Set HOME to a non-existing path to prevent certain programs from using
+       /etc/passwd (or NIS, or whatever) to locate the home directory (for
+       example, wget looks for ~/.wgetrc).  I.e., these tools use /etc/passwd
+       if HOME is not set, but they will just assume that the settings file
+       they are looking for does not exist if HOME is set but points to some
+       non-existing path. */
+    env["HOME"] = homeDir;
+
+    /* Tell the builder where the Nix store is.  Usually they
+       shouldn't care, but this is useful for purity checking (e.g.,
+       the compiler or linker might only want to accept paths to files
+       in the store or in the build directory). */
+    env["NIX_STORE"] = worker.store.storeDir;
+
+    /* The maximum number of cores to utilize for parallel building. */
+    env["NIX_BUILD_CORES"] = (format("%d") % settings.buildCores).str();
+
+    initTmpDir();
 
     /* Compatibility hack with Nix <= 0.7: if this is a fixed-output
        derivation, tell the builder, so that for instance `fetchurl'
@@ -2504,7 +2555,7 @@ void DerivationGoal::initEnv()
        already know the cryptographic hash of the output). */
     if (fixedOutput) {
         for (auto & i : parsedDrv->getStringsAttr("impureEnvVars").value_or(Strings()))
-            env[i] = getEnv(i);
+            env[i] = getEnv(i).value_or("");
     }
 
     /* Currently structured log messages piggyback on stderr, but we
@@ -2622,6 +2673,319 @@ void DerivationGoal::writeStructuredAttrs()
 }
 
 
+/* A wrapper around LocalStore that only allows building/querying of
+   paths that are in the input closures of the build or were added via
+   recursive Nix calls. */
+struct RestrictedStore : public LocalFSStore
+{
+    ref<LocalStore> next;
+
+    DerivationGoal & goal;
+
+    RestrictedStore(const Params & params, ref<LocalStore> next, DerivationGoal & goal)
+        : Store(params), LocalFSStore(params), next(next), goal(goal)
+    { }
+
+    Path getRealStoreDir() override
+    { return next->realStoreDir; }
+
+    std::string getUri() override
+    { return next->getUri(); }
+
+    PathSet queryAllValidPaths() override
+    {
+        PathSet paths;
+        for (auto & p : goal.inputPaths) paths.insert(p);
+        for (auto & p : goal.addedPaths) paths.insert(p);
+        return paths;
+    }
+
+    void queryPathInfoUncached(const Path & path,
+        Callback<std::shared_ptr<const ValidPathInfo>> callback) noexcept override
+    {
+        if (goal.isAllowed(path)) {
+            try {
+                /* Censor impure information. */
+                auto info = std::make_shared<ValidPathInfo>(*next->queryPathInfo(path));
+                info->deriver.clear();
+                info->registrationTime = 0;
+                info->ultimate = false;
+                info->sigs.clear();
+                callback(info);
+            } catch (InvalidPath &) {
+                callback(nullptr);
+            }
+        } else
+            callback(nullptr);
+    };
+
+    void queryReferrers(const Path & path, PathSet & referrers) override
+    { }
+
+    PathSet queryDerivationOutputs(const Path & path) override
+    { throw Error("queryDerivationOutputs"); }
+
+    StringSet queryDerivationOutputNames(const Path & path) override
+    { throw Error("queryDerivationOutputNames"); }
+
+    Path queryPathFromHashPart(const string & hashPart) override
+    { throw Error("queryPathFromHashPart"); }
+
+    Path addToStore(const string & name, const Path & srcPath,
+        bool recursive = true, HashType hashAlgo = htSHA256,
+        PathFilter & filter = defaultPathFilter, RepairFlag repair = NoRepair) override
+    { throw Error("addToStore"); }
+
+    void addToStore(const ValidPathInfo & info, Source & narSource,
+        RepairFlag repair = NoRepair, CheckSigsFlag checkSigs = CheckSigs,
+        std::shared_ptr<FSAccessor> accessor = 0) override
+    {
+        next->addToStore(info, narSource, repair, checkSigs, accessor);
+        goal.addDependency(info.path);
+    }
+
+    Path addToStoreFromDump(const string & dump, const string & name,
+        bool recursive = true, HashType hashAlgo = htSHA256, RepairFlag repair = NoRepair) override
+    {
+        auto path = next->addToStoreFromDump(dump, name, recursive, hashAlgo, repair);
+        goal.addDependency(path);
+        return path;
+    }
+
+    Path addTextToStore(const string & name, const string & s,
+        const PathSet & references, RepairFlag repair = NoRepair) override
+    {
+        auto path = next->addTextToStore(name, s, references, repair);
+        goal.addDependency(path);
+        return path;
+    }
+
+    void narFromPath(const Path & path, Sink & sink) override 
+    {
+        if (!goal.isAllowed(path))
+            throw InvalidPath("cannot dump unknown path '%s' in recursive Nix", path);
+        LocalFSStore::narFromPath(path, sink);
+    }
+
+    void ensurePath(const Path & path) override
+    {
+        if (!goal.isAllowed(path))
+            throw InvalidPath("cannot substitute unknown path '%s' in recursive Nix", path);
+        /* Nothing to be done; 'path' must already be valid. */
+    }
+
+    void buildPaths(const PathSet & paths, BuildMode buildMode) override
+    {
+        if (buildMode != bmNormal) throw Error("unsupported build mode");
+
+        PathSet newPaths;
+
+        for (auto & path : paths) {
+            DrvPathWithOutputs i = parseDrvPathWithOutputs(path);
+            if (isDerivation(i.first)) {
+                if (!goal.isAllowed(i.first))
+                    throw InvalidPath("cannot build unknown path '%s' in recursive Nix", i.first);
+                auto drv = derivationFromPath(i.first);
+                for (auto & output : drv.outputs)
+                    if (wantOutput(output.first, i.second))
+                        newPaths.insert(output.second.path);
+            } else if (!goal.isAllowed(path))
+                throw InvalidPath("cannot build unknown path '%s' in recursive Nix", path);
+        }
+
+        next->buildPaths(paths, buildMode);
+
+        PathSet closure;
+        next->computeFSClosure(newPaths, closure);
+        for (auto & path : closure)
+            goal.addDependency(path);
+    }
+
+    BuildResult buildDerivation(const Path & drvPath, const BasicDerivation & drv,
+        BuildMode buildMode = bmNormal) override
+    { unsupported("buildDerivation"); }
+
+    void addTempRoot(const Path & path)
+    { }
+
+    void addIndirectRoot(const Path & path)
+    { }
+
+    Roots findRoots()
+    { return Roots(); }
+
+    void collectGarbage(const GCOptions & options, GCResults & results)
+    { }
+
+    void addSignatures(const Path & storePath, const StringSet & sigs)
+    { unsupported("addSignatures"); }
+
+    void queryMissing(const PathSet & targets,
+        PathSet & willBuild, PathSet & willSubstitute, PathSet & unknown,
+        unsigned long long & downloadSize, unsigned long long & narSize)
+    {
+        /* This is slightly impure since it leaks information to the
+           client about what paths will be built/substituted or are
+           already present. Probably not a big deal. */
+
+        PathSet allowed;
+        for (auto & path : targets) {
+            DrvPathWithOutputs i = parseDrvPathWithOutputs(path);
+            if (goal.isAllowed(i.first))
+                allowed.insert(i.first);
+            else
+                unknown.insert(i.first);
+        }
+
+        next->queryMissing(allowed, willBuild, willSubstitute,
+            unknown, downloadSize, narSize);
+    }
+};
+
+
+void DerivationGoal::startDaemon()
+{
+    settings.requireExperimentalFeature("recursive-nix");
+
+    Store::Params params;
+    params["path-info-cache-size"] = "0";
+    params["store"] = worker.store.storeDir;
+    params["root"] = worker.store.rootDir;
+    params["state"] = "/no-such-path";
+    params["log"] = "/no-such-path";
+    auto store = make_ref<RestrictedStore>(params,
+        ref<LocalStore>(std::dynamic_pointer_cast<LocalStore>(worker.store.shared_from_this())),
+        *this);
+
+    addedPaths.clear();
+
+    auto socketName = ".nix-socket";
+    Path socketPath = tmpDir + "/" + socketName;
+    env["NIX_REMOTE"] = "unix://" + tmpDirInSandbox + "/" + socketName;
+
+    daemonSocket = createUnixDomainSocket(socketPath, 0600);
+
+    chownToBuilder(socketPath);
+
+    daemonThread = std::thread([this, store]() {
+
+        while (true) {
+
+            /* Accept a connection. */
+            struct sockaddr_un remoteAddr;
+            socklen_t remoteAddrLen = sizeof(remoteAddr);
+
+            AutoCloseFD remote = accept(daemonSocket.get(),
+                (struct sockaddr *) &remoteAddr, &remoteAddrLen);
+            if (!remote) {
+                if (errno == EINTR) continue;
+                if (errno == EINVAL) break;
+                throw SysError("accepting connection");
+            }
+
+            closeOnExec(remote.get());
+
+            debug("received daemon connection");
+
+            auto workerThread = std::thread([this, store, remote{std::move(remote)}]() {
+                FdSource from(remote.get());
+                FdSink to(remote.get());
+                try {
+                    daemon::processConnection(store, from, to,
+                        daemon::NotTrusted, daemon::Recursive, "nobody", 65535);
+                    debug("terminated daemon connection");
+                } catch (SysError &) {
+                    ignoreException();
+                }
+            });
+
+            daemonWorkerThreads.push_back(std::move(workerThread));
+        }
+
+        debug("daemon shutting down");
+    });
+}
+
+
+void DerivationGoal::stopDaemon()
+{
+    if (daemonSocket && shutdown(daemonSocket.get(), SHUT_RDWR) == -1)
+        throw SysError("shutting down daemon socket");
+
+    if (daemonThread.joinable())
+        daemonThread.join();
+
+    // FIXME: should prune worker threads more quickly.
+    // FIXME: shutdown the client socket to speed up worker termination.
+    for (auto & thread : daemonWorkerThreads)
+        thread.join();
+    daemonWorkerThreads.clear();
+
+    daemonSocket = -1;
+}
+
+
+void DerivationGoal::addDependency(const Path & path)
+{
+    worker.store.assertStorePath(path);
+
+    if (isAllowed(path)) return;
+
+    addedPaths.insert(path);
+
+    /* If we're doing a sandbox build, then we have to make the path
+       appear in the sandbox. */
+    if (useChroot) {
+
+        debug("materialising '%s' in the sandbox", path);
+
+        #if __linux__
+
+            Path source = worker.store.toRealPath(path);
+            Path target = chrootRootDir + path;
+            debug("bind-mounting %s -> %s", target, source);
+
+            if (pathExists(target))
+                throw Error("store path '%s' already exists in the sandbox", path);
+
+            struct stat st;
+            if (lstat(source.c_str(), &st))
+                throw SysError("getting attributes of path '%s'", source);
+
+            if (S_ISDIR(st.st_mode)) {
+
+                /* Bind-mount the path into the sandbox. This requires
+                   entering its mount namespace, which is not possible
+                   in multithreaded programs. So we do this in a
+                   child process.*/
+                Pid child(startProcess([&]() {
+
+                    if (setns(sandboxMountNamespace.get(), 0) == -1)
+                        throw SysError("entering sandbox mount namespace");
+
+                    createDirs(target);
+
+                    if (mount(source.c_str(), target.c_str(), "", MS_BIND, 0) == -1)
+                        throw SysError("bind mount from '%s' to '%s' failed", source, target);
+
+                    _exit(0);
+                }));
+
+                int status = child.wait();
+                if (status != 0)
+                    throw Error("could not add path '%s' to sandbox", path);
+
+            } else
+                linkOrCopy(source, target);
+
+        #else
+            throw Error("don't know how to make path '%s' (produced by a recursive Nix call) appear in the sandbox", path);
+        #endif
+
+    }
+}
+
+
 void DerivationGoal::chownToBuilder(const Path & path)
 {
     if (!buildUser) return;
@@ -2757,15 +3121,30 @@ void DerivationGoal::runChild()
                outside of the namespace.  Making a subtree private is
                local to the namespace, though, so setting MS_PRIVATE
                does not affect the outside world. */
-            if (mount(0, "/", 0, MS_REC|MS_PRIVATE, 0) == -1) {
-                throw SysError("unable to make '/' private mount");
-            }
+            if (mount(0, "/", 0, MS_PRIVATE | MS_REC, 0) == -1)
+                throw SysError("unable to make '/' private");
 
             /* Bind-mount chroot directory to itself, to treat it as a
                different filesystem from /, as needed for pivot_root. */
             if (mount(chrootRootDir.c_str(), chrootRootDir.c_str(), 0, MS_BIND, 0) == -1)
                 throw SysError(format("unable to bind mount '%1%'") % chrootRootDir);
 
+            /* Bind-mount the sandbox's Nix store onto itself so that
+               we can mark it as a "shared" subtree, allowing bind
+               mounts made in *this* mount namespace to be propagated
+               into the child namespace created by the
+               unshare(CLONE_NEWNS) call below.
+
+               Marking chrootRootDir as MS_SHARED causes pivot_root()
+               to fail with EINVAL. Don't know why. */
+            Path chrootStoreDir = chrootRootDir + worker.store.storeDir;
+
+            if (mount(chrootStoreDir.c_str(), chrootStoreDir.c_str(), 0, MS_BIND, 0) == -1)
+                throw SysError("unable to bind mount the Nix store", chrootStoreDir);
+
+            if (mount(0, chrootStoreDir.c_str(), 0, MS_SHARED, 0) == -1)
+                throw SysError("unable to make '%s' shared", chrootStoreDir);
+
             /* Set up a nearly empty /dev, unless the user asked to
                bind-mount the host /dev. */
             Strings ss;
@@ -2867,6 +3246,19 @@ void DerivationGoal::runChild()
                 }
             }
 
+            /* Unshare this mount namespace. This is necessary because
+               pivot_root() below changes the root of the mount
+               namespace. This means that the call to setns() in
+               addDependency() would hide the host's filesystem,
+               making it impossible to bind-mount paths from the host
+               Nix store into the sandbox. Therefore, we save the
+               pre-pivot_root namespace in
+               sandboxMountNamespace. Since we made /nix/store a
+               shared subtree above, this allows addDependency() to
+               make paths appear in the sandbox. */
+            if (unshare(CLONE_NEWNS) == -1)
+                throw SysError("unsharing mount namespace");
+
             /* Do the chroot(). */
             if (chdir(chrootRootDir.c_str()) == -1)
                 throw SysError(format("cannot change directory to '%1%'") % chrootRootDir);
@@ -3076,7 +3468,7 @@ void DerivationGoal::runChild()
 
             /* 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. */
-            Path globalTmpDir = canonPath(getEnv("TMPDIR", "/tmp"), true);
+            Path globalTmpDir = canonPath(getEnv("TMPDIR").value_or("/tmp"), true);
 
             /* They don't like trailing slashes on subpath directives */
             if (globalTmpDir.back() == '/') globalTmpDir.pop_back();
@@ -3121,6 +3513,8 @@ void DerivationGoal::runChild()
                     builtinFetchurl(drv2, netrcData);
                 else if (drv->builder == "builtin:buildenv")
                     builtinBuildenv(drv2);
+                else if (drv->builder == "builtin:unpack-channel")
+                    builtinUnpackChannel(drv2);
                 else
                     throw Error(format("unsupported builtin function '%1%'") % string(drv->builder, 8));
                 _exit(0);
@@ -3183,6 +3577,14 @@ void DerivationGoal::registerOutputs()
 
     std::exception_ptr delayedException;
 
+    /* The paths that can be referenced are the input closures, the
+       output paths, and any paths that have been built via recursive
+       Nix calls. */
+    PathSet referenceablePaths;
+    for (auto & p : inputPaths) referenceablePaths.insert(p);
+    for (auto & i : drv->outputs) referenceablePaths.insert(i.second.path);
+    for (auto & p : addedPaths) referenceablePaths.insert(p);
+
     /* Check whether the output paths were created, and grep each
        output path to determine what other paths it references.  Also make all
        output paths read-only. */
@@ -3318,7 +3720,7 @@ void DerivationGoal::registerOutputs()
            verify later on whether nobody has messed with the store. */
         debug("scanning for references inside '%1%'", path);
         HashResult hash;
-        PathSet references = scanForReferences(actualPath, allPaths, hash);
+        PathSet references = scanForReferences(actualPath, referenceablePaths, hash);
 
         if (buildMode == bmCheck) {
             if (!worker.store.isValidPath(path)) continue;
diff --git a/src/libstore/builtins.hh b/src/libstore/builtins.hh
index f9b5f7900..66597e456 100644
--- a/src/libstore/builtins.hh
+++ b/src/libstore/builtins.hh
@@ -6,5 +6,6 @@ namespace nix {
 
 // TODO: make pluggable.
 void builtinFetchurl(const BasicDerivation & drv, const std::string & netrcData);
+void builtinUnpackChannel(const BasicDerivation & drv);
 
 }
diff --git a/src/libstore/builtins/unpack-channel.cc b/src/libstore/builtins/unpack-channel.cc
new file mode 100644
index 000000000..d18e3ddaf
--- /dev/null
+++ b/src/libstore/builtins/unpack-channel.cc
@@ -0,0 +1,29 @@
+#include "builtins.hh"
+#include "tarfile.hh"
+
+namespace nix {
+
+void builtinUnpackChannel(const BasicDerivation & drv)
+{
+    auto getAttr = [&](const string & name) {
+        auto i = drv.env.find(name);
+        if (i == drv.env.end()) throw Error("attribute '%s' missing", name);
+        return i->second;
+    };
+
+    Path out = getAttr("out");
+    auto channelName = getAttr("channelName");
+    auto src = getAttr("src");
+
+    createDirs(out);
+
+    unpackTarfile(src, out);
+
+    auto entries = readDirectory(out);
+    if (entries.size() != 1)
+        throw Error("channel tarball '%s' contains more than one file", src);
+    if (rename((out + "/" + entries[0].name).c_str(), (out + "/" + channelName).c_str()) == -1)
+        throw SysError("renaming channel directory");
+}
+
+}
diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc
index 64147027b..d3c17e772 100644
--- a/src/libstore/daemon.cc
+++ b/src/libstore/daemon.cc
@@ -186,8 +186,75 @@ struct RetrieveRegularNARSink : ParseSink
     }
 };
 
+struct ClientSettings
+{
+    bool keepFailed;
+    bool keepGoing;
+    bool tryFallback;
+    Verbosity verbosity;
+    unsigned int maxBuildJobs;
+    time_t maxSilentTime;
+    bool verboseBuild;
+    unsigned int buildCores;
+    bool useSubstitutes;
+    StringMap overrides;
+
+    void apply(TrustedFlag trusted)
+    {
+        settings.keepFailed = keepFailed;
+        settings.keepGoing = keepGoing;
+        settings.tryFallback = tryFallback;
+        nix::verbosity = verbosity;
+        settings.maxBuildJobs.assign(maxBuildJobs);
+        settings.maxSilentTime = maxSilentTime;
+        settings.verboseBuild = verboseBuild;
+        settings.buildCores = buildCores;
+        settings.useSubstitutes = useSubstitutes;
+
+        for (auto & i : overrides) {
+            auto & name(i.first);
+            auto & value(i.second);
+
+            auto setSubstituters = [&](Setting<Strings> & res) {
+                if (name != res.name && res.aliases.count(name) == 0)
+                    return false;
+                StringSet trusted = settings.trustedSubstituters;
+                for (auto & s : settings.substituters.get())
+                    trusted.insert(s);
+                Strings subs;
+                auto ss = tokenizeString<Strings>(value);
+                for (auto & s : ss)
+                    if (trusted.count(s))
+                        subs.push_back(s);
+                    else
+                        warn("ignoring untrusted substituter '%s'", s);
+                res = subs;
+                return true;
+            };
+
+            try {
+                if (name == "ssh-auth-sock") // obsolete
+                    ;
+                else if (trusted
+                    || name == settings.buildTimeout.name
+                    || name == "connect-timeout"
+                    || (name == "builders" && value == ""))
+                    settings.set(name, value);
+                else if (setSubstituters(settings.substituters))
+                    ;
+                else if (setSubstituters(settings.extraSubstituters))
+                    ;
+                else
+                    warn("ignoring the user-specified setting '%s', because it is a restricted setting and you are not a trusted user", name);
+            } catch (UsageError & e) {
+                warn(e.what());
+            }
+        }
+    }
+};
+
 static void performOp(TunnelLogger * logger, ref<Store> store,
-    bool trusted, unsigned int clientVersion,
+    TrustedFlag trusted, RecursiveFlag recursive, unsigned int clientVersion,
     Source & from, BufferedSink & to, unsigned int op)
 {
     switch (op) {
@@ -464,70 +531,37 @@ static void performOp(TunnelLogger * logger, ref<Store> store,
     }
 
     case wopSetOptions: {
-        settings.keepFailed = readInt(from);
-        settings.keepGoing = readInt(from);
-        settings.tryFallback = readInt(from);
-        verbosity = (Verbosity) readInt(from);
-        settings.maxBuildJobs.assign(readInt(from));
-        settings.maxSilentTime = readInt(from);
+
+        ClientSettings clientSettings;
+
+        clientSettings.keepFailed = readInt(from);
+        clientSettings.keepGoing = readInt(from);
+        clientSettings.tryFallback = readInt(from);
+        clientSettings.verbosity = (Verbosity) readInt(from);
+        clientSettings.maxBuildJobs = readInt(from);
+        clientSettings.maxSilentTime = readInt(from);
         readInt(from); // obsolete useBuildHook
-        settings.verboseBuild = lvlError == (Verbosity) readInt(from);
+        clientSettings.verboseBuild = lvlError == (Verbosity) readInt(from);
         readInt(from); // obsolete logType
         readInt(from); // obsolete printBuildTrace
-        settings.buildCores = readInt(from);
-        settings.useSubstitutes  = readInt(from);
+        clientSettings.buildCores = readInt(from);
+        clientSettings.useSubstitutes = readInt(from);
 
-        StringMap overrides;
         if (GET_PROTOCOL_MINOR(clientVersion) >= 12) {
             unsigned int n = readInt(from);
             for (unsigned int i = 0; i < n; i++) {
                 string name = readString(from);
                 string value = readString(from);
-                overrides.emplace(name, value);
+                clientSettings.overrides.emplace(name, value);
             }
         }
 
         logger->startWork();
 
-        for (auto & i : overrides) {
-            auto & name(i.first);
-            auto & value(i.second);
-
-            auto setSubstituters = [&](Setting<Strings> & res) {
-                if (name != res.name && res.aliases.count(name) == 0)
-                    return false;
-                StringSet trusted = settings.trustedSubstituters;
-                for (auto & s : settings.substituters.get())
-                    trusted.insert(s);
-                Strings subs;
-                auto ss = tokenizeString<Strings>(value);
-                for (auto & s : ss)
-                    if (trusted.count(s))
-                        subs.push_back(s);
-                    else
-                        warn("ignoring untrusted substituter '%s'", s);
-                res = subs;
-                return true;
-            };
-
-            try {
-                if (name == "ssh-auth-sock") // obsolete
-                    ;
-                else if (trusted
-                    || name == settings.buildTimeout.name
-                    || name == "connect-timeout"
-                    || (name == "builders" && value == ""))
-                    settings.set(name, value);
-                else if (setSubstituters(settings.substituters))
-                    ;
-                else if (setSubstituters(settings.extraSubstituters))
-                    ;
-                else
-                    warn("ignoring the user-specified setting '%s', because it is a restricted setting and you are not a trusted user", name);
-            } catch (UsageError & e) {
-                warn(e.what());
-            }
-        }
+        // FIXME: use some setting in recursive mode. Will need to use
+        // non-global variables.
+        if (!recursive)
+            clientSettings.apply(trusted);
 
         logger->stopWork();
         break;
@@ -694,11 +728,12 @@ void processConnection(
     ref<Store> store,
     FdSource & from,
     FdSink & to,
-    bool trusted,
+    TrustedFlag trusted,
+    RecursiveFlag recursive,
     const std::string & userName,
     uid_t userId)
 {
-    MonitorFdHup monitor(from.fd);
+    auto monitor = !recursive ? std::make_unique<MonitorFdHup>(from.fd) : nullptr;
 
     /* Exchange the greeting. */
     unsigned int magic = readInt(from);
@@ -712,7 +747,9 @@ void processConnection(
 
     auto tunnelLogger = new TunnelLogger(to, clientVersion);
     auto prevLogger = nix::logger;
-    logger = tunnelLogger;
+    // FIXME
+    if (!recursive)
+        logger = tunnelLogger;
 
     unsigned int opCount = 0;
 
@@ -721,8 +758,10 @@ void processConnection(
         prevLogger->log(lvlDebug, fmt("%d operations", opCount));
     });
 
-    if (GET_PROTOCOL_MINOR(clientVersion) >= 14 && readInt(from))
-        setAffinityTo(readInt(from));
+    if (GET_PROTOCOL_MINOR(clientVersion) >= 14 && readInt(from)) {
+        auto affinity = readInt(from);
+        setAffinityTo(affinity);
+    }
 
     readInt(from); // obsolete reserveSpace
 
@@ -760,7 +799,7 @@ void processConnection(
             opCount++;
 
             try {
-                performOp(tunnelLogger, store, trusted, clientVersion, from, to, op);
+                performOp(tunnelLogger, store, trusted, recursive, clientVersion, from, to, op);
             } catch (Error & e) {
                 /* If we're not in a state where we can send replies, then
                    something went wrong processing the input of the
diff --git a/src/libstore/daemon.hh b/src/libstore/daemon.hh
index fa26ba9c2..266932013 100644
--- a/src/libstore/daemon.hh
+++ b/src/libstore/daemon.hh
@@ -3,11 +3,15 @@
 
 namespace nix::daemon {
 
+enum TrustedFlag : bool { NotTrusted = false, Trusted = true };
+enum RecursiveFlag : bool { NotRecursive = false, Recursive = true };
+
 void processConnection(
     ref<Store> store,
     FdSource & from,
     FdSink & to,
-    bool trusted,
+    TrustedFlag trusted,
+    RecursiveFlag recursive,
     const std::string & userName,
     uid_t userId);
 
diff --git a/src/libstore/download.cc b/src/libstore/download.cc
index d49d63912..38faa1bfd 100644
--- a/src/libstore/download.cc
+++ b/src/libstore/download.cc
@@ -8,6 +8,7 @@
 #include "compression.hh"
 #include "pathlocks.hh"
 #include "finally.hh"
+#include "tarfile.hh"
 
 #ifdef ENABLE_S3
 #include <aws/core/client/ClientConfiguration.h>
@@ -289,6 +290,7 @@ struct CurlDownloader : public Downloader
             }
 
             if (request.verifyTLS) {
+                debug("verify TLS: Nix CA file = '%s'", settings.caFile);
                 if (settings.caFile != "")
                     curl_easy_setopt(req, CURLOPT_CAINFO, settings.caFile.c_str());
             } else {
@@ -905,11 +907,10 @@ CachedDownloadResult Downloader::downloadCached(
                 result.lastModified = lstat(unpackedLink).st_mtime;
         }
         if (unpackedStorePath.empty()) {
-            printInfo(format("unpacking '%1%'...") % url);
+            printInfo("unpacking '%s'...", url);
             Path tmpDir = createTempDir();
             AutoDelete autoDelete(tmpDir, true);
-            // FIXME: this requires GNU tar for decompression.
-            runProgram("tar", true, {"xf", store->toRealPath(storePath), "-C", tmpDir});
+            unpackTarfile(store->toRealPath(storePath), tmpDir, baseNameOf(url));
             auto members = readDirectory(tmpDir);
             if (members.size() != 1)
                 throw nix::Error("tarball '%s' contains an unexpected number of top-level files", url);
diff --git a/src/libstore/gc.cc b/src/libstore/gc.cc
index 7d3da1cc8..842980fb7 100644
--- a/src/libstore/gc.cc
+++ b/src/libstore/gc.cc
@@ -870,11 +870,11 @@ void LocalStore::collectGarbage(const GCOptions & options, GCResults & results)
 
 void LocalStore::autoGC(bool sync)
 {
-    static auto fakeFreeSpaceFile = getEnv("_NIX_TEST_FREE_SPACE_FILE", "");
+    static auto fakeFreeSpaceFile = getEnv("_NIX_TEST_FREE_SPACE_FILE");
 
     auto getAvail = [this]() -> uint64_t {
-        if (!fakeFreeSpaceFile.empty())
-            return std::stoll(readFile(fakeFreeSpaceFile));
+        if (fakeFreeSpaceFile)
+            return std::stoll(readFile(*fakeFreeSpaceFile));
 
         struct statvfs st;
         if (statvfs(realStoreDir.c_str(), &st))
diff --git a/src/libstore/globals.cc b/src/libstore/globals.cc
index 249c36673..6fedf3d56 100644
--- a/src/libstore/globals.cc
+++ b/src/libstore/globals.cc
@@ -32,20 +32,20 @@ static GlobalConfig::Register r1(&settings);
 
 Settings::Settings()
     : nixPrefix(NIX_PREFIX)
-    , nixStore(canonPath(getEnv("NIX_STORE_DIR", getEnv("NIX_STORE", NIX_STORE_DIR))))
-    , nixDataDir(canonPath(getEnv("NIX_DATA_DIR", NIX_DATA_DIR)))
-    , nixLogDir(canonPath(getEnv("NIX_LOG_DIR", NIX_LOG_DIR)))
-    , nixStateDir(canonPath(getEnv("NIX_STATE_DIR", NIX_STATE_DIR)))
-    , nixConfDir(canonPath(getEnv("NIX_CONF_DIR", NIX_CONF_DIR)))
-    , nixLibexecDir(canonPath(getEnv("NIX_LIBEXEC_DIR", NIX_LIBEXEC_DIR)))
-    , nixBinDir(canonPath(getEnv("NIX_BIN_DIR", NIX_BIN_DIR)))
+    , nixStore(canonPath(getEnv("NIX_STORE_DIR").value_or(getEnv("NIX_STORE").value_or(NIX_STORE_DIR))))
+    , nixDataDir(canonPath(getEnv("NIX_DATA_DIR").value_or(NIX_DATA_DIR)))
+    , nixLogDir(canonPath(getEnv("NIX_LOG_DIR").value_or(NIX_LOG_DIR)))
+    , nixStateDir(canonPath(getEnv("NIX_STATE_DIR").value_or(NIX_STATE_DIR)))
+    , nixConfDir(canonPath(getEnv("NIX_CONF_DIR").value_or(NIX_CONF_DIR)))
+    , nixLibexecDir(canonPath(getEnv("NIX_LIBEXEC_DIR").value_or(NIX_LIBEXEC_DIR)))
+    , nixBinDir(canonPath(getEnv("NIX_BIN_DIR").value_or(NIX_BIN_DIR)))
     , nixManDir(canonPath(NIX_MAN_DIR))
     , nixDaemonSocketFile(canonPath(nixStateDir + DEFAULT_SOCKET_PATH))
 {
     buildUsersGroup = getuid() == 0 ? "nixbld" : "";
-    lockCPU = getEnv("NIX_AFFINITY_HACK", "1") == "1";
+    lockCPU = getEnv("NIX_AFFINITY_HACK") == "1";
 
-    caFile = getEnv("NIX_SSL_CERT_FILE", getEnv("SSL_CERT_FILE", ""));
+    caFile = getEnv("NIX_SSL_CERT_FILE").value_or(getEnv("SSL_CERT_FILE").value_or(""));
     if (caFile == "") {
         for (auto & fn : {"/etc/ssl/certs/ca-certificates.crt", "/nix/var/nix/profiles/default/etc/ssl/certs/ca-bundle.crt"})
             if (pathExists(fn)) {
@@ -56,9 +56,9 @@ Settings::Settings()
 
     /* Backwards compatibility. */
     auto s = getEnv("NIX_REMOTE_SYSTEMS");
-    if (s != "") {
+    if (s) {
         Strings ss;
-        for (auto & p : tokenizeString<Strings>(s, ":"))
+        for (auto & p : tokenizeString<Strings>(*s, ":"))
             ss.push_back("@" + p);
         builders = concatStringsSep(" ", ss);
     }
@@ -95,7 +95,7 @@ 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"};
+    StringSet features{"nixos-test", "benchmark", "big-parallel", "recursive-nix"};
 
     #if __linux__
     if (access("/dev/kvm", R_OK | W_OK) == 0)
@@ -105,10 +105,15 @@ StringSet Settings::getDefaultSystemFeatures()
     return features;
 }
 
-void Settings::requireExperimentalFeature(const std::string & name)
+bool Settings::isExperimentalFeatureEnabled(const std::string & name)
 {
     auto & f = experimentalFeatures.get();
-    if (std::find(f.begin(), f.end(), name) == f.end())
+    return std::find(f.begin(), f.end(), name) != f.end();
+}
+
+void Settings::requireExperimentalFeature(const std::string & name)
+{
+    if (!isExperimentalFeatureEnabled(name))
         throw Error("experimental Nix feature '%s' is disabled", name);
 }
 
diff --git a/src/libstore/globals.hh b/src/libstore/globals.hh
index 0b1a8dac5..d9e44e976 100644
--- a/src/libstore/globals.hh
+++ b/src/libstore/globals.hh
@@ -66,7 +66,7 @@ public:
     /* File name of the socket the daemon listens to.  */
     Path nixDaemonSocketFile;
 
-    Setting<std::string> storeUri{this, getEnv("NIX_REMOTE", "auto"), "store",
+    Setting<std::string> storeUri{this, getEnv("NIX_REMOTE").value_or("auto"), "store",
         "The default Nix store to use."};
 
     Setting<bool> keepFailed{this, false, "keep-failed",
@@ -319,7 +319,7 @@ public:
         "A program to run just before a build to set derivation-specific build settings."};
 
     Setting<std::string> postBuildHook{this, "", "post-build-hook",
-        "A program to run just after each succesful build."};
+        "A program to run just after each successful build."};
 
     Setting<std::string> netrcFile{this, fmt("%s/%s", nixConfDir, "netrc"), "netrc-file",
         "Path to the netrc file used to obtain usernames/passwords for downloads."};
@@ -360,6 +360,8 @@ public:
     Setting<Strings> experimentalFeatures{this, {}, "experimental-features",
         "Experimental Nix features to enable."};
 
+    bool isExperimentalFeatureEnabled(const std::string & name);
+
     void requireExperimentalFeature(const std::string & name);
 };
 
diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc
index 46e8138be..023c90cd8 100644
--- a/src/libstore/local-store.cc
+++ b/src/libstore/local-store.cc
@@ -54,6 +54,7 @@ LocalStore::LocalStore(const Params & params)
     , trashDir(realStoreDir + "/trash")
     , tempRootsDir(stateDir + "/temproots")
     , fnTempRoots(fmt("%s/%d", tempRootsDir, getpid()))
+    , locksHeld(tokenizeString<PathSet>(getEnv("NIX_HELD_LOCKS").value_or("")))
 {
     auto state(_state.lock());
 
@@ -577,6 +578,8 @@ void LocalStore::checkDerivationOutputs(const Path & drvPath, const Derivation &
 uint64_t LocalStore::addValidPath(State & state,
     const ValidPathInfo & info, bool checkOutputs)
 {
+    checkStoreName(storePathToName(info.path));
+
     if (info.ca != "" && !info.isContentAddressed(*this))
         throw Error("cannot add path '%s' to the Nix store because it claims to be content-addressed but isn't", info.path);
 
@@ -1231,7 +1234,29 @@ bool LocalStore::verifyStore(bool checkContents, RepairFlag repair)
 
     /* Optionally, check the content hashes (slow). */
     if (checkContents) {
-        printInfo("checking hashes...");
+
+        printInfo("checking link hashes...");
+
+        for (auto & link : readDirectory(linksDir)) {
+            printMsg(lvlTalkative, "checking contents of '%s'", link.name);
+            Path linkPath = linksDir + "/" + link.name;
+            string hash = hashPath(htSHA256, linkPath).first.to_string(Base32, false);
+            if (hash != link.name) {
+                printError(
+                    "link '%s' was modified! expected hash '%s', got '%s'",
+                    linkPath, link.name, hash);
+                if (repair) {
+                    if (unlink(linkPath.c_str()) == 0)
+                        printError("removed link '%s'", linkPath);
+                    else
+                        throw SysError("removing corrupt link '%s'", linkPath);
+                } else {
+                    errors = true;
+                }
+            }
+        }
+
+        printInfo("checking store hashes...");
 
         Hash nullHash(htSHA256);
 
diff --git a/src/libstore/local-store.hh b/src/libstore/local-store.hh
index 5aa6b0519..1d4b88898 100644
--- a/src/libstore/local-store.hh
+++ b/src/libstore/local-store.hh
@@ -107,7 +107,7 @@ private:
 public:
 
     // Hack for build-remote.cc.
-    PathSet locksHeld = tokenizeString<PathSet>(getEnv("NIX_HELD_LOCKS"));
+    PathSet locksHeld;
 
     /* Initialise the local store, upgrading the schema if
        necessary. */
diff --git a/src/libstore/remote-store.hh b/src/libstore/remote-store.hh
index 84de4c460..728df8b00 100644
--- a/src/libstore/remote-store.hh
+++ b/src/libstore/remote-store.hh
@@ -148,7 +148,7 @@ public:
 
     std::string getUri() override;
 
-    bool sameMachine()
+    bool sameMachine() override
     { return true; }
 
 private:
diff --git a/src/libstore/ssh-store.cc b/src/libstore/ssh-store.cc
index 93e117389..b9d0c41ea 100644
--- a/src/libstore/ssh-store.cc
+++ b/src/libstore/ssh-store.cc
@@ -35,7 +35,7 @@ public:
         return uriScheme + host;
     }
 
-    bool sameMachine()
+    bool sameMachine() override
     { return false; }
 
     void narFromPath(const Path & path, Sink & sink) override;
diff --git a/src/libstore/ssh.cc b/src/libstore/ssh.cc
index 5e0e44935..ac3ccd63d 100644
--- a/src/libstore/ssh.cc
+++ b/src/libstore/ssh.cc
@@ -16,7 +16,7 @@ SSHMaster::SSHMaster(const std::string & host, const std::string & keyFile, bool
 
 void SSHMaster::addCommonSSHOpts(Strings & args)
 {
-    for (auto & i : tokenizeString<Strings>(getEnv("NIX_SSHOPTS")))
+    for (auto & i : tokenizeString<Strings>(getEnv("NIX_SSHOPTS").value_or("")))
         args.push_back(i);
     if (!keyFile.empty())
         args.insert(args.end(), {"-i", keyFile});
diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc
index ea91bf20e..f134f7967 100644
--- a/src/libstore/store-api.cc
+++ b/src/libstore/store-api.cc
@@ -90,17 +90,22 @@ void checkStoreName(const string & name)
         "Path names are alphanumeric and can include the symbols %1% "
         "and must not begin with a period. "
         "Note: If '%2%' is a source file and you cannot rename it on "
-        "disk, builtins.path { name = ... } can be used to give it an "
+        "disk, 'builtins.path { name = ... }' can be used to give it an "
         "alternative name.") % validChars % name;
 
+    if (name.empty())
+        throw Error(baseError % "it is an empty string");
+
     /* Disallow names starting with a dot for possible security
        reasons (e.g., "." and ".."). */
-    if (string(name, 0, 1) == ".")
+    if (name[0] == '.')
         throw Error(baseError % "it is illegal to start the name with a period");
+
     /* Disallow names longer than 211 characters. ext4’s max is 256,
        but we need extra space for the hash and .chroot extensions. */
     if (name.length() > 211)
         throw Error(baseError % "name must be less than 212 characters");
+
     for (auto & i : name)
         if (!((i >= 'A' && i <= 'Z') ||
               (i >= 'a' && i <= 'z') ||
@@ -211,7 +216,7 @@ static std::string makeType(string && type, const PathSet & references)
         type += ":";
         type += i;
     }
-    return type;
+    return std::move(type);
 }
 
 
diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh
index 814ec8d29..e2e720fc4 100644
--- a/src/libstore/store-api.hh
+++ b/src/libstore/store-api.hh
@@ -20,13 +20,13 @@
 namespace nix {
 
 
-MakeError(SubstError, Error)
-MakeError(BuildError, Error) /* denotes a permanent build failure */
-MakeError(InvalidPath, Error)
-MakeError(Unsupported, Error)
-MakeError(SubstituteGone, Error)
-MakeError(SubstituterDisabled, Error)
-MakeError(NotInStore, Error)
+MakeError(SubstError, Error);
+MakeError(BuildError, Error); // denotes a permanent build failure
+MakeError(InvalidPath, Error);
+MakeError(Unsupported, Error);
+MakeError(SubstituteGone, Error);
+MakeError(SubstituterDisabled, Error);
+MakeError(NotInStore, Error);
 
 
 struct BasicDerivation;
@@ -570,7 +570,7 @@ public:
         unsigned long long & downloadSize, unsigned long long & narSize);
 
     /* Sort a set of paths topologically under the references
-       relation.  If p refers to q, then p preceeds q in this list. */
+       relation.  If p refers to q, then p precedes q in this list. */
     Paths topoSortPaths(const PathSet & paths);
 
     /* Export multiple paths in the format expected by ‘nix-store