diff options
15 files changed, 191 insertions, 181 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/CqueryBuildTool.java b/src/main/java/com/google/devtools/build/lib/buildtool/CqueryBuildTool.java index 2bdd2b7750..1b9628e952 100644 --- a/src/main/java/com/google/devtools/build/lib/buildtool/CqueryBuildTool.java +++ b/src/main/java/com/google/devtools/build/lib/buildtool/CqueryBuildTool.java @@ -47,7 +47,7 @@ public class CqueryBuildTool extends PostAnalysisQueryBuildTool { extraFunctions, targetConfig, hostConfiguration, - env.newTargetPatternEvaluator().getOffset(), + env.getRelativeWorkingDirectory().getPathString(), env.getPackageManager().getPackagePath(), () -> walkableGraph, cqueryOptions); diff --git a/src/main/java/com/google/devtools/build/lib/pkgcache/PackageManager.java b/src/main/java/com/google/devtools/build/lib/pkgcache/PackageManager.java index 0bbfae7ced..13f38cd1e5 100644 --- a/src/main/java/com/google/devtools/build/lib/pkgcache/PackageManager.java +++ b/src/main/java/com/google/devtools/build/lib/pkgcache/PackageManager.java @@ -59,7 +59,7 @@ public interface PackageManager extends PackageProvider, CachingPackageLocator { /** * Retrieve a target pattern parser that works with this package manager. */ - TargetPatternEvaluator newTargetPatternEvaluator(); + TargetPatternPreloader newTargetPatternPreloader(); /** * Construct a new {@link TransitivePackageLoader}. diff --git a/src/main/java/com/google/devtools/build/lib/pkgcache/TargetPatternEvaluator.java b/src/main/java/com/google/devtools/build/lib/pkgcache/TargetPatternEvaluator.java index a9e69bd5b4..ea2651d6ab 100644 --- a/src/main/java/com/google/devtools/build/lib/pkgcache/TargetPatternEvaluator.java +++ b/src/main/java/com/google/devtools/build/lib/pkgcache/TargetPatternEvaluator.java @@ -14,17 +14,13 @@ package com.google.devtools.build.lib.pkgcache; -import com.google.common.annotations.VisibleForTesting; import com.google.devtools.build.lib.cmdline.ResolvedTargets; import com.google.devtools.build.lib.cmdline.TargetParsingException; import com.google.devtools.build.lib.concurrent.ThreadSafety; -import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadHostile; import com.google.devtools.build.lib.events.ExtendedEventHandler; import com.google.devtools.build.lib.packages.Target; import com.google.devtools.build.lib.vfs.PathFragment; -import java.util.Collection; import java.util.List; -import java.util.Map; /** * A parser for target patterns. Target patterns are a generalisation of @@ -50,49 +46,10 @@ public interface TargetPatternEvaluator { * computed. Implements the specification described in the class-level comment. */ ResolvedTargets<Target> parseTargetPatternList( + PathFragment relativeWorkingDirectory, ExtendedEventHandler eventHandler, List<String> targetPatterns, FilteringPolicy policy, boolean keepGoing) throws TargetParsingException, InterruptedException; - - /** - * Attempts to parse a single target pattern while consulting the package cache to check for the - * existence of packages and directories and the build targets in them. Implements the - * specification described in the class-level comment. Returns a {@link ResolvedTargets} object. - * - * <p>If an error is encountered, a {@link TargetParsingException} is thrown, unless {@code - * keepGoing} is set to true. In that case, the returned object will have its error bit set. - */ - ResolvedTargets<Target> parseTargetPattern( - ExtendedEventHandler eventHandler, String pattern, boolean keepGoing) - throws TargetParsingException, InterruptedException; - - /** - * Attempts to parse and load the given collection of patterns; the returned map contains the - * results for each pattern successfully parsed. - * - * <p>If an error is encountered, a {@link TargetParsingException} is thrown, unless {@code - * keepGoing} is set to true. In that case, the patterns that failed to load have the error flag - * set. - */ - Map<String, ResolvedTargets<Target>> preloadTargetPatterns( - ExtendedEventHandler eventHandler, Collection<String> patterns, boolean keepGoing) - throws TargetParsingException, InterruptedException; - - /** - * Update the parser's offset, given the workspace and working directory. - * - * @param relativeWorkingDirectory the working directory relative to the workspace - */ - @ThreadHostile - void updateOffset(PathFragment relativeWorkingDirectory); - - /** - * @return the offset of this parser from the root of the workspace. - * Non-absolute package-names will be resolved relative - * to this offset. - */ - @VisibleForTesting - String getOffset(); } diff --git a/src/main/java/com/google/devtools/build/lib/pkgcache/TargetPatternPreloader.java b/src/main/java/com/google/devtools/build/lib/pkgcache/TargetPatternPreloader.java new file mode 100644 index 0000000000..3b89098be2 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/pkgcache/TargetPatternPreloader.java @@ -0,0 +1,46 @@ +// Copyright 2018 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.pkgcache; + +import com.google.devtools.build.lib.cmdline.ResolvedTargets; +import com.google.devtools.build.lib.cmdline.TargetParsingException; +import com.google.devtools.build.lib.concurrent.ThreadSafety; +import com.google.devtools.build.lib.events.ExtendedEventHandler; +import com.google.devtools.build.lib.packages.Target; +import com.google.devtools.build.lib.vfs.PathFragment; +import java.util.Collection; +import java.util.Map; + +/** + * A preloader for target patterns. See {@link TargetPatternEvaluator} for more details. + */ +@ThreadSafety.ThreadSafe +public interface TargetPatternPreloader { + /** + * Attempts to parse and load the given collection of patterns; the returned map contains the + * results for each pattern successfully parsed. As a side effect, calling this method populates + * the Skyframe graph, so subsequent calls are faster. + * + * <p>If an error is encountered, a {@link TargetParsingException} is thrown, unless {@code + * keepGoing} is set to true. In that case, the patterns that failed to load have the error flag + * set. + */ + Map<String, ResolvedTargets<Target>> preloadTargetPatterns( + ExtendedEventHandler eventHandler, + PathFragment relativeWorkingDirectory, + Collection<String> patterns, + boolean keepGoing) + throws TargetParsingException, InterruptedException; +} diff --git a/src/main/java/com/google/devtools/build/lib/query2/BlazeQueryEnvironment.java b/src/main/java/com/google/devtools/build/lib/query2/BlazeQueryEnvironment.java index 68ab117f37..62cc674d31 100644 --- a/src/main/java/com/google/devtools/build/lib/query2/BlazeQueryEnvironment.java +++ b/src/main/java/com/google/devtools/build/lib/query2/BlazeQueryEnvironment.java @@ -35,7 +35,7 @@ import com.google.devtools.build.lib.packages.Package; import com.google.devtools.build.lib.packages.Rule; import com.google.devtools.build.lib.packages.Target; import com.google.devtools.build.lib.pkgcache.TargetEdgeObserver; -import com.google.devtools.build.lib.pkgcache.TargetPatternEvaluator; +import com.google.devtools.build.lib.pkgcache.TargetPatternPreloader; import com.google.devtools.build.lib.pkgcache.TargetProvider; import com.google.devtools.build.lib.pkgcache.TransitivePackageLoader; import com.google.devtools.build.lib.query2.engine.Callback; @@ -71,7 +71,8 @@ import java.util.Set; public class BlazeQueryEnvironment extends AbstractBlazeQueryEnvironment<Target> { private static final int MAX_DEPTH_FULL_SCAN_LIMIT = 20; private final Map<String, Set<Target>> resolvedTargetPatterns = new HashMap<>(); - private final TargetPatternEvaluator targetPatternEvaluator; + private final TargetPatternPreloader targetPatternPreloader; + private final PathFragment relativeWorkingDirectory; private final TransitivePackageLoader transitivePackageLoader; private final TargetProvider targetProvider; private final CachingPackageLocator cachingPackageLocator; @@ -98,7 +99,8 @@ public class BlazeQueryEnvironment extends AbstractBlazeQueryEnvironment<Target> TransitivePackageLoader transitivePackageLoader, TargetProvider targetProvider, CachingPackageLocator cachingPackageLocator, - TargetPatternEvaluator targetPatternEvaluator, + TargetPatternPreloader targetPatternPreloader, + PathFragment relativeWorkingDirectory, boolean keepGoing, boolean strictScope, int loadingPhaseThreads, @@ -107,7 +109,8 @@ public class BlazeQueryEnvironment extends AbstractBlazeQueryEnvironment<Target> Set<Setting> settings, Iterable<QueryFunction> extraFunctions) { super(keepGoing, strictScope, labelFilter, eventHandler, settings, extraFunctions); - this.targetPatternEvaluator = targetPatternEvaluator; + this.targetPatternPreloader = targetPatternPreloader; + this.relativeWorkingDirectory = relativeWorkingDirectory; this.transitivePackageLoader = transitivePackageLoader; this.targetProvider = targetProvider; this.cachingPackageLocator = cachingPackageLocator; @@ -441,7 +444,8 @@ public class BlazeQueryEnvironment extends AbstractBlazeQueryEnvironment<Target> // being called from within a SkyFunction. resolvedTargetPatterns.putAll( Maps.transformValues( - targetPatternEvaluator.preloadTargetPatterns(eventHandler, patterns, keepGoing), + targetPatternPreloader.preloadTargetPatterns( + eventHandler, relativeWorkingDirectory, patterns, keepGoing), ResolvedTargets::getTargets)); } } diff --git a/src/main/java/com/google/devtools/build/lib/query2/QueryEnvironmentFactory.java b/src/main/java/com/google/devtools/build/lib/query2/QueryEnvironmentFactory.java index 3c02da6fe8..e7cc8820d4 100644 --- a/src/main/java/com/google/devtools/build/lib/query2/QueryEnvironmentFactory.java +++ b/src/main/java/com/google/devtools/build/lib/query2/QueryEnvironmentFactory.java @@ -21,11 +21,12 @@ import com.google.devtools.build.lib.packages.CachingPackageLocator; import com.google.devtools.build.lib.packages.Rule; import com.google.devtools.build.lib.packages.Target; import com.google.devtools.build.lib.pkgcache.PathPackageLocator; -import com.google.devtools.build.lib.pkgcache.TargetPatternEvaluator; +import com.google.devtools.build.lib.pkgcache.TargetPatternPreloader; import com.google.devtools.build.lib.pkgcache.TargetProvider; import com.google.devtools.build.lib.pkgcache.TransitivePackageLoader; import com.google.devtools.build.lib.query2.engine.QueryEnvironment.QueryFunction; import com.google.devtools.build.lib.query2.engine.QueryEnvironment.Setting; +import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.skyframe.WalkableGraph.WalkableGraphFactory; import java.util.List; import java.util.Set; @@ -40,7 +41,8 @@ public class QueryEnvironmentFactory { WalkableGraphFactory graphFactory, TargetProvider targetProvider, CachingPackageLocator cachingPackageLocator, - TargetPatternEvaluator targetPatternEvaluator, + TargetPatternPreloader targetPatternPreloader, + PathFragment relativeWorkingDirectory, boolean keepGoing, boolean strictScope, boolean orderedResults, @@ -60,7 +62,7 @@ public class QueryEnvironmentFactory { eventHandler, settings, functions, - targetPatternEvaluator.getOffset(), + relativeWorkingDirectory.getPathString(), graphFactory, universeScope, packagePath, @@ -70,7 +72,8 @@ public class QueryEnvironmentFactory { transitivePackageLoader, targetProvider, cachingPackageLocator, - targetPatternEvaluator, + targetPatternPreloader, + relativeWorkingDirectory, keepGoing, strictScope, loadingPhaseThreads, diff --git a/src/main/java/com/google/devtools/build/lib/rules/genquery/GenQuery.java b/src/main/java/com/google/devtools/build/lib/rules/genquery/GenQuery.java index 00a51524df..4ae0c1a0b8 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/genquery/GenQuery.java +++ b/src/main/java/com/google/devtools/build/lib/rules/genquery/GenQuery.java @@ -52,9 +52,8 @@ import com.google.devtools.build.lib.packages.NoSuchTargetException; import com.google.devtools.build.lib.packages.Package; import com.google.devtools.build.lib.packages.Target; import com.google.devtools.build.lib.pkgcache.FilteringPolicies; -import com.google.devtools.build.lib.pkgcache.FilteringPolicy; import com.google.devtools.build.lib.pkgcache.PackageProvider; -import com.google.devtools.build.lib.pkgcache.TargetPatternEvaluator; +import com.google.devtools.build.lib.pkgcache.TargetPatternPreloader; import com.google.devtools.build.lib.query2.BlazeQueryEnvironment; import com.google.devtools.build.lib.query2.QueryEnvironmentFactory; import com.google.devtools.build.lib.query2.engine.DigraphQueryEvalResult; @@ -279,10 +278,10 @@ public class GenQuery implements RuleConfiguredTargetFactory { ImmutableMap<Label, Target> validTargetsMap = closureInfo.second; PreloadedMapPackageProvider packageProvider = new PreloadedMapPackageProvider(packageMap, validTargetsMap); - TargetPatternEvaluator evaluator = new SkyframeEnvTargetPatternEvaluator(env); + TargetPatternPreloader preloader = new SkyframeEnvTargetPatternEvaluator(env); Predicate<Label> labelFilter = Predicates.in(validTargetsMap.keySet()); - return doQuery(queryOptions, packageProvider, labelFilter, evaluator, query, ruleContext); + return doQuery(queryOptions, packageProvider, labelFilter, preloader, query, ruleContext); } @SuppressWarnings("unchecked") @@ -291,7 +290,7 @@ public class GenQuery implements RuleConfiguredTargetFactory { QueryOptions queryOptions, PreloadedMapPackageProvider packageProvider, Predicate<Label> labelFilter, - TargetPatternEvaluator evaluator, + TargetPatternPreloader preloader, String query, RuleContext ruleContext) throws InterruptedException { @@ -330,7 +329,8 @@ public class GenQuery implements RuleConfiguredTargetFactory { /* graphFactory= */ null, packageProvider, packageProvider, - evaluator, + preloader, + PathFragment.EMPTY_FRAGMENT, /*keepGoing=*/ false, ruleContext.attributes().get("strict", Type.BOOLEAN), /*orderedResults=*/ !QueryOutputUtils.shouldStreamResults( @@ -396,7 +396,7 @@ public class GenQuery implements RuleConfiguredTargetFactory { * Provide target pattern evaluation to the query operations using Skyframe dep lookup. For thread * safety, we must synchronize access to the SkyFunction.Environment. */ - private static final class SkyframeEnvTargetPatternEvaluator implements TargetPatternEvaluator { + private static final class SkyframeEnvTargetPatternEvaluator implements TargetPatternPreloader { private final SkyFunction.Environment env; public SkyframeEnvTargetPatternEvaluator(SkyFunction.Environment env) { @@ -415,9 +415,13 @@ public class GenQuery implements RuleConfiguredTargetFactory { @Override public Map<String, ResolvedTargets<Target>> preloadTargetPatterns( - ExtendedEventHandler eventHandler, Collection<String> patterns, boolean keepGoing) - throws TargetParsingException, InterruptedException { + ExtendedEventHandler eventHandler, + PathFragment relativeWorkingDirectory, + Collection<String> patterns, + boolean keepGoing) + throws TargetParsingException, InterruptedException { Preconditions.checkArgument(!keepGoing); + Preconditions.checkArgument(relativeWorkingDirectory.isEmpty()); boolean ok = true; Map<String, ResolvedTargets<Target>> preloadedPatterns = Maps.newHashMapWithExpectedSize(patterns.size()); @@ -498,33 +502,6 @@ public class GenQuery implements RuleConfiguredTargetFactory { String.format("recursive target patterns are not permitted: '%s''", pattern)); } } - - @Override - public ResolvedTargets<Target> parseTargetPatternList( - ExtendedEventHandler eventHandler, - List<String> targetPatterns, - FilteringPolicy policy, - boolean keepGoing) - throws TargetParsingException { - throw new UnsupportedOperationException(); - } - - @Override - public ResolvedTargets<Target> parseTargetPattern( - ExtendedEventHandler eventHandler, String pattern, boolean keepGoing) - throws TargetParsingException { - throw new UnsupportedOperationException(); - } - - @Override - public void updateOffset(PathFragment relativeWorkingDirectory) { - throw new UnsupportedOperationException(); - } - - @Override - public String getOffset() { - throw new UnsupportedOperationException(); - } } /** diff --git a/src/main/java/com/google/devtools/build/lib/runtime/CommandEnvironment.java b/src/main/java/com/google/devtools/build/lib/runtime/CommandEnvironment.java index 2ad145d1a3..32d31a2979 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/CommandEnvironment.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/CommandEnvironment.java @@ -31,7 +31,7 @@ import com.google.devtools.build.lib.packages.SkylarkSemanticsOptions; import com.google.devtools.build.lib.packages.Target; import com.google.devtools.build.lib.pkgcache.PackageCacheOptions; import com.google.devtools.build.lib.pkgcache.PackageManager; -import com.google.devtools.build.lib.pkgcache.TargetPatternEvaluator; +import com.google.devtools.build.lib.pkgcache.TargetPatternPreloader; import com.google.devtools.build.lib.profiler.Profiler; import com.google.devtools.build.lib.profiler.ProfilerTask; import com.google.devtools.build.lib.profiler.SilentCloseable; @@ -316,12 +316,10 @@ public final class CommandEnvironment { } /** - * Creates and returns a new target pattern parser. + * Creates and returns a new target pattern preloader. */ - public TargetPatternEvaluator newTargetPatternEvaluator() { - TargetPatternEvaluator result = getPackageManager().newTargetPatternEvaluator(); - result.updateOffset(relativeWorkingDirectory); - return result; + public TargetPatternPreloader newTargetPatternPreloader() { + return getPackageManager().newTargetPatternPreloader(); } public PackageRootResolver getPackageRootResolver() { diff --git a/src/main/java/com/google/devtools/build/lib/runtime/commands/QueryCommand.java b/src/main/java/com/google/devtools/build/lib/runtime/commands/QueryCommand.java index b9e071ce6a..8b06f1cbaf 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/commands/QueryCommand.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/commands/QueryCommand.java @@ -300,7 +300,8 @@ public final class QueryCommand implements BlazeCommand { env.getSkyframeExecutor(), targetProviderForQueryEnvironment, env.getPackageManager(), - env.newTargetPatternEvaluator(), + env.newTargetPatternPreloader(), + env.getRelativeWorkingDirectory(), keepGoing, /*strictScope=*/ true, orderedResults, diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java index 633b8b873b..69a9b436ed 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java @@ -117,6 +117,7 @@ import com.google.devtools.build.lib.pkgcache.PackageManager; import com.google.devtools.build.lib.pkgcache.PackageManager.PackageManagerStatistics; import com.google.devtools.build.lib.pkgcache.PathPackageLocator; import com.google.devtools.build.lib.pkgcache.TargetParsingPhaseTimeEvent; +import com.google.devtools.build.lib.pkgcache.TargetPatternEvaluator; import com.google.devtools.build.lib.pkgcache.TestFilter; import com.google.devtools.build.lib.pkgcache.TransitivePackageLoader; import com.google.devtools.build.lib.profiler.AutoProfiler; @@ -2026,6 +2027,11 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory { return packageManager; } + @VisibleForTesting + public TargetPatternEvaluator newTargetPatternEvaluator() { + return new SkyframeTargetPatternEvaluator(this); + } + public ActionKeyContext getActionKeyContext() { return actionKeyContext; } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframePackageManager.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframePackageManager.java index b0e36037ba..d89593ca07 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframePackageManager.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframePackageManager.java @@ -24,7 +24,7 @@ import com.google.devtools.build.lib.packages.Package; import com.google.devtools.build.lib.packages.Target; import com.google.devtools.build.lib.pkgcache.PackageManager; import com.google.devtools.build.lib.pkgcache.PathPackageLocator; -import com.google.devtools.build.lib.pkgcache.TargetPatternEvaluator; +import com.google.devtools.build.lib.pkgcache.TargetPatternPreloader; import com.google.devtools.build.lib.pkgcache.TransitivePackageLoader; import com.google.devtools.build.lib.skyframe.SkyframeExecutor.SkyframePackageLoader; import com.google.devtools.build.lib.vfs.Path; @@ -125,7 +125,7 @@ class SkyframePackageManager implements PackageManager, CachingPackageLocator { } @Override - public TargetPatternEvaluator newTargetPatternEvaluator() { + public TargetPatternPreloader newTargetPatternPreloader() { return new SkyframeTargetPatternEvaluator(skyframeExecutor); } } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeTargetPatternEvaluator.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeTargetPatternEvaluator.java index 4c8cf80a27..01d68f4cc7 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeTargetPatternEvaluator.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeTargetPatternEvaluator.java @@ -27,6 +27,7 @@ import com.google.devtools.build.lib.pkgcache.FilteringPolicies; import com.google.devtools.build.lib.pkgcache.FilteringPolicy; import com.google.devtools.build.lib.pkgcache.ParsingFailedEvent; import com.google.devtools.build.lib.pkgcache.TargetPatternEvaluator; +import com.google.devtools.build.lib.pkgcache.TargetPatternPreloader; import com.google.devtools.build.lib.skyframe.TargetPatternValue.TargetPatternKey; import com.google.devtools.build.lib.skyframe.TargetPatternValue.TargetPatternSkyKeyOrException; import com.google.devtools.build.lib.vfs.PathFragment; @@ -40,47 +41,38 @@ import java.util.Map; /** * Skyframe-based target pattern parsing. */ -final class SkyframeTargetPatternEvaluator implements TargetPatternEvaluator { +final class SkyframeTargetPatternEvaluator + implements TargetPatternEvaluator, TargetPatternPreloader { private final SkyframeExecutor skyframeExecutor; - private String offset = ""; SkyframeTargetPatternEvaluator(SkyframeExecutor skyframeExecutor) { this.skyframeExecutor = skyframeExecutor; } + // Only used by AnalyzeCommand at this point. All build commands use SkyframeLoadingPhaseRunner. @Override public ResolvedTargets<Target> parseTargetPatternList( + PathFragment relativeWorkingDirectory, ExtendedEventHandler eventHandler, List<String> targetPatterns, FilteringPolicy policy, boolean keepGoing) throws TargetParsingException, InterruptedException { return parseTargetPatternList( - offset, eventHandler, ImmutableList.copyOf(targetPatterns), policy, keepGoing); - } - - @Override - public ResolvedTargets<Target> parseTargetPattern( - ExtendedEventHandler eventHandler, String pattern, boolean keepGoing) - throws TargetParsingException, InterruptedException { - return parseTargetPatternList(eventHandler, ImmutableList.of(pattern), - DEFAULT_FILTERING_POLICY, keepGoing); - } - - @Override - public void updateOffset(PathFragment relativeWorkingDirectory) { - offset = relativeWorkingDirectory.getPathString(); - } - - @Override - public String getOffset() { - return offset; + relativeWorkingDirectory.getPathString(), + eventHandler, + ImmutableList.copyOf(targetPatterns), + policy, + keepGoing); } @Override public Map<String, ResolvedTargets<Target>> preloadTargetPatterns( - ExtendedEventHandler eventHandler, Collection<String> patterns, boolean keepGoing) - throws TargetParsingException, InterruptedException { + ExtendedEventHandler eventHandler, + PathFragment relativeWorkingDirectory, + Collection<String> patterns, + boolean keepGoing) + throws TargetParsingException, InterruptedException { // TODO(bazel-team): This is used only in "blaze query". There are plans to dramatically change // how query works on Skyframe, in which case this method is likely to go away. ImmutableList.Builder<TargetPatternsAndKeysAndResultBuilder> @@ -91,7 +83,11 @@ final class SkyframeTargetPatternEvaluator implements TargetPatternEvaluator { targetPatternsAndKeysAndResultListBuilder.add(new TargetPatternsAndKeysAndResultBuilder( singletonPatternList, getTargetPatternKeys( - offset, eventHandler, singletonPatternList, policy, keepGoing), + relativeWorkingDirectory.getPathString(), + eventHandler, + singletonPatternList, + policy, + keepGoing), createTargetPatternEvaluatorUtil(policy, eventHandler, keepGoing))); } diff --git a/src/test/java/com/google/devtools/build/lib/pkgcache/AbstractTargetPatternEvaluatorTest.java b/src/test/java/com/google/devtools/build/lib/pkgcache/AbstractTargetPatternEvaluatorTest.java index 10f8779909..85424470f7 100644 --- a/src/test/java/com/google/devtools/build/lib/pkgcache/AbstractTargetPatternEvaluatorTest.java +++ b/src/test/java/com/google/devtools/build/lib/pkgcache/AbstractTargetPatternEvaluatorTest.java @@ -21,10 +21,12 @@ import com.google.devtools.build.lib.cmdline.ResolvedTargets; import com.google.devtools.build.lib.cmdline.TargetParsingException; import com.google.devtools.build.lib.events.DelegatingEventHandler; import com.google.devtools.build.lib.events.ExtendedEventHandler; +import com.google.devtools.build.lib.events.ExtendedEventHandler.Postable; import com.google.devtools.build.lib.packages.ConstantRuleVisibility; import com.google.devtools.build.lib.packages.Target; import com.google.devtools.build.lib.packages.util.PackageLoadingTestCase; import com.google.devtools.build.lib.util.Pair; +import com.google.devtools.build.lib.vfs.PathFragment; import java.util.ArrayList; import java.util.Arrays; import java.util.HashSet; @@ -49,17 +51,40 @@ public abstract class AbstractTargetPatternEvaluatorTest extends PackageLoadingT boolean keepGoing) throws TargetParsingException, InterruptedException { return parseTargetPatternList( - parser, eventHandler, targetPatterns, FilteringPolicies.NO_FILTER, keepGoing); + PathFragment.EMPTY_FRAGMENT, + parser, + eventHandler, + targetPatterns, + FilteringPolicies.NO_FILTER, + keepGoing); } protected static ResolvedTargets<Target> parseTargetPatternList( + PathFragment relativeWorkingDirectory, + TargetPatternEvaluator parser, + ExtendedEventHandler eventHandler, + List<String> targetPatterns, + boolean keepGoing) + throws TargetParsingException, InterruptedException { + return parseTargetPatternList( + relativeWorkingDirectory, + parser, + eventHandler, + targetPatterns, + FilteringPolicies.NO_FILTER, + keepGoing); + } + + protected static ResolvedTargets<Target> parseTargetPatternList( + PathFragment relativeWorkingDirectory, TargetPatternEvaluator parser, ExtendedEventHandler eventHandler, List<String> targetPatterns, FilteringPolicy policy, boolean keepGoing) throws TargetParsingException, InterruptedException { - return parser.parseTargetPatternList(eventHandler, targetPatterns, policy, keepGoing); + return parser.parseTargetPatternList( + relativeWorkingDirectory, eventHandler, targetPatterns, policy, keepGoing); } /** @@ -77,7 +102,7 @@ public abstract class AbstractTargetPatternEvaluatorTest extends PackageLoadingT @Before public final void initializeParser() throws Exception { setUpSkyframe(ConstantRuleVisibility.PRIVATE, loadingMock.getDefaultsPackageContent()); - parser = skyframeExecutor.getPackageManager().newTargetPatternEvaluator(); + parser = skyframeExecutor.newTargetPatternEvaluator(); parsingListener = new RecordingParsingListener(reporter); } diff --git a/src/test/java/com/google/devtools/build/lib/pkgcache/CompileOneDependencyTransformerTest.java b/src/test/java/com/google/devtools/build/lib/pkgcache/CompileOneDependencyTransformerTest.java index d421e9229c..3c3a7739a3 100644 --- a/src/test/java/com/google/devtools/build/lib/pkgcache/CompileOneDependencyTransformerTest.java +++ b/src/test/java/com/google/devtools/build/lib/pkgcache/CompileOneDependencyTransformerTest.java @@ -24,6 +24,7 @@ import com.google.devtools.build.lib.events.Reporter; import com.google.devtools.build.lib.packages.Target; import com.google.devtools.build.lib.packages.util.PackageLoadingTestCase; import com.google.devtools.build.lib.vfs.Path; +import com.google.devtools.build.lib.vfs.PathFragment; import java.io.IOException; import java.util.Arrays; import java.util.HashSet; @@ -49,7 +50,7 @@ public class CompileOneDependencyTransformerTest extends PackageLoadingTestCase @Before public final void createTransformer() throws Exception { - parser = getPackageManager().newTargetPatternEvaluator(); + parser = skyframeExecutor.newTargetPatternEvaluator(); transformer = new CompileOneDependencyTransformer(getPackageManager()); } @@ -75,7 +76,8 @@ public class CompileOneDependencyTransformerTest extends PackageLoadingTestCase TargetPatternEvaluator parser, Reporter reporter, List<String> targetPatterns, FilteringPolicy policy, boolean keepGoing) throws Exception { - return parser.parseTargetPatternList(reporter, targetPatterns, policy, keepGoing); + return parser.parseTargetPatternList( + PathFragment.EMPTY_FRAGMENT, reporter, targetPatterns, policy, keepGoing); } private ResolvedTargets<Target> parseCompileOneDep(String... patterns) throws Exception { @@ -91,12 +93,14 @@ public class CompileOneDependencyTransformerTest extends PackageLoadingTestCase private Set<Label> parseListCompileOneDepRelative(String... patterns) throws TargetParsingException, IOException, InterruptedException { Path foo = scratch.dir("foo"); - TargetPatternEvaluator fooOffsetParser = getPackageManager().newTargetPatternEvaluator(); - fooOffsetParser.updateOffset(foo.relativeTo(rootDirectory)); + TargetPatternEvaluator fooOffsetParser = skyframeExecutor.newTargetPatternEvaluator(); ResolvedTargets<Target> result; try { result = fooOffsetParser.parseTargetPatternList( - reporter, Arrays.asList(patterns), FilteringPolicies.NO_FILTER, false); + foo.relativeTo(rootDirectory), + reporter, + Arrays.asList(patterns), + FilteringPolicies.NO_FILTER, false); } catch (InterruptedException e) { throw new RuntimeException(e); } diff --git a/src/test/java/com/google/devtools/build/lib/pkgcache/TargetPatternEvaluatorTest.java b/src/test/java/com/google/devtools/build/lib/pkgcache/TargetPatternEvaluatorTest.java index a7f81a23e0..94029ac5c7 100644 --- a/src/test/java/com/google/devtools/build/lib/pkgcache/TargetPatternEvaluatorTest.java +++ b/src/test/java/com/google/devtools/build/lib/pkgcache/TargetPatternEvaluatorTest.java @@ -152,11 +152,6 @@ public class TargetPatternEvaluatorTest extends AbstractTargetPatternEvaluatorTe skyframeExecutor.setDeletedPackages(deletedPackages); } - private TargetPatternEvaluator shiftOffset() { - parser.updateOffset(fooOffset); - return parser; - } - private Set<Label> parseList(String... patterns) throws TargetParsingException, InterruptedException { return targetsToLabels( @@ -176,13 +171,25 @@ public class TargetPatternEvaluatorTest extends AbstractTargetPatternEvaluatorTe FilteringPolicy policy, String... patterns) throws TargetParsingException, InterruptedException { return targetsToLabels(getFailFast( - parseTargetPatternList(parser, parsingListener, Arrays.asList(patterns), policy, false))); + parseTargetPatternList( + PathFragment.EMPTY_FRAGMENT, + parser, + parsingListener, + Arrays.asList(patterns), + policy, + false))); + } + + private Set<Label> parseListRelative(PathFragment offset, String... patterns) + throws TargetParsingException, InterruptedException { + return targetsToLabels(getFailFast(parseTargetPatternList( + offset, parser, parsingListener, Arrays.asList(patterns), false))); } private Set<Label> parseListRelative(String... patterns) throws TargetParsingException, InterruptedException { return targetsToLabels(getFailFast(parseTargetPatternList( - shiftOffset(), parsingListener, Arrays.asList(patterns), false))); + fooOffset, parser, parsingListener, Arrays.asList(patterns), false))); } private static Set<Target> getFailFast(ResolvedTargets<Target> result) { @@ -190,10 +197,14 @@ public class TargetPatternEvaluatorTest extends AbstractTargetPatternEvaluatorTe return result.getTargets(); } - private void expectError(TargetPatternEvaluator parser, String expectedError, - String target) throws InterruptedException { + private void expectError( + PathFragment offset, + TargetPatternEvaluator parser, + String expectedError, + String target) + throws InterruptedException { try { - parser.parseTargetPattern(parsingListener, target, false); + parseTargetPatternList(offset, parser, parsingListener, ImmutableList.of(target), false); fail("target='" + target + "', expected error: " + expectedError); } catch (TargetParsingException e) { assertThat(e).hasMessageThat().contains(expectedError); @@ -201,23 +212,27 @@ public class TargetPatternEvaluatorTest extends AbstractTargetPatternEvaluatorTe } private void expectError(String expectedError, String target) throws InterruptedException { - expectError(parser, expectedError, target); + expectError(PathFragment.EMPTY_FRAGMENT, parser, expectedError, target); } private void expectErrorRelative(String expectedError, String target) throws InterruptedException { - expectError(shiftOffset(), expectedError, target); + expectError(fooOffset, parser, expectedError, target); } private Label parseIndividualTarget(String targetLabel) throws Exception { return Iterables.getOnlyElement( - getFailFast(parser.parseTargetPattern(parsingListener, targetLabel, false))).getLabel(); + getFailFast( + parseTargetPatternList(parser, parsingListener, ImmutableList.of(targetLabel), false))) + .getLabel(); } private Label parseIndividualTargetRelative(String targetLabel) throws Exception { return Iterables.getOnlyElement( getFailFast( - shiftOffset().parseTargetPattern(parsingListener, targetLabel, false))).getLabel(); + parseTargetPatternList( + fooOffset, parser, parsingListener, ImmutableList.of(targetLabel), false))) + .getLabel(); } @Test @@ -336,14 +351,10 @@ public class TargetPatternEvaluatorTest extends AbstractTargetPatternEvaluatorTe scratch.file("nest/nest/BUILD", "cc_library(name = 'nested2', srcs = [ ])"); - updateOffset(PathFragment.create("nest")); - assertThat(parseList(":all")).containsExactlyElementsIn(labels("//nest:nested1")); - updateOffset(PathFragment.create("nest/nest")); - assertThat(parseList(":all")).containsExactlyElementsIn(labels("//nest/nest:nested2")); - } - - protected void updateOffset(PathFragment rel) { - parser.updateOffset(rel); + assertThat(parseListRelative(PathFragment.create("nest"), ":all")) + .containsExactlyElementsIn(labels("//nest:nested1")); + assertThat(parseListRelative(PathFragment.create("nest/nest"), ":all")) + .containsExactlyElementsIn(labels("//nest/nest:nested2")); } private void runFindTargetsInPackage(String suffix) throws Exception { @@ -517,9 +528,14 @@ public class TargetPatternEvaluatorTest extends AbstractTargetPatternEvaluatorTe scratch.file("x/z/BUILD", "cc_library(name='z')"); setDeletedPackages(Sets.newHashSet(PackageIdentifier.createInMainRepo("x/y"))); - parser.updateOffset(PathFragment.create("x")); assertThat( - targetsToLabels(getFailFast(parser.parseTargetPattern(parsingListener, "...", false)))) + targetsToLabels(getFailFast( + parseTargetPatternList( + PathFragment.create("x"), + parser, + parsingListener, + ImmutableList.of("..."), + false)))) .isEqualTo(Sets.newHashSet(Label.parseAbsolute("//x/z"))); } @@ -599,27 +615,6 @@ public class TargetPatternEvaluatorTest extends AbstractTargetPatternEvaluatorTe assertThat(parseListRelative("//foo/...:*")).containsExactlyElementsIn(targetsBeneathFoo); } - @Test - public void testFactoryMethod() throws Exception { - Path workspace = scratch.dir("/client/workspace"); - Path underWorkspace = scratch.dir("/client/workspace/foo"); - Path notUnderWorkspace = scratch.dir("/client/otherclient"); - - updateOffset(workspace, underWorkspace); - updateOffset(workspace, workspace); - - // The client must be equal to or underneath the workspace. - try { - updateOffset(workspace, notUnderWorkspace); - fail("Should have failed because client was not underneath the workspace"); - } catch (IllegalArgumentException expected) { - } - } - - private void updateOffset(Path workspace, Path workingDir) { - parser.updateOffset(workingDir.relativeTo(workspace)); - } - private void setupSubDirectoryCircularSymlink() throws Exception { Path parent = scratch.file("parent/BUILD", "sh_library(name = 'parent')").getParentDirectory(); Path child = parent.getRelative("child"); @@ -725,7 +720,7 @@ public class TargetPatternEvaluatorTest extends AbstractTargetPatternEvaluatorTe // Ensure that validateTargetPattern's checking is strictly weaker than // that of parseTargetPattern. try { - parser.parseTargetPattern(parsingListener, target, false); + parseTargetPatternList(parser, parsingListener, ImmutableList.of(target), false); fail("parseTargetPattern(" + target + ") inconsistent with parseTargetPattern!"); } catch (TargetParsingException expected) { /* ignore */ @@ -745,9 +740,7 @@ public class TargetPatternEvaluatorTest extends AbstractTargetPatternEvaluatorTe @Test public void testSetOffset() throws Exception { assertThat(parseIndividualTarget("foo:foo1").toString()).isEqualTo("//foo:foo1"); - - parser.updateOffset(PathFragment.create("foo")); - assertThat(parseIndividualTarget(":foo1").toString()).isEqualTo("//foo:foo1"); + assertThat(parseIndividualTargetRelative(":foo1").toString()).isEqualTo("//foo:foo1"); } @Test |