Installable::build(): Report succeeding/failing installables

If any installable fails to build, it will print something like

  ✅ flake:nixpkgs#hello
  ❌ git+file:///home/eelco/Dev/patchelf#packages.x86_64-linux.default
  error: Cannot build '/nix/store/nh2dx9cqcy9lw4d4rvd0dbsflwdsbzdy-patchelf-0.18.0.drv'.
         Reason: builder failed with exit code 2.
         ...
  ❓ git+file:///home/eelco/Dev/patchelf#hydraJobs.coverage (cancelled)

It doesn't print anything if all installables succeed.

Author: edolstra

src/libcmd/include/nix/cmd/installables.hh

@@ -183,6 +183,8 @@ struct Installable
         const Installables & installables,
         BuildMode bMode = bmNormal);
 
+    static void throwBuildErrors(std::vector<InstallableWithBuildResult> & buildResults, const Store & store);
+
     static std::set<StorePath> toStorePathSet(
         ref<Store> evalStore, ref<Store> store, Realise mode, OperateOn operateOn, const Installables & installables);
 

src/libcmd/installables.cc

@@ -21,6 +21,7 @@
 #include "nix/util/url.hh"
 #include "nix/fetchers/registry.hh"
 #include "nix/store/build-result.hh"
+#include "nix/util/exit.hh"
 
 #include <regex>
 #include <queue>
@@ -600,45 +601,54 @@ const BuiltPathWithResult & InstallableWithBuildResult::getSuccess() const
         return *std::get_if<Success>(&result);
 }
 
