Rename --experimental_bep_exec_request_include_residue to `--experi…

…mental_run_bep_event_include_residue` For run commands with `--experimental_run_bep_event_include_residue=false`, remove `OriginalUnstructuredCommandLineEvent` from the BEP entirely and remove the residue from `CommandLineEvent`s Also throw `AbruptExitException` in `BuildEventServiceModule#beforeCommand` PiperOrigin-RevId: 556008578 Change-Id: I5f058440bb1e6dcbbc858df1fac3c93257860750
bazelbuild · Aug 11, 2023 · dab6383 · dab6383
1 parent 658b3a6
commit dab6383
Show file tree

Hide file tree

Showing 8 changed files with 76 additions and 19 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/buildeventservice/BuildEventServiceModule.java b/src/main/java/com/google/devtools/build/lib/buildeventservice/BuildEventServiceModule.java
@@ -290,7 +290,7 @@ private void waitForPreviousInvocation(boolean isShutdown) {
   }
 
   @Override
-  public void beforeCommand(CommandEnvironment cmdEnv) {
+  public void beforeCommand(CommandEnvironment cmdEnv) throws AbruptExitException {
     this.invocationId = cmdEnv.getCommandId().toString();
     this.buildRequestId = cmdEnv.getBuildRequestId();
     this.reporter = cmdEnv.getReporter();

diff --git a/src/main/java/com/google/devtools/build/lib/buildeventstream/BuildEventProtocolOptions.java b/src/main/java/com/google/devtools/build/lib/buildeventstream/BuildEventProtocolOptions.java
@@ -97,10 +97,13 @@ public class BuildEventProtocolOptions extends OptionsBase {
   public boolean publishTargetSummary;
 
   @Option(
-      name = "experimental_bep_exec_request_include_residue",
+      name = "experimental_run_bep_event_include_residue",
       defaultValue = "false",
       documentationCategory = OptionDocumentationCategory.LOGGING,
       effectTags = {OptionEffectTag.AFFECTS_OUTPUTS},
-      help = "Whether to include the residue in ExecRequest events.")
-  public boolean includeResidueInExecRequest;
+      help =
+          "Whether to include the command-line residue in run build events which could contain the"
+              + " residue. By default, the residue is not included in run command build events that"
+              + " could contain the residue.")
+  public boolean includeResidueInRunBepEvent;
 }
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcher.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcher.java
@@ -154,8 +154,6 @@ public BlazeCommandResult exec(
       Optional<List<Pair<String, String>>> startupOptionsTaggedWithBazelRc,
       List<Any> commandExtensions)
       throws InterruptedException {
-    OriginalUnstructuredCommandLineEvent originalCommandLine =
-        new OriginalUnstructuredCommandLineEvent(args);
     Preconditions.checkNotNull(clientDescription);
     if (args.isEmpty()) { // Default to help command if no arguments specified.
       args = HELP_COMMAND;
@@ -243,7 +241,6 @@ public BlazeCommandResult exec(
         try {
           result =
               execExclusively(
-                  originalCommandLine,
                   invocationPolicy,
                   args,
                   outErr,
@@ -299,7 +296,6 @@ public BlazeCommandResult exec(List<String> args, String clientDescription, OutE
   }
 
   private BlazeCommandResult execExclusively(
-      OriginalUnstructuredCommandLineEvent unstructuredServerCommandLineEvent,
       InvocationPolicy invocationPolicy,
       List<String> args,
       OutErr outErr,
@@ -641,6 +637,15 @@ private BlazeCommandResult execExclusively(
               runtime, commandName, options, startupOptionsTaggedWithBazelRc);
       CommandLineEvent canonicalCommandLineEvent =
           new CommandLineEvent.CanonicalCommandLineEvent(runtime, commandName, options);
+      BuildEventProtocolOptions bepOptions =
+          env.getOptions().getOptions(BuildEventProtocolOptions.class);
+      OriginalUnstructuredCommandLineEvent unstructuredServerCommandLineEvent;
+      if (commandName.equals("run") && !bepOptions.includeResidueInRunBepEvent) {
+        unstructuredServerCommandLineEvent =
+            OriginalUnstructuredCommandLineEvent.REDACTED_UNSTRUCTURED_COMMAND_LINE_EVENT;
+      } else {
+        unstructuredServerCommandLineEvent = new OriginalUnstructuredCommandLineEvent(args);
+      }
       env.getEventBus().post(unstructuredServerCommandLineEvent);
       env.getEventBus().post(originalCommandLineEvent);
       env.getEventBus().post(canonicalCommandLineEvent);

diff --git a/src/main/java/com/google/devtools/build/lib/runtime/CommandLineEvent.java b/src/main/java/com/google/devtools/build/lib/runtime/CommandLineEvent.java
@@ -19,6 +19,7 @@
 import com.google.common.io.BaseEncoding;
 import com.google.devtools.build.lib.buildeventstream.BuildEventContext;
 import com.google.devtools.build.lib.buildeventstream.BuildEventIdUtil;
+import com.google.devtools.build.lib.buildeventstream.BuildEventProtocolOptions;
 import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos;
 import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos.BuildEvent;
 import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos.BuildEventId;
@@ -186,10 +187,16 @@ CommandLineSection getResidual() {
       // project files, as in runtime.commands.ProjectFileSupport. To properly report this, we would
       // need to let the command customize how the residual is listed. This catch-all could serve
       // as a default in this case.
-      return CommandLineSection.newBuilder()
-          .setSectionLabel("residual")
-          .setChunkList(ChunkList.newBuilder().addAllChunk(commandOptions.getResidue()))
-          .build();
+      CommandLineSection.Builder builder =
+          CommandLineSection.newBuilder().setSectionLabel("residual");
+      if (commandName.equals("run")
+          && !commandOptions.getOptions(BuildEventProtocolOptions.class)
+              .includeResidueInRunBepEvent) {
+        builder.setChunkList(ChunkList.newBuilder().addChunk("REDACTED"));
+      } else {
+        builder.setChunkList(ChunkList.newBuilder().addAllChunk(commandOptions.getResidue()));
+      }
+      return builder.build();
     }
   }
 

diff --git a/...main/java/com/google/devtools/build/lib/runtime/OriginalUnstructuredCommandLineEvent.java b/...main/java/com/google/devtools/build/lib/runtime/OriginalUnstructuredCommandLineEvent.java
@@ -26,10 +26,11 @@
 
 /** A build event reporting the original commandline by which bazel was invoked. */
 public class OriginalUnstructuredCommandLineEvent implements BuildEventWithOrderConstraint {
-
+  static final OriginalUnstructuredCommandLineEvent REDACTED_UNSTRUCTURED_COMMAND_LINE_EVENT =
+      new OriginalUnstructuredCommandLineEvent(ImmutableList.of("REDACTED"));
   private final ImmutableList<String> args;
 
-  public OriginalUnstructuredCommandLineEvent(List<String> args) {
+  OriginalUnstructuredCommandLineEvent(List<String> args) {
     this.args = ImmutableList.copyOf(args);
   }
 

diff --git a/src/main/java/com/google/devtools/build/lib/runtime/commands/RunCommand.java b/src/main/java/com/google/devtools/build/lib/runtime/commands/RunCommand.java
@@ -300,7 +300,7 @@ public BlazeCommandResult exec(CommandEnvironment env, OptionsParsingResult opti
               builtTargets.configuration,
               builtTargets.stopTime);
       boolean includeResidueInExecRequest =
-          options.getOptions(BuildEventProtocolOptions.class).includeResidueInExecRequest;
+          options.getOptions(BuildEventProtocolOptions.class).includeResidueInRunBepEvent;
       env.getReporter().post(new ExecRequestEvent(execRequest, includeResidueInExecRequest));
       return BlazeCommandResult.execute(execRequest);
     } catch (RunCommandException e) {

diff --git a/src/main/protobuf/failure_details.proto b/src/main/protobuf/failure_details.proto
@@ -278,7 +278,7 @@ message BuildProgress {
         [(metadata) = { exit_code: 45 }];
     BES_UPLOAD_RETRY_LIMIT_EXCEEDED_FAILURE = 17
         [(metadata) = { exit_code: 38 }];
-    reserved 1, 2, 18; // For internal use
+    reserved 1, 2, 18, 20; // For internal use
   }
   Code code = 1;
   // Additional data could include the build progress upload endpoint.

diff --git a/src/test/shell/bazel/bazel_build_event_stream_test.sh b/src/test/shell/bazel/bazel_build_event_stream_test.sh
@@ -222,7 +222,7 @@ EOF
   expect_not_log "argv"
 }
 
-function test_residue_in_exec_request_flag(){
+function test_residue_in_run_bep(){
   mkdir -p a
   cat > a/BUILD <<'EOF'
 sh_binary(
@@ -242,17 +242,58 @@ done
 EOF
 
   chmod +x a/arg.sh
-  bazel run --experimental_bep_exec_request_include_residue --build_event_json_file=bep.json //a:arg -- \
-    'arg1' 'arg2' \
+  bazel run --experimental_run_bep_event_include_residue=true \
+   --build_event_json_file=bep.json //a:arg -- 'arg1' 'arg2' \
     >&"$TEST_log" || fail "run failed"
 
   expect_log "ARG 1: arg1"
   expect_log "ARG 2: arg2"
 
+  ls >& "$TEST_log"
   cat bep.json >> "$TEST_log"
 
   expect_log "execRequest"
   expect_log "argv"
+  expect_log "arg1"
+  expect_log "arg2"
+}
+
+function test_no_residue_in_run_bep(){
+  mkdir -p a
+  cat > a/BUILD <<'EOF'
+sh_binary(
+    name = 'arg',
+    srcs = ['arg.sh'],
+)
+EOF
+
+  cat > a/arg.sh <<'EOF'
+#!/bin/bash
+
+COUNTER=1
+for i in "$@"; do
+  echo "ARG $COUNTER": $i;
+  COUNTER=$(($COUNTER+1))
+done
+EOF
+
+  chmod +x a/arg.sh
+  bazel run --build_event_json_file=bep.json \
+  --experimental_run_bep_event_include_residue=false //a:arg -- \
+    'arg1' 'arg2' \
+    >&"$TEST_log" || fail "run failed"
+
+  expect_log "ARG 1: arg1"
+  expect_log "ARG 2: arg2"
+
+  ls >& "$TEST_log"
+  cat bep.json >> "$TEST_log"
+
+  expect_log "execRequest"
+  expect_not_log "argv"
+  expect_log "REDACTED"
+  expect_not_log "arg1"
+  expect_not_log "arg2"
 }
 
 run_suite "Bazel-specific integration tests for the build-event stream"