Merge pull request #281 from DeterminateSystems/show-build-result

edolstra · web-flow · commit 055dfbc6b22d · 2025-12-04T20:58:55.000Z
nix build, nix profile: Report failing/succeeding installables
diff --git a/src/libcmd/include/nix/cmd/installables.hh b/src/libcmd/include/nix/cmd/installables.hh
@@ -96,6 +96,22 @@ typedef std::vector<DerivedPathWithInfo> DerivedPathsWithInfo;
 
 struct Installable;
 
+struct InstallableWithBuildResult
+{
+    ref<Installable> installable;
+
+    using Success = BuiltPathWithResult;
+
+    using Failure = BuildResult; // must be a `BuildResult::Failure`
+
+    std::variant<Success, Failure> result;
+
+    /**
+     * Throw an exception if this represents a failure, otherwise returns a `BuiltPathWithResult`.
+     */
+    const BuiltPathWithResult & getSuccess() const;
+};
+
 /**
  * Shorthand, for less typing and helping us keep the choice of
  * collection in sync.
@@ -160,13 +176,15 @@ struct Installable
         const Installables & installables,
         BuildMode bMode = bmNormal);
 
-    static std::vector<std::pair<ref<Installable>, BuiltPathWithResult>> build2(
+    static std::vector<InstallableWithBuildResult> build2(
         ref<Store> evalStore,
         ref<Store> store,
         Realise mode,
         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);
 
diff --git a/src/libcmd/installables.cc b/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>
@@ -590,46 +591,69 @@ static SingleBuiltPath getBuiltPath(ref<Store> evalStore, ref<Store> store, cons
         b.raw());
 }
 
-std::vector<BuiltPathWithResult> Installable::build(
-    ref<Store> evalStore, ref<Store> store, Realise mode, const Installables & installables, BuildMode bMode)
+const BuiltPathWithResult & InstallableWithBuildResult::getSuccess() const
 {
-    std::vector<BuiltPathWithResult> res;
-    for (auto & [_, builtPathWithResult] : build2(evalStore, store, mode, installables, bMode))
-        res.push_back(builtPathWithResult);
-    return res;
+    if (auto * failure = std::get_if<Failure>(&result)) {
+        auto failure2 = failure->tryGetFailure();
+        assert(failure2);
+        failure2->rethrow();
+    } else
+        return *std::get_if<Success>(&result);
 }
 
-static void throwBuildErrors(std::vector<KeyedBuildResult> & buildResults, const Store & store)
+void Installable::throwBuildErrors(std::vector<InstallableWithBuildResult> & 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 cancelled builds.
+            for (auto & buildResult : buildResults) {
+                if (auto failure = std::get_if<InstallableWithBuildResult::Failure>(&buildResult.result)) {
+                    if (failure->isCancelled())
+                        notice(
+                            "❓ " ANSI_BOLD "%s" ANSI_NORMAL ANSI_FAINT " (cancelled)",
+                            buildResult.installable->what());
+                }
+            }
+
+            // Then failures.
+            for (auto & buildResult : buildResults) {
+                if (auto failure = std::get_if<InstallableWithBuildResult::Failure>(&buildResult.result)) {
+                    if (failure->isCancelled())
+                        continue;
+                    auto failure2 = failure->tryGetFailure();
+                    assert(failure2);
+                    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<std::pair<ref<Installable>, BuiltPathWithResult>> Installable::build2(
+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)
 {
     if (mode == Realise::Nothing)
@@ -651,7 +675,7 @@ std::vector<std::pair<ref<Installable>, BuiltPathWithResult>> Installable::build
         }
     }
 
-    std::vector<std::pair<ref<Installable>, BuiltPathWithResult>> res;
+    std::vector<InstallableWithBuildResult> res;
 
     switch (mode) {
 
@@ -666,17 +690,21 @@ std::vector<std::pair<ref<Installable>, BuiltPathWithResult>> Installable::build
                         [&](const DerivedPath::Built & bfd) {
                             auto outputs = resolveDerivedPath(*store, bfd, &*evalStore);
                             res.push_back(
-                                {aux.installable,
-                                 {.path =
-                                      BuiltPath::Built{
-                                          .drvPath =
-                                              make_ref<SingleBuiltPath>(getBuiltPath(evalStore, store, *bfd.drvPath)),
-                                          .outputs = outputs,
-                                      },
-                                  .info = aux.info}});
+                                {.installable = aux.installable,
+                                 .result = InstallableWithBuildResult::Success{
+                                     .path =
+                                         BuiltPath::Built{
+                                             .drvPath = make_ref<SingleBuiltPath>(
+                                                 getBuiltPath(evalStore, store, *bfd.drvPath)),
+                                             .outputs = outputs,
+                                         },
+                                     .info = aux.info}});
                         },
                         [&](const DerivedPath::Opaque & bo) {
-                            res.push_back({aux.installable, {.path = BuiltPath::Opaque{bo.path}, .info = aux.info}});
+                            res.push_back(
+                                {.installable = aux.installable,
+                                 .result = InstallableWithBuildResult::Success{
+                                     .path = BuiltPath::Opaque{bo.path}, .info = aux.info}});
                         },
                     },
                     path.raw());
@@ -690,9 +718,13 @@ std::vector<std::pair<ref<Installable>, BuiltPathWithResult>> Installable::build
             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 sucesses
+            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(
@@ -702,20 +734,22 @@ std::vector<std::pair<ref<Installable>, BuiltPathWithResult>> Installable::build
                             for (auto & [outputName, realisation] : success.builtOutputs)
                                 outputs.emplace(outputName, realisation.outPath);
                             res.push_back(
-                                {aux.installable,
-                                 {.path =
-                                      BuiltPath::Built{
-                                          .drvPath =
-                                              make_ref<SingleBuiltPath>(getBuiltPath(evalStore, store, *bfd.drvPath)),
-                                          .outputs = outputs,
-                                      },
-                                  .info = aux.info,
-                                  .result = buildResult}});
+                                {.installable = aux.installable,
+                                 .result = InstallableWithBuildResult::Success{
+                                     .path =
+                                         BuiltPath::Built{
+                                             .drvPath = make_ref<SingleBuiltPath>(
+                                                 getBuiltPath(evalStore, store, *bfd.drvPath)),
+                                             .outputs = outputs,
+                                         },
+                                     .info = aux.info,
+                                     .result = buildResult}});
                         },
                         [&](const DerivedPath::Opaque & bo) {
                             res.push_back(
-                                {aux.installable,
-                                 {.path = BuiltPath::Opaque{bo.path}, .info = aux.info, .result = buildResult}});
+                                {.installable = aux.installable,
+                                 .result = InstallableWithBuildResult::Success{
+                                     .path = BuiltPath::Opaque{bo.path}, .info = aux.info, .result = buildResult}});
                         },
                     },
                     buildResult.path.raw());
diff --git a/src/libstore/build-result.cc b/src/libstore/build-result.cc
@@ -13,7 +13,7 @@ std::strong_ordering BuildResult::Success::operator<=>(const BuildResult::Succes
 bool BuildResult::Failure::operator==(const BuildResult::Failure &) const noexcept = default;
 std::strong_ordering BuildResult::Failure::operator<=>(const BuildResult::Failure &) const noexcept = default;
 
-static std::string_view statusToString(BuildResult::Success::Status status)
+std::string_view BuildResult::Success::statusToString(BuildResult::Success::Status status)
 {
     switch (status) {
     case BuildResult::Success::Built:
@@ -29,7 +29,7 @@ static std::string_view statusToString(BuildResult::Success::Status status)
     }
 }
 
-static std::string_view statusToString(BuildResult::Failure::Status status)
+std::string_view BuildResult::Failure::statusToString(BuildResult::Failure::Status status)
 {
     switch (status) {
     case BuildResult::Failure::PermanentFailure:
@@ -56,6 +56,8 @@ static std::string_view statusToString(BuildResult::Failure::Status status)
         return "NoSubstituters";
     case BuildResult::Failure::HashMismatch:
         return "HashMismatch";
+    case BuildResult::Failure::Cancelled:
+        return "Cancelled";
     default:
         unreachable();
     }
@@ -66,9 +68,9 @@ void to_json(nlohmann::json & json, const BuildResult & buildResult)
     json = nlohmann::json::object();
     // FIXME: change this to have `success` and `failure` objects.
     if (auto success = buildResult.tryGetSuccess()) {
-        json["status"] = statusToString(success->status);
+        json["status"] = BuildResult::Success::statusToString(success->status);
     } else if (auto failure = buildResult.tryGetFailure()) {
-        json["status"] = statusToString(failure->status);
+        json["status"] = BuildResult::Failure::statusToString(failure->status);
         if (failure->errorMsg != "")
             json["errorMsg"] = failure->errorMsg;
         if (failure->isNonDeterministic)
diff --git a/src/libstore/include/nix/store/build-result.hh b/src/libstore/include/nix/store/build-result.hh
@@ -31,6 +31,8 @@ struct BuildResult
             ResolvesToAlreadyValid = 13,
         } status;
 
+        static std::string_view statusToString(Status status);
+
         /**
          * For derivations, a mapping from the names of the wanted outputs
          * to actual paths.
@@ -75,8 +77,11 @@ struct BuildResult
             /// know about this one, so change it back to `OutputRejected`
             /// before serialization.
             HashMismatch = 15,
+            Cancelled = 16,
         } status = MiscFailure;
 
+        static std::string_view statusToString(Status status);
+
         /**
          * Information about the error if the build failed.
          *
@@ -98,7 +103,7 @@ struct BuildResult
 
         [[noreturn]] void rethrow() const
         {
-            throw Error("%s", errorMsg);
+            throw Error("%s", errorMsg.empty() ? statusToString(status) : errorMsg);
         }
     };
 
@@ -142,6 +147,13 @@ struct BuildResult
 
     bool operator==(const BuildResult &) const noexcept;
     std::strong_ordering operator<=>(const BuildResult &) const noexcept;
+
+    bool isCancelled() const
+    {
+        auto failure = tryGetFailure();
+        // FIXME: remove MiscFailure eventually.
+        return failure && (failure->status == Failure::Cancelled || failure->status == Failure::MiscFailure);
+    }
 };
 
 /**
diff --git a/src/libstore/include/nix/store/build/goal.hh b/src/libstore/include/nix/store/build/goal.hh
@@ -109,7 +109,7 @@ public:
     /**
      * Build result.
      */
