Factor out checkOutputs

We currently just use this during the build of a derivation, but there is no
reason we wouldn't want to use it elsewhere, e.g. to check the outputs
of someone else's build after the fact.

Moreover, I like pulling things out of `DerivationBuilder` that are
simple and don't need access to all that state. While
`DerivationBuilder` is unix-only, this refactor also make the code more
portable "for free".

The header is private, at Eelco's request.

src/libstore/build/derivation-check.cc

@@ -0,0 +1,156 @@
+#include <queue>
+
+#include "nix/store/store-api.hh"
+
+#include "derivation-check.hh"
+
+namespace nix {
+
+void checkOutputs(
+    Store & store,
+    const StorePath & drvPath,
+    const decltype(DerivationOptions::outputChecks) & outputChecks,
+    const std::map<std::string, ValidPathInfo> & outputs)
+{
+    std::map<Path, const ValidPathInfo &> outputsByPath;
+    for (auto & output : outputs)
+        outputsByPath.emplace(store.printStorePath(output.second.path), output.second);
+
+    for (auto & output : outputs) {
+        auto & outputName = output.first;
+        auto & info = output.second;
+
+        /* Compute the closure and closure size of some output. This
+           is slightly tricky because some of its references (namely
+           other outputs) may not be valid yet. */
+        auto getClosure = [&](const StorePath & path) {
+            uint64_t closureSize = 0;
+            StorePathSet pathsDone;
+            std::queue<StorePath> pathsLeft;
+            pathsLeft.push(path);
+
+            while (!pathsLeft.empty()) {
+                auto path = pathsLeft.front();
+                pathsLeft.pop();
+                if (!pathsDone.insert(path).second)
+                    continue;
+
+                auto i = outputsByPath.find(store.printStorePath(path));
+                if (i != outputsByPath.end()) {
+                    closureSize += i->second.narSize;
+                    for (auto & ref : i->second.references)
+                        pathsLeft.push(ref);
+                } else {
+                    auto info = store.queryPathInfo(path);
+                    closureSize += info->narSize;
+                    for (auto & ref : info->references)
+                        pathsLeft.push(ref);
+                }
+            }
+
+            return std::make_pair(std::move(pathsDone), closureSize);
+        };
+
+        auto applyChecks = [&](const DerivationOptions::OutputChecks & checks) {
+            if (checks.maxSize && info.narSize > *checks.maxSize)
+                throw BuildError(
+                    "path '%s' is too large at %d bytes; limit is %d bytes",
+                    store.printStorePath(info.path),
+                    info.narSize,
+                    *checks.maxSize);
+
+            if (checks.maxClosureSize) {
+                uint64_t closureSize = getClosure(info.path).second;
+                if (closureSize > *checks.maxClosureSize)
+                    throw BuildError(
+                        "closure of path '%s' is too large at %d bytes; limit is %d bytes",
+                        store.printStorePath(info.path),
+                        closureSize,
+                        *checks.maxClosureSize);
+            }
+
+            auto checkRefs = [&](const StringSet & value, bool allowed, bool recursive) {
+                /* Parse a list of reference specifiers.  Each element must
+                   either be a store path, or the symbolic name of the output
+                   of the derivation (such as `out'). */
+                StorePathSet spec;
+                for (auto & i : value) {
+                    if (store.isStorePath(i))
+                        spec.insert(store.parseStorePath(i));
+                    else if (auto output = get(outputs, i))
+                        spec.insert(output->path);
+                    else {
+                        std::string outputsListing =
+                            concatMapStringsSep(", ", outputs, [](auto & o) { return o.first; });
+                        throw BuildError(
+                            "derivation '%s' output check for '%s' contains an illegal reference specifier '%s',"
+                            " expected store path or output name (one of [%s])",
+                            store.printStorePath(drvPath),
+                            outputName,
+                            i,
+                            outputsListing);
+                    }
+                }
+
+                auto used = recursive ? getClosure(info.path).first : info.references;
+
+                if (recursive && checks.ignoreSelfRefs)
+                    used.erase(info.path);
+
+                StorePathSet badPaths;
+
+                for (auto & i : used)
+                    if (allowed) {
+                        if (!spec.count(i))
+                            badPaths.insert(i);
+                    } else {
+                        if (spec.count(i))
+                            badPaths.insert(i);
+                    }
+
+                if (!badPaths.empty()) {
+                    std::string badPathsStr;
+                    for (auto & i : badPaths) {
+                        badPathsStr += "\n  ";
+                        badPathsStr += store.printStorePath(i);
+                    }
+                    throw BuildError(
+                        "output '%s' is not allowed to refer to the following paths:%s",
+                        store.printStorePath(info.path),
+                        badPathsStr);
+                }
+            };
+
+            /* Mandatory check: absent whitelist, and present but empty
+               whitelist mean very different things. */
+            if (auto & refs = checks.allowedReferences) {
+                checkRefs(*refs, true, false);
+            }
+            if (auto & refs = checks.allowedRequisites) {
+                checkRefs(*refs, true, true);
+            }
+
+            /* Optimization: don't need to do anything when
+               disallowed and empty set. */
+            if (!checks.disallowedReferences.empty()) {
+                checkRefs(checks.disallowedReferences, false, false);
+            }
+            if (!checks.disallowedRequisites.empty()) {
+                checkRefs(checks.disallowedRequisites, false, true);
+            }
+        };
+
+        std::visit(
+            overloaded{
+                [&](const DerivationOptions::OutputChecks & checks) { applyChecks(checks); },
+                [&](const std::map<std::string, DerivationOptions::OutputChecks> & checksPerOutput) {
+                    if (auto outputChecks = get(checksPerOutput, outputName))
+
+                        applyChecks(*outputChecks);
+                },
+            },
+            outputChecks);
+    }
+}
+
+} // namespace nix

