Rollback of accidentally submitted change.
--
MOS_MIGRATED_REVID=97648982
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 3d677990..f6badad 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 @@
* <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 43669a8..de9bfdf 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.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 @@
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 @@
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 @@
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 803fb48..f2f5b8e 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 @@
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 @@
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 daba8f1..e57fe0d 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 @@
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 @@
+ "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 9bce541..5363cb5 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 @@
* @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 27b072f..c8204c6 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.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 static final Set<String> DEFAULT_TOP_LEVEL_EXCLUDES = ImmutableSet.of("experimental");
- private final ImmutableList<Path> pathEntries;
- private final Path outputBase;
+ /**
+ * An interface which accepts {@link PathFragment}s.
+ */
+ public interface AcceptsPathFragment {
- private PathPackageLocator(Path outputBase, List<Path> pathEntries) {
- this.outputBase = outputBase;
- this.pathEntries = ImmutableList.copyOf(pathEntries);
+ /**
+ * 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 @@
* <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 @@
* @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 @@
* 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 @@
* @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 @@
resolvedPaths.add(rootPath);
}
}
- return new PathPackageLocator(outputBase, resolvedPaths);
+ return new PathPackageLocator(resolvedPaths);
}
/**
@@ -210,7 +194,7 @@
@Override
public int hashCode() {
- return Objects.hashCode(pathEntries, outputBase);
+ return pathEntries.hashCode();
}
@Override
@@ -221,15 +205,6 @@
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 bbc432e..2aae12f 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 @@
}
@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 fcd11e5..fca2ae9 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 @@
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 133179f..bec0deb 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 @@
// 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 ac5b6a2..9ea6210 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 @@
}
@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 942a201..d1ee81c 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.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 @@
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 @@
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 4b701dd..e2c83a0 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 @@
}
@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 c634d08..a4996a4 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 @@
*/
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 @@
+ packageNameErrorMsg);
}
- if (deletedPackages.get().contains(packageKey)) {
+ if (deletedPackages.get().contains(packageKey.getPackageFragment().toString())) {
return PackageLookupValue.deletedPackage();
}
@@ -142,7 +142,7 @@
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 2fa6ca9..bf32843 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 @@
@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 b913628..942ed83 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 @@
@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 a1ac8e2..ef22ab9 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 @@
}
@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 @@
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 @@
*/
@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 79acbe0..b2ed3c6 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 @@
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 @@
* 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 @@
/**
* 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 @@
@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 @@
}
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 7ffb79ce..3e9b6ca 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 @@
}
@Override
- public boolean isPackage(EventHandler eventHandler, PackageIdentifier packageName) {
+ public boolean isPackage(EventHandler eventHandler, String packageName) {
return getBuildFileForPackage(packageName) != null;
}
@@ -146,10 +147,11 @@
@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 ce7680c..070e928 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 @@
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 @@
* 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 f91222c..4966fb8 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.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 @@
/** 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 f2f7c2c..2a9d103 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 @@
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;
}
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java
index 94975bb..e5bfd83 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java
@@ -242,7 +242,7 @@
PackageCacheOptions packageCacheOptions = optionsParser.getOptions(PackageCacheOptions.class);
PathPackageLocator pathPackageLocator = PathPackageLocator.create(
- null, packageCacheOptions.packagePath, reporter, rootDirectory, rootDirectory);
+ packageCacheOptions.packagePath, reporter, rootDirectory, rootDirectory);
skyframeExecutor.preparePackageLoading(pathPackageLocator,
packageCacheOptions.defaultVisibility, true,
7, ruleClassProvider.getDefaultsPackageContent(), UUID.randomUUID());
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java
index 283f231..e6fa833 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java
@@ -274,7 +274,7 @@
private void setUpSkyframe() {
PathPackageLocator pkgLocator = PathPackageLocator.create(
- null, packageCacheOptions.packagePath, reporter, rootDirectory, rootDirectory);
+ packageCacheOptions.packagePath, reporter, rootDirectory, rootDirectory);
skyframeExecutor.preparePackageLoading(pkgLocator,
packageCacheOptions.defaultVisibility, true,
7, ruleClassProvider.getDefaultsPackageContent(optionsParser),
diff --git a/src/test/java/com/google/devtools/build/lib/packages/GlobCacheTest.java b/src/test/java/com/google/devtools/build/lib/packages/GlobCacheTest.java
index 392c750..bd8d536 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/GlobCacheTest.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/GlobCacheTest.java
@@ -94,8 +94,7 @@
cache = new GlobCache(packageDirectory, PackageIdentifier.createInDefaultRepo("isolated"),
new CachingPackageLocator() {
@Override
- public Path getBuildFileForPackage(PackageIdentifier packageId) {
- String packageName = packageId.getPackageFragment().getPathString();
+ public Path getBuildFileForPackage(String packageName) {
if (packageName.equals("isolated")) {
return scratch.resolve("isolated/BUILD");
} else if (packageName.equals("isolated/sub")) {
diff --git a/src/test/java/com/google/devtools/build/lib/packages/util/PackageFactoryApparatus.java b/src/test/java/com/google/devtools/build/lib/packages/util/PackageFactoryApparatus.java
index e769e38..bc641dd 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/util/PackageFactoryApparatus.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/util/PackageFactoryApparatus.java
@@ -160,7 +160,7 @@
public static CachingPackageLocator createEmptyLocator() {
return new CachingPackageLocator() {
@Override
- public Path getBuildFileForPackage(PackageIdentifier packageName) {
+ public Path getBuildFileForPackage(String packageName) {
return null;
}
};
diff --git a/src/test/java/com/google/devtools/build/lib/packages/util/PackageLoadingTestCase.java b/src/test/java/com/google/devtools/build/lib/packages/util/PackageLoadingTestCase.java
index 133e76d..e789d51 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/util/PackageLoadingTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/util/PackageLoadingTestCase.java
@@ -94,7 +94,7 @@
private void setUpSkyframe(PackageCacheOptions packageCacheOptions) {
PathPackageLocator pkgLocator = PathPackageLocator.create(
- null, packageCacheOptions.packagePath, reporter, rootDirectory, rootDirectory);
+ packageCacheOptions.packagePath, reporter, rootDirectory, rootDirectory);
skyframeExecutor.preparePackageLoading(pkgLocator,
packageCacheOptions.defaultVisibility, true,
7, ruleClassProvider.getDefaultsPackageContent(),
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/BuildFileASTTest.java b/src/test/java/com/google/devtools/build/lib/syntax/BuildFileASTTest.java
index a1b108e..8456235 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/BuildFileASTTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/BuildFileASTTest.java
@@ -25,7 +25,6 @@
import com.google.devtools.build.lib.events.Reporter;
import com.google.devtools.build.lib.events.util.EventCollectionApparatus;
import com.google.devtools.build.lib.packages.CachingPackageLocator;
-import com.google.devtools.build.lib.packages.PackageIdentifier;
import com.google.devtools.build.lib.testutil.JunitTestUtils;
import com.google.devtools.build.lib.testutil.Scratch;
import com.google.devtools.build.lib.vfs.Path;
@@ -46,8 +45,8 @@
private class ScratchPathPackageLocator implements CachingPackageLocator {
@Override
- public Path getBuildFileForPackage(PackageIdentifier packageName) {
- return scratch.resolve(packageName.getPackageFragment()).getRelative("BUILD");
+ public Path getBuildFileForPackage(String packageName) {
+ return scratch.resolve(packageName).getRelative("BUILD");
}
}
@@ -265,7 +264,7 @@
private class EmptyPackageLocator implements CachingPackageLocator {
@Override
- public Path getBuildFileForPackage(PackageIdentifier packageName) {
+ public Path getBuildFileForPackage(String packageName) {
return null;
}
}
diff --git a/src/test/shell/bazel/local_repository_test.sh b/src/test/shell/bazel/local_repository_test.sh
index bba1234..5a587e6 100755
--- a/src/test/shell/bazel/local_repository_test.sh
+++ b/src/test/shell/bazel/local_repository_test.sh
@@ -481,29 +481,4 @@
assert_contains "Michaelangelo" bazel-genfiles/tmnt
}
-function test_globs() {
- local r=$TEST_TMPDIR/r
- mkdir -p $r
- cat > WORKSPACE <<EOF
-local_repository(
- name = "r",
- path = "$r",
-)
-
-EOF
-
- cat > $r/BUILD <<EOF
-filegroup(
- name = "fg",
- srcs = glob(["**"]),
-)
-EOF
-
- touch $r/a
- mkdir -p $r/b
- touch $r/b/{BUILD,b}
-
- bazel build @r//:fg || fail "build failed"
-}
-
run_suite "local repository tests"