-    BuildResult buildResult;
+    BuildResult buildResult = {.inner = BuildResult::Failure{.status = BuildResult::Failure::Cancelled}};
 
     /**
      * Suspend our goal and wait until we get `work`-ed again.
diff --git a/src/nix/profile.cc b/src/nix/profile.cc
@@ -313,11 +313,11 @@ struct ProfileManifest
 };
 
 static std::map<Installable *, std::pair<BuiltPaths, ref<ExtraPathInfo>>>
-builtPathsPerInstallable(const std::vector<std::pair<ref<Installable>, BuiltPathWithResult>> & builtPaths)
+builtPathsPerInstallable(const std::vector<InstallableWithBuildResult> & builtPaths)
 {
     std::map<Installable *, std::pair<BuiltPaths, ref<ExtraPathInfo>>> res;
-    for (auto & [installable, builtPath] : builtPaths) {
-        auto & r = res.insert({&*installable,
+    for (auto & b : builtPaths) {
+        auto & r = res.insert({&*b.installable,
                                {
                                    {},
                                    make_ref<ExtraPathInfo>(),
@@ -327,6 +327,7 @@ builtPathsPerInstallable(const std::vector<std::pair<ref<Installable>, BuiltPath
            (e.g. meta.priority fields) if the installable returned
            multiple derivations. So pick one arbitrarily. FIXME:
            print a warning? */
+        auto builtPath = b.getSuccess();
         r.first.push_back(builtPath.path);
         r.second = builtPath.info;
     }
@@ -363,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;
@@ -766,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);
diff --git a/tests/functional/build.sh b/tests/functional/build.sh
