Merge branch 'master' into referenceablePaths

src/libstore/build/derivation-goal.cc

@@ -63,7 +63,7 @@
 namespace nix {
 
 DerivationGoal::DerivationGoal(const StorePath & drvPath,
-    const StringSet & wantedOutputs, Worker & worker, BuildMode buildMode)
+    const OutputsSpec & wantedOutputs, Worker & worker, BuildMode buildMode)
     : Goal(worker, DerivedPath::Built { .drvPath = drvPath, .outputs = wantedOutputs })
     , useDerivation(true)
     , drvPath(drvPath)
@@ -82,7 +82,7 @@ DerivationGoal::DerivationGoal(const StorePath & drvPath,
 
 
 DerivationGoal::DerivationGoal(const StorePath & drvPath, const BasicDerivation & drv,
-    const StringSet & wantedOutputs, Worker & worker, BuildMode buildMode)
+    const OutputsSpec & wantedOutputs, Worker & worker, BuildMode buildMode)
     : Goal(worker, DerivedPath::Built { .drvPath = drvPath, .outputs = wantedOutputs })
     , useDerivation(false)
     , drvPath(drvPath)
@@ -142,18 +142,12 @@ void DerivationGoal::work()
     (this->*state)();
 }
 
-void DerivationGoal::addWantedOutputs(const StringSet & outputs)
+void DerivationGoal::addWantedOutputs(const OutputsSpec & outputs)
 {
-    /* If we already want all outputs, there is nothing to do. */
-    if (wantedOutputs.empty()) return;
-
-    if (outputs.empty()) {
-        wantedOutputs.clear();
+    auto newWanted = wantedOutputs.union_(outputs);
+    if (!newWanted.isSubsetOf(wantedOutputs))
         needRestart = true;
-    } else
-        for (auto & i : outputs)
-            if (wantedOutputs.insert(i).second)
-                needRestart = true;
+    wantedOutputs = newWanted;
 }
 
 
@@ -390,7 +384,7 @@ void DerivationGoal::repairClosure()
     auto outputs = queryDerivationOutputMap();
     StorePathSet outputClosure;
     for (auto & i : outputs) {
-        if (!wantOutput(i.first, wantedOutputs)) continue;
+        if (!wantedOutputs.contains(i.first)) continue;
         worker.store.computeFSClosure(i.second, outputClosure);
     }
 
@@ -422,7 +416,7 @@ void DerivationGoal::repairClosure()
         if (drvPath2 == outputsToDrv.end())
             addWaitee(upcast_goal(worker.makePathSubstitutionGoal(i, Repair)));
         else
