Add the --experimental_disable_external package command line option that (unsurprisingly) disables the //external package
This has the side effect of allowing labels in the unnamed package to start
with "external" (e.g. //:external/foo/bar)
RELNOTES: None.
PiperOrigin-RevId: 296237592
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/ninja/actions/NinjaGraph.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/ninja/actions/NinjaGraph.java
index db819f5..42e8d8e 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/ninja/actions/NinjaGraph.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/ninja/actions/NinjaGraph.java
@@ -240,6 +240,10 @@
Environment env = ruleContext.getAnalysisEnvironment().getSkyframeEnv();
ImmutableSortedSet<String> notSymlinkedDirs =
ExternalPackageUtil.getNotSymlinkedInExecrootDirectories(env);
+ if (env.valuesMissing()) {
+ return;
+ }
+
// We can compare strings because notSymlinkedDirs contains normalized directory names
if (!notSymlinkedDirs.contains(outputRoot.getPathString())) {
ruleContext.attributeError(
diff --git a/src/main/java/com/google/devtools/build/lib/cmdline/LabelConstants.java b/src/main/java/com/google/devtools/build/lib/cmdline/LabelConstants.java
index 1608ed0..a110064 100644
--- a/src/main/java/com/google/devtools/build/lib/cmdline/LabelConstants.java
+++ b/src/main/java/com/google/devtools/build/lib/cmdline/LabelConstants.java
@@ -17,9 +17,22 @@
/** Constants associated with {@code Label}s */
public class LabelConstants {
+ /** The subdirectory under the output base which contains external repositories. */
+ public static final PathFragment EXTERNAL_REPOSITORY_LOCATION = PathFragment.create("external");
+
+ /**
+ * The path name under which external repositories are accessible if {@code
+ * --experimental_sibling_repository_layout} is not in effect.
+ */
public static final PathFragment EXTERNAL_PACKAGE_NAME = PathFragment.create("external");
+
+ /**
+ * The name of the package containing information about external repositories if {@code
+ * --experimental_disable_external_package} is not in effect.
+ */
public static final PackageIdentifier EXTERNAL_PACKAGE_IDENTIFIER =
PackageIdentifier.createInMainRepo(EXTERNAL_PACKAGE_NAME);
+
public static final PathFragment WORKSPACE_FILE_NAME = PathFragment.create("WORKSPACE");
public static final PathFragment WORKSPACE_DOT_BAZEL_FILE_NAME =
PathFragment.create("WORKSPACE.bazel");
diff --git a/src/main/java/com/google/devtools/build/lib/cmdline/RepositoryName.java b/src/main/java/com/google/devtools/build/lib/cmdline/RepositoryName.java
index 5f4d455..3e14d05 100644
--- a/src/main/java/com/google/devtools/build/lib/cmdline/RepositoryName.java
+++ b/src/main/java/com/google/devtools/build/lib/cmdline/RepositoryName.java
@@ -250,7 +250,7 @@
public PathFragment getSourceRoot() {
return isDefault() || isMain()
? PathFragment.EMPTY_FRAGMENT
- : LabelConstants.EXTERNAL_PACKAGE_NAME.getRelative(strippedName());
+ : LabelConstants.EXTERNAL_REPOSITORY_LOCATION.getRelative(strippedName());
}
/**
diff --git a/src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java b/src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java
index 31c3218..3c82d52 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java
@@ -215,6 +215,20 @@
public boolean experimentalRepoRemoteExec;
@Option(
+ name = "experimental_disable_external_package",
+ defaultValue = "false",
+ documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS,
+ effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS, OptionEffectTag.LOSES_INCREMENTAL_STATE},
+ metadataTags = {
+ OptionMetadataTag.EXPERIMENTAL,
+ },
+ help =
+ "If set to true, the auto-generated //external package will not be available anymore. "
+ + "Bazel will still be unable to parse the file 'external/BUILD', but globs reaching "
+ + "into external/ from the unnamed package will work.")
+ public boolean experimentalDisableExternalPackage;
+
+ @Option(
name = "experimental_sibling_repository_layout",
defaultValue = "false",
documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS,
@@ -654,6 +668,7 @@
.experimentalStarlarkUnusedInputsList(experimentalStarlarkUnusedInputsList)
.experimentalCcSharedLibrary(experimentalCcSharedLibrary)
.experimentalRepoRemoteExec(experimentalRepoRemoteExec)
+ .experimentalDisableExternalPackage(experimentalDisableExternalPackage)
.experimentalSiblingRepositoryLayout(experimentalSiblingRepositoryLayout)
.incompatibleApplicableLicenses(incompatibleApplicableLicenses)
.incompatibleBzlDisallowLoadAfterStatement(incompatibleBzlDisallowLoadAfterStatement)
diff --git a/src/main/java/com/google/devtools/build/lib/query2/ParallelSkyQueryUtils.java b/src/main/java/com/google/devtools/build/lib/query2/ParallelSkyQueryUtils.java
index 24267e0..9e81dbd 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/ParallelSkyQueryUtils.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/ParallelSkyQueryUtils.java
@@ -154,7 +154,7 @@
/*resultUniquifier=*/ env.createSkyKeyUniquifier(),
context,
callback);
- visitor.visitAndWaitForCompletion(env.getFileStateKeysForFileFragments(fileIdentifiers));
+ visitor.visitAndWaitForCompletion(env.getSkyKeysForFileFragments(fileIdentifiers));
}
static QueryTaskFuture<Void> getDepsUnboundedParallel(
@@ -234,4 +234,3 @@
}
}
}
-
diff --git a/src/main/java/com/google/devtools/build/lib/query2/RBuildFilesVisitor.java b/src/main/java/com/google/devtools/build/lib/query2/RBuildFilesVisitor.java
index ed5dcd5..73b638c 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/RBuildFilesVisitor.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/RBuildFilesVisitor.java
@@ -16,7 +16,6 @@
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import com.google.devtools.build.lib.cmdline.Label;
-import com.google.devtools.build.lib.cmdline.LabelConstants;
import com.google.devtools.build.lib.cmdline.PackageIdentifier;
import com.google.devtools.build.lib.collect.compacthashset.CompactHashSet;
import com.google.devtools.build.lib.packages.Target;
@@ -25,7 +24,6 @@
import com.google.devtools.build.lib.query2.engine.QueryException;
import com.google.devtools.build.lib.query2.engine.QueryExpressionContext;
import com.google.devtools.build.lib.query2.engine.Uniquifier;
-import com.google.devtools.build.lib.skyframe.PackageValue;
import com.google.devtools.build.lib.skyframe.SkyFunctions;
import com.google.devtools.build.skyframe.SkyFunctionName;
import com.google.devtools.build.skyframe.SkyKey;
@@ -54,8 +52,6 @@
SkyFunctions.PREPARE_DEPS_OF_PATTERN,
SkyFunctions.PREPARE_DEPS_OF_PATTERNS);
- private static final SkyKey EXTERNAL_PACKAGE_KEY =
- PackageValue.key(LabelConstants.EXTERNAL_PACKAGE_IDENTIFIER);
private final SkyQueryEnvironment env;
private final QueryExpressionContext<Target> context;
private final Uniquifier<SkyKey> visitUniquifier;
@@ -89,13 +85,6 @@
if (resultUniquifier.unique(rdep)) {
keysToUseForResult.add((PackageIdentifier) rdep.argument());
}
- // PackageValue(//p) has a transitive dep on the PackageValue(//external), so we need to
- // make sure these dep paths are traversed. These dep paths go through the singleton
- // WorkspaceNameValue(), and that node has a direct dep on PackageValue(//external), so it
- // suffices to ensure we visit PackageValue(//external).
- if (rdep.equals(EXTERNAL_PACKAGE_KEY)) {
- keysToVisitNext.add(rdep);
- }
} else if (!NODES_TO_PRUNE_TRAVERSAL.contains(rdep.functionName())) {
processNonPackageRdepAndDetermineVisitations(rdep, keysToVisitNext, keysToUseForResult);
}
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 3678032..6f5e062 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
@@ -1211,15 +1211,8 @@
return target;
}
- /**
- * Returns package lookup keys for looking up the package root for which there may be a relevant
- * (from the perspective of {@link #getRBuildFiles}) {@link FileStateValue} node in the graph for
- * {@code originalFileFragment}, which is assumed to be a file path.
- *
- * <p>This is a helper function for {@link #getFileStateKeysForFileFragments}.
- */
- private static Iterable<SkyKey> getPkgLookupKeysForFile(PathFragment originalFileFragment,
- PathFragment currentPathFragment) {
+ private static ImmutableList<SkyKey> getDependenciesForWorkspaceFile(
+ PathFragment originalFileFragment, PathFragment currentPathFragment) {
if (originalFileFragment.equals(currentPathFragment)
&& WorkspaceFileHelper.matchWorkspaceFileName(originalFileFragment)) {
// TODO(mschaller): this should not be checked at runtime. These are constants!
@@ -1228,10 +1221,29 @@
.getParentDirectory()
.equals(PathFragment.EMPTY_FRAGMENT),
LabelConstants.WORKSPACE_FILE_NAME);
- return ImmutableList.of(
- PackageLookupValue.key(LabelConstants.EXTERNAL_PACKAGE_IDENTIFIER),
- PackageLookupValue.key(PackageIdentifier.createInMainRepo(PathFragment.EMPTY_FRAGMENT)));
+ // The WORKSPACE file is a transitive dependency of every package. Unfortunately, there is no
+ // specific SkyValue that we can use to figure out under which package path entries it lives,
+ // so we add another dependency that's a transitive dependency of every package.
+ //
+ // The PackageLookupValue for //external is not a good choice, because it's not requested when
+ // that package is built so that we can have a directory called external/ in the workspace
+ // (this is special-cased in PackageFunction). The principled solution would be to not rely
+ // on external/ and introduce a SkyValue for the concept of "the WORKSPACE file of the main
+ // repository", but we don't have that yet, so we improvise.
+ return ImmutableList.of(PackageValue.key(LabelConstants.EXTERNAL_PACKAGE_IDENTIFIER));
}
+
+ return ImmutableList.of();
+ }
+
+ /**
+ * Returns package lookup keys for looking up the package root for which there may be a relevant
+ * (from the perspective of {@link #getRBuildFiles}) {@link FileStateValue} node in the graph for
+ * {@code originalFileFragment}, which is assumed to be a file path.
+ *
+ * <p>This is a helper function for {@link #getSkyKeysForFileFragments}.
+ */
+ private static Iterable<SkyKey> getPkgLookupKeysForFile(PathFragment currentPathFragment) {
PathFragment parentPathFragment = currentPathFragment.getParentDirectory();
return parentPathFragment == null
? ImmutableList.of()
@@ -1240,19 +1252,26 @@
}
/**
- * Returns FileStateValue keys for which there may be relevant (from the perspective of {@link
+ * Returns SkyKeys for which there may be relevant (from the perspective of {@link
* #getRBuildFiles}) FileStateValues in the graph corresponding to the given {@code
* pathFragments}, which are assumed to be file paths.
*
- * <p>To do this, we emulate the {@link ContainingPackageLookupFunction} logic: for each given
- * file path, we look for the nearest ancestor directory (starting with its parent directory), if
- * any, that has a package. The {@link PackageLookupValue} for this package tells us the package
- * root that we should use for the {@link RootedPath} for the {@link FileStateValue} key.
+ * <p>The passed in {@link PathFragment}s can be associated uniquely to a {@link FileStateValue}
+ * with the following logic (the same logic that's in {@link ContainingPackageLookupFunction}):
+ * For each given file path, we look for the nearest ancestor directory (starting with its parent
+ * directory), if any, that has a package. The {@link PackageLookupValue} for this package tells
+ * us the package root that we should use for the {@link RootedPath} for the {@link
+ * FileStateValue} key.
+ *
+ * <p>For the reverse graph traversal in {@link #getRBuildFiles}, we are looking for all packages
+ * that are transitively reverse dependencies of those {@link FileStateValue} keys. This function
+ * returns a collection of SkyKeys whose transitive reverse dependencies must contain the exact
+ * same set of packages.
*
* <p>Note that there may not be nodes in the graph corresponding to the returned SkyKeys.
*/
- protected Collection<SkyKey> getFileStateKeysForFileFragments(
- Iterable<PathFragment> pathFragments) throws InterruptedException {
+ protected Collection<SkyKey> getSkyKeysForFileFragments(Iterable<PathFragment> pathFragments)
+ throws InterruptedException {
Set<SkyKey> result = new HashSet<>();
Multimap<PathFragment, PathFragment> currentToOriginal = ArrayListMultimap.create();
for (PathFragment pathFragment : pathFragments) {
@@ -1264,10 +1283,12 @@
for (Map.Entry<PathFragment, PathFragment> entry : currentToOriginal.entries()) {
PathFragment current = entry.getKey();
PathFragment original = entry.getValue();
- for (SkyKey packageLookupKey : getPkgLookupKeysForFile(original, current)) {
+ for (SkyKey packageLookupKey : getPkgLookupKeysForFile(current)) {
packageLookupKeysToOriginal.put(packageLookupKey, original);
packageLookupKeysToCurrent.put(packageLookupKey, current);
}
+
+ result.addAll(getDependenciesForWorkspaceFile(original, current));
}
Map<SkyKey, SkyValue> lookupValues =
graph.getSuccessfulValues(packageLookupKeysToOriginal.keySet());
diff --git a/src/main/java/com/google/devtools/build/lib/repository/ExternalPackageUtil.java b/src/main/java/com/google/devtools/build/lib/repository/ExternalPackageUtil.java
index d5d558d..5b91936 100644
--- a/src/main/java/com/google/devtools/build/lib/repository/ExternalPackageUtil.java
+++ b/src/main/java/com/google/devtools/build/lib/repository/ExternalPackageUtil.java
@@ -14,14 +14,21 @@
package com.google.devtools.build.lib.repository;
+import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSortedSet;
+import com.google.common.collect.Iterables;
+import com.google.devtools.build.lib.actions.FileValue;
import com.google.devtools.build.lib.cmdline.LabelConstants;
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.packages.BuildFileContainsErrorsException;
+import com.google.devtools.build.lib.packages.BuildFileName;
import com.google.devtools.build.lib.packages.Package;
import com.google.devtools.build.lib.packages.Rule;
import com.google.devtools.build.lib.packages.WorkspaceFileValue;
-import com.google.devtools.build.lib.skyframe.PackageLookupValue;
+import com.google.devtools.build.lib.pkgcache.PathPackageLocator;
+import com.google.devtools.build.lib.skyframe.PrecomputedValue;
+import com.google.devtools.build.lib.vfs.PathFragment;
+import com.google.devtools.build.lib.vfs.Root;
import com.google.devtools.build.lib.vfs.RootedPath;
import com.google.devtools.build.skyframe.SkyFunction.Environment;
import com.google.devtools.build.skyframe.SkyFunctionException.Transience;
@@ -68,16 +75,63 @@
return extractor.getRule();
}
+ private static RootedPath checkWorkspaceFile(
+ Environment env, Root root, PathFragment workspaceFile) throws InterruptedException {
+ RootedPath candidate = RootedPath.toRootedPath(root, workspaceFile);
+ FileValue fileValue = (FileValue) env.getValue(FileValue.key(candidate));
+ if (env.valuesMissing()) {
+ return null;
+ }
+
+ return fileValue.isFile() ? candidate : null;
+ }
+
+ /**
+ * Returns the path of the main WORKSPACE file or null when a Skyframe restart is required.
+ *
+ * <p>Should also return null when the WORKSPACE file is not present, but then some tests break,
+ * so then it lies and returns the RootedPath corresponding to the last package path entry.
+ */
+ @Nullable
+ public static RootedPath findWorkspaceFile(Environment env) throws InterruptedException {
+ PathPackageLocator packageLocator = PrecomputedValue.PATH_PACKAGE_LOCATOR.get(env);
+ ImmutableList<Root> packagePath = packageLocator.getPathEntries();
+ for (Root candidateRoot : packagePath) {
+ RootedPath path =
+ checkWorkspaceFile(
+ env, candidateRoot, BuildFileName.WORKSPACE_DOT_BAZEL.getFilenameFragment());
+ if (env.valuesMissing()) {
+ return null;
+ }
+
+ if (path != null) {
+ return path;
+ }
+
+ path = checkWorkspaceFile(env, candidateRoot, BuildFileName.WORKSPACE.getFilenameFragment());
+ if (env.valuesMissing()) {
+ return null;
+ }
+
+ if (path != null) {
+ return path;
+ }
+ }
+
+ // TODO(lberki): Technically, this means that the WORKSPACE file was not found. I'd love to not
+ // have this here, but a lot of tests break without it because they rely on Bazel kinda working
+ // even if the WORKSPACE file is not present.
+ return RootedPath.toRootedPath(
+ Iterables.getLast(packagePath), BuildFileName.WORKSPACE.getFilenameFragment());
+ }
+
/** Returns false if some SkyValues were missing. */
private static boolean iterateWorkspaceFragments(
Environment env, WorkspaceFileValueProcessor processor) throws InterruptedException {
- SkyKey packageLookupKey = PackageLookupValue.key(LabelConstants.EXTERNAL_PACKAGE_IDENTIFIER);
- PackageLookupValue packageLookupValue = (PackageLookupValue) env.getValue(packageLookupKey);
- if (packageLookupValue == null) {
+ RootedPath workspacePath = findWorkspaceFile(env);
+ if (env.valuesMissing()) {
return false;
}
- RootedPath workspacePath =
- packageLookupValue.getRootedPath(LabelConstants.EXTERNAL_PACKAGE_IDENTIFIER);
SkyKey workspaceKey = WorkspaceFileValue.key(workspacePath);
WorkspaceFileValue value;
diff --git a/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryFunction.java b/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryFunction.java
index 8b8d82f..95b211b 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryFunction.java
@@ -533,7 +533,7 @@
}
protected static Path getExternalRepositoryDirectory(BlazeDirectories directories) {
- return directories.getOutputBase().getRelative(LabelConstants.EXTERNAL_PACKAGE_NAME);
+ return directories.getOutputBase().getRelative(LabelConstants.EXTERNAL_REPOSITORY_LOCATION);
}
/**
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ExternalFilesHelper.java b/src/main/java/com/google/devtools/build/lib/skyframe/ExternalFilesHelper.java
index 1fcbc4e..df04a16 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ExternalFilesHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ExternalFilesHelper.java
@@ -245,7 +245,7 @@
return FileType.EXTERNAL;
}
if (rootedPath.asPath().startsWith(outputBase)) {
- Path externalRepoDir = outputBase.getRelative(LabelConstants.EXTERNAL_PACKAGE_NAME);
+ Path externalRepoDir = outputBase.getRelative(LabelConstants.EXTERNAL_REPOSITORY_LOCATION);
if (rootedPath.asPath().startsWith(externalRepoDir)) {
anyNonOutputExternalFilesSeen = true;
return FileType.EXTERNAL_REPO;
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ExternalPackageFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ExternalPackageFunction.java
index 47791d5..71fa63a 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ExternalPackageFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ExternalPackageFunction.java
@@ -17,6 +17,7 @@
import com.google.common.collect.Interner;
import com.google.devtools.build.lib.concurrent.BlazeInterners;
import com.google.devtools.build.lib.packages.WorkspaceFileValue;
+import com.google.devtools.build.lib.repository.ExternalPackageUtil;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
import com.google.devtools.build.lib.vfs.RootedPath;
import com.google.devtools.build.skyframe.AbstractSkyKey;
@@ -27,17 +28,23 @@
import javax.annotation.Nullable;
/**
- * A SkyFunction for resolving //external:* bindings.
+ * A SkyFunction for parsing the {@code //external} package.
*
* <p>This function iterates through the WorkspaceFileValue-s to get the last WorkspaceFileValue
- * that will contains all the bind statements from the workspace file.
+ * that will contain all the bind statements from the WORKSPACE file.
*/
public class ExternalPackageFunction implements SkyFunction {
@Nullable
@Override
public SkyValue compute(SkyKey skyKey, Environment env) throws InterruptedException {
- RootedPath workspacePath = (RootedPath) skyKey.argument();
+ RootedPath workspacePath = ExternalPackageUtil.findWorkspaceFile(env);
+ if (env.valuesMissing()) {
+ return null;
+ }
+
+ // This currently cannot be null due to a hack in ExternalPackageUtil.findWorkspaceFile()
+ // TODO(lberki): Remove that hack and handle the case when the WORKSPACE file is not found.
SkyKey key = WorkspaceFileValue.key(workspacePath);
WorkspaceFileValue value = (WorkspaceFileValue) env.getValue(key);
if (value == null) {
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/LocalRepositoryLookupFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/LocalRepositoryLookupFunction.java
index 0e87677..86e21c1 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/LocalRepositoryLookupFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/LocalRepositoryLookupFunction.java
@@ -18,18 +18,17 @@
import com.google.common.collect.Iterables;
import com.google.devtools.build.lib.actions.FileValue;
import com.google.devtools.build.lib.actions.InconsistentFilesystemException;
-import com.google.devtools.build.lib.cmdline.LabelConstants;
import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
import com.google.devtools.build.lib.cmdline.RepositoryName;
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.packages.AggregatingAttributeMapper;
-import com.google.devtools.build.lib.packages.BuildFileNotFoundException;
import com.google.devtools.build.lib.packages.ErrorDeterminingRepositoryException;
import com.google.devtools.build.lib.packages.Package;
import com.google.devtools.build.lib.packages.Package.NameConflictException;
import com.google.devtools.build.lib.packages.Rule;
import com.google.devtools.build.lib.packages.Type;
import com.google.devtools.build.lib.packages.WorkspaceFileValue;
+import com.google.devtools.build.lib.repository.ExternalPackageUtil;
import com.google.devtools.build.lib.rules.repository.LocalRepositoryRule;
import com.google.devtools.build.lib.rules.repository.WorkspaceFileHelper;
import com.google.devtools.build.lib.skyframe.PackageFunction.PackageFunctionException;
@@ -136,33 +135,11 @@
private Optional<LocalRepositoryLookupValue> maybeCheckWorkspaceForRepository(
Environment env, final RootedPath directory)
throws InterruptedException, LocalRepositoryLookupFunctionException {
- // Look up the main WORKSPACE file by the external package, to find all repositories.
- PackageLookupValue externalPackageLookupValue;
- try {
- externalPackageLookupValue =
- (PackageLookupValue)
- env.getValueOrThrow(
- PackageLookupValue.key(LabelConstants.EXTERNAL_PACKAGE_IDENTIFIER),
- BuildFileNotFoundException.class,
- InconsistentFilesystemException.class);
- if (externalPackageLookupValue == null) {
- return Optional.absent();
- }
- } catch (BuildFileNotFoundException e) {
- throw new LocalRepositoryLookupFunctionException(
- new ErrorDeterminingRepositoryException(
- "BuildFileNotFoundException while loading the //external package", e),
- Transience.PERSISTENT);
- } catch (InconsistentFilesystemException e) {
- throw new LocalRepositoryLookupFunctionException(
- new ErrorDeterminingRepositoryException(
- "InconsistentFilesystemException while loading the //external package", e),
- Transience.PERSISTENT);
+ RootedPath workspacePath = ExternalPackageUtil.findWorkspaceFile(env);
+ if (env.valuesMissing()) {
+ return Optional.absent();
}
- RootedPath workspacePath =
- externalPackageLookupValue.getRootedPath(LabelConstants.EXTERNAL_PACKAGE_IDENTIFIER);
-
SkyKey workspaceKey = WorkspaceFileValue.key(workspacePath);
do {
WorkspaceFileValue value;
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java
index 92454ad..a4b7240 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java
@@ -51,6 +51,7 @@
import com.google.devtools.build.lib.profiler.Profiler;
import com.google.devtools.build.lib.profiler.ProfilerTask;
import com.google.devtools.build.lib.profiler.SilentCloseable;
+import com.google.devtools.build.lib.repository.ExternalPackageUtil;
import com.google.devtools.build.lib.rules.repository.WorkspaceFileHelper;
import com.google.devtools.build.lib.skyframe.GlobValue.InvalidGlobPatternException;
import com.google.devtools.build.lib.skyframe.SkylarkImportLookupFunction.SkylarkImportFailedException;
@@ -292,26 +293,14 @@
* @throws PackageFunctionException if there is an error computing the workspace file or adding
* its rules to the //external package.
*/
- private SkyValue getExternalPackage(Environment env, Root packageLookupPath)
+ private SkyValue getExternalPackage(Environment env)
throws PackageFunctionException, InterruptedException {
StarlarkSemantics starlarkSemantics = PrecomputedValue.STARLARK_SEMANTICS.get(env);
- if (starlarkSemantics == null) {
+ RootedPath workspacePath = ExternalPackageUtil.findWorkspaceFile(env);
+ if (env.valuesMissing()) {
return null;
}
- RootedPath workspacePath;
- try {
- workspacePath = WorkspaceFileHelper.getWorkspaceRootedFile(packageLookupPath, env);
- if (workspacePath == null) {
- return null;
- }
- } catch (IOException e) {
- throw new PackageFunctionException(
- new NoSuchPackageException(
- LabelConstants.EXTERNAL_PACKAGE_IDENTIFIER,
- "Could not determine workspace file (\"WORKSPACE.bazel\" or \"WORKSPACE\"): "
- + e.getMessage()),
- Transience.PERSISTENT);
- }
+
SkyKey workspaceKey = ExternalPackageFunction.key(workspacePath);
PackageValue workspace = null;
try {
@@ -355,6 +344,9 @@
public SkyValue compute(SkyKey key, Environment env) throws PackageFunctionException,
InterruptedException {
PackageIdentifier packageId = (PackageIdentifier) key.argument();
+ if (packageId.equals(LabelConstants.EXTERNAL_PACKAGE_IDENTIFIER)) {
+ return getExternalPackage(env);
+ }
SkyKey packageLookupKey = PackageLookupValue.key(packageId);
PackageLookupValue packageLookupValue;
@@ -394,9 +386,6 @@
}
}
- if (packageId.equals(LabelConstants.EXTERNAL_PACKAGE_IDENTIFIER)) {
- return getExternalPackage(env, packageLookupValue.getRoot());
- }
WorkspaceNameValue workspaceNameValue =
(WorkspaceNameValue) env.getValue(WorkspaceNameValue.key());
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 187b5d8..0249681 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
@@ -14,7 +14,7 @@
package com.google.devtools.build.lib.skyframe;
import com.google.common.base.Preconditions;
-import com.google.common.collect.ImmutableList;
+import com.google.common.base.Verify;
import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.actions.FileValue;
import com.google.devtools.build.lib.actions.InconsistentFilesystemException;
@@ -27,6 +27,7 @@
import com.google.devtools.build.lib.packages.NoSuchPackageException;
import com.google.devtools.build.lib.packages.RepositoryFetchException;
import com.google.devtools.build.lib.pkgcache.PathPackageLocator;
+import com.google.devtools.build.lib.repository.ExternalPackageUtil;
import com.google.devtools.build.lib.syntax.EvalException;
import com.google.devtools.build.lib.syntax.StarlarkSemantics;
import com.google.devtools.build.lib.vfs.PathFragment;
@@ -71,6 +72,7 @@
public SkyValue compute(SkyKey skyKey, Environment env)
throws PackageLookupFunctionException, InterruptedException {
PathPackageLocator pkgLocator = PrecomputedValue.PATH_PACKAGE_LOCATOR.get(env);
+ StarlarkSemantics semantics = PrecomputedValue.STARLARK_SEMANTICS.get(env);
PackageIdentifier packageKey = (PackageIdentifier) skyKey.argument();
@@ -87,8 +89,12 @@
if (!packageKey.getRepository().isMain()) {
return computeExternalPackageLookupValue(skyKey, env, packageKey);
- } else if (packageKey.equals(LabelConstants.EXTERNAL_PACKAGE_IDENTIFIER)) {
- return computeWorkspacePackageLookupValue(env, pkgLocator.getPathEntries());
+ }
+
+ if (packageKey.equals(LabelConstants.EXTERNAL_PACKAGE_IDENTIFIER)) {
+ return semantics.experimentalDisableExternalPackage()
+ ? PackageLookupValue.NO_BUILD_FILE_VALUE
+ : computeWorkspacePackageLookupValue(env);
}
// Check .bazelignore file under main repository.
@@ -194,30 +200,6 @@
private PackageLookupValue getPackageLookupValue(
Environment env,
- ImmutableList<Root> packagePathEntries,
- PackageIdentifier packageIdentifier,
- BuildFileName buildFileName)
- throws PackageLookupFunctionException, InterruptedException {
-
- // TODO(bazel-team): The following is O(n^2) on the number of elements on the package path due
- // to having restart the SkyFunction after every new dependency. However, if we try to batch
- // the missing value keys, more dependencies than necessary will be declared. This wart can be
- // fixed once we have nicer continuation support [skyframe-loading]
- for (Root packagePathEntry : packagePathEntries) {
- PackageLookupValue result =
- getPackageLookupValue(env, packagePathEntry, packageIdentifier, buildFileName);
- if (result == null) {
- return null;
- }
- if (result != PackageLookupValue.NO_BUILD_FILE_VALUE) {
- return result;
- }
- }
- return PackageLookupValue.NO_BUILD_FILE_VALUE;
- }
-
- private PackageLookupValue getPackageLookupValue(
- Environment env,
Root packagePathEntry,
PackageIdentifier packageIdentifier,
BuildFileName buildFileName)
@@ -306,51 +288,29 @@
return false;
}
- private PackageLookupValue computeWorkspacePackageLookupValue(
- Environment env, ImmutableList<Root> packagePathEntries)
- throws PackageLookupFunctionException, InterruptedException {
- PackageLookupValue resultForWorkspaceDotBazel =
- getPackageLookupValue(
- env,
- packagePathEntries,
- LabelConstants.EXTERNAL_PACKAGE_IDENTIFIER,
- BuildFileName.WORKSPACE_DOT_BAZEL);
- if (resultForWorkspaceDotBazel == null) {
+ private static PackageLookupValue computeWorkspacePackageLookupValue(Environment env)
+ throws InterruptedException {
+ RootedPath workspaceFile = ExternalPackageUtil.findWorkspaceFile(env);
+ if (env.valuesMissing()) {
return null;
}
- if (resultForWorkspaceDotBazel.packageExists()) {
- return resultForWorkspaceDotBazel;
- }
- PackageLookupValue resultForWorkspace =
- getPackageLookupValue(
- env,
- packagePathEntries,
- LabelConstants.EXTERNAL_PACKAGE_IDENTIFIER,
- BuildFileName.WORKSPACE);
- if (resultForWorkspace == null) {
- return null;
- }
- if (resultForWorkspace.packageExists()) {
- return resultForWorkspace;
- }
- // Fall back on the last package path entry if there were any and nothing else worked.
- // TODO(kchodorow): get rid of this, the semantics are wrong (successful package lookup should
- // mean the package exists). a bunch of tests need to be rewritten first though.
- if (packagePathEntries.isEmpty()) {
+
+ if (workspaceFile == null) {
return PackageLookupValue.NO_BUILD_FILE_VALUE;
+ } else {
+ BuildFileName filename = null;
+ for (BuildFileName candidate : BuildFileName.values()) {
+ if (workspaceFile.getRootRelativePath().equals(candidate.getFilenameFragment())) {
+ filename = candidate;
+ break;
+ }
+ }
+
+ // Otherwise ExternalPackageUtil.findWorkspaceFile() returned something whose name is not in
+ // BuildFileName
+ Verify.verify(filename != null);
+ return PackageLookupValue.success(workspaceFile.getRoot(), filename);
}
- Root lastPackagePath = packagePathEntries.get(packagePathEntries.size() - 1);
- FileValue lastPackagePackagePathFileValue =
- getFileValue(
- RootedPath.toRootedPath(lastPackagePath, PathFragment.EMPTY_FRAGMENT),
- env,
- LabelConstants.EXTERNAL_PACKAGE_IDENTIFIER);
- if (lastPackagePackagePathFileValue == null) {
- return null;
- }
- return lastPackagePackagePathFileValue.exists()
- ? PackageLookupValue.success(lastPackagePath, BuildFileName.WORKSPACE)
- : PackageLookupValue.NO_BUILD_FILE_VALUE;
}
/**
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunction.java
index 4736075..0e7c87f 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunction.java
@@ -41,9 +41,7 @@
import java.util.function.Function;
import java.util.stream.Collectors;
-/**
- * A SkyFunction to parse WORKSPACE files.
- */
+/** A SkyFunction to parse the WORKSPACE file at a given path. */
public class WorkspaceFileFunction implements SkyFunction {
private final PackageFactory packageFactory;
@@ -68,9 +66,9 @@
throws WorkspaceFileFunctionException, InterruptedException {
WorkspaceFileKey key = (WorkspaceFileKey) skyKey.argument();
- RootedPath workspaceRoot = key.getPath();
- WorkspaceASTValue workspaceASTValue = (WorkspaceASTValue) env.getValue(
- WorkspaceASTValue.key(workspaceRoot));
+ RootedPath workspaceFile = key.getPath();
+ WorkspaceASTValue workspaceASTValue =
+ (WorkspaceASTValue) env.getValue(WorkspaceASTValue.key(workspaceFile));
if (workspaceASTValue == null) {
return null;
}
@@ -79,11 +77,9 @@
return null;
}
- RootedPath repoWorkspace =
- RootedPath.toRootedPath(workspaceRoot.getRoot(), workspaceRoot.getRootRelativePath());
Package.Builder builder =
packageFactory.newExternalPackageBuilder(
- repoWorkspace, ruleClassProvider.getRunfilesPrefix(), starlarkSemantics);
+ workspaceFile, ruleClassProvider.getRunfilesPrefix(), starlarkSemantics);
if (workspaceASTValue.getASTs().isEmpty()) {
try {
@@ -92,7 +88,7 @@
/* importMap = */ ImmutableMap.<String, Extension>of(),
/* importToChunkMap = */ ImmutableMap.<String, Integer>of(),
/* bindings = */ ImmutableMap.<String, Object>of(),
- workspaceRoot,
+ workspaceFile,
/* idx = */ 0, // first fragment
/* hasNext = */ false,
ImmutableMap.of(),
@@ -103,7 +99,7 @@
}
WorkspaceFactory parser;
WorkspaceFileValue prevValue = null;
- try (Mutability mutability = Mutability.create("workspace", repoWorkspace)) {
+ try (Mutability mutability = Mutability.create("workspace", workspaceFile)) {
parser =
new WorkspaceFactory(
builder,
@@ -118,7 +114,7 @@
if (key.getIndex() > 0) {
prevValue =
(WorkspaceFileValue)
- env.getValue(WorkspaceFileValue.key(key.getPath(), key.getIndex() - 1));
+ env.getValue(WorkspaceFileValue.key(workspaceFile, key.getIndex() - 1));
if (prevValue == null) {
return null;
}
@@ -130,7 +126,7 @@
StarlarkFile ast = workspaceASTValue.getASTs().get(key.getIndex());
PackageFunction.SkylarkImportResult importResult =
PackageFunction.fetchImportsFromBuildFile(
- repoWorkspace,
+ workspaceFile,
rootPackage,
/*repoMapping=*/ ImmutableMap.of(),
ast,
@@ -153,7 +149,7 @@
parser.getImportMap(),
createImportToChunkMap(prevValue, parser, key),
parser.getVariableBindings(),
- workspaceRoot,
+ workspaceFile,
key.getIndex(),
key.getIndex() < workspaceASTValue.getASTs().size() - 1,
ImmutableMap.copyOf(parser.getManagedDirectories()),
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java b/src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java
index 26bb70c..3278452 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java
@@ -53,6 +53,8 @@
public static final String EXPERIMENTAL_ACTION_ARGS = "experimental_action_args";
public static final String EXPERIMENTAL_ALLOW_INCREMENTAL_REPOSITORY_UPDATES =
"experimental_allow_incremental_repository_updates";
+ public static final String EXPERIMENTAL_DISABLE_EXTERNAL_PACKGE =
+ "experimental_disable_external_package";
public static final String EXPERIMENTAL_SIBLING_REPOSITORY_LAYOUT =
"experimental_sibling_repository_layout";
public static final String EXPERIMENTAL_ASPECT_OUTPUT_PROPAGATION =
@@ -96,6 +98,8 @@
return experimentalActionArgs();
case FlagIdentifier.EXPERIMENTAL_ALLOW_INCREMENTAL_REPOSITORY_UPDATES:
return experimentalAllowIncrementalRepositoryUpdates();
+ case FlagIdentifier.EXPERIMENTAL_DISABLE_EXTERNAL_PACKGE:
+ return experimentalDisableExternalPackage();
case FlagIdentifier.EXPERIMENTAL_SIBLING_REPOSITORY_LAYOUT:
return experimentalSiblingRepositoryLayout();
case FlagIdentifier.EXPERIMENTAL_ASPECT_OUTPUT_PROPAGATION:
@@ -203,6 +207,8 @@
public abstract boolean experimentalRepoRemoteExec();
+ public abstract boolean experimentalDisableExternalPackage();
+
public abstract boolean experimentalSiblingRepositoryLayout();
public abstract boolean incompatibleAlwaysCheckDepsetElements();
@@ -307,6 +313,7 @@
.experimentalStarlarkUnusedInputsList(true)
.experimentalCcSharedLibrary(false)
.experimentalRepoRemoteExec(false)
+ .experimentalDisableExternalPackage(false)
.experimentalSiblingRepositoryLayout(false)
.incompatibleAlwaysCheckDepsetElements(false)
.incompatibleApplicableLicenses(false)
@@ -371,6 +378,8 @@
public abstract Builder experimentalRepoRemoteExec(boolean value);
+ public abstract Builder experimentalDisableExternalPackage(boolean value);
+
public abstract Builder experimentalSiblingRepositoryLayout(boolean value);
public abstract Builder incompatibleAlwaysCheckDepsetElements(boolean value);
diff --git a/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java b/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java
index b13f2dc..743d169 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java
@@ -121,6 +121,7 @@
// <== Add new options here in alphabetic order ==>
"--debug_depset_depth=" + rand.nextBoolean(),
"--experimental_action_args=" + rand.nextBoolean(),
+ "--experimental_disable_external_package=" + rand.nextBoolean(),
"--experimental_sibling_repository_layout=" + rand.nextBoolean(),
"--experimental_allow_incremental_repository_updates=" + rand.nextBoolean(),
"--experimental_aspect_output_propagation=" + rand.nextBoolean(),
@@ -175,6 +176,7 @@
// <== Add new options here in alphabetic order ==>
.debugDepsetDepth(rand.nextBoolean())
.experimentalActionArgs(rand.nextBoolean())
+ .experimentalDisableExternalPackage(rand.nextBoolean())
.experimentalSiblingRepositoryLayout(rand.nextBoolean())
.experimentalAllowIncrementalRepositoryUpdates(rand.nextBoolean())
.experimentalAspectOutputPropagation(rand.nextBoolean())
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/FileFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/FileFunctionTest.java
index ea3efbf..d173a20 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/FileFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/FileFunctionTest.java
@@ -391,7 +391,7 @@
public void testAbsoluteSymlinkToExternal() throws Exception {
String externalPath =
outputBase
- .getRelative(LabelConstants.EXTERNAL_PACKAGE_NAME)
+ .getRelative(LabelConstants.EXTERNAL_REPOSITORY_LOCATION)
.getRelative("a/b")
.getPathString();
symlink("a", externalPath);
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunctionTest.java
index 69e78aa..02b80ad 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunctionTest.java
@@ -35,6 +35,7 @@
import com.google.devtools.build.lib.packages.Rule;
import com.google.devtools.build.lib.packages.WorkspaceFileValue;
import com.google.devtools.build.lib.packages.WorkspaceFileValue.WorkspaceFileKey;
+import com.google.devtools.build.lib.pkgcache.PathPackageLocator;
import com.google.devtools.build.lib.rules.repository.ManagedDirectoriesKnowledge;
import com.google.devtools.build.lib.rules.repository.ManagedDirectoriesKnowledgeImpl;
import com.google.devtools.build.lib.rules.repository.ManagedDirectoriesKnowledgeImpl.ManagedDirectoriesListener;
@@ -53,7 +54,6 @@
import com.google.devtools.build.skyframe.SkyFunction;
import com.google.devtools.build.skyframe.SkyFunctionName;
import com.google.devtools.build.skyframe.SkyKey;
-import com.google.devtools.build.skyframe.SkyValue;
import java.io.IOException;
import java.util.Objects;
import java.util.Set;
@@ -66,8 +66,6 @@
import org.junit.runners.JUnit4;
import org.mockito.Mockito;
import org.mockito.hamcrest.MockitoHamcrest;
-import org.mockito.invocation.InvocationOnMock;
-import org.mockito.stubbing.Answer;
/**
* Test for {@link WorkspaceFileFunction}.
@@ -107,6 +105,11 @@
}
@Override
+ public boolean isFile() {
+ return exists;
+ }
+
+ @Override
public ImmutableList<RootedPath> logicalChainDuringResolution() {
throw new UnsupportedOperationException();
}
@@ -183,46 +186,50 @@
}
private SkyFunction.Environment getEnv() throws InterruptedException {
+ PathPackageLocator locator = Mockito.mock(PathPackageLocator.class);
+ Mockito.when(locator.getPathEntries())
+ .thenReturn(ImmutableList.of(Root.fromPath(directories.getWorkspace())));
+
SkyFunction.Environment env = Mockito.mock(SkyFunction.Environment.class);
Mockito.when(env.getValue(MockitoHamcrest.argThat(new SkyKeyMatchers(FileValue.FILE))))
- .thenReturn(fakeWorkspaceFileValue);
+ .then(
+ invocation -> {
+ SkyKey key = (SkyKey) invocation.getArguments()[0];
+ String path = ((RootedPath) key.argument()).getRootRelativePath().getPathString();
+ FakeFileValue result = new FakeFileValue();
+ result.setExists(path.equals("WORKSPACE"));
+ return result;
+ });
Mockito.when(
env.getValue(
MockitoHamcrest.argThat(new SkyKeyMatchers(WorkspaceFileValue.WORKSPACE_FILE))))
.then(
- new Answer<SkyValue>() {
- @Override
- public SkyValue answer(InvocationOnMock invocation) throws Throwable {
- SkyKey key = (SkyKey) invocation.getArguments()[0];
- return workspaceSkyFunc.compute(key, getEnv());
- }
+ invocation -> {
+ SkyKey key = (SkyKey) invocation.getArguments()[0];
+ return workspaceSkyFunc.compute(key, getEnv());
});
Mockito.when(
env.getValue(MockitoHamcrest.argThat(new SkyKeyMatchers(SkyFunctions.WORKSPACE_AST))))
.then(
- new Answer<SkyValue>() {
- @Override
- public SkyValue answer(InvocationOnMock invocation) throws Throwable {
- SkyKey key = (SkyKey) invocation.getArguments()[0];
- return astSkyFunc.compute(key, getEnv());
- }
+ invocation -> {
+ SkyKey key = (SkyKey) invocation.getArguments()[0];
+ return astSkyFunc.compute(key, getEnv());
});
Mockito.when(
env.getValue(MockitoHamcrest.argThat(new SkyKeyMatchers(SkyFunctions.PRECOMPUTED))))
.then(
- new Answer<SkyValue>() {
- @Override
- public SkyValue answer(InvocationOnMock invocation) throws Throwable {
- SkyKey key = (SkyKey) invocation.getArguments()[0];
- if (key.equals(PrecomputedValue.STARLARK_SEMANTICS.getKeyForTesting())) {
- return new PrecomputedValue(StarlarkSemantics.DEFAULT_SEMANTICS);
- } else if (key.equals(
- RepositoryDelegatorFunction.RESOLVED_FILE_INSTEAD_OF_WORKSPACE
- .getKeyForTesting())) {
- return new PrecomputedValue(Optional.<RootedPath>absent());
- } else {
- return null;
- }
+ invocation -> {
+ SkyKey key = (SkyKey) invocation.getArguments()[0];
+ if (key.equals(PrecomputedValue.STARLARK_SEMANTICS.getKeyForTesting())) {
+ return new PrecomputedValue(StarlarkSemantics.DEFAULT_SEMANTICS);
+ } else if (key.equals(
+ RepositoryDelegatorFunction.RESOLVED_FILE_INSTEAD_OF_WORKSPACE
+ .getKeyForTesting())) {
+ return new PrecomputedValue(Optional.<RootedPath>absent());
+ } else if (key.equals(PrecomputedValue.PATH_PACKAGE_LOCATOR.getKeyForTesting())) {
+ return new PrecomputedValue(locator);
+ } else {
+ return null;
}
});
return env;
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/WorkspaceNameFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/WorkspaceNameFunctionTest.java
index b290e8d..4d68c18 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/WorkspaceNameFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/WorkspaceNameFunctionTest.java
@@ -68,12 +68,10 @@
reporter.removeHandler(failFastHandler);
scratch.deleteFile("WORKSPACE");
FileSystemUtils.ensureSymbolicLink(scratch.resolve("WORKSPACE"), "WORKSPACE");
- // Transitive errors from WorkspaceNameValue should manifest themselves as
- // NoSuchPackageExceptions.
assertThatEvaluationResult(eval())
.hasErrorEntryForKeyThat(key)
.hasExceptionThat()
- .isInstanceOf(NoSuchPackageException.class);
+ .isInstanceOf(FileSymlinkCycleException.class);
}
@Test
diff --git a/src/test/shell/bazel/execroot_test.sh b/src/test/shell/bazel/execroot_test.sh
index f0b842c..9c312e8 100755
--- a/src/test/shell/bazel/execroot_test.sh
+++ b/src/test/shell/bazel/execroot_test.sh
@@ -144,4 +144,68 @@
}
+function test_external_directory_globs() {
+ touch WORKSPACE
+
+ mkdir -p external/a external/c
+ echo file_ab > external/a/b
+ echo file_cd > external/c/d
+ echo file_e > external/e
+ touch external/a/b external/c/d external/e
+
+ cat > BUILD <<'EOF'
+filegroup(name='f', srcs=glob(["**/*"]))
+genrule(name="g", srcs=[":f"], outs=["go"], cmd="cat $(locations :f) > $@")
+EOF
+
+ bazel build //:g \
+ --experimental_disable_external_package \
+ --experimental_sibling_repository_layout \
+ || fail "build failed"
+ assert_contains file_ab bazel-bin/go
+ assert_contains file_cd bazel-bin/go
+ assert_contains file_e bazel-bin/go
+}
+
+function test_cc_smoke_with_new_layouts() {
+ touch WORKSPACE
+ mkdir -p external/a
+ cat > external/a/BUILD <<EOF
+cc_binary(name='a', srcs=['a.cc'])
+EOF
+
+ cat > external/a/a.cc <<EOF
+int main(void) {
+ return 0;
+}
+EOF
+
+ bazel build //external/a:a \
+ --experimental_disable_external_package \
+ --experimental_sibling_repository_layout \
+ || fail "build failed"
+}
+
+function test_java_smoke_with_new_layouts() {
+ touch WORKSPACE
+ mkdir -p external/java/a
+ cat > external/java/a/BUILD <<EOF
+java_binary(name='a', srcs=['A.java'])
+EOF
+
+ cat > external/java/a/A.java << EOF
+package a;
+public class A {
+ public static void main(String[] args) {
+ System.out.println("hello world");
+ }
+}
+EOF
+
+ bazel build //external/java/a:a \
+ --experimental_disable_external_package \
+ --experimental_sibling_repository_layout \
+ || fail "build failed"
+}
+
run_suite "execution root tests"