[8.0.0] Match --worker_extra_flag against the worker key mnemonic, no…

…t the action mnemonic, as is the case for other worker flags taking a mnemonic. (#24251)

Fixes #24237.

RELNOTES[INC]: The mnemonic passed to --worker_extra_flag is now matched
against the worker key mnemonic when one is available, instead of the
action mnemonic. This makes it consistent with other worker flags taking
a mnemonic.

PiperOrigin-RevId: 694436563
Change-Id: I7c6e6ede5e3ef2f53aeb847bbfe173042c528d68

Commit
8357fba

Co-authored-by: Googler <[email protected]>

src/main/java/com/google/devtools/build/lib/worker/WorkerParser.java

@@ -211,7 +211,7 @@ ImmutableList<String> splitSpawnArgsIntoWorkerArgsAndFlagFiles(
     ImmutableList.Builder<String> mnemonicFlags = ImmutableList.builder();
 
     workerOptions.workerExtraFlags.stream()
-        .filter(entry -> entry.getKey().equals(spawn.getMnemonic()))
+        .filter(entry -> entry.getKey().equals(Spawns.getWorkerKeyMnemonic(spawn)))
         .forEach(entry -> mnemonicFlags.add(entry.getValue()));
 
     return workerArgs.add("--persistent_worker").addAll(mnemonicFlags.build()).build();

src/test/java/com/google/devtools/build/lib/worker/ExampleWorker.java

@@ -125,6 +125,7 @@ public void processRequests() throws IOException {
 
   public static void main(String[] args) throws Exception {
     if (ImmutableSet.copyOf(args).contains("--persistent_worker")) {
+      System.err.printf("Worker args: %s\n", String.join(" ", args));
       OptionsParser parser =
           OptionsParser.builder()
               .optionsClasses(ExampleWorkerOptions.class)

src/test/java/com/google/devtools/build/lib/worker/ExampleWorkerMultiplexer.java

@@ -78,6 +78,7 @@ private ExampleWorkerMultiplexer() {}
 
   public static void main(String[] args) throws Exception {
     if (ImmutableSet.copyOf(args).contains("--persistent_worker")) {
+      System.err.printf("Worker args: %s\n", String.join(" ", args));
       OptionsParser parser =
           OptionsParser.builder()
               .optionsClasses(ExampleWorkerMultiplexerOptions.class)

src/test/java/com/google/devtools/build/lib/worker/ExampleWorkerOptions.java

@@ -163,6 +163,14 @@ public static class ExampleWorkOptions extends OptionsBase {
       help = "Don't send a response until receiving a cancel request.")
   public boolean waitForCancel;
 
+  @Option(
+      name = "ignored_argument",
+      documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
+      effectTags = {OptionEffectTag.NO_OP},
+      defaultValue = "false",
+      help = "An argument that does nothing, but whose presence can be asserted in a test.")
+  public boolean ignoredArgument;
+
   /** Enum converter for --worker_protocol. */
   public static class WorkerProtocolEnumConverter
       extends EnumConverter<ExecutionRequirements.WorkerProtocolFormat> {

src/test/shell/integration/bazel_worker_test.sh

@@ -322,6 +322,30 @@ EOF
   assert_equals "$worker_uuid_1" "$worker_uuid_2"
 }
 
+function test_worker_extra_flag() {
+  prepare_example_worker
+  cat >>BUILD <<EOF
+work(
+  name = "hello_world",
+  worker = ":worker",
+  worker_args = ["--worker_protocol=${WORKER_PROTOCOL}"],
+  action_mnemonic = "Hello",
+  worker_key_mnemonic = "World",
+)
+EOF
+
+  bazel build :hello_world --worker_extra_flag=World=--ignored_argument \
+      &> "$TEST_log" || fail "build failed"
+
+  local -r worker_log=$(egrep -o -- 'logging to .*/b(azel|laze)-workers/worker-[0-9]+-World.log' "$TEST_log" | sed 's/^logging to //')
+
+  if ! [[ -e "$worker_log" ]]; then
+    fail "Worker log was not found"
+  fi
+
+  assert_contains "Worker args: .* --ignored_argument" "$worker_log"
+}
+
 function test_multiple_flagfiles() {
   prepare_example_worker
   cat >>BUILD <<EOF
@@ -592,10 +616,11 @@ EOF
 
   expect_log "Created new ${WORKER_TYPE} Work worker (id [0-9]\+, key hash -\?[0-9]\+)"
 
-  worker_log=$(egrep -o -- 'logging to .*/b(azel|laze)-workers/worker-[0-9]-Work.log' "$TEST_log" | sed 's/^logging to //')
+  local -r worker_log=$(egrep -o -- 'logging to .*/b(azel|laze)-workers/worker-[0-9]+-Work.log' "$TEST_log" | sed 's/^logging to //')
 
-  [ -e "$worker_log" ] \
-    || fail "Worker log was not found"
+  if ! [[ -e "$worker_log" ]]; then
+    fail "Worker log was not found"
+  fi
 
   # Running a build after a server shutdown should trigger the removal of old worker log files.
   bazel shutdown &> $TEST_log