Implement --target_pattern_file flag for reading patterns from file

An alternative to bazelbuild#10796 to fix bazelbuild#8609.

As bazelbuild#8609 (comment) points out there's currently a --query_file to read a query pattern from file, but there's no analogous feature available for build/test. Generic parameter file support attempted in bazelbuild#10796 turns out to be a little more complex than expected due to bazel's client/server architecture and likely requires some design document.

This PR would fix help bazelbuild#8609 without rushing into any designs, so we can spend time properly designing generic parameter files while also alleviating the immediate problem with a known/simpler pattern.

src/main/java/com/google/devtools/build/lib/buildtool/BuildRequestOptions.java

@@ -455,6 +455,17 @@ public boolean useTopLevelTargetsForSymlinks() {
       help = "If this flag is set, replay action out/err on incremental builds.")
   public boolean replayActionOutErr;
 
+  @Option(
+      name = "target_pattern_file",
+      defaultValue = "",
+      documentationCategory = OptionDocumentationCategory.GENERIC_INPUTS,
+      effectTags = {OptionEffectTag.CHANGES_INPUTS},
+      help =
+          "If set, build will read patterns from the file named here, rather than on the command "
+              + "line. It is an error to specify a file here as well as command-line patterns."
+  )
+  public String targetPatternFile;
+
   /**
    * Converter for jobs: Takes keyword ({@value #FLAG_SYNTAX}). Values must be between 1 and
    * MAX_JOBS.

src/main/java/com/google/devtools/build/lib/runtime/commands/BuildCommand.java

@@ -33,6 +33,7 @@
 import com.google.devtools.build.lib.runtime.KeepGoingOption;
 import com.google.devtools.build.lib.runtime.LoadingPhaseThreadsOption;
 import com.google.devtools.build.lib.util.DetailedExitCode;
+import com.google.devtools.build.lib.util.ExitCode;
 import com.google.devtools.common.options.OptionsParsingResult;
 import java.util.List;
 
@@ -65,9 +66,12 @@ public final class BuildCommand implements BlazeCommand {
   public BlazeCommandResult exec(CommandEnvironment env, OptionsParsingResult options) {
     BlazeRuntime runtime = env.getRuntime();
     List<String> targets;
-    try (SilentCloseable closeable = Profiler.instance().profile("ProjectFileSupport.getTargets")) {
-      // only takes {@code options} to get options.getResidue()
-      targets = ProjectFileSupport.getTargets(runtime.getProjectFileProvider(), options);
+    try {
+      targets = TargetPatternFileSupport.handleTargetPatternFile(env, options);
+    } catch (TargetPatternFileSupport.TargetPatternFileSupportException e) {
+      env.getReporter()
+          .handle(Event.error(e.getMessage()));
+      return BlazeCommandResult.exitCode(ExitCode.COMMAND_LINE_ERROR);
     }
     if (targets.isEmpty()) {
       env.getReporter()

src/main/java/com/google/devtools/build/lib/runtime/commands/TargetPatternFileSupport.java

@@ -0,0 +1,63 @@
+package com.google.devtools.build.lib.runtime.commands;
+
+import com.google.common.collect.Lists;
+import com.google.devtools.build.lib.buildtool.BuildRequestOptions;
+import com.google.devtools.build.lib.profiler.Profiler;
+import com.google.devtools.build.lib.profiler.SilentCloseable;
+import com.google.devtools.build.lib.runtime.BlazeCommand;
+import com.google.devtools.build.lib.runtime.CommandEnvironment;
+import com.google.devtools.build.lib.vfs.FileSystemUtils;
+import com.google.devtools.build.lib.vfs.Path;
+import com.google.devtools.common.options.OptionsParsingResult;
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.util.List;
+
+/**
+ * Provides support for implementations for {@link BlazeCommand} to read target patterns from file.
+ */
+final class TargetPatternFileSupport {
+
+  private TargetPatternFileSupport() {}
+
+  /**
+   * Reads a list of target patterns, either from the command-line residue or by reading newline
+   * delimited target patterns from the --target_pattern_file flag. If --target_pattern_file is
+   * specified and options contain a residue, or file cannot be read it throws an exception instead.
+   */
+  public static List<String> handleTargetPatternFile(
+      CommandEnvironment env, OptionsParsingResult options)
+      throws TargetPatternFileSupportException {
+    List<String> targets = options.getResidue();
+    BuildRequestOptions buildRequestOptions = options.getOptions(BuildRequestOptions.class);
+    if (!targets.isEmpty() && !buildRequestOptions.targetPatternFile.isEmpty()) {
+      throw new TargetPatternFileSupportException(
+          "Command-line target pattern and --target_pattern_file cannot both be specified");
+    } else if (!buildRequestOptions.targetPatternFile.isEmpty()) {
+      // Works for absolute or relative file.
+      Path residuePath =
+          env.getWorkingDirectory().getRelative(buildRequestOptions.targetPatternFile);
+      try {
+        targets =
+            Lists.newArrayList(FileSystemUtils.readLines(residuePath, StandardCharsets.UTF_8));
+      } catch (IOException e) {
+        throw new TargetPatternFileSupportException(
+            "I/O error reading from " + residuePath.getPathString());
+      }
+    } else {
+      try (SilentCloseable closeable =
+          Profiler.instance().profile("ProjectFileSupport.getTargets")) {
+        targets = ProjectFileSupport.getTargets(env.getRuntime().getProjectFileProvider(), options);
+      }
+    }
+    return targets;
+  }
+
+  /**
+   * TargetPatternFileSupportException gets thrown when TargetPatternFileSupport cannot return a
+   * list of targets based on the supplied command line options.
+   */
+  public static class TargetPatternFileSupportException extends Exception {
+    public TargetPatternFileSupportException(String message) { super(message); }
+  }
+}

