Repository: kythe/kythe
Pull request #4779 (2020-12-10 21:56:20)

feat(cxx_common): allow matching with RE2 pattern in addtion to flat_hash_set

shahms: > Are there unit tests for this?

In progress now.

Patch:
diff --git a/kythe/cxx/extractor/bazel_artifact_selector.cc b/kythe/cxx/extractor/bazel_artifact_selector.cc
index b29ebb2b3..7c499c63c 100644
--- a/kythe/cxx/extractor/bazel_artifact_selector.cc
+++ b/kythe/cxx/extractor/bazel_artifact_selector.cc
@@ -22,6 +22,7 @@
 #include "glog/logging.h"
 #include "google/protobuf/any.pb.h"
 #include "kythe/proto/bazel_artifact_selector.pb.h"
+#include "re2/re2.h"
 
 namespace kythe {
 namespace {
@@ -197,11 +198,29 @@ void AspectArtifactSelector::ReadFilesInto(
   state_.pending.emplace(id, target);
 }
 
+ExtraActionSelector::ExtraActionSelector(
+    absl::flat_hash_set<std::string> action_types)
+    : action_matches_([action_types = std::move(action_types)](
+                          absl::string_view action_type) {
+        return action_types.empty() || action_types.contains(action_type);
+      }) {}
+
+ExtraActionSelector::ExtraActionSelector(const RE2* action_pattern)
+    : action_matches_([action_pattern](absl::string_view action_type) {
+        if (action_pattern == nullptr || action_pattern->pattern().empty()) {
+          return false;
+        }
+        return RE2::FullMatch(action_type, *action_pattern);
+      }) {
+  CHECK(action_pattern == nullptr || action_pattern->ok())
+      << "ExtraActionSelector requires a valid pattern: "
+      << action_pattern->error();
+}
+
 absl::optional<BazelArtifact> ExtraActionSelector::Select(
     const build_event_stream::BuildEvent& event) {
   if (event.id().has_action_completed() && event.action().success() &&
-      (action_types_.empty() ||
-       action_types_.contains(event.action().type()))) {
+      action_matches_(event.action().type())) {
     return BazelArtifact{
         .label = event.id().action_completed().label(),
         .files = {{
diff --git a/kythe/cxx/extractor/bazel_artifact_selector.h b/kythe/cxx/extractor/bazel_artifact_selector.h
index 126e44692..d696f804f 100644
--- a/kythe/cxx/extractor/bazel_artifact_selector.h
+++ b/kythe/cxx/extractor/bazel_artifact_selector.h
@@ -28,6 +28,7 @@
 #include "absl/types/span.h"
 #include "google/protobuf/any.pb.h"
 #include "kythe/cxx/extractor/bazel_artifact.h"
+#include "re2/re2.h"
 #include "re2/set.h"
 #include "src/main/java/com/google/devtools/build/lib/buildeventstream/proto/build_event_stream.pb.h"
 
@@ -194,15 +195,18 @@ class ExtraActionSelector final : public BazelArtifactSelector {
   /// to match ActionCompleted events. An empty set will select any successful
   /// action.
   explicit ExtraActionSelector(
-      absl::flat_hash_set<std::string> action_types = {})
-      : action_types_(std::move(action_types)) {}
+      absl::flat_hash_set<std::string> action_types = {});
+
+  /// \brief Constructs an ExtraActionSelector from an allowlist pattern.
+  /// Both a null and an empty pattern will match nothing.
+  explicit ExtraActionSelector(const RE2* action_pattern);
 
   /// \brief Selects artifacts from ExtraAction-based extractors.
   absl::optional<BazelArtifact> Select(
       const build_event_stream::BuildEvent& event) final;
 
  private:
-  absl::flat_hash_set<std::string> action_types_;
+  std::function<bool(absl::string_view)> action_matches_;
 };
 
 }  // namespace kythe


Test patch:
diff --git a/kythe/cxx/extractor/bazel_artifact_selector_test.cc b/kythe/cxx/extractor/bazel_artifact_selector_test.cc
index b01528ef8..9ee0bd4f9 100644
--- a/kythe/cxx/extractor/bazel_artifact_selector_test.cc
+++ b/kythe/cxx/extractor/bazel_artifact_selector_test.cc
@@ -26,6 +26,7 @@
 #include "google/protobuf/text_format.h"
 #include "gtest/gtest.h"
 #include "kythe/cxx/extractor/bazel_artifact.h"
+#include "re2/re2.h"
 #include "re2/set.h"
 #include "src/main/java/com/google/devtools/build/lib/buildeventstream/proto/build_event_stream.pb.h"
 
@@ -169,5 +170,157 @@ TEST(AnyArtifactSelectorTest, ForwardsFunctions) {
   EXPECT_THAT(selector.Deserialize({}), absl::OkStatus());
 }
 
+TEST(ExtraActionSelector, SelectsAllByDefault) {
+  ExtraActionSelector selector;
+  EXPECT_THAT(selector.Select(ParseEventOrDie(R"pb(
+    id {
+      action_completed {
+        primary_output: "path/to/file/dummy.kzip"
+        label: "//kythe/cxx/extractor:bazel_artifact_selector"
+        configuration { id: "hash0" }
+      }
+    }
+    action {
+      success: true
+      label: "//kythe/cxx/extractor:bazel_artifact_selector"
+      primary_output { uri: "file:///home/path/to/file/dummy.kzip" }
+      configuration { id: "hash0" }
+      type: "extract_kzip_cxx_extra_action"
+    }
+  )pb")), Eq(BazelArtifact{
+               .label = "//kythe/cxx/extractor:bazel_artifact_selector",
+               .files = {{
+                   .local_path = "path/to/file/dummy.kzip",
+                   .uri = "file:///home/path/to/file/dummy.kzip",
+               }},
+           }));
+}
+
+TEST(ExtraActionSelector, SelectsFromList) {
+  ExtraActionSelector selector({"matching_action_type"});
+  EXPECT_THAT(selector.Select(ParseEventOrDie(R"pb(
+    id {
+      action_completed {
+        primary_output: "path/to/file/dummy.kzip"
+        label: "//kythe/cxx/extractor:bazel_artifact_selector"
+        configuration { id: "hash0" }
+      }
+    }
+    action {
+      success: true
+      label: "//kythe/cxx/extractor:bazel_artifact_selector"
+      primary_output { uri: "file:///home/path/to/file/dummy.kzip" }
+      configuration { id: "hash0" }
+      type: "matching_action_type"
+    }
+  )pb")), Eq(BazelArtifact{
+               .label = "//kythe/cxx/extractor:bazel_artifact_selector",
+               .files = {{
+                   .local_path = "path/to/file/dummy.kzip",
+                   .uri = "file:///home/path/to/file/dummy.kzip",
+               }},
+           }));
+  EXPECT_THAT(selector.Select(ParseEventOrDie(R"pb(
+    id {
+      action_completed {
+        primary_output: "path/to/file/dummy.kzip"
+        label: "//kythe/cxx/extractor:bazel_artifact_selector"
+        configuration { id: "hash0" }
+      }
+    }
+    action {
+      success: true
+      label: "//kythe/cxx/extractor:bazel_artifact_selector"
+      primary_output { uri: "file:///home/path/to/file/dummy.kzip" }
+      configuration { id: "hash0" }
+      type: "another_action_type"
+    }
+  )pb")), Eq(absl::nullopt));
+}
+
+TEST(ExtraActionSelector, SelectsFromPattern) {
+  const RE2 pattern("matching_action_type");
+  ExtraActionSelector selector(&pattern);
+  EXPECT_THAT(selector.Select(ParseEventOrDie(R"pb(
+    id {
+      action_completed {
+        primary_output: "path/to/file/dummy.kzip"
+        label: "//kythe/cxx/extractor:bazel_artifact_selector"
+        configuration { id: "hash0" }
+      }
+    }
+    action {
+      success: true
+      label: "//kythe/cxx/extractor:bazel_artifact_selector"
+      primary_output { uri: "file:///home/path/to/file/dummy.kzip" }
+      configuration { id: "hash0" }
+      type: "matching_action_type"
+    }
+  )pb")), Eq(BazelArtifact{
+               .label = "//kythe/cxx/extractor:bazel_artifact_selector",
+               .files = {{
+                   .local_path = "path/to/file/dummy.kzip",
+                   .uri = "file:///home/path/to/file/dummy.kzip",
+               }},
+           }));
+  EXPECT_THAT(selector.Select(ParseEventOrDie(R"pb(
+    id {
+      action_completed {
+        primary_output: "path/to/file/dummy.kzip"
+        label: "//kythe/cxx/extractor:bazel_artifact_selector"
+        configuration { id: "hash0" }
+      }
+    }
+    action {
+      success: true
+      label: "//kythe/cxx/extractor:bazel_artifact_selector"
+      primary_output { uri: "file:///home/path/to/file/dummy.kzip" }
+      configuration { id: "hash0" }
+      type: "another_action_type"
+    }
+  )pb")), Eq(absl::nullopt));
+}
+
+TEST(ExtraActionSelector, SelectsNoneWithEmptyPattern) {
+  const RE2 pattern("");
+  ExtraActionSelector selector(&pattern);
+  EXPECT_THAT(selector.Select(ParseEventOrDie(R"pb(
+    id {
+      action_completed {
+        primary_output: "path/to/file/dummy.kzip"
+        label: "//kythe/cxx/extractor:bazel_artifact_selector"
+        configuration { id: "hash0" }
+      }
+    }
+    action {
+      success: true
+      label: "//kythe/cxx/extractor:bazel_artifact_selector"
+      primary_output { uri: "file:///home/path/to/file/dummy.kzip" }
+      configuration { id: "hash0" }
+      type: "another_action_type"
+    }
+  )pb")), Eq(absl::nullopt));
+}
+
+TEST(ExtraActionSelector, SelectsNoneWithNullPattern) {
+  ExtraActionSelector selector(nullptr);
+  EXPECT_THAT(selector.Select(ParseEventOrDie(R"pb(
+    id {
+      action_completed {
+        primary_output: "path/to/file/dummy.kzip"
+        label: "//kythe/cxx/extractor:bazel_artifact_selector"
+        configuration { id: "hash0" }
+      }
+    }
+    action {
+      success: true
+      label: "//kythe/cxx/extractor:bazel_artifact_selector"
+      primary_output { uri: "file:///home/path/to/file/dummy.kzip" }
+      configuration { id: "hash0" }
+      type: "another_action_type"
+    }
+  )pb")), Eq(absl::nullopt));
+}
+
 }  // namespace
 }  // namespace kythe