Split up getTargetsToBuild() into multiple methods so compute() can determine which pattern expanded to which targets.

Plan is to modify either TargetParsingCompleteEvent or LoadingPhaseCompleteEvent (which are both posted to the event handler) so that targets can be traced back to original command line patterns.

PiperOrigin-RevId: 208031588

1 files changed, 48 insertions, 17 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternPhaseFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternPhaseFunction.java
index d5fdabb851..955649065e 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternPhaseFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternPhaseFunction.java
@@ -13,6 +13,7 @@
 // limitations under the License.
 package com.google.devtools.build.lib.skyframe;
 
+import com.google.auto.value.AutoValue;
 import com.google.common.base.Preconditions;
 import com.google.common.base.Predicates;
 import com.google.common.collect.ImmutableSet;
@@ -84,7 +85,11 @@ final class TargetPatternPhaseFunction implements SkyFunction {
 
     // Determine targets to build:
     List<String> failedPatterns = new ArrayList<String>();
-    ResolvedTargets<Target> targets = getTargetsToBuild(env, options, failedPatterns);
+    List<ExpandedPattern> expandedPatterns = getTargetsToBuild(env, options, failedPatterns);
+    ResolvedTargets<Target> targets =
+        env.valuesMissing()
+            ? null
+            : mergeAll(expandedPatterns, !failedPatterns.isEmpty(), env, options);
 
     // If the --build_tests_only option was specified or we want to run tests, we need to determine
     // the list of targets to test. For that, we remove manual tests and apply the command-line
@@ -238,15 +243,17 @@ final class TargetPatternPhaseFunction implements SkyFunction {
   }
 
   /**
-   * Interpret the command-line arguments.
+   * Interprets the command-line arguments by expanding each pattern to targets and populating the
+   * list of {@code failedPatterns}.
    *
+   * @param env the Skylark environment
    * @param options the command-line arguments in structured form
+   * @param failedPatterns a list into which failed patterns are added
    */
-  private static ResolvedTargets<Target> getTargetsToBuild(
+  private static List<ExpandedPattern> getTargetsToBuild(
       Environment env, TargetPatternPhaseKey options, List<String> failedPatterns)
-          throws InterruptedException {
-    List<TargetPatternKey> patternSkyKeys = new ArrayList<>();
-    ResolvedTargets.Builder<Target> builder = ResolvedTargets.builder();
+      throws InterruptedException {
+    List<TargetPatternKey> patternSkyKeys = new ArrayList<>(options.getTargetPatterns().size());
     for (TargetPatternSkyKeyOrException keyOrException :
         TargetPatternValue.keys(
             options.getTargetPatterns(),
@@ -267,7 +274,7 @@ final class TargetPatternPhaseFunction implements SkyFunction {
         // that the TargetPatternFunction otherwise reports these events (but only if the target
         // pattern could be parsed successfully).
         env.getListener().post(
-            new ParsingFailedEvent(keyOrException.getOriginalPattern(),  e.getMessage()));
+            new ParsingFailedEvent(keyOrException.getOriginalPattern(), e.getMessage()));
         try {
           env.getValueOrThrow(
               TargetPatternErrorFunction.key(e.getMessage()), TargetParsingException.class);
@@ -277,12 +284,13 @@ final class TargetPatternPhaseFunction implements SkyFunction {
         env.getListener().handle(
             Event.error(
                 "Skipping '" + keyOrException.getOriginalPattern() + "': " + e.getMessage()));
-        builder.setError();
       }
     }
 
     Map<SkyKey, ValueOrException<TargetParsingException>> resolvedPatterns =
         env.getValuesOrThrow(patternSkyKeys, TargetParsingException.class);
+    List<ExpandedPattern> expandedPatterns = new ArrayList<>(patternSkyKeys.size());
+
     for (TargetPatternKey pattern : patternSkyKeys) {
       TargetPatternValue value;
       try {
@@ -293,7 +301,6 @@ final class TargetPatternPhaseFunction implements SkyFunction {
         failedPatterns.add(rawPattern);
         env.getListener().post(PatternExpandingError.failed(rawPattern, errorMessage));
         env.getListener().handle(Event.error("Skipping '" + rawPattern + "': " + errorMessage));
-        builder.setError();
         continue;
       }
       if (value == null) {
@@ -305,17 +312,29 @@ final class TargetPatternPhaseFunction implements SkyFunction {
       if (asTargets == null) {
         continue;
       }
-      if (pattern.isNegative()) {
-        builder.filter(Predicates.not(Predicates.in(asTargets.getTargets())));
+      expandedPatterns.add(ExpandedPattern.of(pattern, asTargets));
+    }
+
+    return expandedPatterns;
+  }
+
+  /** Merges expansions from all patterns into a single {@link ResolvedTargets} instance. */
+  private static ResolvedTargets<Target> mergeAll(
+      List<ExpandedPattern> expandedPatterns,
+      boolean hasError,
+      Environment env,
+      TargetPatternPhaseKey options)
+      throws InterruptedException {
+    ResolvedTargets.Builder<Target> builder = ResolvedTargets.builder();
+    builder.mergeError(hasError);
+
+    for (ExpandedPattern expansion : expandedPatterns) {
+      if (expansion.pattern().isNegative()) {
+        builder.filter(Predicates.not(Predicates.in(expansion.resolvedTargets().getTargets())));
       } else {
-        builder.merge(asTargets);
+        builder.merge(expansion.resolvedTargets());
       }
     }
-    // Only check for missing values after reporting errors. Otherwise we will miss errors in the
-    // nokeep_going case.
-    if (env.valuesMissing()) {
-      return null;
-    }
 
     ResolvedTargets<Target> result = builder
         .filter(TargetUtils.tagFilter(options.getBuildTargetFilter()))
@@ -422,4 +441,16 @@ final class TargetPatternPhaseFunction implements SkyFunction {
   public String extractTag(SkyKey skyKey) {
     return null;
   }
+
+  /** Represents the expansion of a single target pattern. */
+  @AutoValue
+  abstract static class ExpandedPattern {
+
+    static ExpandedPattern of(TargetPatternKey pattern, ResolvedTargets<Target> resolvedTargets) {
+      return new AutoValue_TargetPatternPhaseFunction_ExpandedPattern(pattern, resolvedTargets);
+    }
+
+    abstract TargetPatternKey pattern();
+    abstract ResolvedTargets<Target> resolvedTargets();
+  }
 }