src/main/java/com/google/devtools/build/lib/runtime/commands/TestCommand.java

@@ -116,7 +116,14 @@ private BlazeCommandResult doTest(
       AnsiTerminalPrinter printer) {
     BlazeRuntime runtime = env.getRuntime();
     // Run simultaneous build and test.
-    List<String> targets = ProjectFileSupport.getTargets(runtime.getProjectFileProvider(), options);
+    List<String> targets;
+    try {
+      targets = TargetPatternFileSupport.handleTargetPatternFile(env, options);
+    } catch (TargetPatternFileSupport.TargetPatternFileSupportException e) {
+      env.getReporter()
+          .handle(Event.error(e.getMessage()));
+      return BlazeCommandResult.exitCode(ExitCode.COMMAND_LINE_ERROR);
+    }
     BuildRequest request = BuildRequest.create(
         getClass().getAnnotation(Command.class).name(), options,
         runtime.getStartupOptionsProvider(), targets,

src/test/java/com/google/devtools/build/lib/BUILD

@@ -1261,6 +1261,7 @@ java_test(
         "//src/main/java/com/google/devtools/build/lib:bazel-modules",
         "//src/main/java/com/google/devtools/build/lib:bazel-rules",
         "//src/main/java/com/google/devtools/build/lib:build-base",
+        "//src/main/java/com/google/devtools/build/lib:build-request-options",
         "//src/main/java/com/google/devtools/build/lib:detailed_exit_code",
         "//src/main/java/com/google/devtools/build/lib:loading-phase-threads-option",
         "//src/main/java/com/google/devtools/build/lib:packages",

src/test/java/com/google/devtools/build/lib/runtime/commands/TargetPatternFileSupportTest.java

@@ -0,0 +1,103 @@
+// Copyright 2020 The Bazel Authors. All rights reserved.
+//
+// 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.google.devtools.build.lib.runtime.commands;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.devtools.build.lib.testutil.MoreAsserts.assertThrows;
+import static org.mockito.Mockito.when;
+
+import com.google.common.collect.ImmutableList;
+import com.google.devtools.build.lib.analysis.ServerDirectories;
+import com.google.devtools.build.lib.buildtool.BuildRequestOptions;
+import com.google.devtools.build.lib.runtime.BlazeRuntime;
+import com.google.devtools.build.lib.runtime.BlazeServerStartupOptions;
+import com.google.devtools.build.lib.runtime.CommandEnvironment;
+import com.google.devtools.build.lib.runtime.commands.TargetPatternFileSupport.TargetPatternFileSupportException;
+import com.google.devtools.build.lib.testutil.Scratch;
+import com.google.devtools.build.lib.testutil.TestConstants;
+import com.google.devtools.common.options.OptionsParser;
+import com.google.devtools.common.options.OptionsParsingException;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+import org.mockito.Mockito;
+
+/** Tests {@link TargetPatternFileSupport}. */
+@RunWith(JUnit4.class)
+public class TargetPatternFileSupportTest {
+
+  private CommandEnvironment env;
+  private Scratch scratch;
+  private OptionsParser options;
+
+  @Before
+  public void setUp() throws Exception {
+    options =
+        OptionsParser.builder().optionsClasses(BuildRequestOptions.class).build();
+    scratch = new Scratch();
+    BlazeRuntime runtime =
+        new BlazeRuntime.Builder()
+            .setFileSystem(scratch.getFileSystem())
+            .setProductName(TestConstants.PRODUCT_NAME)
+            .setServerDirectories(
+                new ServerDirectories(
+                    scratch.resolve("/install"),
+                    scratch.resolve("/base"),
+                    scratch.resolve("/userRoot")))
+            .setStartupOptionsProvider(
+                OptionsParser.builder().optionsClasses(BlazeServerStartupOptions.class).build())
+            .build();
+    env = Mockito.mock(CommandEnvironment.class);
+    when(env.getWorkingDirectory()).thenReturn(scratch.resolve("wd"));
+    when(env.getRuntime()).thenReturn(runtime);
+  }
+
+  @Test
+  public void testTargetPatternFile() throws Exception {
+    scratch.file("/wd/patterns.txt", "//some/...\n//patterns");
+    options.parse("--target_pattern_file=patterns.txt");
+
+    assertThat(TargetPatternFileSupport.handleTargetPatternFile(env, options))
+        .isEqualTo(ImmutableList.of("//some/...", "//patterns"));
+  }
+
+  @Test
+  public void testNoTargetPatternFile() throws TargetPatternFileSupportException {
+    ImmutableList<String> patterns = ImmutableList.of("//some/...", "//patterns");
+    options.setResidue(patterns);
+
+    assertThat(TargetPatternFileSupport.handleTargetPatternFile(env, options))
+        .isEqualTo(patterns);
+  }
+
+  @Test
+  public void testSpecifyPatternAndFileThrows() throws OptionsParsingException {
+    options.parse("--target_pattern_file=patterns.txt");
+    options.setResidue(ImmutableList.of("//some:pattern"));
+
+    assertThrows(
+        TargetPatternFileSupport.TargetPatternFileSupportException.class,
+        () -> TargetPatternFileSupport.handleTargetPatternFile(env, options));
+  }
+
+  @Test
+  public void testSpecifyNonExistingFileThrows() throws OptionsParsingException {
+    options.parse("--target_pattern_file=patterns.txt");
+
+    assertThrows(
+        TargetPatternFileSupport.TargetPatternFileSupportException.class,
+        () -> TargetPatternFileSupport.handleTargetPatternFile(env, options));
+  }
+}

src/test/shell/integration/BUILD

@@ -683,6 +683,15 @@ sh_test(
     deps = ["@bazel_tools//tools/bash/runfiles"],
 )
 
+sh_test(
+    name = "target_pattern_file_test",
+    srcs = ["target_pattern_file_test.sh"],
+    data = [
+        ":test-deps",
+        "@bazel_tools//tools/bash/runfiles",
+    ],
+)
+
 ########################################################################
 # Test suites.
 

src/test/shell/integration/target_pattern_file_test.sh

@@ -0,0 +1,103 @@
+#!/bin/bash
+#
+# Copyright 2020 The Bazel Authors. All rights reserved.
+#
+# 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.
+
+# --- begin runfiles.bash initialization ---
+# Copy-pasted from Bazel's Bash runfiles library (tools/bash/runfiles/runfiles.bash).
+set -euo pipefail
+if [[ ! -d "${RUNFILES_DIR:-/dev/null}" && ! -f "${RUNFILES_MANIFEST_FILE:-/dev/null}" ]]; then
+  if [[ -f "$0.runfiles_manifest" ]]; then
+    export RUNFILES_MANIFEST_FILE="$0.runfiles_manifest"
+  elif [[ -f "$0.runfiles/MANIFEST" ]]; then
+    export RUNFILES_MANIFEST_FILE="$0.runfiles/MANIFEST"
+  elif [[ -f "$0.runfiles/bazel_tools/tools/bash/runfiles/runfiles.bash" ]]; then
+    export RUNFILES_DIR="$0.runfiles"
+  fi
+fi
+if [[ -f "${RUNFILES_DIR:-/dev/null}/bazel_tools/tools/bash/runfiles/runfiles.bash" ]]; then
+  source "${RUNFILES_DIR}/bazel_tools/tools/bash/runfiles/runfiles.bash"
+elif [[ -f "${RUNFILES_MANIFEST_FILE:-/dev/null}" ]]; then
+  source "$(grep -m1 "^bazel_tools/tools/bash/runfiles/runfiles.bash " \
+            "$RUNFILES_MANIFEST_FILE" | cut -d ' ' -f 2-)"
+else
+  echo >&2 "ERROR: cannot find @bazel_tools//tools/bash/runfiles:runfiles.bash"
+  exit 1
+fi
+# --- end runfiles.bash initialization ---
+
+source "$(rlocation "io_bazel/src/test/shell/integration_test_setup.sh")" \
+  || { echo "integration_test_setup.sh not found!" >&2; exit 1; }
+
+# `uname` returns the current platform, e.g "MSYS_NT-10.0" or "Linux".
+# `tr` converts all upper case letters to lower case.
+# `case` matches the result if the `uname | tr` expression to string prefixes
+# that use the same wildcards as names do in Bash, i.e. "msys*" matches strings
+# starting with "msys", and "*" matches everything (it's the default case).
+case "$(uname -s | tr [:upper:] [:lower:])" in
+msys*)
+  # As of 2018-08-14, Bazel on Windows only supports MSYS Bash.
+  declare -r is_windows=true
+  ;;
+*)
+  declare -r is_windows=false
+  ;;
+esac
+
+if "$is_windows"; then
+  # Disable MSYS path conversion that converts path-looking command arguments to
+  # Windows paths (even if they arguments are not in fact paths).
+  export MSYS_NO_PATHCONV=1
+  export MSYS2_ARG_CONV_EXCL="*"
+fi
+
+add_to_bazelrc "build --package_path=%workspace%"
+
+#### SETUP #############################################################
+
+function setup() {
+  cat >BUILD <<'EOF'
+genrule(name = "x", outs = ["x.out"], cmd = "echo true > $@", executable = True)
+sh_test(name = "y", srcs = ["x.out"])
+EOF
+
+  cat >build.params <<'EOF'
+//:x
+//:y
+EOF
+}
+
+#### TESTS #############################################################
+
+function test_target_pattern_file_build() {
+  setup
+  bazel build --target_pattern_file=build.params >& $TEST_log || fail "Expected success"
+  expect_log "2 targets"
+  test -f bazel-genfiles/x.out
+}
+
+function test_target_pattern_file_test() {
+  setup
+  echo //:y > test.params
+  bazel test --target_pattern_file=test.params >& $TEST_log || fail "Expected success"
+  expect_log "1 test passes"
+}
+
+function test_target_pattern_file_and_cli_pattern() {
+  setup
+  bazel build --target_pattern_file=build.params -- //:x >& $TEST_log && fail "Expected failure"
+  expect_log "ERROR: Command-line target pattern and --target_pattern_file cannot both be specified"
+}
+
+run_suite "Tests for using target_pattern_file"