diff options
author | Lukacs Berki <lberki@google.com> | 2015-07-07 07:58:15 +0000 |
---|---|---|
committer | Lukacs Berki <lberki@google.com> | 2015-07-07 08:42:21 +0000 |
commit | b21df3d2a022a24f66c88671ab7694c9b0d9735c (patch) | |
tree | 015c696c5d45076255570e0d05ef6fabafdeac9f /src/main/java/com/google/devtools/build/lib | |
parent | 7d02845041bbd0342f9e6dca69627bff5864aa71 (diff) |
Rollback of accidentally submitted change.
--
MOS_MIGRATED_REVID=97648982
Diffstat (limited to 'src/main/java/com/google/devtools/build/lib')
21 files changed, 80 insertions, 150 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/packages/CachingPackageLocator.java b/src/main/java/com/google/devtools/build/lib/packages/CachingPackageLocator.java index 3d677990f3..f6badad9dd 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/CachingPackageLocator.java +++ b/src/main/java/com/google/devtools/build/lib/packages/CachingPackageLocator.java @@ -40,6 +40,6 @@ public interface CachingPackageLocator { * <p> This method must be thread-safe. */ @ThreadSafe - Path getBuildFileForPackage(PackageIdentifier packageName); + Path getBuildFileForPackage(String packageName); } diff --git a/src/main/java/com/google/devtools/build/lib/packages/GlobCache.java b/src/main/java/com/google/devtools/build/lib/packages/GlobCache.java index 43669a86bf..de9bfdfebc 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/GlobCache.java +++ b/src/main/java/com/google/devtools/build/lib/packages/GlobCache.java @@ -23,6 +23,7 @@ import com.google.common.util.concurrent.SettableFuture; import com.google.devtools.build.lib.concurrent.ThreadSafety; import com.google.devtools.build.lib.util.Pair; import com.google.devtools.build.lib.vfs.Path; +import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.lib.vfs.Symlinks; import com.google.devtools.build.lib.vfs.UnixGlob; @@ -106,6 +107,7 @@ public class GlobCache { this.syscalls = syscalls == null ? new AtomicReference<>(UnixGlob.DEFAULT_SYSCALLS) : syscalls; Preconditions.checkNotNull(locator); + final PathFragment pkgNameFrag = packageId.getPackageFragment(); childDirectoryPredicate = new Predicate<Path>() { @Override public boolean apply(Path directory) { @@ -113,9 +115,7 @@ public class GlobCache { return true; } - PackageIdentifier subPackageId = new PackageIdentifier( - packageId.getRepository(), - packageId.getPackageFragment().getRelative(directory.relativeTo(packageDirectory))); + PathFragment pkgName = pkgNameFrag.getRelative(directory.relativeTo(packageDirectory)); UnixGlob.FilesystemCalls syscalls = GlobCache.this.syscalls.get(); if (syscalls != UnixGlob.DEFAULT_SYSCALLS) { // This is needed because in case the BUILD file exists, we do not call readdir() on its @@ -134,7 +134,7 @@ public class GlobCache { syscalls.statNullable(directory.getChild("BUILD"), Symlinks.FOLLOW); } - return locator.getBuildFileForPackage(subPackageId) == null; + return locator.getBuildFileForPackage(pkgName.getPathString()) == null; } }; } diff --git a/src/main/java/com/google/devtools/build/lib/packages/PackageIdentifier.java b/src/main/java/com/google/devtools/build/lib/packages/PackageIdentifier.java index 803fb48855..f2f5b8e0e0 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/PackageIdentifier.java +++ b/src/main/java/com/google/devtools/build/lib/packages/PackageIdentifier.java @@ -16,7 +16,6 @@ package com.google.devtools.build.lib.packages; import com.google.common.base.Preconditions; import com.google.common.collect.ComparisonChain; -import com.google.devtools.build.lib.cmdline.LabelValidator; import com.google.devtools.build.lib.syntax.Label.SyntaxException; import com.google.devtools.build.lib.util.StringCanonicalizer; import com.google.devtools.build.lib.util.StringUtilities; @@ -218,34 +217,6 @@ public final class PackageIdentifier implements Comparable<PackageIdentifier>, S this.pkgName = Canonicalizer.fragments().intern(pkgName.normalize()); } - public static PackageIdentifier parse(String input) throws SyntaxException { - String repo; - String packageName; - int packageStartPos = input.indexOf("//"); - if (packageStartPos > 0) { - repo = input.substring(0, packageStartPos); - packageName = input.substring(packageStartPos + 2); - } else if (packageStartPos == 0) { - repo = PackageIdentifier.DEFAULT_REPOSITORY; - packageName = input.substring(2); - } else { - repo = PackageIdentifier.DEFAULT_REPOSITORY; - packageName = input; - } - - String error = RepositoryName.validate(repo); - if (error != null) { - throw new SyntaxException(error); - } - - error = LabelValidator.validatePackageName(packageName); - if (error != null) { - throw new SyntaxException(error); - } - - return new PackageIdentifier(repo, new PathFragment(packageName)); - } - private Object writeReplace() throws ObjectStreamException { return new SerializationProxy(this); } diff --git a/src/main/java/com/google/devtools/build/lib/pkgcache/PackageCacheOptions.java b/src/main/java/com/google/devtools/build/lib/pkgcache/PackageCacheOptions.java index daba8f1e88..e57fe0df61 100644 --- a/src/main/java/com/google/devtools/build/lib/pkgcache/PackageCacheOptions.java +++ b/src/main/java/com/google/devtools/build/lib/pkgcache/PackageCacheOptions.java @@ -16,7 +16,6 @@ package com.google.devtools.build.lib.pkgcache; import com.google.devtools.build.lib.Constants; import com.google.devtools.build.lib.packages.ConstantRuleVisibility; -import com.google.devtools.build.lib.packages.PackageIdentifier; import com.google.devtools.build.lib.packages.RuleVisibility; import com.google.devtools.build.lib.syntax.CommaSeparatedPackageNameListConverter; import com.google.devtools.common.options.Converter; @@ -111,7 +110,7 @@ public class PackageCacheOptions extends OptionsBase { + "encounters a label '//x:y/z' if that is still provided by another " + "package_path entry. Specifying --deleted_packages x/y avoids this " + "problem.") - public List<PackageIdentifier> deletedPackages; + public List<String> deletedPackages; @Option(name = "default_visibility", defaultValue = "private", diff --git a/src/main/java/com/google/devtools/build/lib/pkgcache/PackageProvider.java b/src/main/java/com/google/devtools/build/lib/pkgcache/PackageProvider.java index 9bce541a51..5363cb5a8a 100644 --- a/src/main/java/com/google/devtools/build/lib/pkgcache/PackageProvider.java +++ b/src/main/java/com/google/devtools/build/lib/pkgcache/PackageProvider.java @@ -59,5 +59,5 @@ public interface PackageProvider extends TargetProvider { * @param eventHandler the eventHandler on which to report warnings and errors * @param packageName the name of the package. */ - boolean isPackage(EventHandler eventHandler, PackageIdentifier packageName); + boolean isPackage(EventHandler eventHandler, String packageName); } diff --git a/src/main/java/com/google/devtools/build/lib/pkgcache/PathPackageLocator.java b/src/main/java/com/google/devtools/build/lib/pkgcache/PathPackageLocator.java index 27b072f211..c8204c6a26 100644 --- a/src/main/java/com/google/devtools/build/lib/pkgcache/PathPackageLocator.java +++ b/src/main/java/com/google/devtools/build/lib/pkgcache/PathPackageLocator.java @@ -13,8 +13,6 @@ // limitations under the License. package com.google.devtools.build.lib.pkgcache; -import com.google.common.annotations.VisibleForTesting; -import com.google.common.base.Objects; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.devtools.build.lib.events.Event; @@ -28,9 +26,6 @@ import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.lib.vfs.Symlinks; import com.google.devtools.build.lib.vfs.UnixGlob; -import java.io.IOException; -import java.io.ObjectInputStream; -import java.io.ObjectOutputStream; import java.io.Serializable; import java.util.ArrayList; import java.util.Arrays; @@ -50,27 +45,33 @@ public class PathPackageLocator implements Serializable { public static final Set<String> DEFAULT_TOP_LEVEL_EXCLUDES = ImmutableSet.of("experimental"); - private final ImmutableList<Path> pathEntries; - private final Path outputBase; - - private PathPackageLocator(Path outputBase, List<Path> pathEntries) { - this.outputBase = outputBase; - this.pathEntries = ImmutableList.copyOf(pathEntries); + /** + * An interface which accepts {@link PathFragment}s. + */ + public interface AcceptsPathFragment { + + /** + * Accept a {@link PathFragment}. + * + * @param fragment The path fragment. + */ + void accept(PathFragment fragment); } + private final ImmutableList<Path> pathEntries; + /** * Constructs a PathPackageLocator based on the specified list of package root directories. */ - @VisibleForTesting public PathPackageLocator(List<Path> pathEntries) { - this(null, pathEntries); + this.pathEntries = ImmutableList.copyOf(pathEntries); } /** * Constructs a PathPackageLocator based on the specified array of package root directories. */ public PathPackageLocator(Path... pathEntries) { - this(null, Arrays.asList(pathEntries)); + this(Arrays.asList(pathEntries)); } /** @@ -86,10 +87,11 @@ public class PathPackageLocator implements Serializable { * <p>If the same package exists beneath multiple package path entries, the * first path that matches always wins. */ - public Path getPackageBuildFile(PackageIdentifier packageName) throws NoSuchPackageException { + public Path getPackageBuildFile(String packageName) throws NoSuchPackageException { Path buildFile = getPackageBuildFileNullable(packageName, UnixGlob.DEFAULT_SYSCALLS_REF); if (buildFile == null) { - throw new BuildFileNotFoundException(packageName, "BUILD file not found on package path"); + throw new BuildFileNotFoundException(PackageIdentifier.createInDefaultRepo(packageName), + "BUILD file not found on package path"); } return buildFile; } @@ -100,25 +102,9 @@ public class PathPackageLocator implements Serializable { * @param packageName the name of the package. * @param cache a filesystem-level cache of stat() calls. */ - public Path getPackageBuildFileNullable(PackageIdentifier packageName, + public Path getPackageBuildFileNullable(String packageName, AtomicReference<? extends UnixGlob.FilesystemCalls> cache) { - if (packageName.getRepository().isDefault()) { - return getFilePath(packageName.getPackageFragment().getRelative("BUILD"), cache); - } else if (outputBase != null) { - // This works only to some degree, because it relies on the presence of the repository under - // $OUTPUT_BASE/external, which is created by the appropriate RepositoryValue. This is true - // for the invocation in GlobCache, but not for the locator.getBuildFileForPackage() - // invocation in Parser#include(). - Path buildFile = outputBase.getRelative(packageName.getPathFragment()).getRelative("BUILD"); - FileStatus stat = cache.get().statNullable(buildFile, Symlinks.FOLLOW); - if (stat != null && stat.isFile()) { - return buildFile; - } else { - return null; - } - } else { - return null; - } + return getFilePath(new PathFragment(packageName).getRelative("BUILD"), cache); } @@ -138,7 +124,6 @@ public class PathPackageLocator implements Serializable { * A factory of PathPackageLocators from a list of path elements. Elements * may contain "%workspace%", indicating the workspace. * - * @param outputBase the output base. Can be null if remote repositories are not in use. * @param pathElements Each element must be an absolute path, relative path, * or some string "%workspace%" + relative, where relative is itself a * relative path. The special symbol "%workspace%" means to interpret @@ -150,8 +135,7 @@ public class PathPackageLocator implements Serializable { * @param clientWorkingDirectory The client's working directory. * @return a list of {@link Path}s. */ - public static PathPackageLocator create(Path outputBase, - List<String> pathElements, + public static PathPackageLocator create(List<String> pathElements, EventHandler eventHandler, Path workspace, Path clientWorkingDirectory) { @@ -180,7 +164,7 @@ public class PathPackageLocator implements Serializable { resolvedPaths.add(rootPath); } } - return new PathPackageLocator(outputBase, resolvedPaths); + return new PathPackageLocator(resolvedPaths); } /** @@ -210,7 +194,7 @@ public class PathPackageLocator implements Serializable { @Override public int hashCode() { - return Objects.hashCode(pathEntries, outputBase); + return pathEntries.hashCode(); } @Override @@ -221,15 +205,6 @@ public class PathPackageLocator implements Serializable { if (!(other instanceof PathPackageLocator)) { return false; } - return this.getPathEntries().equals(((PathPackageLocator) other).getPathEntries()) - && Objects.equal(this.outputBase, ((PathPackageLocator) other).outputBase); - } - - private void writeObject(ObjectOutputStream out) throws IOException { - out.writeObject(pathEntries); - } - - private void readObject(ObjectInputStream in) throws IOException { - throw new IllegalStateException(); + return this.getPathEntries().equals(((PathPackageLocator) other).getPathEntries()); } } 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 bbc432e7a5..2aae12f91a 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 @@ -461,7 +461,7 @@ public class GenQuery implements RuleConfiguredTargetFactory { } @Override - public boolean isPackage(EventHandler eventHandler, PackageIdentifier packageName) { + public boolean isPackage(EventHandler eventHandler, String packageName) { throw new UnsupportedOperationException(); } } diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java index fcd11e5e46..fca2ae918f 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java @@ -1014,7 +1014,7 @@ public final class BlazeRuntime { if (!skyframeExecutor.hasIncrementalState()) { clearSkyframeRelevantCaches(); } - skyframeExecutor.sync(packageCacheOptions, getOutputBase(), getWorkingDirectory(), + skyframeExecutor.sync(packageCacheOptions, getWorkingDirectory(), defaultsPackageContents, getCommandId()); } diff --git a/src/main/java/com/google/devtools/build/lib/runtime/commands/ProjectFileSupport.java b/src/main/java/com/google/devtools/build/lib/runtime/commands/ProjectFileSupport.java index 133179f07d..bec0deb5d1 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/commands/ProjectFileSupport.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/commands/ProjectFileSupport.java @@ -63,11 +63,8 @@ public final class ProjectFileSupport { // relative to the cwd instead. PathFragment projectFilePath = new PathFragment(targets.get(0).substring(1)); List<Path> packagePath = PathPackageLocator.create( - runtime.getOutputBase(), - optionsParser.getOptions(PackageCacheOptions.class).packagePath, - runtime.getReporter(), - runtime.getWorkspace(), - runtime.getWorkingDirectory()).getPathEntries(); + optionsParser.getOptions(PackageCacheOptions.class).packagePath, runtime.getReporter(), + runtime.getWorkspace(), runtime.getWorkingDirectory()).getPathEntries(); ProjectFile projectFile = projectFileProvider.getProjectFile(packagePath, projectFilePath); runtime.getReporter().handle(Event.info("Using " + projectFile.getName())); 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 ac5b6a2f1f..9ea62103fe 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 @@ -64,9 +64,9 @@ public final class EnvironmentBackedRecursivePackageProvider implements Recursiv } @Override - public boolean isPackage(EventHandler eventHandler, PackageIdentifier packageId) + public boolean isPackage(EventHandler eventHandler, String packageName) throws MissingDepException { - SkyKey packageLookupKey = PackageLookupValue.key(packageId); + SkyKey packageLookupKey = PackageLookupValue.key(new PathFragment(packageName)); try { PackageLookupValue packageLookupValue = (PackageLookupValue) env.getValueOrThrow(packageLookupKey, NoSuchPackageException.class, diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/GlobFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/GlobFunction.java index 942a201770..d1ee81cb07 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/GlobFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/GlobFunction.java @@ -18,7 +18,6 @@ import com.google.common.cache.Cache; import com.google.common.cache.CacheBuilder; import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; -import com.google.devtools.build.lib.packages.PackageIdentifier; import com.google.devtools.build.lib.vfs.Dirent; import com.google.devtools.build.lib.vfs.Dirent.Type; import com.google.devtools.build.lib.vfs.PathFragment; @@ -61,9 +60,8 @@ final class GlobFunction implements SkyFunction { PathFragment globSubdir = glob.getSubdir(); if (!globSubdir.equals(PathFragment.EMPTY_FRAGMENT)) { PackageLookupValue globSubdirPkgLookupValue = (PackageLookupValue) env.getValue( - PackageLookupValue.key(new PackageIdentifier( - glob.getPackageId().getRepository(), - glob.getPackageId().getPackageFragment().getRelative(globSubdir)))); + PackageLookupValue.key(glob.getPackageId().getPackageFragment() + .getRelative(globSubdir))); if (globSubdirPkgLookupValue == null) { return null; } @@ -222,8 +220,7 @@ final class GlobFunction implements SkyFunction { PathFragment directory = glob.getPackageId().getPackageFragment() .getRelative(glob.getSubdir()).getRelative(fileName); PackageLookupValue pkgLookupValue = (PackageLookupValue) - env.getValue(PackageLookupValue.key(new PackageIdentifier( - glob.getPackageId().getRepository(), directory))); + env.getValue(PackageLookupValue.key(directory)); if (pkgLookupValue == null) { return; } 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 4b701dd775..e2c83a024c 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 @@ -68,8 +68,8 @@ public final class GraphBackedRecursivePackageProvider implements RecursivePacka } @Override - public boolean isPackage(EventHandler eventHandler, PackageIdentifier packageName) { - SkyKey packageLookupKey = PackageLookupValue.key(packageName); + public boolean isPackage(EventHandler eventHandler, String packageName) { + SkyKey packageLookupKey = PackageLookupValue.key(new PathFragment(packageName)); if (!graph.exists(packageLookupKey)) { // If the package lookup key does not exist in the graph, then it must not correspond to any // package, because the SkyQuery environment has already loaded the universe. diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PackageLookupFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/PackageLookupFunction.java index c634d08d5a..a4996a4479 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/PackageLookupFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/PackageLookupFunction.java @@ -41,9 +41,9 @@ import javax.annotation.Nullable; */ class PackageLookupFunction implements SkyFunction { - private final AtomicReference<ImmutableSet<PackageIdentifier>> deletedPackages; + private final AtomicReference<ImmutableSet<String>> deletedPackages; - PackageLookupFunction(AtomicReference<ImmutableSet<PackageIdentifier>> deletedPackages) { + PackageLookupFunction(AtomicReference<ImmutableSet<String>> deletedPackages) { this.deletedPackages = deletedPackages; } @@ -64,7 +64,7 @@ class PackageLookupFunction implements SkyFunction { + packageNameErrorMsg); } - if (deletedPackages.get().contains(packageKey)) { + if (deletedPackages.get().contains(packageKey.getPackageFragment().toString())) { return PackageLookupValue.deletedPackage(); } @@ -142,7 +142,7 @@ class PackageLookupFunction implements SkyFunction { throws PackageLookupFunctionException { PackageIdentifier id = (PackageIdentifier) skyKey.argument(); SkyKey repositoryKey = RepositoryValue.key(id.getRepository()); - RepositoryValue repositoryValue; + RepositoryValue repositoryValue = null; try { repositoryValue = (RepositoryValue) env.getValueOrThrow( repositoryKey, NoSuchPackageException.class, IOException.class, EvalException.class); 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 2fa6ca91a8..bf32843787 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 @@ -187,10 +187,7 @@ public class PrepareDepsOfPatternFunction implements SkyFunction { @Override public boolean isPackage(String packageName) { - // TODO(bazel-team): this should get the whole PackageIdentifier. Using only the package name - // makes it impossible to use wildcards to refer to targets in remote repositories. - return packageProvider.isPackage(env.getListener(), - PackageIdentifier.createInDefaultRepo(packageName)); + return packageProvider.isPackage(env.getListener(), packageName); } @Override 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 b91362834e..942ed83fcb 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 @@ -132,10 +132,7 @@ public class RecursivePackageProviderBackedTargetPatternResolver @Override public boolean isPackage(String packageName) { - // TODO(bazel-team): this should get the whole PackageIdentifier. Using only the package name - // makes it impossible to use the //... wildcard to refer to targets in remote repositories. - return recursivePackageProvider.isPackage( - eventHandler, PackageIdentifier.createInDefaultRepo(packageName)); + return recursivePackageProvider.isPackage(eventHandler, packageName); } @Override diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java index a1ac8e2116..ef22ab993b 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java @@ -211,12 +211,11 @@ public final class SequencedSkyframeExecutor extends SkyframeExecutor { } @Override - public void sync(PackageCacheOptions packageCacheOptions, Path outputBase, Path workingDirectory, + public void sync(PackageCacheOptions packageCacheOptions, Path workingDirectory, String defaultsPackageContents, UUID commandId) throws InterruptedException, AbruptExitException { this.valueCacheEvictionLimit = packageCacheOptions.minLoadedPkgCountForCtNodeEviction; - super.sync( - packageCacheOptions, outputBase, workingDirectory, defaultsPackageContents, commandId); + super.sync(packageCacheOptions, workingDirectory, defaultsPackageContents, commandId); handleDiffs(); } @@ -244,10 +243,11 @@ public final class SequencedSkyframeExecutor extends SkyframeExecutor { recordingDiffer.invalidate(Iterables.filter(memoizingEvaluator.getValues().keySet(), pred)); } - private void invalidateDeletedPackages(Iterable<PackageIdentifier> deletedPackages) { + private void invalidateDeletedPackages(Iterable<String> deletedPackages) { ArrayList<SkyKey> packagesToInvalidate = Lists.newArrayList(); - for (PackageIdentifier deletedPackage : deletedPackages) { - packagesToInvalidate.add(PackageLookupValue.key(deletedPackage)); + for (String deletedPackage : deletedPackages) { + PathFragment pathFragment = new PathFragment(deletedPackage); + packagesToInvalidate.add(PackageLookupValue.key(pathFragment)); } recordingDiffer.invalidate(packagesToInvalidate); } @@ -257,7 +257,7 @@ public final class SequencedSkyframeExecutor extends SkyframeExecutor { */ @Override @VisibleForTesting // productionVisibility = Visibility.PRIVATE - public void setDeletedPackages(Iterable<PackageIdentifier> pkgs) { + public void setDeletedPackages(Iterable<String> pkgs) { // Invalidate the old deletedPackages as they may exist now. invalidateDeletedPackages(deletedPackages.get()); deletedPackages.set(ImmutableSet.copyOf(pkgs)); 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 79acbe0985..b2ed3c6c5a 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 @@ -183,8 +183,8 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory { new AtomicReference<>(UnixGlob.DEFAULT_SYSCALLS); protected final AtomicReference<PathPackageLocator> pkgLocator = new AtomicReference<>(); - protected final AtomicReference<ImmutableSet<PackageIdentifier>> deletedPackages = - new AtomicReference<>(ImmutableSet.<PackageIdentifier>of()); + protected final AtomicReference<ImmutableSet<String>> deletedPackages = + new AtomicReference<>(ImmutableSet.<String>of()); private final AtomicReference<EventBus> eventBus = new AtomicReference<>(); private final ImmutableList<BuildInfoFactory> buildInfoFactories; @@ -756,7 +756,7 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory { * Sets the packages that should be treated as deleted and ignored. */ @VisibleForTesting // productionVisibility = Visibility.PRIVATE - public abstract void setDeletedPackages(Iterable<PackageIdentifier> pkgs); + public abstract void setDeletedPackages(Iterable<String> pkgs); /** * Prepares the evaluator for loading. @@ -1302,7 +1302,7 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory { /** * Returns whether the given package should be consider deleted and thus should be ignored. */ - public boolean isPackageDeleted(PackageIdentifier packageName) { + public boolean isPackageDeleted(String packageName) { return deletedPackages.get().contains(packageName); } @@ -1352,13 +1352,12 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory { @ThreadCompatible public abstract void updateLoadedPackageSet(Set<PackageIdentifier> loadedPackages); - public void sync(PackageCacheOptions packageCacheOptions, Path outputBase, Path workingDirectory, + public void sync(PackageCacheOptions packageCacheOptions, Path workingDirectory, String defaultsPackageContents, UUID commandId) throws InterruptedException, AbruptExitException{ preparePackageLoading( - createPackageLocator( - packageCacheOptions, outputBase, directories.getWorkspace(), workingDirectory), + createPackageLocator(packageCacheOptions, directories.getWorkspace(), workingDirectory), packageCacheOptions.defaultVisibility, packageCacheOptions.showLoadingProgress, packageCacheOptions.globbingThreads, defaultsPackageContents, commandId); setDeletedPackages(ImmutableSet.copyOf(packageCacheOptions.deletedPackages)); @@ -1368,9 +1367,9 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory { } protected PathPackageLocator createPackageLocator(PackageCacheOptions packageCacheOptions, - Path outputBase, Path workspace, Path workingDirectory) throws AbruptExitException { + Path workspace, Path workingDirectory) throws AbruptExitException{ return PathPackageLocator.create( - outputBase, packageCacheOptions.packagePath, getReporter(), workspace, workingDirectory); + packageCacheOptions.packagePath, getReporter(), workspace, workingDirectory); } private CyclesReporter createCyclesReporter() { 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 7ffb79ce89..3e9b6cad1f 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 @@ -13,6 +13,7 @@ // limitations under the License. package com.google.devtools.build.lib.skyframe; +import com.google.devtools.build.lib.cmdline.LabelValidator; import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe; import com.google.devtools.build.lib.events.EventHandler; import com.google.devtools.build.lib.packages.NoSuchPackageException; @@ -135,7 +136,7 @@ class SkyframePackageManager implements PackageManager { } @Override - public boolean isPackage(EventHandler eventHandler, PackageIdentifier packageName) { + public boolean isPackage(EventHandler eventHandler, String packageName) { return getBuildFileForPackage(packageName) != null; } @@ -146,10 +147,11 @@ class SkyframePackageManager implements PackageManager { @ThreadSafe @Override - public Path getBuildFileForPackage(PackageIdentifier packageName) { + public Path getBuildFileForPackage(String packageName) { // Note that this method needs to be thread-safe, as it is currently used concurrently by // legacy blaze code. - if (packageLoader.isPackageDeleted(packageName)) { + if (packageLoader.isPackageDeleted(packageName) + || LabelValidator.validatePackageName(packageName) != null) { return null; } // TODO(bazel-team): Use a PackageLookupValue here [skyframe-loading] diff --git a/src/main/java/com/google/devtools/build/lib/syntax/CommaSeparatedPackageNameListConverter.java b/src/main/java/com/google/devtools/build/lib/syntax/CommaSeparatedPackageNameListConverter.java index ce7680ce3e..070e928eb9 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/CommaSeparatedPackageNameListConverter.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/CommaSeparatedPackageNameListConverter.java @@ -16,7 +16,7 @@ package com.google.devtools.build.lib.syntax; import com.google.common.base.Splitter; import com.google.common.base.Strings; import com.google.common.collect.ImmutableList; -import com.google.devtools.build.lib.packages.PackageIdentifier; +import com.google.devtools.build.lib.cmdline.LabelValidator; import com.google.devtools.common.options.Converter; import com.google.devtools.common.options.OptionsParsingException; @@ -26,22 +26,22 @@ import java.util.List; * A converter from strings containing comma-separated names of packages to lists of strings. */ public class CommaSeparatedPackageNameListConverter - implements Converter<List<PackageIdentifier>> { + implements Converter<List<String>> { private static final Splitter SPACE_SPLITTER = Splitter.on(','); @Override - public List<PackageIdentifier> convert(String input) throws OptionsParsingException { + public List<String> convert(String input) throws OptionsParsingException { if (Strings.isNullOrEmpty(input)) { return ImmutableList.of(); } - ImmutableList.Builder<PackageIdentifier> list = ImmutableList.builder(); + ImmutableList.Builder<String> list = ImmutableList.builder(); for (String s : SPACE_SPLITTER.split(input)) { - try { - list.add(PackageIdentifier.parse(s)); - } catch (Label.SyntaxException e) { - throw new OptionsParsingException(e.getMessage()); + String errorMessage = LabelValidator.validatePackageName(s); + if (errorMessage != null) { + throw new OptionsParsingException(errorMessage); } + list.add(s); } return list.build(); } diff --git a/src/main/java/com/google/devtools/build/lib/syntax/EvaluationContext.java b/src/main/java/com/google/devtools/build/lib/syntax/EvaluationContext.java index f91222c740..4966fb8717 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/EvaluationContext.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/EvaluationContext.java @@ -21,7 +21,6 @@ import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.events.EventHandler; import com.google.devtools.build.lib.events.EventKind; import com.google.devtools.build.lib.packages.CachingPackageLocator; -import com.google.devtools.build.lib.packages.PackageIdentifier; import com.google.devtools.build.lib.syntax.Environment.NoSuchVariableException; import com.google.devtools.build.lib.vfs.Path; @@ -91,7 +90,7 @@ public final class EvaluationContext { /** Mock package locator */ private static final class EmptyPackageLocator implements CachingPackageLocator { @Override - public Path getBuildFileForPackage(PackageIdentifier packageName) { + public Path getBuildFileForPackage(String packageName) { return null; } } diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Parser.java b/src/main/java/com/google/devtools/build/lib/syntax/Parser.java index f2f7c2c546..2a9d103beb 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/Parser.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/Parser.java @@ -612,13 +612,10 @@ class Parser { try { Label label = Label.parseAbsolute(labelName); - // Note that this doesn't really work if the label belongs to a different repository, because - // there is no guarantee that its RepositoryValue has been evaluated. In an ideal world, we - // could put a Skyframe dependency the appropriate PackageLookupValue, but we can't do that - // because package loading is not completely Skyframized. - Path packagePath = locator.getBuildFileForPackage(label.getPackageIdentifier()); + String packageName = label.getPackageFragment().getPathString(); + Path packagePath = locator.getBuildFileForPackage(packageName); if (packagePath == null) { - reportError(location, "Package '" + label.getPackageIdentifier() + "' not found"); + reportError(location, "Package '" + packageName + "' not found"); list.add(mocksubincludeExpression(labelName, "", location)); return; } |