-            addWaitee(worker.makeDerivationGoal(drvPath2->second, StringSet(), bmRepair));
+            addWaitee(worker.makeDerivationGoal(drvPath2->second, OutputsSpec::All(), bmRepair));
     }
 
     if (waitees.empty()) {
@@ -544,7 +538,8 @@ void DerivationGoal::inputsRealised()
                    However, the impure derivations feature still relies on this
                    fragile way of doing things, because its builds do not have
                    a representation in the store, which is a usability problem
-                   in itself */
+                   in itself. When implementing this logic entirely with lookups
+                   make sure that they're cached. */
                 if (auto outPath = get(inputDrvOutputs, { depDrvPath, j })) {
                     worker.store.computeFSClosure(*outPath, inputPaths);
                 }
@@ -990,10 +985,15 @@ void DerivationGoal::resolvedFinished()
 
         StorePathSet outputPaths;
 
-        // `wantedOutputs` might be empty, which means “all the outputs”
-        auto realWantedOutputs = wantedOutputs;
-        if (realWantedOutputs.empty())
-            realWantedOutputs = resolvedDrv.outputNames();
+        // `wantedOutputs` might merely indicate “all the outputs”
+        auto realWantedOutputs = std::visit(overloaded {
+            [&](const OutputsSpec::All &) {
+                return resolvedDrv.outputNames();
+            },
+            [&](const OutputsSpec::Names & names) {
+                return static_cast<std::set<std::string>>(names);
+            },
+        }, wantedOutputs.raw());
 
         for (auto & wantedOutput : realWantedOutputs) {
             auto initialOutput = get(initialOutputs, wantedOutput);
@@ -1321,7 +1321,14 @@ std::pair<bool, DrvOutputs> DerivationGoal::checkPathValidity()
     if (!drv->type().isPure()) return { false, {} };
 
     bool checkHash = buildMode == bmRepair;
-    auto wantedOutputsLeft = wantedOutputs;
+    auto wantedOutputsLeft = std::visit(overloaded {
+        [&](const OutputsSpec::All &) {
+            return StringSet {};
+        },
+        [&](const OutputsSpec::Names & names) {
+            return static_cast<StringSet>(names);
+        },
+    }, wantedOutputs.raw());
     DrvOutputs validOutputs;
 
     for (auto & i : queryPartialDerivationOutputMap()) {
@@ -1330,7 +1337,7 @@ std::pair<bool, DrvOutputs> DerivationGoal::checkPathValidity()
             // this is an invalid output, gets catched with (!wantedOutputsLeft.empty())
             continue;
         auto & info = *initialOutput;
-        info.wanted = wantOutput(i.first, wantedOutputs);
+        info.wanted = wantedOutputs.contains(i.first);
         if (info.wanted)
             wantedOutputsLeft.erase(i.first);
         if (i.second) {
@@ -1368,7 +1375,7 @@ std::pair<bool, DrvOutputs> DerivationGoal::checkPathValidity()
             validOutputs.emplace(drvOutput, Realisation { drvOutput, info.known->path });
     }
 
-    // If we requested all the outputs via the empty set, we are always fine.
+    // If we requested all the outputs, we are always fine.
     // If we requested specific elements, the loop above removes all the valid
     // ones, so any that are left must be invalid.
     if (!wantedOutputsLeft.empty())

src/libstore/build/derivation-goal.hh

@@ -2,6 +2,7 @@
 
 #include "parsed-derivations.hh"
 #include "lock.hh"
+#include "outputs-spec.hh"
 #include "store-api.hh"
 #include "pathlocks.hh"
 #include "goal.hh"
@@ -55,7 +56,7 @@ struct DerivationGoal : public Goal
 
     /* The specific outputs that we need to build.  Empty means all of
        them. */
-    StringSet wantedOutputs;
+    OutputsSpec wantedOutputs;
 
     /* Mapping from input derivations + output names to actual store
        paths. This is filled in by waiteeDone() as each dependency
@@ -128,10 +129,10 @@ struct DerivationGoal : public Goal
     std::string machineName;
 
     DerivationGoal(const StorePath & drvPath,
-        const StringSet & wantedOutputs, Worker & worker,
+        const OutputsSpec & wantedOutputs, Worker & worker,
         BuildMode buildMode = bmNormal);
     DerivationGoal(const StorePath & drvPath, const BasicDerivation & drv,
-        const StringSet & wantedOutputs, Worker & worker,
+        const OutputsSpec & wantedOutputs, Worker & worker,
         BuildMode buildMode = bmNormal);
     virtual ~DerivationGoal();
 
@@ -142,7 +143,7 @@ struct DerivationGoal : public Goal
     void work() override;
 
     /* Add wanted outputs to an already existing derivation goal. */
-    void addWantedOutputs(const StringSet & outputs);
+    void addWantedOutputs(const OutputsSpec & outputs);
 
     /* The states. */
     void getDerivation();

src/libstore/build/entry-points.cc

@@ -80,7 +80,7 @@ BuildResult Store::buildDerivation(const StorePath & drvPath, const BasicDerivat
     BuildMode buildMode)
 {
     Worker worker(*this, *this);
-    auto goal = worker.makeBasicDerivationGoal(drvPath, drv, {}, buildMode);
+    auto goal = worker.makeBasicDerivationGoal(drvPath, drv, OutputsSpec::All {}, buildMode);
 
     try {
         worker.run(Goals{goal});
@@ -89,7 +89,10 @@ BuildResult Store::buildDerivation(const StorePath & drvPath, const BasicDerivat
         return BuildResult {
             .status = BuildResult::MiscFailure,
             .errorMsg = e.msg(),
-            .path = DerivedPath::Built { .drvPath = drvPath },
+            .path = DerivedPath::Built {
+                .drvPath = drvPath,
+                .outputs = OutputsSpec::All { },
+            },
         };
     };
 }
@@ -130,7 +133,8 @@ void LocalStore::repairPath(const StorePath & path)
         auto info = queryPathInfo(path);
         if (info->deriver && isValidPath(*info->deriver)) {
             goals.clear();
-            goals.insert(worker.makeDerivationGoal(*info->deriver, StringSet(), bmRepair));
+            // FIXME: Should just build the specific output we need.
+            goals.insert(worker.makeDerivationGoal(*info->deriver, OutputsSpec::All { }, bmRepair));
             worker.run(goals);
         } else
             throw Error(worker.exitStatus(), "cannot repair path '%s'", printStorePath(path));

src/libstore/build/local-derivation-goal.cc

@@ -1459,7 +1459,7 @@ struct RestrictedStore : public virtual RestrictedStoreConfig, public virtual Lo
             unknown, downloadSize, narSize);
     }
 
-    virtual std::optional<std::string> getBuildLog(const StorePath & path) override
+    virtual std::optional<std::string> getBuildLogExact(const StorePath & path) override
     { return std::nullopt; }
 
     virtual void addBuildLog(const StorePath & path, std::string_view log) override
@@ -2050,10 +2050,14 @@ void LocalDerivationGoal::runChild()
                     sandboxProfile += "(deny default (with no-log))\n";
                 }
 
-                sandboxProfile += "(import \"sandbox-defaults.sb\")\n";
+                sandboxProfile +=
+                    #include "sandbox-defaults.sb"
+                    ;
 
                 if (!derivationType.isSandboxed())
-                    sandboxProfile += "(import \"sandbox-network.sb\")\n";
+                    sandboxProfile +=
+                        #include "sandbox-network.sb"
+                        ;
 
                 /* Add the output paths we'll use at build-time to the chroot */
                 sandboxProfile += "(allow file-read* file-write* process-exec\n";
@@ -2096,7 +2100,9 @@ void LocalDerivationGoal::runChild()
 
                 sandboxProfile += additionalSandboxProfile;
             } else
