[BazelProfile] Make BazelVersion available in `BazelProfile

The functionality provided by `BazelProfile` may need to take the Bazel version into consideration. Therefore, this PR makes the version available within and through the `BazelProfile`, while `BazelVersionDataProvider` stays in place to provide easier access, as well as memoization. Contributes to #109. Signed-off-by: Sara Adams <[email protected]>
EngFlow · Nov 22, 2023 · 8c3a403 · 8c3a403
1 parent 5fe8dcf
commit 8c3a403
Show file tree

Hide file tree

Showing 10 changed files with 153 additions and 56 deletions.
diff --git a/analyzer/java/com/engflow/bazel/invocation/analyzer/bazelprofile/BUILD b/analyzer/java/com/engflow/bazel/invocation/analyzer/bazelprofile/BUILD
@@ -13,6 +13,7 @@ java_library(
     deps = [
         ":types",
         "//analyzer/java/com/engflow/bazel/invocation/analyzer/core",
+        "//analyzer/java/com/engflow/bazel/invocation/analyzer/dataproviders:types",
         "//analyzer/java/com/engflow/bazel/invocation/analyzer/time",
         "//analyzer/java/com/engflow/bazel/invocation/analyzer/traceeventformat",
         "//third_party/gson",

diff --git a/analyzer/java/com/engflow/bazel/invocation/analyzer/bazelprofile/BazelProfile.java b/analyzer/java/com/engflow/bazel/invocation/analyzer/bazelprofile/BazelProfile.java
@@ -19,6 +19,7 @@
 import com.engflow.bazel.invocation.analyzer.core.Datum;
 import com.engflow.bazel.invocation.analyzer.core.DatumSupplierSpecification;
 import com.engflow.bazel.invocation.analyzer.core.DuplicateProviderException;
+import com.engflow.bazel.invocation.analyzer.dataproviders.BazelVersion;
 import com.engflow.bazel.invocation.analyzer.time.DurationUtil;
 import com.engflow.bazel.invocation.analyzer.traceeventformat.CounterEvent;
 import com.engflow.bazel.invocation.analyzer.traceeventformat.TraceEventFormatConstants;
@@ -204,6 +205,18 @@ public ImmutableMap<String, String> getOtherData() {
     return ImmutableMap.copyOf(otherData);
   }
 
+  /**
+   * For performance reasons, prefer getting the Datum {@link BazelVersion} provided by {@link
+   * com.engflow.bazel.invocation.analyzer.dataproviders.BazelVersionDataProvider}, which memoizes
+   * the value.
+   *
+   * @return the BazelVersion included in the profile, if any.
+   */
+  public BazelVersion getBazelVersion() {
+    String bazelVersion = getOtherData().get(BazelProfileConstants.OTHER_DATA_BAZEL_VERSION);
+    return BazelVersion.parse(bazelVersion);
+  }
+
   public Stream<ProfileThread> getThreads() {
     return threads.values().stream();
   }
@@ -307,7 +320,9 @@ private void appendThreadSummary(
   @Override
   public String getSummary() {
     StringBuilder sb = new StringBuilder();
-    sb.append("Threads:\n");
+    sb.append("Bazel version:\n");
+    sb.append(getBazelVersion().toString());
+    sb.append("\n\nThreads:\n");
     appendThreadSummary(sb, getMainThread());
     Optional<ProfileThread> optionalCriticalPath = getCriticalPath();
     if (optionalCriticalPath.isPresent()) {

diff --git a/analyzer/java/com/engflow/bazel/invocation/analyzer/dataproviders/BUILD b/analyzer/java/com/engflow/bazel/invocation/analyzer/dataproviders/BUILD
@@ -2,6 +2,7 @@ TYPES = [
     "ActionStats.java",
     "BazelPhaseDescription.java",
     "BazelPhaseDescriptions.java",
+    "BazelVersion.java",
     "Bottleneck.java",
     "CriticalPathDuration.java",
     "EstimatedCores.java",

diff --git a/analyzer/java/com/engflow/bazel/invocation/analyzer/dataproviders/BazelVersion.java b/analyzer/java/com/engflow/bazel/invocation/analyzer/dataproviders/BazelVersion.java
@@ -19,10 +19,20 @@
 import com.google.common.base.Strings;
 import java.util.Objects;
 import java.util.Optional;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
 import javax.annotation.Nullable;
 
 /** The Bazel version used when the analyzed inputs were generated. */
 public class BazelVersion implements Datum {
+  // See https://bazel.build/release#bazel-versioning
+  // See
+  // https://github.com/bazelbuild/bazel/blob/aab19f75cd383c4b09a6ae720f9fa436bf89d271/src/main/java/com/google/devtools/build/lib/profiler/JsonTraceFileWriter.java#L179
+  // See
+  // https://github.com/bazelbuild/bazel/blob/c637041ec145e0964982a2cbf8d5693f0d1d4be0/src/main/java/com/google/devtools/build/lib/analysis/BlazeVersionInfo.java#L119
+  private static final Pattern BAZEL_VERSION_PATTERN =
+      Pattern.compile("^release (\\d+)\\.(\\d+)\\.(\\d+)(|-.*)$");
+
   private final Optional<Integer> major;
   private final Optional<Integer> minor;
   private final Optional<Integer> patch;
@@ -47,6 +57,26 @@ public class BazelVersion implements Datum {
     this.emptyReason = emptyReason;
   }
 
+  public static BazelVersion parse(String version) {
+    if (version == null) {
+      // The version metadata was introduced in https://github.com/bazelbuild/bazel/pull/17562 and
+      // added to release 6.1.0.
+      return new BazelVersion(
+          "No Bazel version was found. Bazel versions before 6.1.0 did not report the version.");
+    }
+    Matcher m = BAZEL_VERSION_PATTERN.matcher(version);
+    if (m.matches()) {
+      return new BazelVersion(
+          Integer.valueOf(m.group(1)),
+          Integer.valueOf(m.group(2)),
+          Integer.valueOf(m.group(3)),
+          m.group(4));
+    } else {
+      return new BazelVersion(
+          String.format("The provided Bazel version could not be parsed: '%s'", version));
+    }
+  }
+
   @Override
   public boolean isEmpty() {
     return emptyReason != null;

diff --git a/...er/java/com/engflow/bazel/invocation/analyzer/dataproviders/BazelVersionDataProvider.java b/...er/java/com/engflow/bazel/invocation/analyzer/dataproviders/BazelVersionDataProvider.java
@@ -17,30 +17,18 @@
 import static com.engflow.bazel.invocation.analyzer.core.DatumSupplier.memoized;
 
 import com.engflow.bazel.invocation.analyzer.bazelprofile.BazelProfile;
-import com.engflow.bazel.invocation.analyzer.bazelprofile.BazelProfileConstants;
 import com.engflow.bazel.invocation.analyzer.core.DataProvider;
 import com.engflow.bazel.invocation.analyzer.core.DatumSupplierSpecification;
 import com.engflow.bazel.invocation.analyzer.core.InvalidProfileException;
 import com.engflow.bazel.invocation.analyzer.core.MissingInputException;
 import com.engflow.bazel.invocation.analyzer.core.NullDatumException;
-import com.google.common.annotations.VisibleForTesting;
 import java.util.List;
-import java.util.regex.Matcher;
-import java.util.regex.Pattern;
 
 /**
  * A {@link DataProvider} that supplies data on the Bazel version used when the Bazel profile was
  * generated.
  */
 public class BazelVersionDataProvider extends DataProvider {
-  // See https://bazel.build/release#bazel-versioning
-  // See
-  // https://github.com/bazelbuild/bazel/blob/aab19f75cd383c4b09a6ae720f9fa436bf89d271/src/main/java/com/google/devtools/build/lib/profiler/JsonTraceFileWriter.java#L179
-  // See
-  // https://github.com/bazelbuild/bazel/blob/c637041ec145e0964982a2cbf8d5693f0d1d4be0/src/main/java/com/google/devtools/build/lib/analysis/BlazeVersionInfo.java#L119
-  private static final Pattern BAZEL_VERSION_PATTERN =
-      Pattern.compile("^release (\\d+)\\.(\\d+)\\.(\\d+)(|-.*)$");
-
   @Override
   public List<DatumSupplierSpecification<?>> getSuppliers() {
     return List.of(
@@ -49,30 +37,6 @@ public List<DatumSupplierSpecification<?>> getSuppliers() {
 
   public BazelVersion getBazelVersion()
       throws InvalidProfileException, MissingInputException, NullDatumException {
-    BazelProfile bazelProfile = getDataManager().getDatum(BazelProfile.class);
-    String bazelVersion =
-        bazelProfile.getOtherData().get(BazelProfileConstants.OTHER_DATA_BAZEL_VERSION);
-    return parse(bazelVersion);
-  }
-
-  @VisibleForTesting
-  static BazelVersion parse(String version) {
-    if (version == null) {
-      // The version metadata was introduced in https://github.com/bazelbuild/bazel/pull/17562 and
-      // added to release 6.1.0.
-      return new BazelVersion(
-          "No Bazel version was found. Bazel versions before 6.1.0 did not report the version.");
-    }
-    Matcher m = BAZEL_VERSION_PATTERN.matcher(version);
-    if (m.matches()) {
-      return new BazelVersion(
-          Integer.valueOf(m.group(1)),
-          Integer.valueOf(m.group(2)),
-          Integer.valueOf(m.group(3)),
-          m.group(4));
-    } else {
-      return new BazelVersion(
-          String.format("The provided Bazel version could not be parsed: '%s'", version));
-    }
+    return getDataManager().getDatum(BazelProfile.class).getBazelVersion();
   }
 }
diff --git a/analyzer/javatests/com/engflow/bazel/invocation/analyzer/bazelprofile/BUILD b/analyzer/javatests/com/engflow/bazel/invocation/analyzer/bazelprofile/BUILD
@@ -7,6 +7,7 @@ java_test(
         "//analyzer/java/com/engflow/bazel/invocation/analyzer/bazelprofile",
         "//analyzer/java/com/engflow/bazel/invocation/analyzer/bazelprofile:types",
         "//analyzer/java/com/engflow/bazel/invocation/analyzer/core",
+        "//analyzer/java/com/engflow/bazel/invocation/analyzer/dataproviders:types",
         "//analyzer/java/com/engflow/bazel/invocation/analyzer/time",
         "//analyzer/java/com/engflow/bazel/invocation/analyzer/traceeventformat",
         "//analyzer/javatests/com/engflow/bazel/invocation/analyzer:test_base",

diff --git a/analyzer/javatests/com/engflow/bazel/invocation/analyzer/bazelprofile/BazelProfileTest.java b/analyzer/javatests/com/engflow/bazel/invocation/analyzer/bazelprofile/BazelProfileTest.java
@@ -32,6 +32,7 @@
 import com.engflow.bazel.invocation.analyzer.core.InvalidProfileException;
 import com.engflow.bazel.invocation.analyzer.core.MissingInputException;
 import com.engflow.bazel.invocation.analyzer.core.NullDatumException;
+import com.engflow.bazel.invocation.analyzer.dataproviders.BazelVersion;
 import com.engflow.bazel.invocation.analyzer.time.TimeUtil;
 import com.engflow.bazel.invocation.analyzer.time.Timestamp;
 import com.engflow.bazel.invocation.analyzer.traceeventformat.CompleteEvent;
@@ -398,4 +399,67 @@ public void isGarbageCollectorShouldReturnTrueOnOldName() {
         new ProfileThread(new ThreadId(0, 0), "Service Thread", null, null, null, null, null, null);
     assertThat(BazelProfile.isGarbageCollectorThread(thread)).isTrue();
   }
+
+  @Test
+  public void getBazelVersionShouldReturnEmptyOnMissingBazelVersion()
+      throws DuplicateProviderException,
+          InvalidProfileException,
+          MissingInputException,
+          NullDatumException {
+    var profile = useProfile(metaData(), trace(mainThread()));
+    assertThat(profile.getBazelVersion().isEmpty()).isTrue();
+  }
+
+  @Test
+  public void getBazelVersionShouldReturnEmptyOnInvalidBazelVersion()
+      throws DuplicateProviderException,
+          InvalidProfileException,
+          MissingInputException,
+          NullDatumException {
+    var profile =
+        useProfile(
+            metaData(
+                WriteBazelProfile.Property.put(
+                    BazelProfileConstants.OTHER_DATA_BAZEL_VERSION, "invalid")),
+            trace(mainThread()));
+    assertThat(profile.getBazelVersion().isEmpty()).isTrue();
+  }
+
+  @Test
+  public void getBazelVersionShouldReturnVersionWithoutPreRelease()
+      throws DuplicateProviderException,
+          InvalidProfileException,
+          MissingInputException,
+          NullDatumException {
+    String validBazelVersion = "release 6.1.0";
+    var profile =
+        useProfile(
+            metaData(
+                WriteBazelProfile.Property.put(
+                    BazelProfileConstants.OTHER_DATA_BAZEL_VERSION, validBazelVersion)),
+            trace(mainThread()));
+
+    BazelVersion version = profile.getBazelVersion();
+    assertThat(version.isEmpty()).isFalse();
+    assertThat(version.getSummary()).isEqualTo(validBazelVersion);
+  }
+
+  @Test
+  public void getBazelVersionShouldReturnBazelVersionWithPreRelease()
+      throws DuplicateProviderException,
+          InvalidProfileException,
+          MissingInputException,
+          NullDatumException {
+    String validBazelVersion = "release 8.0.0-pre.20231030.2";
+    var profile =
+        useProfile(
+            metaData(
+                WriteBazelProfile.Property.put(
+                    BazelProfileConstants.OTHER_DATA_BAZEL_VERSION, validBazelVersion)),
+            trace(mainThread()));
+
+    BazelVersion version = profile.getBazelVersion();
+    assertThat(version.isEmpty()).isFalse();
+    assertThat(version.getSummary()).isEqualTo(validBazelVersion);
+  }
 }
diff --git a/...sts/com/engflow/bazel/invocation/analyzer/dataproviders/BazelVersionDataProviderTest.java b/...sts/com/engflow/bazel/invocation/analyzer/dataproviders/BazelVersionDataProviderTest.java
@@ -97,22 +97,4 @@ public void shouldReturnBazelVersionWithPreRelease()
     assertThat(version.isEmpty()).isFalse();
     assertThat(version.getSummary()).isEqualTo(validBazelVersion);
   }
-
-  @Test
-  public void parseReturnsEmpty() {
-    assertThat(BazelVersionDataProvider.parse("").isEmpty()).isTrue();
-    assertThat(BazelVersionDataProvider.parse("1.2.3").isEmpty()).isTrue();
-    assertThat(BazelVersionDataProvider.parse("1.2.3-foo").isEmpty()).isTrue();
-    assertThat(BazelVersionDataProvider.parse("release 1").isEmpty()).isTrue();
-    assertThat(BazelVersionDataProvider.parse("release 1.2").isEmpty()).isTrue();
-    assertThat(BazelVersionDataProvider.parse("release 1.2.3foo").isEmpty()).isTrue();
-  }
-
-  @Test
-  public void parseReturnsNonempty() {
-    assertThat(BazelVersionDataProvider.parse("release 1.23.4"))
-        .isEqualTo(new BazelVersion(1, 23, 4, ""));
-    assertThat(BazelVersionDataProvider.parse("release 12.3.45-foo"))
-        .isEqualTo(new BazelVersion(12, 3, 45, "-foo"));
-  }
 }
diff --git a/analyzer/javatests/com/engflow/bazel/invocation/analyzer/dataproviders/BazelVersionTest.java b/analyzer/javatests/com/engflow/bazel/invocation/analyzer/dataproviders/BazelVersionTest.java
@@ -0,0 +1,38 @@
+/*
+ * Copyright 2023 EngFlow Inc.
+ *
+ * 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.
+ */
+
+package com.engflow.bazel.invocation.analyzer.dataproviders;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import org.junit.Test;
+
+public class BazelVersionTest {
+  @Test
+  public void parseReturnsEmpty() {
+    assertThat(BazelVersion.parse("").isEmpty()).isTrue();
+    assertThat(BazelVersion.parse("1.2.3").isEmpty()).isTrue();
+    assertThat(BazelVersion.parse("1.2.3-foo").isEmpty()).isTrue();
+    assertThat(BazelVersion.parse("release 1").isEmpty()).isTrue();
+    assertThat(BazelVersion.parse("release 1.2").isEmpty()).isTrue();
+    assertThat(BazelVersion.parse("release 1.2.3foo").isEmpty()).isTrue();
+  }
+
+  @Test
+  public void parseReturnsNonempty() {
+    assertThat(BazelVersion.parse("release 1.23.4")).isEqualTo(new BazelVersion(1, 23, 4, ""));
+    assertThat(BazelVersion.parse("release 12.3.45-foo"))
+        .isEqualTo(new BazelVersion(12, 3, 45, "-foo"));
+  }
+}
diff --git a/...javatests/com/engflow/bazel/invocation/analyzer/dataproviders/DataProvidersTestSuite.java b/...javatests/com/engflow/bazel/invocation/analyzer/dataproviders/DataProvidersTestSuite.java
@@ -22,6 +22,7 @@
   ActionStatsDataProviderTest.class,
   BazelPhasesDataProviderTest.class,
   BazelPhaseDescriptionsTest.class,
+  BazelVersionTest.class,
   BazelVersionDataProviderTest.class,
   BazelProfilePhaseTest.class,
   CriticalPathDurationDataProviderTest.class,