From 7b73c5f6abc9e9b574849f760873252ee987e184 Mon Sep 17 00:00:00 2001 From: Googler Date: Thu, 9 Aug 2018 05:17:29 -0700 Subject: 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 --- .../lib/skyframe/TargetPatternPhaseFunction.java | 65 ++++++++++++++++------ 1 file 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 failedPatterns = new ArrayList(); - ResolvedTargets targets = getTargetsToBuild(env, options, failedPatterns); + List expandedPatterns = getTargetsToBuild(env, options, failedPatterns); + ResolvedTargets 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 getTargetsToBuild( + private static List getTargetsToBuild( Environment env, TargetPatternPhaseKey options, List failedPatterns) - throws InterruptedException { - List patternSkyKeys = new ArrayList<>(); - ResolvedTargets.Builder builder = ResolvedTargets.builder(); + throws InterruptedException { + List 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> resolvedPatterns = env.getValuesOrThrow(patternSkyKeys, TargetParsingException.class); + List 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 mergeAll( + List expandedPatterns, + boolean hasError, + Environment env, + TargetPatternPhaseKey options) + throws InterruptedException { + ResolvedTargets.Builder 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 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 resolvedTargets) { + return new AutoValue_TargetPatternPhaseFunction_ExpandedPattern(pattern, resolvedTargets); + } + + abstract TargetPatternKey pattern(); + abstract ResolvedTargets resolvedTargets(); + } } -- cgit v1.2.3