src/libstore/build/derivation-check.hh

@@ -0,0 +1,23 @@
+#include "nix/store/derivations.hh"
+#include "nix/store/derivation-options.hh"
+#include "nix/store/path-info.hh"
+
+namespace nix {
+
+/**
+ * Check that outputs meets the requirements specified by the
+ * 'outputChecks' attribute (or the legacy
+ * '{allowed,disallowed}{References,Requisites}' attributes).
+ *
+ * The outputs may not be valid yet, hence outputs needs to contain all
+ * needed info like the NAR size. However, the external (not other
+ * output) references of the output must be valid, so we can compute the
+ * closure size.
+ */
+void checkOutputs(
+    Store & store,
+    const StorePath & drvPath,
+    const decltype(DerivationOptions::outputChecks) & drvOptions,
+    const std::map<std::string, ValidPathInfo> & outputs);
+
+} // namespace nix

src/libstore/meson.build

@@ -265,6 +265,7 @@ sources = files(
   'binary-cache-store.cc',
   'build-result.cc',
   'build/derivation-building-goal.cc',
+  'build/derivation-check.cc',
   'build/derivation-goal.cc',
   'build/derivation-trampoline-goal.cc',
   'build/drv-output-substitution-goal.cc',

src/libstore/unix/build/derivation-builder.cc

@@ -42,6 +42,7 @@
 #include "nix/util/signals.hh"
 
 #include "store-config-private.hh"
+#include "build/derivation-check.hh"
 
 namespace nix {
 
@@ -335,13 +336,6 @@ private:
      */
     SingleDrvOutputs registerOutputs();
 
-    /**
-     * Check that an output meets the requirements specified by the
-     * 'outputChecks' attribute (or the legacy
-     * '{allowed,disallowed}{References,Requisites}' attributes).
-     */
-    void checkOutputs(const std::map<std::string, ValidPathInfo> & outputs);
-
 public:
 
     void deleteTmpDir(bool force) override;
@@ -1810,7 +1804,7 @@ SingleDrvOutputs DerivationBuilderImpl::registerOutputs()
     }
 
     /* Apply output checks. */
-    checkOutputs(infos);
+    checkOutputs(store, drvPath, drvOptions.outputChecks, infos);
 
     /* Register each output path as valid, and register the sets of
        paths referenced by each of them.  If there are cycles in the
@@ -1849,149 +1843,6 @@ SingleDrvOutputs DerivationBuilderImpl::registerOutputs()
     return builtOutputs;
 }
 
-void DerivationBuilderImpl::checkOutputs(const std::map<std::string, ValidPathInfo> & outputs)
-{
-    std::map<Path, const ValidPathInfo &> outputsByPath;
-    for (auto & output : outputs)
-        outputsByPath.emplace(store.printStorePath(output.second.path), output.second);
-
-    for (auto & output : outputs) {
-        auto & outputName = output.first;
-        auto & info = output.second;
-
-        /* Compute the closure and closure size of some output. This
-           is slightly tricky because some of its references (namely
-           other outputs) may not be valid yet. */
-        auto getClosure = [&](const StorePath & path) {
-            uint64_t closureSize = 0;
-            StorePathSet pathsDone;
-            std::queue<StorePath> pathsLeft;
-            pathsLeft.push(path);
-
-            while (!pathsLeft.empty()) {
-                auto path = pathsLeft.front();
-                pathsLeft.pop();
-                if (!pathsDone.insert(path).second)
-                    continue;
-
-                auto i = outputsByPath.find(store.printStorePath(path));
-                if (i != outputsByPath.end()) {
-                    closureSize += i->second.narSize;
-                    for (auto & ref : i->second.references)
-                        pathsLeft.push(ref);
-                } else {
-                    auto info = store.queryPathInfo(path);
-                    closureSize += info->narSize;
-                    for (auto & ref : info->references)
-                        pathsLeft.push(ref);
-                }
-            }
-
-            return std::make_pair(std::move(pathsDone), closureSize);
-        };
-
-        auto applyChecks = [&](const DerivationOptions::OutputChecks & checks) {
-            if (checks.maxSize && info.narSize > *checks.maxSize)
-                throw BuildError(
-                    "path '%s' is too large at %d bytes; limit is %d bytes",
-                    store.printStorePath(info.path),
-                    info.narSize,
-                    *checks.maxSize);
-
-            if (checks.maxClosureSize) {
-                uint64_t closureSize = getClosure(info.path).second;
-                if (closureSize > *checks.maxClosureSize)
-                    throw BuildError(
-                        "closure of path '%s' is too large at %d bytes; limit is %d bytes",
-                        store.printStorePath(info.path),
-                        closureSize,
-                        *checks.maxClosureSize);
-            }
-
-            auto checkRefs = [&](const StringSet & value, bool allowed, bool recursive) {
-                /* Parse a list of reference specifiers.  Each element must
-                   either be a store path, or the symbolic name of the output
-                   of the derivation (such as `out'). */
-                StorePathSet spec;
-                for (auto & i : value) {
-                    if (store.isStorePath(i))
-                        spec.insert(store.parseStorePath(i));
-                    else if (auto output = get(outputs, i))
-                        spec.insert(output->path);
-                    else {
-                        std::string outputsListing =
-                            concatMapStringsSep(", ", outputs, [](auto & o) { return o.first; });
-                        throw BuildError(
-                            "derivation '%s' output check for '%s' contains an illegal reference specifier '%s',"
-                            " expected store path or output name (one of [%s])",
-                            store.printStorePath(drvPath),
-                            outputName,
-                            i,
-                            outputsListing);
-                    }
-                }
-
-                auto used = recursive ? getClosure(info.path).first : info.references;
-
-                if (recursive && checks.ignoreSelfRefs)
-                    used.erase(info.path);
-
-                StorePathSet badPaths;
-
-                for (auto & i : used)
-                    if (allowed) {
-                        if (!spec.count(i))
-                            badPaths.insert(i);
-                    } else {
-                        if (spec.count(i))
-                            badPaths.insert(i);
-                    }
-
-                if (!badPaths.empty()) {
-                    std::string badPathsStr;
-                    for (auto & i : badPaths) {
-                        badPathsStr += "\n  ";
-                        badPathsStr += store.printStorePath(i);
-                    }
-                    throw BuildError(
-                        "output '%s' is not allowed to refer to the following paths:%s",
-                        store.printStorePath(info.path),
-                        badPathsStr);
-                }
-            };
-
-            /* Mandatory check: absent whitelist, and present but empty
-               whitelist mean very different things. */
-            if (auto & refs = checks.allowedReferences) {
-                checkRefs(*refs, true, false);
-            }
-            if (auto & refs = checks.allowedRequisites) {
-                checkRefs(*refs, true, true);
-            }
-
-            /* Optimization: don't need to do anything when
-               disallowed and empty set. */
-            if (!checks.disallowedReferences.empty()) {
-                checkRefs(checks.disallowedReferences, false, false);
-            }
-            if (!checks.disallowedRequisites.empty()) {
-                checkRefs(checks.disallowedRequisites, false, true);
-            }
-        };
-
-        std::visit(
-            overloaded{
-                [&](const DerivationOptions::OutputChecks & checks) { applyChecks(checks); },
-                [&](const std::map<std::string, DerivationOptions::OutputChecks> & checksPerOutput) {
-                    if (auto outputChecks = get(checksPerOutput, outputName))
-
-                        applyChecks(*outputChecks);
-                },
-            },
-            drvOptions.outputChecks);
-    }
-}
-
 void DerivationBuilderImpl::deleteTmpDir(bool force)
 {
     if (topTmpDir != "") {