Progress reporting: Extract generic logic from just base progress reporter class

9 files changed, 142 insertions, 72 deletions

diff --git a/src/buildtool/graph_traverser/TARGETS b/src/buildtool/graph_traverser/TARGETS
index 91c50702..04b5f746 100644
--- a/src/buildtool/graph_traverser/TARGETS
+++ b/src/buildtool/graph_traverser/TARGETS
@@ -19,6 +19,7 @@
     , ["src/buildtool/common", "common"]
     , ["src/buildtool/execution_api/bazel_msg", "bazel_msg"]
     , ["src/buildtool/logging", "logging"]
+    , ["src/buildtool/progress_reporting", "base_progress_reporter"]
     ]
   , "stage": ["src", "buildtool", "graph_traverser"]
   }
diff --git a/src/buildtool/graph_traverser/graph_traverser.hpp b/src/buildtool/graph_traverser/graph_traverser.hpp
index d5e155fa..e34df5cf 100644
--- a/src/buildtool/graph_traverser/graph_traverser.hpp
+++ b/src/buildtool/graph_traverser/graph_traverser.hpp
@@ -45,6 +45,7 @@
 #include "src/buildtool/logging/log_sink_cmdline.hpp"
 #include "src/buildtool/logging/log_sink_file.hpp"
 #include "src/buildtool/logging/logger.hpp"
+#include "src/buildtool/progress_reporting/base_progress_reporter.hpp"
 #include "src/utils/cpp/json.hpp"
 
 class GraphTraverser {
@@ -64,12 +65,6 @@ class GraphTraverser {
         bool failed_artifacts;
     };
 
-    // Type of a progress reporter. The reporter
-    // may only block in such a way that it return on a notification of the
-    // condition variable; moreover, it has to exit once the boolean is true.
-    using progress_reporter_t =
-        std::function<void(std::atomic<bool>*, std::condition_variable*)>;
-
     explicit GraphTraverser(CommandLineArguments clargs)
         : clargs_{std::move(clargs)},
           local_api_{CreateExecutionApi(std::nullopt)},
diff --git a/src/buildtool/main/TARGETS b/src/buildtool/main/TARGETS
index 725bd83a..818c24af 100644
--- a/src/buildtool/main/TARGETS
+++ b/src/buildtool/main/TARGETS
@@ -9,7 +9,7 @@
     , ["src/buildtool/compatibility", "compatibility"]
     , ["src/buildtool/graph_traverser", "graph_traverser"]
     , ["src/buildtool/logging", "logging"]
-    , ["src/buildtool/progress_reporting", "base_progress_reporter"]
+    , ["src/buildtool/progress_reporting", "progress_reporter"]
     , ["src/buildtool/execution_api/local", "garbage_collector"]
     , ["src/buildtool/build_engine/target_map", "result_map"]
     , ["src/buildtool/build_engine/target_map", "target_cache"]
diff --git a/src/buildtool/main/main.cpp b/src/buildtool/main/main.cpp
index d3911bf1..98812abd 100644
--- a/src/buildtool/main/main.cpp
+++ b/src/buildtool/main/main.cpp
@@ -43,7 +43,7 @@
 #include "src/buildtool/execution_api/execution_service/server_implementation.hpp"
 #include "src/buildtool/execution_api/local/garbage_collector.hpp"
 #include "src/buildtool/graph_traverser/graph_traverser.hpp"
-#include "src/buildtool/progress_reporting/base_progress_reporter.hpp"
+#include "src/buildtool/progress_reporting/progress_reporter.hpp"
 #endif
 #include "src/buildtool/logging/log_config.hpp"
 #include "src/buildtool/logging/log_sink_cmdline.hpp"
@@ -1286,7 +1286,7 @@ auto main(int argc, char* argv[]) -> int {
                                         std::move(arguments.build),
                                         std::move(stage_args),
                                         std::move(rebuild_args)},
-                                       BaseProgressReporter::Reporter()};
+                                       ProgressReporter::Reporter()};
 
         if (arguments.cmd == SubCommand::kInstallCas) {
             return FetchAndInstallArtifacts(traverser.GetRemoteApi(),
diff --git a/src/buildtool/progress_reporting/TARGETS b/src/buildtool/progress_reporting/TARGETS
index 23f4bc37..38009108 100644
--- a/src/buildtool/progress_reporting/TARGETS
+++ b/src/buildtool/progress_reporting/TARGETS
@@ -15,16 +15,27 @@
   , "stage": ["src", "buildtool", "progress_reporting"]
   , "deps": [["src/buildtool/logging", "logging"]]
   }
+, "progress_reporter":
+  { "type": ["@", "rules", "CC", "library"]
+  , "name": ["progress_reporter"]
+  , "hdrs": ["progress_reporter.hpp"]
+  , "srcs": ["progress_reporter.cpp"]
+  , "stage": ["src", "buildtool", "progress_reporting"]
+  , "deps": ["base_progress_reporter"]
+  , "private-deps":
+    [ "progress"
+    , ["@", "fmt", "", "fmt"]
+    , ["@", "gsl-lite", "", "gsl-lite"]
+    , ["src/buildtool/common", "common"]
+    , ["src/buildtool/logging", "logging"]
+    ]
+  }
 , "base_progress_reporter":
   { "type": ["@", "rules", "CC", "library"]
   , "name": ["base_progress_reporter"]
   , "hdrs": ["base_progress_reporter.hpp"]
   , "srcs": ["base_progress_reporter.cpp"]
   , "stage": ["src", "buildtool", "progress_reporting"]
-  , "deps":
-    [ ["src/buildtool/graph_traverser", "graph_traverser"]
-    , ["@", "gsl-lite", "", "gsl-lite"]
-    ]
-  , "private-deps": [["src/buildtool/common", "common"], "progress"]
+  , "private-deps": [["src/buildtool/logging", "logging"]]
   }
 }
