Make InstrumentationOutput framework support relative to `TEMP_LOGG…

…ING_DIRECTORY`

In more details:

* A new `PathToReplace#Type` enum value (`TEMP_LOGGING_DIRECTORY`) is added.
* `localTempLoggingDirPathStr` can be injected into `InstrumentationOutputFactory`.
* For local output, blaze crashes if relative to type is `TEMP_LOGGING_DIRECTORY` but `localTempLoggingDirPathStr` is not provided.
* `createInstrumentationOutput()` might optionally take in `createParent` flag, which defaults to `false`.

PiperOrigin-RevId: 710150887
Change-Id: Ic08d3966441b1b9b6db951ec7edbfaf28d980946

src/main/java/com/google/devtools/build/lib/runtime/InstrumentationOutputFactory.java

@@ -14,6 +14,7 @@
 package com.google.devtools.build.lib.runtime;
 
 import static com.google.common.base.Preconditions.checkNotNull;
+import static com.google.common.base.StandardSystemProperty.JAVA_IO_TMPDIR;
 
 import com.google.devtools.build.lib.buildeventstream.BuildEventArtifactUploader;
 import com.google.devtools.build.lib.events.Event;
@@ -35,17 +36,20 @@ public final class InstrumentationOutputFactory {
   @Nullable
   final Supplier<InstrumentationOutputBuilder> redirectInstrumentationOutputBuilderSupplier;
 
+  private final String localTempLoggingDirPathStr;
+
   private InstrumentationOutputFactory(
       Supplier<LocalInstrumentationOutput.Builder> localInstrumentationOutputBuilderSupplier,
       Supplier<BuildEventArtifactInstrumentationOutput.Builder>
           buildEventArtifactInstrumentationOutputBuilderSupplier,
-      @Nullable
-          Supplier<InstrumentationOutputBuilder> redirectInstrumentationOutputBuilderSupplier) {
+      @Nullable Supplier<InstrumentationOutputBuilder> redirectInstrumentationOutputBuilderSupplier,
+      String localTempLoggingDirPathStr) {
     this.localInstrumentationOutputBuilderSupplier = localInstrumentationOutputBuilderSupplier;
     this.buildEventArtifactInstrumentationOutputBuilderSupplier =
         buildEventArtifactInstrumentationOutputBuilderSupplier;
     this.redirectInstrumentationOutputBuilderSupplier =
         redirectInstrumentationOutputBuilderSupplier;
+    this.localTempLoggingDirPathStr = localTempLoggingDirPathStr;
   }
 
   /**
@@ -79,7 +83,33 @@ public enum DestinationRelativeTo {
     WORKING_DIRECTORY_OR_HOME,
 
     /** Output is relative to the {@code output_base} directory. */
-    OUTPUT_BASE
+    OUTPUT_BASE,
+
+    /**
+     * Output is relative to the specified system logging directory.
+     *
+     * <p>Used only when {@link #localTempLoggingDirPathStr} is set.
+     */
+    TEMP_LOGGING_DIRECTORY
+  }
+
+  public InstrumentationOutput createInstrumentationOutput(
+      String name,
+      PathFragment destination,
+      DestinationRelativeTo destinationRelativeTo,
+      CommandEnvironment env,
+      EventHandler eventHandler,
+      @Nullable Boolean append,
+      @Nullable Boolean internal) {
+    return createInstrumentationOutput(
+        name,
+        destination,
+        destinationRelativeTo,
+        env,
+        eventHandler,
+        append,
+        internal,
+        /* createParent= */ false);
   }
 
   /**
@@ -92,6 +122,8 @@ public enum DestinationRelativeTo {
    *
    * @param append Whether to open the {@link LocalInstrumentationOutput} file in append mode
    * @param internal Whether the {@link LocalInstrumentationOutput} file is a Bazel internal file.
+   * @param createParent Whether to recursively create parent directories when the file path's
+   *     parent directory does not exist.
    */
   public InstrumentationOutput createInstrumentationOutput(
       String name,
@@ -100,7 +132,8 @@ public InstrumentationOutput createInstrumentationOutput(
       CommandEnvironment env,
       EventHandler eventHandler,
       @Nullable Boolean append,
-      @Nullable Boolean internal) {
+      @Nullable Boolean internal,
+      boolean createParent) {
     boolean isRedirect =
         env.getOptions()
             .getOptions(CommonCommandOptions.class)
@@ -123,21 +156,27 @@ public InstrumentationOutput createInstrumentationOutput(
 
     // Since PathFragmentConverter for flag value replaces prefixed `~/` with user's home path, the
     // destination path could be (1) an absolute path, or (2) a path relative to
-    // output_base/workspace/cwd.
+    // output_base, workspace, cwd or some temporary logging directory.
     Path localOutputPath =
         (switch (destinationRelativeTo) {
               case OUTPUT_BASE -> env.getOutputBase();
               case WORKSPACE_OR_HOME -> env.getWorkspace();
               case WORKING_DIRECTORY_OR_HOME -> env.getWorkingDirectory();
+              case TEMP_LOGGING_DIRECTORY ->
+                  env.getRuntime().getFileSystem().getPath(localTempLoggingDirPathStr);
             })
             .getRelative(destination);
-    return localInstrumentationOutputBuilderSupplier
-        .get()
+    LocalInstrumentationOutput.Builder localOutputBuilder =
+        localInstrumentationOutputBuilderSupplier.get();
+    localOutputBuilder
         .setName(name)
         .setPath(localOutputPath)
         .setAppend(append)
-        .setInternal(internal)
-        .build();
+        .setInternal(internal);
+    if (createParent) {
+      localOutputBuilder.enableCreateParent();
+    }
+    return localOutputBuilder.build();
   }
 
   public BuildEventArtifactInstrumentationOutput createBuildEventArtifactInstrumentationOutput(
@@ -161,6 +200,8 @@ public static class Builder {
     @Nullable
     private Supplier<InstrumentationOutputBuilder> redirectInstrumentationOutputBuilderSupplier;
 
+    @Nullable private String localTempLoggingDirPathStr;
+
     @CanIgnoreReturnValue
     public Builder setLocalInstrumentationOutputBuilderSupplier(
         Supplier<LocalInstrumentationOutput.Builder> localInstrumentationOutputBuilderSupplier) {
@@ -185,6 +226,12 @@ public Builder setRedirectInstrumentationOutputBuilderSupplier(
       return this;
     }
 
+    @CanIgnoreReturnValue
+    public Builder setLocalTempLoggingDirPathStr(String localTempLoggingDirPathStr) {
+      this.localTempLoggingDirPathStr = localTempLoggingDirPathStr;
+      return this;
+    }
+
     public InstrumentationOutputFactory build() {
       return new InstrumentationOutputFactory(
           checkNotNull(
@@ -193,7 +240,8 @@ public InstrumentationOutputFactory build() {
           checkNotNull(
               buildEventArtifactInstrumentationOutputBuilderSupplier,
               "Cannot create InstrumentationOutputFactory without bepOutputBuilderSupplier"),
-          redirectInstrumentationOutputBuilderSupplier);
+          redirectInstrumentationOutputBuilderSupplier,
+          localTempLoggingDirPathStr != null ? localTempLoggingDirPathStr : JAVA_IO_TMPDIR.value());
     }
   }
 }

src/main/protobuf/command_server.proto

@@ -128,6 +128,7 @@ message PathToReplace {
     BUILD_WORKSPACE_DIRECTORY = 3;
     TEST_LOG_SUBDIR = 4;
     HOME = 5;
+    TEMP_LOGGING_DIRECTORY = 7;
   }
   Type type = 1;
   bytes value = 2;

src/test/java/com/google/devtools/build/lib/runtime/InstrumentationOutputFactoryTest.java

@@ -13,6 +13,7 @@
 // limitations under the License.
 package com.google.devtools.build.lib.runtime;
 
+import static com.google.common.base.StandardSystemProperty.JAVA_IO_TMPDIR;
 import static com.google.common.truth.Truth.assertThat;
 import static org.junit.Assert.assertThrows;
 import static org.mockito.Mockito.mock;
@@ -61,20 +62,25 @@ public void testInstrumentationOutputFactory_cannotCreateFactorIfBepSupplierUnse
         factoryBuilder::build);
   }
 
-  private static InstrumentationOutputFactory createInstrumentationOutputFactory() {
+  private static InstrumentationOutputFactory createInstrumentationOutputFactory(
+      boolean setLocalTempLoggingDir) {
     InstrumentationOutputFactory.Builder factoryBuilder =
         new InstrumentationOutputFactory.Builder();
     factoryBuilder.setLocalInstrumentationOutputBuilderSupplier(
         LocalInstrumentationOutput.Builder::new);
     factoryBuilder.setBuildEventArtifactInstrumentationOutputBuilderSupplier(
         BuildEventArtifactInstrumentationOutput.Builder::new);
+    if (setLocalTempLoggingDir) {
+      factoryBuilder.setLocalTempLoggingDirPathStr("/tmp");
+    }
     return factoryBuilder.build();
   }
 
   @Test
   public void testInstrumentationOutputFactory_successfullyCreateLocalOutputWithConvenientLink()
       throws Exception {
-    InstrumentationOutputFactory outputFactory = createInstrumentationOutputFactory();
+    InstrumentationOutputFactory outputFactory =
+        createInstrumentationOutputFactory(/* setLocalTempLoggingDir= */ false);
 
     CommandEnvironment env = runtimeWrapper.newCommand();
     InstrumentationOutput output =
@@ -90,7 +96,8 @@ public void testInstrumentationOutputFactory_successfullyCreateLocalOutputWithCo
 
   @Test
   public void testInstrumentationOutputFactory_localRelativeToOutputBase() throws Exception {
-    InstrumentationOutputFactory outputFactory = createInstrumentationOutputFactory();
+    InstrumentationOutputFactory outputFactory =
+        createInstrumentationOutputFactory(/* setLocalTempLoggingDir= */ false);
 
     CommandEnvironment env = runtimeWrapper.newCommand();
     InstrumentationOutput output =
@@ -110,7 +117,8 @@ public void testInstrumentationOutputFactory_localRelativeToOutputBase() throws
 
   @Test
   public void testInstrumentationOutputFactory_localAbsolutePath() throws Exception {
-    InstrumentationOutputFactory outputFactory = createInstrumentationOutputFactory();
+    InstrumentationOutputFactory outputFactory =
+        createInstrumentationOutputFactory(/* setLocalTempLoggingDir= */ false);
 
     CommandEnvironment env = runtimeWrapper.newCommand();
     InstrumentationOutput output =
@@ -134,7 +142,8 @@ public void testInstrumentationOutputFactory_localRelativePath(
       @TestParameter({"WORKSPACE_OR_HOME", "WORKING_DIRECTORY_OR_HOME"})
           DestinationRelativeTo relativeTo)
       throws Exception {
-    InstrumentationOutputFactory outputFactory = createInstrumentationOutputFactory();
+    InstrumentationOutputFactory outputFactory =
+        createInstrumentationOutputFactory(/* setLocalTempLoggingDir= */ false);
 
     CommandEnvironment env = runtimeWrapper.newCommand();
     InstrumentationOutput output =
@@ -157,6 +166,28 @@ public void testInstrumentationOutputFactory_localRelativePath(
                 .getPathString());
   }
 
+  @Test
+  public void testInstrumentationOutputFactory_localRelativeToTempLogging(
+      @TestParameter boolean setLocalTempLoggingDir) throws Exception {
+    InstrumentationOutputFactory outputFactory =
+        createInstrumentationOutputFactory(setLocalTempLoggingDir);
+
+    CommandEnvironment env = runtimeWrapper.newCommand();
+    InstrumentationOutput output =
+        outputFactory.createInstrumentationOutput(
+            /* name= */ "output-relative",
+            PathFragment.create("relative-output"),
+            DestinationRelativeTo.TEMP_LOGGING_DIRECTORY,
+            env,
+            mock(EventHandler.class),
+            /* append= */ null,
+            /* internal= */ null);
+
+    assertThat(output).isInstanceOf(LocalInstrumentationOutput.class);
+    String expectedOutputBaseDir = setLocalTempLoggingDir ? "/tmp" : JAVA_IO_TMPDIR.value();
+    assertThat(output.getPathString()).isEqualTo(expectedOutputBaseDir + "/relative-output");
+  }
+
   @Test
   public void testInstrumentationOutputFactory_successfulFactoryCreation(
       @TestParameter boolean injectRedirectOutputBuilderSupplier,