-std::vector<BuiltPathWithResult> Installable::build(
-    ref<Store> evalStore, ref<Store> store, Realise mode, const Installables & installables, BuildMode bMode)
+void Installable::throwBuildErrors(std::vector<InstallableWithBuildResult> & buildResults, const Store & store)
 {
-    std::vector<BuiltPathWithResult> res;
-    for (auto & b : build2(evalStore, store, mode, installables, bMode))
-        res.push_back(b.getSuccess());
-    return res;
-}
-
-static void throwBuildErrors(std::vector<KeyedBuildResult> & buildResults, const Store & store)
-{
-    std::vector<std::pair<const KeyedBuildResult *, const KeyedBuildResult::Failure *>> failed;
     for (auto & buildResult : buildResults) {
-        if (auto * failure = buildResult.tryGetFailure()) {
-            failed.push_back({&buildResult, failure});
-        }
-    }
+        if (std::get_if<InstallableWithBuildResult::Failure>(&buildResult.result)) {
+            // Report success first.
+            for (auto & buildResult : buildResults) {
+                if (std::get_if<InstallableWithBuildResult::Success>(&buildResult.result))
+                    notice("✅ " ANSI_BOLD "%s" ANSI_NORMAL, buildResult.installable->what());
+            }
 
-    auto failedResult = failed.begin();
-    if (failedResult != failed.end()) {
-        if (failed.size() == 1) {
-            failedResult->second->rethrow();
-        } else {
-            StringSet failedPaths;
-            for (; failedResult != failed.end(); failedResult++) {
-                if (!failedResult->second->errorMsg.empty()) {
-                    logError(
-                        ErrorInfo{
-                            .level = lvlError,
-                            .msg = failedResult->second->errorMsg,
-                        });
+            // Then failures.
+            for (auto & buildResult : buildResults) {
+                if (auto failure = std::get_if<InstallableWithBuildResult::Failure>(&buildResult.result)) {
+                    auto failure2 = failure->tryGetFailure();
+                    assert(failure2);
+                    if (failure2->status == BuildResult::Failure::Cancelled
+                        // FIXME: remove MiscFailure eventually
+                        || failure2->status == BuildResult::Failure::MiscFailure)
+                        notice(
+                            "❓ " ANSI_BOLD "%s" ANSI_NORMAL ANSI_FAINT " (cancelled)",
+                            buildResult.installable->what());
+                    else {
+                        printError("❌ " ANSI_RED "%s" ANSI_NORMAL, buildResult.installable->what());
+                        try {
+                            failure2->rethrow();
+                        } catch (Error & e) {
+                            logError(e.info());
+                        }
+                    }
                 }
-                failedPaths.insert(failedResult->first->path.to_string(store));
             }
-            throw Error("build of %s failed", concatStringsSep(", ", quoteStrings(failedPaths)));
+
+            throw Exit(1);
         }
     }
 }
 
+std::vector<BuiltPathWithResult> Installable::build(
+    ref<Store> evalStore, ref<Store> store, Realise mode, const Installables & installables, BuildMode bMode)
+{
+    auto results = build2(evalStore, store, mode, installables, bMode);
+    throwBuildErrors(results, *store);
+    std::vector<BuiltPathWithResult> res;
+    for (auto & b : results)
+        res.push_back(b.getSuccess());
+    return res;
+}
+
 std::vector<InstallableWithBuildResult> Installable::build2(
     ref<Store> evalStore, ref<Store> store, Realise mode, const Installables & installables, BuildMode bMode)
 {
@@ -704,9 +714,13 @@ std::vector<InstallableWithBuildResult> Installable::build2(
             printMissing(store, pathsToBuild, lvlInfo);
 
         auto buildResults = store->buildPathsWithResults(pathsToBuild, bMode, evalStore);
-        throwBuildErrors(buildResults, *store);
         for (auto & buildResult : buildResults) {
-            // If we didn't throw, they must all be successes.
+            if (buildResult.tryGetFailure()) {
+                for (auto & aux : backmap[buildResult.path]) {
+                    res.push_back({.installable = aux.installable, .result = buildResult});
+                }
+                continue;
+            }
             auto & success = std::get<nix::BuildResult::Success>(buildResult.inner);
             for (auto & aux : backmap[buildResult.path]) {
                 std::visit(

src/nix/profile.cc

@@ -364,8 +364,10 @@ struct CmdProfileAdd : InstallablesCommand, MixDefaultProfile
     {
         ProfileManifest manifest(*getEvalState(), *profile);
 
-        auto builtPaths = builtPathsPerInstallable(
-            Installable::build2(getEvalStore(), store, Realise::Outputs, installables, bmNormal));
+        auto buildResults = Installable::build2(getEvalStore(), store, Realise::Outputs, installables, bmNormal);
+        Installable::throwBuildErrors(buildResults, *store);
+
+        auto builtPaths = builtPathsPerInstallable(buildResults);
 
         for (auto & installable : installables) {
             ProfileElement element;
@@ -767,8 +769,10 @@ struct CmdProfileUpgrade : virtual SourceExprCommand, MixDefaultProfile, MixProf
             return;
         }
 
-        auto builtPaths = builtPathsPerInstallable(
-            Installable::build2(getEvalStore(), store, Realise::Outputs, installables, bmNormal));
+        auto buildResults = Installable::build2(getEvalStore(), store, Realise::Outputs, installables, bmNormal);
+        Installable::throwBuildErrors(buildResults, *store);
+
+        auto builtPaths = builtPathsPerInstallable(buildResults);
 
         for (size_t i = 0; i < installables.size(); ++i) {
             auto & installable = installables.at(i);

tests/functional/build.sh

@@ -161,20 +161,17 @@ printf "%s\n" "$drv^*" | nix build --no-link --stdin --json | jq --exit-status '
 out="$(nix build -f fod-failing.nix -L 2>&1)" && status=0 || status=$?
 test "$status" = 1
 # one "hash mismatch" error, one "build of ... failed"
-test "$(<<<"$out" grep -cE '^error:')" = 2
-<<<"$out" grepQuiet -E "hash mismatch in fixed-output derivation '.*-x1\\.drv'"
-<<<"$out" grepQuiet -vE "hash mismatch in fixed-output derivation '.*-x3\\.drv'"
-<<<"$out" grepQuiet -vE "hash mismatch in fixed-output derivation '.*-x2\\.drv'"
-<<<"$out" grepQuiet -E "error: build of '.*-x[1-4]\\.drv\\^out', '.*-x[1-4]\\.drv\\^out', '.*-x[1-4]\\.drv\\^out', '.*-x[1-4]\\.drv\\^out' failed"
+test "$(<<<"$out" grep -cE '^error:')" = 1
+test "$(<<<"$out" grep -cE '(cancelled)')" = 3
+<<<"$out" grepQuiet -E "hash mismatch in fixed-output derivation"
 
 out="$(nix build -f fod-failing.nix -L x1 x2 x3 --keep-going 2>&1)" && status=0 || status=$?
 test "$status" = 1
-# three "hash mismatch" errors - for each failing fod, one "build of ... failed"
-test "$(<<<"$out" grep -cE '^error:')" = 4
+# three "hash mismatch" errors - for each failing fod
+test "$(<<<"$out" grep -cE '^error:')" = 3
 <<<"$out" grepQuiet -E "hash mismatch in fixed-output derivation '.*-x1\\.drv'"
 <<<"$out" grepQuiet -E "hash mismatch in fixed-output derivation '.*-x3\\.drv'"
 <<<"$out" grepQuiet -E "hash mismatch in fixed-output derivation '.*-x2\\.drv'"
-<<<"$out" grepQuiet -E "error: build of '.*-x[1-3]\\.drv\\^out', '.*-x[1-3]\\.drv\\^out', '.*-x[1-3]\\.drv\\^out' failed"
 
 out="$(nix build -f fod-failing.nix -L x4 2>&1)" && status=0 || status=$?
 test "$status" = 1