diff options
Diffstat (limited to 'src')
10 files changed, 129 insertions, 56 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/cmdline/TargetPattern.java b/src/main/java/com/google/devtools/build/lib/cmdline/TargetPattern.java index 4a693e8148..739afe63f6 100644 --- a/src/main/java/com/google/devtools/build/lib/cmdline/TargetPattern.java +++ b/src/main/java/com/google/devtools/build/lib/cmdline/TargetPattern.java @@ -446,7 +446,7 @@ public abstract class TargetPattern implements Serializable { ImmutableSet<String> excludedSubdirectories) throws TargetParsingException, InterruptedException { return resolver.findTargetsBeneathDirectory( - PackageIdentifier.DEFAULT_REPOSITORY_NAME, getOriginalPattern(), + directory.getRepository(), getOriginalPattern(), directory.getPackageFragment().getPathString(), rulesOnly, excludedSubdirectories); } @@ -610,7 +610,8 @@ public abstract class TargetPattern implements Serializable { String realPackagePart = removeSuffix(packagePart, "/..."); PackageIdentifier packageIdentifier; try { - packageIdentifier = PackageIdentifier.parse("//" + realPackagePart); + packageIdentifier = PackageIdentifier.parse( + repository.getName() + "//" + realPackagePart); } catch (LabelSyntaxException e) { throw new TargetParsingException( "Invalid package name '" + realPackagePart + "': " + e.getMessage()); diff --git a/src/main/java/com/google/devtools/build/lib/pkgcache/RecursivePackageProvider.java b/src/main/java/com/google/devtools/build/lib/pkgcache/RecursivePackageProvider.java index c80f51a1d8..24866b665f 100644 --- a/src/main/java/com/google/devtools/build/lib/pkgcache/RecursivePackageProvider.java +++ b/src/main/java/com/google/devtools/build/lib/pkgcache/RecursivePackageProvider.java @@ -29,6 +29,6 @@ public interface RecursivePackageProvider extends PackageProvider { * @param excludedSubdirectories a set of {@link PathFragment}s, all of which are beneath * {@code directory}, specifying transitive subdirectories to exclude */ - Iterable<PathFragment> getPackagesUnderDirectory(RepositoryName repository, RootedPath directory, - ImmutableSet<PathFragment> excludedSubdirectories); + Iterable<PathFragment> getPackagesUnderDirectory(RepositoryName repository, + PathFragment directory, ImmutableSet<PathFragment> excludedSubdirectories); } diff --git a/src/main/java/com/google/devtools/build/lib/query2/SkyQueryEnvironment.java b/src/main/java/com/google/devtools/build/lib/query2/SkyQueryEnvironment.java index f6c2c2d16f..302410144e 100644 --- a/src/main/java/com/google/devtools/build/lib/query2/SkyQueryEnvironment.java +++ b/src/main/java/com/google/devtools/build/lib/query2/SkyQueryEnvironment.java @@ -418,7 +418,7 @@ public class SkyQueryEnvironment extends AbstractBlazeQueryEnvironment<Target> { QueryExpression caller, Collection<String> patterns) throws QueryException, TargetParsingException { GraphBackedRecursivePackageProvider provider = - new GraphBackedRecursivePackageProvider(graph, universeTargetPatternKeys); + new GraphBackedRecursivePackageProvider(graph, universeTargetPatternKeys, pkgPath); Map<String, Set<Target>> result = Maps.newHashMapWithExpectedSize(patterns.size()); Map<String, SkyKey> keys = new HashMap<>(patterns.size()); @@ -457,7 +457,7 @@ public class SkyQueryEnvironment extends AbstractBlazeQueryEnvironment<Target> { RecursivePackageProviderBackedTargetPatternResolver resolver = new RecursivePackageProviderBackedTargetPatternResolver(provider, eventHandler, - targetPatternKey.getPolicy(), pkgPath); + targetPatternKey.getPolicy()); TargetPattern parsedPattern = targetPatternKey.getParsedPattern(); try { patternsWithTargetsToFilter.put(pattern, parsedPattern.eval(resolver).getTargets()); diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/EnvironmentBackedRecursivePackageProvider.java b/src/main/java/com/google/devtools/build/lib/skyframe/EnvironmentBackedRecursivePackageProvider.java index e81858e93e..34ab492120 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/EnvironmentBackedRecursivePackageProvider.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/EnvironmentBackedRecursivePackageProvider.java @@ -19,6 +19,7 @@ import com.google.common.collect.Iterables; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.cmdline.PackageIdentifier; import com.google.devtools.build.lib.cmdline.PackageIdentifier.RepositoryName; +import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.events.EventHandler; import com.google.devtools.build.lib.packages.BuildFileContainsErrorsException; @@ -26,12 +27,17 @@ import com.google.devtools.build.lib.packages.NoSuchPackageException; 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.PathPackageLocator; import com.google.devtools.build.lib.pkgcache.RecursivePackageProvider; +import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.lib.vfs.RootedPath; import com.google.devtools.build.skyframe.SkyFunction.Environment; import com.google.devtools.build.skyframe.SkyKey; +import java.util.ArrayList; +import java.util.List; + /** * A {@link RecursivePackageProvider} backed by an {@link Environment}. Its methods * may throw {@link MissingDepException} if the package values this depends on haven't been @@ -91,26 +97,48 @@ public final class EnvironmentBackedRecursivePackageProvider implements Recursiv @Override public Iterable<PathFragment> getPackagesUnderDirectory( - RepositoryName repository, RootedPath directory, + RepositoryName repository, PathFragment directory, ImmutableSet<PathFragment> excludedSubdirectories) throws MissingDepException { - PathFragment rootedPathFragment = directory.getRelativePath(); - PathFragment.checkAllPathsAreUnder(excludedSubdirectories, rootedPathFragment); - RecursivePkgValue lookup = (RecursivePkgValue) env.getValue( - RecursivePkgValue.key(repository, directory, excludedSubdirectories)); - if (lookup == null) { - // Typically a null value from Environment.getValue(k) means that either the key k is missing - // a dependency or an exception was thrown during evaluation of k. Here, if this getValue - // call returns null in a keep_going build, it can only mean a missing dependency, because - // RecursivePkgFunction#compute never throws. - // In a nokeep_going build, a lower-level exception that RecursivePkgFunction ignored may - // bubble up to here, but we ignore it and depend on the top-level caller to be flexible in - // the exception types it can accept. + PathPackageLocator packageLocator = PrecomputedValue.PATH_PACKAGE_LOCATOR.get(env); + if (packageLocator == null) { throw new MissingDepException(); } + + List<Path> roots = new ArrayList<>(); + if (repository.isDefault()) { + roots.addAll(packageLocator.getPathEntries()); + } else { + RepositoryValue repositoryValue = + (RepositoryValue) env.getValue(RepositoryValue.key(repository)); + if (repositoryValue == null) { + throw new MissingDepException(); + } + + roots.add(repositoryValue.getPath()); + } + + NestedSetBuilder<String> packageNames = NestedSetBuilder.stableOrder(); + for (Path root : roots) { + PathFragment.checkAllPathsAreUnder(excludedSubdirectories, directory); + RecursivePkgValue lookup = (RecursivePkgValue) env.getValue(RecursivePkgValue.key( + repository, RootedPath.toRootedPath(root, directory), excludedSubdirectories)); + if (lookup == null) { + // Typically a null value from Environment.getValue(k) means that either the key k is + // missing a dependency or an exception was thrown during evaluation of k. Here, if this + // getValue call returns null in a keep_going build, it can only mean a missing dependency + // because RecursivePkgFunction#compute never throws. + // In a nokeep_going build, a lower-level exception that RecursivePkgFunction ignored may + // bubble up to here, but we ignore it and depend on the top-level caller to be flexible in + // the exception types it can accept. + throw new MissingDepException(); + } + + packageNames.addTransitive(lookup.getPackages()); + } // TODO(bazel-team): Make RecursivePkgValue return NestedSet<PathFragment> so this transform is // unnecessary. - return Iterables.transform(lookup.getPackages(), PathFragment.TO_PATH_FRAGMENT); + return Iterables.transform(packageNames.build(), PathFragment.TO_PATH_FRAGMENT); } @Override diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/GraphBackedRecursivePackageProvider.java b/src/main/java/com/google/devtools/build/lib/skyframe/GraphBackedRecursivePackageProvider.java index fffcd38c36..8b11e2ee1c 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/GraphBackedRecursivePackageProvider.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/GraphBackedRecursivePackageProvider.java @@ -31,13 +31,18 @@ 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.PathPackageLocator; import com.google.devtools.build.lib.pkgcache.RecursivePackageProvider; import com.google.devtools.build.lib.skyframe.TargetPatternValue.TargetPatternKey; +import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.lib.vfs.RootedPath; import com.google.devtools.build.skyframe.SkyKey; import com.google.devtools.build.skyframe.WalkableGraph; +import java.util.ArrayList; +import java.util.List; + /** * A {@link RecursivePackageProvider} backed by a {@link WalkableGraph}, used by * {@code SkyQueryEnvironment} to look up the packages and targets matching the universe that's @@ -46,10 +51,13 @@ import com.google.devtools.build.skyframe.WalkableGraph; public final class GraphBackedRecursivePackageProvider implements RecursivePackageProvider { private final WalkableGraph graph; + private final PathPackageLocator pkgPath; private final ImmutableList<TargetPatternKey> universeTargetPatternKeys; public GraphBackedRecursivePackageProvider(WalkableGraph graph, - ImmutableList<TargetPatternKey> universeTargetPatternKeys) { + ImmutableList<TargetPatternKey> universeTargetPatternKeys, + PathPackageLocator pkgPath) { + this.pkgPath = pkgPath; this.graph = Preconditions.checkNotNull(graph); this.universeTargetPatternKeys = Preconditions.checkNotNull(universeTargetPatternKeys); } @@ -100,9 +108,9 @@ public final class GraphBackedRecursivePackageProvider implements RecursivePacka @Override public Iterable<PathFragment> getPackagesUnderDirectory( - RepositoryName repository, RootedPath directory, + RepositoryName repository, PathFragment directory, ImmutableSet<PathFragment> excludedSubdirectories) { - PathFragment.checkAllPathsAreUnder(excludedSubdirectories, directory.getRelativePath()); + PathFragment.checkAllPathsAreUnder(excludedSubdirectories, directory); // Find the filtering policy of a TargetsBelowDirectory pattern, if any, in the universe that // contains this directory. @@ -111,7 +119,7 @@ public final class GraphBackedRecursivePackageProvider implements RecursivePacka TargetPattern pattern = patternKey.getParsedPattern(); boolean isTBD = pattern.getType().equals(Type.TARGETS_BELOW_DIRECTORY); PackageIdentifier packageIdentifier = PackageIdentifier.create( - repository, directory.getRelativePath()); + repository, directory); if (isTBD && pattern.containsBelowDirectory(packageIdentifier)) { filteringPolicy = pattern.getRulesOnly() ? FilteringPolicies.RULES_ONLY : FilteringPolicies.NO_FILTER; @@ -119,12 +127,29 @@ public final class GraphBackedRecursivePackageProvider implements RecursivePacka } } + List<Path> roots = new ArrayList<>(); + if (repository.isDefault()) { + roots.addAll(pkgPath.getPathEntries()); + } else { + RepositoryValue repositoryValue = + (RepositoryValue) graph.getValue(RepositoryValue.key(repository)); + if (repositoryValue == null) { + // If this key doesn't exist, the repository is outside the universe, so we return + // "nothing". + return ImmutableList.of(); + } + roots.add(repositoryValue.getPath()); + } + // If we found a TargetsBelowDirectory pattern in the universe that contains this directory, // then we can look for packages in and under it in the graph. If we didn't find one, then the // directory wasn't in the universe, so return an empty list. ImmutableList.Builder<PathFragment> builder = ImmutableList.builder(); if (filteringPolicy != null) { - collectPackagesUnder(repository, directory, excludedSubdirectories, builder, filteringPolicy); + for (Path root : roots) { + collectPackagesUnder(repository, RootedPath.toRootedPath(root, directory), + excludedSubdirectories, builder, filteringPolicy); + } } return builder.build(); } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternFunction.java index cebe4298b8..e776281b75 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternFunction.java @@ -42,6 +42,8 @@ import com.google.devtools.build.skyframe.SkyFunctionException; import com.google.devtools.build.skyframe.SkyKey; import com.google.devtools.build.skyframe.SkyValue; +import java.util.ArrayList; +import java.util.List; import java.util.concurrent.atomic.AtomicReference; import javax.annotation.Nullable; @@ -214,9 +216,20 @@ public class PrepareDepsOfPatternFunction implements SkyFunction { ImmutableSet<PathFragment> excludedPathFragments = TargetPatternResolverUtil.getPathFragments(excludedSubdirectories); PathFragment pathFragment = TargetPatternResolverUtil.getPathFragment(directory); - // TODO(bazel-team): This is where we need to depend on the RepositoryValue of a remote - // repository in order figure out its root and thus support recursive package search in them. - for (Path root : pkgPath.getPathEntries()) { + List<Path> roots = new ArrayList<>(); + if (repository.isDefault()) { + roots.addAll(pkgPath.getPathEntries()); + } else { + RepositoryValue repositoryValue = + (RepositoryValue) env.getValue(RepositoryValue.key(repository)); + if (repositoryValue == null) { + throw new MissingDepException(); + } + + roots.add(repositoryValue.getPath()); + } + + for (Path root : roots) { RootedPath rootedPath = RootedPath.toRootedPath(root, pathFragment); SkyValue token = env.getValue(PrepareDepsOfTargetsUnderDirectoryValue.key( repository, rootedPath, excludedPathFragments, policy)); diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/RecursivePackageProviderBackedTargetPatternResolver.java b/src/main/java/com/google/devtools/build/lib/skyframe/RecursivePackageProviderBackedTargetPatternResolver.java index 91a9bd9fd5..3f5f76e4f2 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/RecursivePackageProviderBackedTargetPatternResolver.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/RecursivePackageProviderBackedTargetPatternResolver.java @@ -28,12 +28,9 @@ 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.PathPackageLocator; import com.google.devtools.build.lib.pkgcache.RecursivePackageProvider; import com.google.devtools.build.lib.pkgcache.TargetPatternResolverUtil; -import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; -import com.google.devtools.build.lib.vfs.RootedPath; /** * A {@link TargetPatternResolver} backed by a {@link RecursivePackageProvider}. @@ -44,17 +41,14 @@ public class RecursivePackageProviderBackedTargetPatternResolver private final RecursivePackageProvider recursivePackageProvider; private final EventHandler eventHandler; private final FilteringPolicy policy; - private final PathPackageLocator pkgPath; public RecursivePackageProviderBackedTargetPatternResolver( RecursivePackageProvider recursivePackageProvider, EventHandler eventHandler, - FilteringPolicy policy, - PathPackageLocator pkgPath) { + FilteringPolicy policy) { this.recursivePackageProvider = recursivePackageProvider; this.eventHandler = eventHandler; this.policy = policy; - this.pkgPath = pkgPath; } @Override @@ -141,18 +135,13 @@ public class RecursivePackageProviderBackedTargetPatternResolver TargetPatternResolverUtil.getPathFragments(excludedSubdirectories); PathFragment pathFragment = TargetPatternResolverUtil.getPathFragment(directory); ResolvedTargets.Builder<Target> targetBuilder = ResolvedTargets.builder(); - // TODO(bazel-team): This is where we need to depend on the RepositoryValue of a remote - // repository in order figure out its root and thus support recursive package search in them. - for (Path root : pkgPath.getPathEntries()) { - RootedPath rootedPath = RootedPath.toRootedPath(root, pathFragment); - Iterable<PathFragment> packagesUnderDirectory = - recursivePackageProvider.getPackagesUnderDirectory( - repository, rootedPath, excludedPathFragments); - for (PathFragment pkg : packagesUnderDirectory) { - targetBuilder.merge(getTargetsInPackage(originalPattern, - PackageIdentifier.createInDefaultRepo(pkg), - FilteringPolicies.NO_FILTER)); - } + Iterable<PathFragment> packagesUnderDirectory = + recursivePackageProvider.getPackagesUnderDirectory( + repository, pathFragment, excludedPathFragments); + for (PathFragment pkg : packagesUnderDirectory) { + targetBuilder.merge(getTargetsInPackage(originalPattern, + PackageIdentifier.create(repository, pkg), + FilteringPolicies.NO_FILTER)); } // Perform the no-targets-found check before applying the filtering policy so we only return the 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 a67eced903..a75e6c53a3 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 @@ -327,7 +327,7 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory { newSkylarkImportLookupFunction(ruleClassProvider, pkgFactory)); map.put(SkyFunctions.SKYLARK_IMPORT_CYCLE, new SkylarkImportUniqueCycleFunction()); map.put(SkyFunctions.GLOB, newGlobFunction()); - map.put(SkyFunctions.TARGET_PATTERN, new TargetPatternFunction(pkgLocator)); + map.put(SkyFunctions.TARGET_PATTERN, new TargetPatternFunction()); map.put(SkyFunctions.PREPARE_DEPS_OF_PATTERNS, new PrepareDepsOfPatternsFunction()); map.put(SkyFunctions.PREPARE_DEPS_OF_PATTERN, new PrepareDepsOfPatternFunction(pkgLocator)); map.put(SkyFunctions.PREPARE_DEPS_OF_TARGETS_UNDER_DIRECTORY, diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternFunction.java index 404b1d4227..0ac65a9af3 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternFunction.java @@ -20,15 +20,12 @@ import com.google.devtools.build.lib.cmdline.ResolvedTargets; import com.google.devtools.build.lib.cmdline.TargetParsingException; import com.google.devtools.build.lib.cmdline.TargetPattern; import com.google.devtools.build.lib.packages.Target; -import com.google.devtools.build.lib.pkgcache.PathPackageLocator; import com.google.devtools.build.lib.skyframe.EnvironmentBackedRecursivePackageProvider.MissingDepException; import com.google.devtools.build.skyframe.SkyFunction; import com.google.devtools.build.skyframe.SkyFunctionException; import com.google.devtools.build.skyframe.SkyKey; import com.google.devtools.build.skyframe.SkyValue; -import java.util.concurrent.atomic.AtomicReference; - import javax.annotation.Nullable; /** @@ -37,10 +34,7 @@ import javax.annotation.Nullable; */ public class TargetPatternFunction implements SkyFunction { - private final AtomicReference<PathPackageLocator> pkgPath; - - public TargetPatternFunction(AtomicReference<PathPackageLocator> pkgPath) { - this.pkgPath = pkgPath; + public TargetPatternFunction() { } @Override @@ -54,7 +48,7 @@ public class TargetPatternFunction implements SkyFunction { new EnvironmentBackedRecursivePackageProvider(env); RecursivePackageProviderBackedTargetPatternResolver resolver = new RecursivePackageProviderBackedTargetPatternResolver(provider, env.getListener(), - patternKey.getPolicy(), pkgPath.get()); + patternKey.getPolicy()); TargetPattern parsedPattern = patternKey.getParsedPattern(); ImmutableSet<String> excludedSubdirectories = patternKey.getExcludedSubdirectories(); resolvedTargets = parsedPattern.eval(resolver, excludedSubdirectories); diff --git a/src/test/shell/bazel/local_repository_test.sh b/src/test/shell/bazel/local_repository_test.sh index 985e92fc2d..536dbbd3e1 100755 --- a/src/test/shell/bazel/local_repository_test.sh +++ b/src/test/shell/bazel/local_repository_test.sh @@ -848,4 +848,27 @@ EOF expect_log "@r//a:h" } +function test_recursive_wildcard_in_remote_repository() { + local r=$TEST_TMPDIR/r + rm -fr $r + mkdir -p $r/a/{x,y/z} + touch $r/a/{x,y/z}/{m,n} + + echo 'exports_files(["m", "n"])' > $r/a/x/BUILD + echo 'exports_files(["m", "n"])' > $r/a/y/z/BUILD + + echo "local_repository(name='r', path='$r')" > WORKSPACE + bazel query @r//...:all-targets >& $TEST_log || fail "query failed" + expect_log "@r//a/x:m" + expect_log "@r//a/x:n" + expect_log "@r//a/y/z:m" + expect_log "@r//a/y/z:n" + + bazel query @r//a/x:all-targets >& $TEST_log || fail "query failed" + expect_log "@r//a/x:m" + expect_log "@r//a/x:n" + expect_not_log "@r//a/y/z:m" + expect_not_log "@r//a/y/z:n" +} + run_suite "local repository tests" |