-                sandboxProfile += "(import \"sandbox-minimal.sb\")\n";
+                sandboxProfile +=
+                    #include "sandbox-minimal.sb"
+                    ;
 
             debug("Generated sandbox profile:");
             debug(sandboxProfile);
@@ -2121,8 +2127,6 @@ void LocalDerivationGoal::runChild()
                 args.push_back(sandboxFile);
                 args.push_back("-D");
                 args.push_back("_GLOBAL_TMP_DIR=" + globalTmpDir);
-                args.push_back("-D");
-                args.push_back("IMPORT_DIR=" + settings.nixDataDir + "/nix/sandbox/");
                 if (allowLocalNetworking) {
                     args.push_back("-D");
                     args.push_back(std::string("_ALLOW_LOCAL_NETWORKING=1"));
@@ -2748,7 +2752,7 @@ DrvOutputs LocalDerivationGoal::registerOutputs()
             signRealisation(thisRealisation);
             worker.store.registerDrvOutput(thisRealisation);
         }
-        if (wantOutput(outputName, wantedOutputs))
+        if (wantedOutputs.contains(outputName))
             builtOutputs.emplace(thisRealisation.id, thisRealisation);
     }
 

src/libstore/build/sandbox-defaults.sb

@@ -0,0 +1,110 @@
+R""(
+
+(define TMPDIR (param "_GLOBAL_TMP_DIR"))
+
+(deny default)
+
+; Disallow creating setuid/setgid binaries, since that
+; would allow breaking build user isolation.
+(deny file-write-setugid)
+
+; Allow forking.
+(allow process-fork)
+
+; Allow reading system information like #CPUs, etc.
+(allow sysctl-read)
+
+; Allow POSIX semaphores and shared memory.
+(allow ipc-posix*)
+
+; Allow socket creation.
+(allow system-socket)
+
+; Allow sending signals within the sandbox.
+(allow signal (target same-sandbox))
+
+; Allow getpwuid.
+(allow mach-lookup (global-name "com.apple.system.opendirectoryd.libinfo"))
+
+; Access to /tmp.
+; The network-outbound/network-inbound ones are for unix domain sockets, which
+; we allow access to in TMPDIR (but if we allow them more broadly, you could in
+; theory escape the sandbox)
+(allow file* process-exec network-outbound network-inbound
+       (literal "/tmp") (subpath TMPDIR))
+
+; Some packages like to read the system version.
+(allow file-read*
+       (literal "/System/Library/CoreServices/SystemVersion.plist")
+       (literal "/System/Library/CoreServices/SystemVersionCompat.plist"))
+
+; Without this line clang cannot write to /dev/null, breaking some configure tests.
+(allow file-read-metadata (literal "/dev"))
+
+; Many packages like to do local networking in their test suites, but let's only
+; allow it if the package explicitly asks for it.
+(if (param "_ALLOW_LOCAL_NETWORKING")
+    (begin
+      (allow network* (local ip) (local tcp) (local udp))
+
+      ; Allow access to /etc/resolv.conf (which is a symlink to
+      ; /private/var/run/resolv.conf).
+      ; TODO: deduplicate with sandbox-network.sb
+      (allow file-read-metadata
+             (literal "/var")
+             (literal "/etc")
+             (literal "/etc/resolv.conf")
+             (literal "/private/etc/resolv.conf"))
+
+      (allow file-read*
+             (literal "/private/var/run/resolv.conf"))
+
+      ; Allow DNS lookups. This is even needed for localhost, which lots of tests rely on
+      (allow file-read-metadata (literal "/etc/hosts"))
+      (allow file-read*         (literal "/private/etc/hosts"))
+      (allow network-outbound (remote unix-socket (path-literal "/private/var/run/mDNSResponder")))))
+
+; Standard devices.
+(allow file*
+       (literal "/dev/null")
+       (literal "/dev/random")
+       (literal "/dev/stdin")
+       (literal "/dev/stdout")
+       (literal "/dev/tty")
+       (literal "/dev/urandom")
+       (literal "/dev/zero")
+       (subpath "/dev/fd"))
+
+; Allow pseudo-terminals.
+(allow file*
+       (literal "/dev/ptmx")
+       (regex #"^/dev/pty[a-z]+")
+       (regex #"^/dev/ttys[0-9]+"))
+
+; Does nothing, but reduces build noise.
+(allow file* (literal "/dev/dtracehelper"))
+
+; Allow access to zoneinfo since libSystem needs it.
+(allow file-read* (subpath "/usr/share/zoneinfo"))
+
+(allow file-read* (subpath "/usr/share/locale"))
+
+; This is mostly to get more specific log messages when builds try to
+; access something in /etc or /var.
+(allow file-read-metadata
+       (literal "/etc")
+       (literal "/var")
+       (literal "/private/var/tmp"))
+
+; This is used by /bin/sh on macOS 10.15 and later.
+(allow file*
+       (literal "/private/var/select/sh"))
+
+; Allow Rosetta 2 to run x86_64 binaries on aarch64-darwin (and vice versa).
+(allow file-read*
+       (subpath "/Library/Apple/usr/libexec/oah")
+       (subpath "/System/Library/Apple/usr/libexec/oah")
+       (subpath "/System/Library/LaunchDaemons/com.apple.oahd.plist")
+       (subpath "/Library/Apple/System/Library/LaunchDaemons/com.apple.oahd.plist"))
+
+)""

src/libstore/build/sandbox-minimal.sb

@@ -0,0 +1,9 @@
+R""(
+
+(allow default)
+
+; Disallow creating setuid/setgid binaries, since that
+; would allow breaking build user isolation.
+(deny file-write-setugid)
+
+)""

src/libstore/build/sandbox-network.sb

@@ -0,0 +1,24 @@
+R""(
+
+; Allow local and remote network traffic.
+(allow network* (local ip) (remote ip))
+
+; Allow access to /etc/resolv.conf (which is a symlink to
+; /private/var/run/resolv.conf).
+(allow file-read-metadata
+       (literal "/var")
+       (literal "/etc")
+       (literal "/etc/resolv.conf")
+       (literal "/private/etc/resolv.conf"))
+
+(allow file-read*
+       (literal "/private/var/run/resolv.conf"))
+
+; Allow DNS lookups.
+(allow network-outbound (remote unix-socket (path-literal "/private/var/run/mDNSResponder")))
+
+; Allow access to trustd.
+(allow mach-lookup (global-name "com.apple.trustd"))
+(allow mach-lookup (global-name "com.apple.trustd.agent"))
+
+)""

src/libstore/build/worker.cc

@@ -42,7 +42,7 @@ Worker::~Worker()
 
 std::shared_ptr<DerivationGoal> Worker::makeDerivationGoalCommon(
     const StorePath & drvPath,
-    const StringSet & wantedOutputs,
+    const OutputsSpec & wantedOutputs,
     std::function<std::shared_ptr<DerivationGoal>()> mkDrvGoal)
 {
     std::weak_ptr<DerivationGoal> & goal_weak = derivationGoals[drvPath];
@@ -59,7 +59,7 @@ std::shared_ptr<DerivationGoal> Worker::makeDerivationGoalCommon(
 
 
 std::shared_ptr<DerivationGoal> Worker::makeDerivationGoal(const StorePath & drvPath,
-    const StringSet & wantedOutputs, BuildMode buildMode)
+    const OutputsSpec & wantedOutputs, BuildMode buildMode)
 {
     return makeDerivationGoalCommon(drvPath, wantedOutputs, [&]() -> std::shared_ptr<DerivationGoal> {
         return !dynamic_cast<LocalStore *>(&store)
@@ -70,7 +70,7 @@ std::shared_ptr<DerivationGoal> Worker::makeDerivationGoal(const StorePath & drv
 
 
 std::shared_ptr<DerivationGoal> Worker::makeBasicDerivationGoal(const StorePath & drvPath,
-    const BasicDerivation & drv, const StringSet & wantedOutputs, BuildMode buildMode)
+    const BasicDerivation & drv, const OutputsSpec & wantedOutputs, BuildMode buildMode)
 {
     return makeDerivationGoalCommon(drvPath, wantedOutputs, [&]() -> std::shared_ptr<DerivationGoal> {
         return !dynamic_cast<LocalStore *>(&store)

src/libstore/build/worker.hh

@@ -140,15 +140,15 @@ public:
     /* derivation goal */
 private:
     std::shared_ptr<DerivationGoal> makeDerivationGoalCommon(
-        const StorePath & drvPath, const StringSet & wantedOutputs,
+        const StorePath & drvPath, const OutputsSpec & wantedOutputs,
         std::function<std::shared_ptr<DerivationGoal>()> mkDrvGoal);
 public:
     std::shared_ptr<DerivationGoal> makeDerivationGoal(
         const StorePath & drvPath,
-        const StringSet & wantedOutputs, BuildMode buildMode = bmNormal);
+        const OutputsSpec & wantedOutputs, BuildMode buildMode = bmNormal);
     std::shared_ptr<DerivationGoal> makeBasicDerivationGoal(
         const StorePath & drvPath, const BasicDerivation & drv,
-        const StringSet & wantedOutputs, BuildMode buildMode = bmNormal);
+        const OutputsSpec & wantedOutputs, BuildMode buildMode = bmNormal);
 
     /* substitution goal */
     std::shared_ptr<PathSubstitutionGoal> makePathSubstitutionGoal(const StorePath & storePath, RepairFlag repair = NoRepair, std::optional<ContentAddress> ca = std::nullopt);