diff --git a/src/buildtool/progress_reporting/base_progress_reporter.cpp b/src/buildtool/progress_reporting/base_progress_reporter.cpp
index 6bc72d86..11354af8 100644
--- a/src/buildtool/progress_reporting/base_progress_reporter.cpp
+++ b/src/buildtool/progress_reporting/base_progress_reporter.cpp
@@ -12,72 +12,38 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-#ifndef BOOTSTRAP_BUILD_TOOL
-
 #include "src/buildtool/progress_reporting/base_progress_reporter.hpp"
 
+#include <chrono>
+#include <exception>
 #include <mutex>
+#include <utility>
 
-#include <gsl-lite/gsl-lite.hpp>
-
-#include "fmt/core.h"
-#include "src/buildtool/common/statistics.hpp"
+#include "src/buildtool/logging/log_level.hpp"
 #include "src/buildtool/logging/logger.hpp"
-#include "src/buildtool/progress_reporting/progress.hpp"
 
-auto BaseProgressReporter::Reporter() -> GraphTraverser::progress_reporter_t {
-    return [](std::atomic<bool>* done, std::condition_variable* cv) {
+auto BaseProgressReporter::Reporter(std::function<void(void)> report) noexcept
+    -> progress_reporter_t {
+    return [report = std::move(report)](std::atomic<bool>* done,
+                                        std::condition_variable* cv) {
         std::mutex m;
-        auto const& stats = Statistics::Instance();
         std::unique_lock<std::mutex> lock(m);
         int64_t delay = kStartDelayMillis;
-        int total = gsl::narrow<int>(Progress::Instance().OriginMap().size());
         while (not *done) {
             cv->wait_for(lock, std::chrono::milliseconds(delay));
             if (not *done) {
-                // Note: order matters; queued has to be queried last
-                std::string sample =
-                    Progress::Instance().TaskTracker().Sample();
-                int cached = stats.ActionsCachedCounter();
-                int run = stats.ActionsExecutedCounter();
-                int queued = stats.ActionsQueuedCounter();
-                int active = queued - run - cached;
-                std::string now_msg;
-                if (active > 0 and !sample.empty()) {
-                    auto const& origin_map = Progress::Instance().OriginMap();
-                    auto origins = origin_map.find(sample);
-                    if (origins != origin_map.end() and
-                        !origins->second.empty()) {
-                        auto const& origin = origins->second[0];
-                        now_msg = fmt::format(" ({}#{}{})",
-                                              origin.first.target.ToString(),
-                                              origin.second,
-                                              active > 1 ? ", ..." : "");
-                    }
-                    else {
-                        now_msg = fmt::format(
-                            " ({}{})", sample, active > 1 ? ", ..." : "");
-                    }
+                try {
+                    report();
+                } catch (std::exception const& ex) {
+                    Logger::Log(
+                        LogLevel::Warning,
+                        "calling progress report function failed with:\n{}",
+                        ex.what());
+                    // continue with progress reporting
                 }
-                constexpr int kOneHundred{100};
-                int total_work = total - cached;
-                int progress =
-                    kOneHundred;  // default if no work has to be done
-                if (total_work > 0) {
-                    progress = run * kOneHundred / total_work;
-                }
-                Logger::Log(LogLevel::Progress,
-                            "[{:3}%] {} cached, {} run, {} processing{}.",
-                            progress,
-                            cached,
-                            run,
-                            active,
-                            now_msg);
             }
             delay = delay * kDelayScalingFactorNumerator /
                     kDelayScalingFactorDenominator;
         }
     };
 }
-
-#endif
diff --git a/src/buildtool/progress_reporting/base_progress_reporter.hpp b/src/buildtool/progress_reporting/base_progress_reporter.hpp
index 0f298da6..f03a68d1 100644
--- a/src/buildtool/progress_reporting/base_progress_reporter.hpp
+++ b/src/buildtool/progress_reporting/base_progress_reporter.hpp
@@ -12,16 +12,24 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-#ifndef INCLUDED_SRC_BUILDTOOL_PROGRESS_REPORTING_BASE_REPORTER_HPP
-#define INCLUDED_SRC_BUILDTOOL_PROGRESS_REPORTING_BASE_REPORTER_HPP
+#ifndef INCLUDED_SRC_BUILDTOOL_PROGRESS_REPORTING_BASE_PROGRESS_REPORTER_HPP
+#define INCLUDED_SRC_BUILDTOOL_PROGRESS_REPORTING_BASE_PROGRESS_REPORTER_HPP
 
-#ifndef BOOTSTRAP_BUILD_TOOL
+#include <atomic>
+#include <condition_variable>
+#include <cstdint>
+#include <functional>
 
-#include "src/buildtool/graph_traverser/graph_traverser.hpp"
+// Type of a progress reporter. The reporter may only block in such a way that
+// it return on a notification of the condition variable; moreover, it has to
+// exit once the boolean is true.
+using progress_reporter_t =
+    std::function<void(std::atomic<bool>*, std::condition_variable*)>;
 
 class BaseProgressReporter {
   public:
-    static auto Reporter() -> GraphTraverser::progress_reporter_t;
+    [[nodiscard]] static auto Reporter(
+        std::function<void(void)> report) noexcept -> progress_reporter_t;
 
   private:
     constexpr static int64_t kStartDelayMillis = 3000;
@@ -30,6 +38,4 @@ class BaseProgressReporter {
     constexpr static int64_t kDelayScalingFactorDenominator = 70;
 };
 
-#endif
-
-#endif
+#endif  // INCLUDED_SRC_BUILDTOOL_PROGRESS_REPORTING_BASE_PROGRESS_REPORTER_HPP
diff --git a/src/buildtool/progress_reporting/progress_reporter.cpp b/src/buildtool/progress_reporting/progress_reporter.cpp
new file mode 100644
index 00000000..976df89d
--- /dev/null
+++ b/src/buildtool/progress_reporting/progress_reporter.cpp
@@ -0,0 +1,66 @@
+// Copyright 2023 Huawei Cloud Computing Technology Co., Ltd.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "src/buildtool/progress_reporting/progress_reporter.hpp"
+
+#include <string>
+
+#include "fmt/core.h"
+#include "gsl-lite/gsl-lite.hpp"
+#include "src/buildtool/common/statistics.hpp"
+#include "src/buildtool/logging/log_level.hpp"
+#include "src/buildtool/logging/logger.hpp"
+#include "src/buildtool/progress_reporting/progress.hpp"
+
+auto ProgressReporter::Reporter() noexcept -> progress_reporter_t {
+    return BaseProgressReporter::Reporter([]() {
+        int total = gsl::narrow<int>(Progress::Instance().OriginMap().size());
+        // Note: order matters; queued has to be queried last
+        auto const& sample = Progress::Instance().TaskTracker().Sample();
+        auto const& stats = Statistics::Instance();
+        int cached = stats.ActionsCachedCounter();
+        int run = stats.ActionsExecutedCounter();
+        int queued = stats.ActionsQueuedCounter();
+        int active = queued - run - cached;
+        std::string now_msg;
+        if (active > 0 and !sample.empty()) {
+            auto const& origin_map = Progress::Instance().OriginMap();
+            auto origins = origin_map.find(sample);
+            if (origins != origin_map.end() and !origins->second.empty()) {
+                auto const& origin = origins->second[0];
+                now_msg = fmt::format(" ({}#{}{})",
+                                      origin.first.target.ToString(),
+                                      origin.second,
+                                      active > 1 ? ", ..." : "");
+            }
+            else {
+                now_msg =
+                    fmt::format(" ({}{})", sample, active > 1 ? ", ..." : "");
+            }
+        }
+        constexpr int kOneHundred{100};
+        int total_work = total - cached;
+        int progress = kOneHundred;  // default if no work has to be done
+        if (total_work > 0) {
+            progress = run * kOneHundred / total_work;
+        }
+        Logger::Log(LogLevel::Progress,
+                    "[{:3}%] {} cached, {} run, {} processing{}.",
+                    progress,
+                    cached,
+                    run,
+                    active,
+                    now_msg);
+    });
+}
diff --git a/src/buildtool/progress_reporting/progress_reporter.hpp b/src/buildtool/progress_reporting/progress_reporter.hpp
new file mode 100644
index 00000000..5323578f
--- /dev/null
+++ b/src/buildtool/progress_reporting/progress_reporter.hpp
@@ -0,0 +1,25 @@
+// Copyright 2023 Huawei Cloud Computing Technology Co., Ltd.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#ifndef INCLUDED_SRC_BUILDTOOL_PROGRESS_REPORTING_PROGRESS_REPORTER_HPP
+#define INCLUDED_SRC_BUILDTOOL_PROGRESS_REPORTING_PROGRESS_REPORTER_HPP
+
+#include "src/buildtool/progress_reporting/base_progress_reporter.hpp"
+
+class ProgressReporter {
+  public:
+    [[nodiscard]] static auto Reporter() noexcept -> progress_reporter_t;
+};
+
+#endif  // INCLUDED_SRC_BUILDTOOL_PROGRESS_REPORTING_PROGRESS_REPORTER_HPP