Support sibling repository execution root layout.
Enable building and depending on targets in subpackages of //external.
Closes #10604.
1) Added --experimental_sibling_repository_layout starlark semantics flag
2) When enabled, the flag changes the execution root of external repository source artifacts (non-default/main repository) from <execroot cwd>/external/reponame to <execroot cwd>/../reponame. Therefore, this allows plantSymlinkForest to plant the "external" top level directory in the execution root, like any other top level source directory in the main repository.
3) The artifact roots (and therefore root relative path) of external repos' source artifacts are left unchanged. This enables RepositoryName#getRunfilesPath and RepositoryName#getSourceRoot to remain unchanged. Thus we only needed to thread StarlarkSemantics#experimentalAllowExternalDirectory from BazelRepositoryModule / ruleContext to callsites of RepositoryName#getExecPath(boolean allowExternalRepository).
This took a while because I had to revert some changes in order to make the other changes less intrusive.
Specifically, for external source artifacts, rosica's change set output_base/external as a source root, instead output_base. This causes all external source artifact's rootRelativePath to drop "external" as a prefix. By retaining the output_base source root, I was able to undo a lot of the intrusive changes.
Another caveat: this change currently clobbers output artifacts from external repositories and subpackages of //external in the same derived output root (bazel-bin/external). This will be a problem if external repository names conflicts with directory names in the top-level external directory. We'll probably have to draft up a design following this change.
PiperOrigin-RevId: 294991422
diff --git a/src/main/java/com/google/devtools/build/lib/actions/Artifact.java b/src/main/java/com/google/devtools/build/lib/actions/Artifact.java
index a455087..4f46206 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/Artifact.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/Artifact.java
@@ -606,6 +606,11 @@
@Override
public PathFragment getRootRelativePath() {
+ // flag-less way of checking of the root is <execroot>/.., or sibling of __main__.
+ if (getExecPath().startsWith(LabelConstants.EXPERIMENTAL_EXTERNAL_PATH_PREFIX)) {
+ return LabelConstants.EXTERNAL_PATH_PREFIX.getRelative(getExecPath().subFragment(1));
+ }
+
return getExecPath();
}
diff --git a/src/main/java/com/google/devtools/build/lib/actions/ArtifactFactory.java b/src/main/java/com/google/devtools/build/lib/actions/ArtifactFactory.java
index 8eeeb4f..e56f4ae 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/ArtifactFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/ArtifactFactory.java
@@ -44,6 +44,7 @@
private final Path execRootParent;
private final PathFragment derivedPathPrefix;
private ImmutableMap<Root, ArtifactRoot> sourceArtifactRoots;
+ private boolean siblingRepositoryLayout = false;
/**
* Cache of source artifacts.
@@ -152,6 +153,10 @@
this.sourceArtifactRoots = sourceArtifactRoots;
}
+ public void setSiblingRepositoryLayout(boolean siblingRepositoryLayout) {
+ this.siblingRepositoryLayout = siblingRepositoryLayout;
+ }
+
/**
* Set the set of known packages and their corresponding source artifact roots. Must be called
* exactly once after construction or clear().
@@ -397,10 +402,18 @@
!relativePath.isEmpty(), "%s %s %s", relativePath, baseExecPath, baseRoot);
PathFragment execPath =
baseExecPath != null ? baseExecPath.getRelative(relativePath) : relativePath;
- if (execPath.containsUplevelReferences()) {
- // Source exec paths cannot escape the source root.
+
+ // Source exec paths cannot escape the source root.
+ if (siblingRepositoryLayout) {
+ // The exec path may start with .. if using --experimental_sibling_repository_layout, so test
+ // the subfragment from index 1 onwards.
+ if (execPath.subFragment(1).containsUplevelReferences()) {
+ return null;
+ }
+ } else if (execPath.containsUplevelReferences()) {
return null;
}
+
// Don't create an artifact if it's derived.
if (isDerivedArtifact(execPath)) {
return null;
@@ -430,7 +443,8 @@
return null;
}
- Pair<RepositoryName, PathFragment> repo = RepositoryName.fromPathFragment(dir);
+ Pair<RepositoryName, PathFragment> repo =
+ RepositoryName.fromPathFragment(dir, siblingRepositoryLayout);
if (repo != null) {
repositoryName = repo.getFirst();
dir = repo.getSecond();
diff --git a/src/main/java/com/google/devtools/build/lib/actions/ArtifactRoot.java b/src/main/java/com/google/devtools/build/lib/actions/ArtifactRoot.java
index 22abaf9..3316026 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/ArtifactRoot.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/ArtifactRoot.java
@@ -19,6 +19,7 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Interner;
import com.google.common.collect.Interners;
+import com.google.devtools.build.lib.cmdline.LabelConstants;
import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
import com.google.devtools.build.lib.skylarkbuildapi.FileRootApi;
@@ -60,6 +61,11 @@
return new ArtifactRoot(root, PathFragment.EMPTY_FRAGMENT, RootType.Source);
}
+ public static ArtifactRoot asExternalSourceRoot(Root root) {
+ return new ArtifactRoot(
+ root, LabelConstants.EXPERIMENTAL_EXTERNAL_PATH_PREFIX, RootType.Source);
+ }
+
/**
* Constructs an ArtifactRoot given the output prefixes. (eg, "bin"), and (eg, "testlogs")
* relative to the execRoot.
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java
index bba38e4..4e6c4eb 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java
@@ -238,7 +238,8 @@
visibility);
SourceArtifact artifact =
artifactFactory.getSourceArtifact(
- inputFile.getExecPath(),
+ inputFile.getExecPath(
+ analysisEnvironment.getSkylarkSemantics().experimentalSiblingRepositoryLayout()),
inputFile.getPackage().getSourceRoot(),
ConfiguredTargetKey.of(target.getLabel(), config));
return new InputFileConfiguredTarget(targetContext, inputFile, artifact);
diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionTool.java b/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionTool.java
index ad49c9c..e4ad3c1 100644
--- a/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionTool.java
+++ b/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionTool.java
@@ -66,6 +66,7 @@
import com.google.devtools.build.lib.exec.ExecutorLifecycleListener;
import com.google.devtools.build.lib.exec.SpawnActionContextMaps;
import com.google.devtools.build.lib.exec.SymlinkTreeStrategy;
+import com.google.devtools.build.lib.packages.StarlarkSemanticsOptions;
import com.google.devtools.build.lib.profiler.AutoProfiler;
import com.google.devtools.build.lib.profiler.ProfilePhase;
import com.google.devtools.build.lib.profiler.Profiler;
@@ -432,7 +433,9 @@
packageRootMap.get(),
getExecRoot(),
runtime.getProductName(),
- nonSymlinkedDirectoriesUnderExecRoot);
+ nonSymlinkedDirectoriesUnderExecRoot,
+ request.getOptions(StarlarkSemanticsOptions.class)
+ .experimentalSiblingRepositoryLayout);
symlinkForest.plantSymlinkForest();
} catch (IOException e) {
throw new ExecutorInitException("Source forest creation failed", e);
diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/SymlinkForest.java b/src/main/java/com/google/devtools/build/lib/buildtool/SymlinkForest.java
index eda2aaa..5a52959 100644
--- a/src/main/java/com/google/devtools/build/lib/buildtool/SymlinkForest.java
+++ b/src/main/java/com/google/devtools/build/lib/buildtool/SymlinkForest.java
@@ -51,11 +51,12 @@
private final String productName;
private final String prefix;
private final ImmutableSortedSet<String> notSymlinkedInExecrootDirectories;
+ private final boolean siblingRepositoryLayout;
/** Constructor for a symlink forest creator without non-symlinked directories parameter. */
public SymlinkForest(
ImmutableMap<PackageIdentifier, Root> packageRoots, Path execroot, String productName) {
- this(packageRoots, execroot, productName, ImmutableSortedSet.of());
+ this(packageRoots, execroot, productName, ImmutableSortedSet.of(), false);
}
/**
@@ -74,12 +75,14 @@
ImmutableMap<PackageIdentifier, Root> packageRoots,
Path execroot,
String productName,
- ImmutableSortedSet<String> notSymlinkedInExecrootDirectories) {
+ ImmutableSortedSet<String> notSymlinkedInExecrootDirectories,
+ boolean siblingRepositoryLayout) {
this.packageRoots = packageRoots;
this.execroot = execroot;
this.productName = productName;
this.prefix = productName + "-";
this.notSymlinkedInExecrootDirectories = notSymlinkedInExecrootDirectories;
+ this.siblingRepositoryLayout = siblingRepositoryLayout;
}
/**
@@ -123,8 +126,13 @@
// directory.
// From <output_base>/execroot/<main repo name>/external/<external repo name>
// to <output_base>/external/<external repo name>
- Path execrootLink = execroot.getRelative(repository.getPathUnderExecRoot());
- if (externalRepoLinks.isEmpty()) {
+ //
+ // However, if --experimental_sibling_repository_layout is true, symlink:
+ // From <output_base>/execroot/<external repo name>
+ // to <output_base>/external/<external repo name>
+ Path execrootLink = execroot.getRelative(repository.getExecPath(siblingRepositoryLayout));
+
+ if (!siblingRepositoryLayout && externalRepoLinks.isEmpty()) {
execroot.getRelative(LabelConstants.EXTERNAL_PACKAGE_NAME).createDirectoryAndParents();
}
if (!externalRepoLinks.add(execrootLink)) {
@@ -136,6 +144,9 @@
private void plantSymlinkForestWithFullMainRepository(Path mainRepoRoot) throws IOException {
// For the main repo top-level directory, generate symlinks to everything in the directory
// instead of the directory itself.
+ if (siblingRepositoryLayout) {
+ execroot.createDirectory();
+ }
for (Path target : mainRepoRoot.getDirectoryEntries()) {
String baseName = target.getBaseName();
if (this.notSymlinkedInExecrootDirectories.contains(baseName)) {
@@ -145,14 +156,19 @@
// Create any links that don't start with bazel-, and ignore external/ directory if
// user has it in the source tree because it conflicts with external repository location.
if (!baseName.startsWith(prefix)
- && !baseName.equals(LabelConstants.EXTERNAL_PATH_PREFIX.getBaseName())) {
+ && (siblingRepositoryLayout
+ || !baseName.equals(LabelConstants.EXTERNAL_PATH_PREFIX.getBaseName()))) {
execPath.createSymbolicLink(target);
+ // TODO(jingwen-external): is this creating execroot/io_bazel/external?
}
}
}
private void plantSymlinkForestWithPartialMainRepository(Map<Path, Path> mainRepoLinks)
throws IOException, AbruptExitException {
+ if (siblingRepositoryLayout) {
+ execroot.createDirectory();
+ }
for (Map.Entry<Path, Path> entry : mainRepoLinks.entrySet()) {
Path link = entry.getKey();
Path target = entry.getValue();
@@ -205,14 +221,14 @@
for (PackageIdentifier dir : dirsParentsFirst) {
if (!dir.getRepository().isMain()) {
execroot
- .getRelative(dir.getRepository().getPathUnderExecRoot())
+ .getRelative(dir.getRepository().getExecPath(siblingRepositoryLayout))
.createDirectoryAndParents();
}
if (dirRootsMap.get(dir).size() > 1) {
if (LOG_FINER) {
- logger.finer("mkdir " + execroot.getRelative(dir.getPathUnderExecRoot()));
+ logger.finer("mkdir " + execroot.getRelative(dir.getExecPath(siblingRepositoryLayout)));
}
- execroot.getRelative(dir.getPathUnderExecRoot()).createDirectoryAndParents();
+ execroot.getRelative(dir.getExecPath(siblingRepositoryLayout)).createDirectoryAndParents();
}
}
@@ -232,9 +248,10 @@
"ln -s "
+ root.getRelative(dir.getSourceRoot())
+ " "
- + execroot.getRelative(dir.getPathUnderExecRoot()));
+ + execroot.getRelative(dir.getExecPath(siblingRepositoryLayout)));
}
- execroot.getRelative(dir.getPathUnderExecRoot())
+ execroot
+ .getRelative(dir.getExecPath(siblingRepositoryLayout))
.createSymbolicLink(root.getRelative(dir.getSourceRoot()));
}
}
@@ -275,7 +292,7 @@
if (!pkgId.getPackageFragment().equals(PathFragment.EMPTY_FRAGMENT)) {
continue;
}
- Path execrootDirectory = execroot.getRelative(pkgId.getPathUnderExecRoot());
+ Path execrootDirectory = execroot.getRelative(pkgId.getExecPath(siblingRepositoryLayout));
// If there were no subpackages, this directory might not exist yet.
if (!execrootDirectory.exists()) {
execrootDirectory.createDirectoryAndParents();
@@ -298,6 +315,15 @@
public void plantSymlinkForest() throws IOException, AbruptExitException {
deleteTreesBelowNotPrefixed(execroot, prefix);
+ if (siblingRepositoryLayout) {
+ // Delete execroot/../<symlinks> to directories representing external repositories.
+ for (Path p : execroot.getParentDirectory().getDirectoryEntries()) {
+ if (p.isSymbolicLink()) {
+ p.deleteTree();
+ }
+ }
+ }
+
boolean shouldLinkAllTopLevelItems = false;
Map<Path, Path> mainRepoLinks = Maps.newLinkedHashMap();
Set<Root> mainRepoRoots = Sets.newLinkedHashSet();
@@ -306,7 +332,7 @@
for (Map.Entry<PackageIdentifier, Root> entry : packageRoots.entrySet()) {
PackageIdentifier pkgId = entry.getKey();
- if (pkgId.equals(LabelConstants.EXTERNAL_PACKAGE_IDENTIFIER)) {
+ if (!siblingRepositoryLayout && pkgId.equals(LabelConstants.EXTERNAL_PACKAGE_IDENTIFIER)) {
// This isn't a "real" package, don't add it to the symlink tree.
continue;
}
@@ -327,9 +353,10 @@
shouldLinkAllTopLevelItems = true;
} else {
String baseName = pkgId.getPackageFragment().getSegment(0);
- // ignore external/ directory if user has it in the source tree
- // because it conflicts with external repository location.
- if (baseName.equals(LabelConstants.EXTERNAL_PATH_PREFIX.getBaseName())) {
+ if (!siblingRepositoryLayout
+ && baseName.equals(LabelConstants.EXTERNAL_PATH_PREFIX.getBaseName())) {
+ // ignore external/ directory if user has it in the source tree
+ // because it conflicts with external repository location.
continue;
}
Path execrootLink = execroot.getRelative(baseName);
diff --git a/src/main/java/com/google/devtools/build/lib/cmdline/Label.java b/src/main/java/com/google/devtools/build/lib/cmdline/Label.java
index 41374dd..e4154be 100644
--- a/src/main/java/com/google/devtools/build/lib/cmdline/Label.java
+++ b/src/main/java/com/google/devtools/build/lib/cmdline/Label.java
@@ -30,6 +30,7 @@
import com.google.devtools.build.lib.skylarkinterface.SkylarkModuleCategory;
import com.google.devtools.build.lib.syntax.Printer;
import com.google.devtools.build.lib.syntax.SkylarkType;
+import com.google.devtools.build.lib.syntax.StarlarkSemantics;
import com.google.devtools.build.lib.syntax.StarlarkThread;
import com.google.devtools.build.lib.syntax.StarlarkValue;
import com.google.devtools.build.lib.util.StringUtilities;
@@ -368,16 +369,20 @@
* it will returns an empty string.
*/
@SkylarkCallable(
- name = "workspace_root",
- structField = true,
- doc =
- "Returns the execution root for the workspace of this label, relative to the execroot. "
- + "For instance:<br>"
- + "<pre class=language-python>Label(\"@repo//pkg/foo:abc\").workspace_root =="
- + " \"external/repo\"</pre>"
- )
- public String getWorkspaceRoot() {
- return packageIdentifier.getRepository().getSourceRoot().toString();
+ name = "workspace_root",
+ structField = true,
+ doc =
+ "Returns the execution root for the workspace of this label, relative to the execroot. "
+ + "For instance:<br>"
+ + "<pre class=language-python>Label(\"@repo//pkg/foo:abc\").workspace_root =="
+ + " \"external/repo\"</pre>",
+ useStarlarkSemantics = true)
+ public String getWorkspaceRoot(StarlarkSemantics semantics) {
+ if (semantics.experimentalSiblingRepositoryLayout()) {
+ return packageIdentifier.getRepository().getExecPath(true).toString();
+ } else {
+ return packageIdentifier.getRepository().getSourceRoot().toString();
+ }
}
/**
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 042b049..1608ed0 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
@@ -20,9 +20,15 @@
public static final PathFragment EXTERNAL_PACKAGE_NAME = PathFragment.create("external");
public static final PackageIdentifier EXTERNAL_PACKAGE_IDENTIFIER =
PackageIdentifier.createInMainRepo(EXTERNAL_PACKAGE_NAME);
- public static final PathFragment EXTERNAL_PATH_PREFIX = PathFragment.create("external");
public static final PathFragment WORKSPACE_FILE_NAME = PathFragment.create("WORKSPACE");
public static final PathFragment WORKSPACE_DOT_BAZEL_FILE_NAME =
PathFragment.create("WORKSPACE.bazel");
public static final String DEFAULT_REPOSITORY_DIRECTORY = "__main__";
+
+ // With this prefix, non-main repositories are symlinked under
+ // $output_base/execution_root/__main__/external
+ public static final PathFragment EXTERNAL_PATH_PREFIX = PathFragment.create("external");
+ // With this prefix, non-main repositories are sibling symlinks of
+ // $output_base/execution_root/__main__
+ public static final PathFragment EXPERIMENTAL_EXTERNAL_PATH_PREFIX = PathFragment.create("..");
}
diff --git a/src/main/java/com/google/devtools/build/lib/cmdline/PackageIdentifier.java b/src/main/java/com/google/devtools/build/lib/cmdline/PackageIdentifier.java
index e4f285d..eecec56 100644
--- a/src/main/java/com/google/devtools/build/lib/cmdline/PackageIdentifier.java
+++ b/src/main/java/com/google/devtools/build/lib/cmdline/PackageIdentifier.java
@@ -76,19 +76,21 @@
* @throws LabelSyntaxException if the exec path seems to be for an external repository that does
* not have a valid repository name (see {@link RepositoryName#create})
*/
- public static PackageIdentifier discoverFromExecPath(PathFragment execPath, boolean forFiles)
- throws LabelSyntaxException {
+ public static PackageIdentifier discoverFromExecPath(
+ PathFragment execPath, boolean forFiles, boolean siblingRepositoryLayout) {
Preconditions.checkArgument(!execPath.isAbsolute(), execPath);
PathFragment tofind = forFiles
? Preconditions.checkNotNull(
execPath.getParentDirectory(), "Must pass in files, not root directory")
: execPath;
- if (tofind.startsWith(LabelConstants.EXTERNAL_PATH_PREFIX)) {
- // TODO(ulfjack): Remove this when kchodorow@'s exec root rearrangement has been rolled out.
- RepositoryName repository = RepositoryName.create("@" + tofind.getSegment(1));
- return PackageIdentifier.create(repository, tofind.subFragment(2));
- } else if (tofind.containsUplevelReferences()) {
- RepositoryName repository = RepositoryName.create("@" + tofind.getSegment(1));
+ PathFragment prefix =
+ siblingRepositoryLayout
+ ? LabelConstants.EXPERIMENTAL_EXTERNAL_PATH_PREFIX
+ : LabelConstants.EXTERNAL_PATH_PREFIX;
+ if (tofind.startsWith(prefix)) {
+ // Using the path prefix can be either "external" or "..", depending on whether the sibling
+ // repository layout is used.
+ RepositoryName repository = RepositoryName.createFromValidStrippedName(tofind.getSegment(1));
return PackageIdentifier.create(repository, tofind.subFragment(2));
} else {
return PackageIdentifier.createInMainRepo(tofind);
@@ -175,8 +177,8 @@
return repository.getSourceRoot().getRelative(pkgName);
}
- public PathFragment getPathUnderExecRoot() {
- return repository.getPathUnderExecRoot().getRelative(pkgName);
+ public PathFragment getExecPath(boolean siblingRepositoryLayout) {
+ return repository.getExecPath(siblingRepositoryLayout).getRelative(pkgName);
}
/**
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 4e133bb..5f4d455 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
@@ -132,17 +132,27 @@
}
/**
- * Extracts the repository name from a PathFragment that was created with
- * {@code PackageIdentifier.getSourceRoot}.
+ * Extracts the repository name from a PathFragment that was created with {@code
+ * PackageIdentifier.getSourceRoot}.
*
- * @return a {@code Pair} of the extracted repository name and the path fragment with stripped
- * of "external/"-prefix and repository name, or null if none was found or the repository name
- * was invalid.
+ * @return a {@code Pair} of the extracted repository name and the path fragment with stripped of
+ * "external/"-prefix and repository name, or null if none was found or the repository name
+ * was invalid.
*/
- public static Pair<RepositoryName, PathFragment> fromPathFragment(PathFragment path) {
- if (path.segmentCount() < 2 || !path.startsWith(LabelConstants.EXTERNAL_PATH_PREFIX)) {
+ public static Pair<RepositoryName, PathFragment> fromPathFragment(
+ PathFragment path, boolean siblingRepositoryLayout) {
+ if (path.segmentCount() < 2) {
return null;
}
+
+ PathFragment prefix =
+ siblingRepositoryLayout
+ ? LabelConstants.EXPERIMENTAL_EXTERNAL_PATH_PREFIX
+ : LabelConstants.EXTERNAL_PATH_PREFIX;
+ if (!path.startsWith(prefix)) {
+ return null;
+ }
+
try {
RepositoryName repoName = RepositoryName.create("@" + path.getSegment(1));
PathFragment subPath = path.subFragment(2);
@@ -244,13 +254,30 @@
}
/**
+ * Returns the relative path to the repository's source for derived artifacts. This behavior is
+ * currently the same for source artifacts, but we create a new method name to keep call sites
+ * readable and not misleading.
+ */
+ public PathFragment getDerivedArtifactSourceRoot() {
+ return getSourceRoot();
+ }
+
+ /**
* Returns the runfiles/execRoot path for this repository. If we don't know the name of this repo
* (i.e., it is in the main repository), return an empty path fragment.
+ *
+ * <p>If --experimental_sibling_repository_layout is true, return "$execroot/../repo" (sibling of
+ * __main__), instead of "$execroot/external/repo".
*/
- public PathFragment getPathUnderExecRoot() {
- return isDefault() || isMain()
- ? PathFragment.EMPTY_FRAGMENT
- : LabelConstants.EXTERNAL_PATH_PREFIX.getRelative(strippedName());
+ public PathFragment getExecPath(boolean siblingRepositoryLayout) {
+ if (isDefault() || isMain()) {
+ return PathFragment.EMPTY_FRAGMENT;
+ }
+ PathFragment prefix =
+ siblingRepositoryLayout
+ ? LabelConstants.EXPERIMENTAL_EXTERNAL_PATH_PREFIX
+ : LabelConstants.EXTERNAL_PATH_PREFIX;
+ return prefix.getRelative(strippedName());
}
/**
diff --git a/src/main/java/com/google/devtools/build/lib/packages/InputFile.java b/src/main/java/com/google/devtools/build/lib/packages/InputFile.java
index 3667643..d2e24c1 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/InputFile.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/InputFile.java
@@ -101,10 +101,16 @@
}
/**
- * Returns the exec path of the file, i.e. the path relative to the package source root.
+ * Returns the exec path of the file, i.e. the path relative to the execution root working
+ * directory.
*/
- public PathFragment getExecPath() {
- return label.getPackageIdentifier().getSourceRoot().getRelative(label.getName());
+ public PathFragment getExecPath(boolean siblingRepositoryLayout) {
+ return label
+ .getPackageIdentifier()
+ .getRepository()
+ .getExecPath(siblingRepositoryLayout)
+ .getRelative(label.getPackageName())
+ .getRelative(label.getName());
}
@Override
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 ec60aa2..31c3218 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,27 @@
public boolean experimentalRepoRemoteExec;
@Option(
+ name = "experimental_sibling_repository_layout",
+ defaultValue = "false",
+ documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS,
+ effectTags = {
+ OptionEffectTag.ACTION_COMMAND_LINES,
+ OptionEffectTag.BAZEL_INTERNAL_CONFIGURATION,
+ OptionEffectTag.LOADING_AND_ANALYSIS,
+ OptionEffectTag.LOSES_INCREMENTAL_STATE
+ },
+ metadataTags = {
+ OptionMetadataTag.EXPERIMENTAL,
+ },
+ help =
+ "If set to true, non-main repositories are planted as symlinks to the main repository in"
+ + " the execution root. That is, all repositories are direct children of the"
+ + " $output_base/execution_root directory. This has the side effect of freeing up"
+ + " $output_base/execution_root/__main__/external for the real top-level 'external' "
+ + "directory.")
+ public boolean experimentalSiblingRepositoryLayout;
+
+ @Option(
name = "incompatible_bzl_disallow_load_after_statement",
defaultValue = "true",
documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS,
@@ -633,6 +654,7 @@
.experimentalStarlarkUnusedInputsList(experimentalStarlarkUnusedInputsList)
.experimentalCcSharedLibrary(experimentalCcSharedLibrary)
.experimentalRepoRemoteExec(experimentalRepoRemoteExec)
+ .experimentalSiblingRepositoryLayout(experimentalSiblingRepositoryLayout)
.incompatibleApplicableLicenses(incompatibleApplicableLicenses)
.incompatibleBzlDisallowLoadAfterStatement(incompatibleBzlDisallowLoadAfterStatement)
.incompatibleDepsetUnion(incompatibleDepsetUnion)
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java
index f962a98..b8d67f7 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java
@@ -589,12 +589,19 @@
* Determines a list of loose include directories that are only allowed to be referenced when
* headers checking is {@link HeadersCheckingMode#LOOSE}.
*/
- Set<PathFragment> getLooseIncludeDirs() {
+ Set<PathFragment> getLooseIncludeDirs() throws InterruptedException {
ImmutableSet.Builder<PathFragment> result = ImmutableSet.builder();
// The package directory of the rule contributes includes. Note that this also covers all
// non-subpackage sub-directories.
- PathFragment rulePackage = ruleContext.getLabel().getPackageIdentifier()
- .getPathUnderExecRoot();
+ PathFragment rulePackage =
+ ruleContext
+ .getLabel()
+ .getPackageIdentifier()
+ .getExecPath(
+ ruleContext
+ .getAnalysisEnvironment()
+ .getSkylarkSemantics()
+ .experimentalSiblingRepositoryLayout());
result.add(rulePackage);
if (ruleContext
@@ -604,17 +611,29 @@
.experimentalIncludesAttributeSubpackageTraversal
&& ruleContext.getRule().isAttributeValueExplicitlySpecified("includes")) {
PathFragment packageFragment =
- ruleContext.getLabel().getPackageIdentifier().getPathUnderExecRoot();
+ ruleContext
+ .getLabel()
+ .getPackageIdentifier()
+ .getExecPath(
+ ruleContext
+ .getAnalysisEnvironment()
+ .getSkylarkSemantics()
+ .experimentalSiblingRepositoryLayout());
// For now, anything with an 'includes' needs a blanket declaration
result.add(packageFragment.getRelative("**"));
}
return result.build();
}
- List<PathFragment> getSystemIncludeDirs() {
+ List<PathFragment> getSystemIncludeDirs() throws InterruptedException {
+ boolean siblingRepositoryLayout =
+ ruleContext
+ .getAnalysisEnvironment()
+ .getSkylarkSemantics()
+ .experimentalSiblingRepositoryLayout();
List<PathFragment> result = new ArrayList<>();
PackageIdentifier packageIdentifier = ruleContext.getLabel().getPackageIdentifier();
- PathFragment packageFragment = packageIdentifier.getPathUnderExecRoot();
+ PathFragment packageFragment = packageIdentifier.getExecPath(siblingRepositoryLayout);
for (String includesAttr : ruleContext.getExpander().list("includes")) {
if (includesAttr.startsWith("/")) {
ruleContext.attributeWarning("includes",
@@ -622,7 +641,7 @@
continue;
}
PathFragment includesPath = packageFragment.getRelative(includesAttr);
- if (includesPath.containsUplevelReferences()) {
+ if (!siblingRepositoryLayout && includesPath.containsUplevelReferences()) {
ruleContext.attributeError("includes",
"Path references a path above the execution root.");
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java
index 24011e6..079185f 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java
@@ -335,7 +335,8 @@
}
/** Sets fields that overlap for cc_library and cc_binary rules. */
- public CcCompilationHelper fromCommon(CcCommon common, ImmutableList<String> additionalCopts) {
+ public CcCompilationHelper fromCommon(CcCommon common, ImmutableList<String> additionalCopts)
+ throws InterruptedException {
Preconditions.checkNotNull(additionalCopts);
setCopts(ImmutableList.copyOf(Iterables.concat(common.getCopts(), additionalCopts)));
@@ -754,7 +755,7 @@
*
* @throws RuleErrorException
*/
- public CompilationInfo compile() throws RuleErrorException {
+ public CompilationInfo compile() throws RuleErrorException, InterruptedException {
if (!generatePicAction && !generateNoPicAction) {
ruleErrorConsumer.ruleError("Either PIC or no PIC actions have to be created.");
@@ -835,7 +836,7 @@
}
}
- private PublicHeaders computePublicHeaders() {
+ private PublicHeaders computePublicHeaders() throws InterruptedException {
PathFragment prefix = null;
if (includePrefix != null) {
prefix = PathFragment.create(includePrefix);
@@ -944,10 +945,8 @@
virtualToOriginalHeaders.build());
}
- /**
- * Create {@code CcCompilationContext} for cc compile action from generated inputs.
- */
- private CcCompilationContext initializeCcCompilationContext() {
+ /** Create {@code CcCompilationContext} for cc compile action from generated inputs. */
+ private CcCompilationContext initializeCcCompilationContext() throws InterruptedException {
CcCompilationContext.Builder ccCompilationContextBuilder =
CcCompilationContext.builder(actionConstructionContext, configuration, label);
@@ -959,8 +958,13 @@
// generated files. It is important that the execRoot (EMPTY_FRAGMENT) comes
// before the genfilesFragment to preferably pick up source files. Otherwise
// we might pick up stale generated files.
+ boolean siblingRepositoryLayout =
+ actionConstructionContext
+ .getAnalysisEnvironment()
+ .getSkylarkSemantics()
+ .experimentalSiblingRepositoryLayout();
PathFragment repositoryPath =
- label.getPackageIdentifier().getRepository().getPathUnderExecRoot();
+ label.getPackageIdentifier().getRepository().getExecPath(siblingRepositoryLayout);
ccCompilationContextBuilder.addQuoteIncludeDir(repositoryPath);
ccCompilationContextBuilder.addQuoteIncludeDir(
configuration.getGenfilesFragment().getRelative(repositoryPath));
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcModule.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcModule.java
index 4527907..87a30f4 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcModule.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcModule.java
@@ -826,7 +826,10 @@
.getRuleContext()
.getLabel()
.getPackageIdentifier()
- .getPathUnderExecRoot()
+ .getExecPath(
+ skylarkRuleContext
+ .getSkylarkSemantics()
+ .experimentalSiblingRepositoryLayout())
.getRelative(PathFragment.create(tool.second))
.getPathString();
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProviderHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProviderHelper.java
index 3dcd11e..bdc62d4 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProviderHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProviderHelper.java
@@ -70,7 +70,13 @@
CcToolchainConfigInfo toolchainConfigInfo = attributes.getCcToolchainConfigInfo();
ImmutableMap<String, PathFragment> toolPaths;
CcToolchainFeatures toolchainFeatures;
- PathFragment toolsDirectory = getToolsDirectory(ruleContext.getLabel());
+ PathFragment toolsDirectory =
+ getToolsDirectory(
+ ruleContext.getLabel(),
+ ruleContext
+ .getAnalysisEnvironment()
+ .getSkylarkSemantics()
+ .experimentalSiblingRepositoryLayout());
try {
toolPaths = computeToolPaths(toolchainConfigInfo, toolsDirectory);
toolchainFeatures = new CcToolchainFeatures(toolchainConfigInfo, toolsDirectory);
@@ -463,8 +469,8 @@
return toolPaths.get(tool.getNamePart());
}
- static PathFragment getToolsDirectory(Label ccToolchainLabel) {
- return ccToolchainLabel.getPackageIdentifier().getPathUnderExecRoot();
+ static PathFragment getToolsDirectory(Label ccToolchainLabel, boolean siblingRepositoryLayout) {
+ return ccToolchainLabel.getPackageIdentifier().getExecPath(siblingRepositoryLayout);
}
private static ImmutableMap<String, String> computeAdditionalMakeVariables(
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java
index 3c92fe2..59dab83 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java
@@ -61,6 +61,7 @@
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
import com.google.devtools.build.lib.collect.nestedset.Order;
import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadCompatible;
+import com.google.devtools.build.lib.packages.StarlarkSemanticsOptions;
import com.google.devtools.build.lib.profiler.AutoProfiler;
import com.google.devtools.build.lib.profiler.Profiler;
import com.google.devtools.build.lib.profiler.ProfilerTask;
@@ -594,7 +595,12 @@
.setCmdlineIncludes(getCmdlineIncludes(options))
.build());
if (needsIncludeValidation) {
- verifyActionIncludePaths(systemIncludeDirs);
+ verifyActionIncludePaths(
+ systemIncludeDirs,
+ actionExecutionContext
+ .getOptions()
+ .getOptions(StarlarkSemanticsOptions.class)
+ .experimentalSiblingRepositoryLayout);
}
if (!shouldScanIncludes) {
@@ -1035,7 +1041,8 @@
}
@VisibleForTesting
- void verifyActionIncludePaths(List<PathFragment> systemIncludeDirs)
+ void verifyActionIncludePaths(
+ List<PathFragment> systemIncludeDirs, boolean siblingRepositoryLayout)
throws ActionExecutionException {
ImmutableSet<PathFragment> ignoredDirs = ImmutableSet.copyOf(getValidationIgnoredDirs());
// We currently do not check the output of:
@@ -1052,9 +1059,16 @@
|| FileSystemUtils.startsWithAny(includePath, ignoredDirs)) {
continue;
}
- // One starting ../ is okay for getting to a sibling repository.
- if (includePath.startsWith(LabelConstants.EXTERNAL_PATH_PREFIX)) {
- includePath = includePath.relativeTo(LabelConstants.EXTERNAL_PATH_PREFIX);
+
+ // Two conditions:
+ // 1. Paths cannot be absolute (e.g. multiple uplevels to /etc/passwd)
+ // 2. For relative paths, one starting ../ is okay for getting to a sibling repository.
+ PathFragment prefix =
+ siblingRepositoryLayout
+ ? LabelConstants.EXPERIMENTAL_EXTERNAL_PATH_PREFIX
+ : LabelConstants.EXTERNAL_PATH_PREFIX;
+ if (includePath.startsWith(prefix)) {
+ includePath = includePath.relativeTo(prefix);
}
if (includePath.isAbsolute() || includePath.containsUplevelReferences()) {
throw new ActionExecutionException(
@@ -1465,7 +1479,8 @@
Path execRoot,
ArtifactResolver artifactResolver,
ShowIncludesFilter showIncludesFilterForStdout,
- ShowIncludesFilter showIncludesFilterForStderr)
+ ShowIncludesFilter showIncludesFilterForStderr,
+ boolean siblingRepositoryLayout)
throws ActionExecutionException {
if (!needsDotdInputPruning) {
return NestedSetBuilder.emptySet(Order.STABLE_ORDER);
@@ -1485,7 +1500,9 @@
discoveryBuilder.shouldValidateInclusions();
}
- return discoveryBuilder.build().discoverInputsFromDependencies(execRoot, artifactResolver);
+ return discoveryBuilder
+ .build()
+ .discoverInputsFromDependencies(execRoot, artifactResolver, siblingRepositoryLayout);
}
@VisibleForTesting
@@ -1493,7 +1510,8 @@
ActionExecutionContext actionExecutionContext,
Path execRoot,
ArtifactResolver artifactResolver,
- byte[] dotDContents)
+ byte[] dotDContents,
+ boolean siblingRepositoryLayout)
throws ActionExecutionException {
if (!needsDotdInputPruning || getDotdFile() == null) {
return NestedSetBuilder.emptySet(Order.STABLE_ORDER);
@@ -1511,7 +1529,9 @@
discoveryBuilder.shouldValidateInclusions();
}
- return discoveryBuilder.build().discoverInputsFromDependencies(execRoot, artifactResolver);
+ return discoveryBuilder
+ .build()
+ .discoverInputsFromDependencies(execRoot, artifactResolver, siblingRepositoryLayout);
}
public DependencySet processDepset(
@@ -1786,6 +1806,11 @@
CppIncludeExtractionContext scanningContext =
actionExecutionContext.getContext(CppIncludeExtractionContext.class);
Path execRoot = actionExecutionContext.getExecRoot();
+ boolean siblingRepositoryLayout =
+ actionExecutionContext
+ .getOptions()
+ .getOptions(StarlarkSemanticsOptions.class)
+ .experimentalSiblingRepositoryLayout;
NestedSet<Artifact> discoveredInputs;
if (featureConfiguration.isEnabled(CppRuleClasses.PARSE_SHOWINCLUDES)) {
@@ -1794,14 +1819,16 @@
execRoot,
scanningContext.getArtifactResolver(),
showIncludesFilterForStdout,
- showIncludesFilterForStderr);
+ showIncludesFilterForStderr,
+ siblingRepositoryLayout);
} else {
discoveredInputs =
discoverInputsFromDotdFiles(
actionExecutionContext,
execRoot,
scanningContext.getArtifactResolver(),
- dotDContents);
+ dotDContents,
+ siblingRepositoryLayout);
}
dotDContents = null; // Garbage collect in-memory .d contents.
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/FakeCppCompileAction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/FakeCppCompileAction.java
index 683f24a..076fbdb 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/FakeCppCompileAction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/FakeCppCompileAction.java
@@ -37,6 +37,7 @@
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadCompatible;
import com.google.devtools.build.lib.events.Event;
+import com.google.devtools.build.lib.packages.StarlarkSemanticsOptions;
import com.google.devtools.build.lib.rules.cpp.CcCommon.CoptsFilter;
import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration;
import com.google.devtools.build.lib.util.ShellEscaper;
@@ -170,10 +171,17 @@
discoveryBuilder.shouldValidateInclusions();
}
+ boolean siblingRepositoryLayout =
+ actionExecutionContext
+ .getOptions()
+ .getOptions(StarlarkSemanticsOptions.class)
+ .experimentalSiblingRepositoryLayout;
+
discoveredInputs =
discoveryBuilder
.build()
- .discoverInputsFromDependencies(execRoot, scanningContext.getArtifactResolver());
+ .discoverInputsFromDependencies(
+ execRoot, scanningContext.getArtifactResolver(), siblingRepositoryLayout);
}
dotDContents = null; // Garbage collect in-memory .d contents.
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoHelper.java
index e2107eb..b54b5f0 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoHelper.java
@@ -442,7 +442,8 @@
}
private static FdoInputFile fdoInputFileFromArtifacts(
- RuleContext ruleContext, CcToolchainAttributesProvider attributes) {
+ RuleContext ruleContext, CcToolchainAttributesProvider attributes)
+ throws InterruptedException {
ImmutableList<Artifact> fdoArtifacts = attributes.getFdoOptimizeArtifacts();
if (fdoArtifacts.size() != 1) {
ruleContext.ruleError("--fdo_optimize does not point to a single target");
@@ -458,7 +459,11 @@
Label fdoLabel = attributes.getFdoOptimize().getLabel();
if (!fdoLabel
.getPackageIdentifier()
- .getPathUnderExecRoot()
+ .getExecPath(
+ ruleContext
+ .getAnalysisEnvironment()
+ .getSkylarkSemantics()
+ .experimentalSiblingRepositoryLayout())
.getRelative(fdoLabel.getName())
.equals(fdoArtifact.getExecPath())) {
ruleContext.ruleError("--fdo_optimize points to a target that is not an input file");
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/HeaderDiscovery.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/HeaderDiscovery.java
index b72b0d7..013e47d 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/HeaderDiscovery.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/HeaderDiscovery.java
@@ -21,7 +21,7 @@
import com.google.devtools.build.lib.actions.ActionExecutionException;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.ArtifactResolver;
-import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
+import com.google.devtools.build.lib.cmdline.LabelConstants;
import com.google.devtools.build.lib.cmdline.PackageIdentifier;
import com.google.devtools.build.lib.cmdline.RepositoryName;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
@@ -126,7 +126,8 @@
*/
@ThreadCompatible
NestedSet<Artifact> discoverInputsFromDependencies(
- Path execRoot, ArtifactResolver artifactResolver) throws ActionExecutionException {
+ Path execRoot, ArtifactResolver artifactResolver, boolean siblingRepositoryLayout)
+ throws ActionExecutionException {
NestedSetBuilder<Artifact> inputs = NestedSetBuilder.stableOrder();
if (dependencies == null) {
return inputs.build();
@@ -147,6 +148,11 @@
// the build with an error.
if (execPath.startsWith(execRoot)) {
execPathFragment = execPath.relativeTo(execRoot); // funky but tolerable path
+ } else if (siblingRepositoryLayout && execPath.startsWith(execRoot.getParentDirectory())) {
+ // for --experimental_sibling_repository_layout
+ execPathFragment =
+ LabelConstants.EXPERIMENTAL_EXTERNAL_PATH_PREFIX.getRelative(
+ execPath.relativeTo(execRoot.getParentDirectory()));
} else {
problems.add(execPathFragment.getPathString());
continue;
@@ -154,19 +160,10 @@
}
Artifact artifact = allowedDerivedInputsMap.get(execPathFragment);
if (artifact == null) {
- try {
- RepositoryName repository =
- PackageIdentifier.discoverFromExecPath(execPathFragment, false).getRepository();
- artifact = artifactResolver.resolveSourceArtifact(execPathFragment, repository);
- } catch (LabelSyntaxException e) {
- throw new ActionExecutionException(
- String.format(
- "Could not find the external repository for %s: " + e.getMessage(),
- execPathFragment),
- e,
- action,
- false);
- }
+ RepositoryName repository =
+ PackageIdentifier.discoverFromExecPath(execPathFragment, false, siblingRepositoryLayout)
+ .getRepository();
+ artifact = artifactResolver.resolveSourceArtifact(execPathFragment, repository);
}
if (artifact != null) {
// We don't need to add the sourceFile itself as it is a mandatory input.
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoAspect.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoAspect.java
index 1014c39..6005815 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoAspect.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoAspect.java
@@ -337,7 +337,15 @@
protoRootFragment = protoRootFragment.relativeTo(binOrGenfiles);
}
PathFragment repositoryPath =
- ruleContext.getLabel().getPackageIdentifier().getRepository().getPathUnderExecRoot();
+ ruleContext
+ .getLabel()
+ .getPackageIdentifier()
+ .getRepository()
+ .getExecPath(
+ ruleContext
+ .getAnalysisEnvironment()
+ .getSkylarkSemantics()
+ .experimentalSiblingRepositoryLayout());
if (protoRootFragment.startsWith(repositoryPath)) {
protoRootFragment = protoRootFragment.relativeTo(repositoryPath);
}
@@ -420,7 +428,8 @@
helper.addAdditionalExportedHeaders(headers.build());
}
- private void createProtoCompileAction(Collection<Artifact> outputs) {
+ private void createProtoCompileAction(Collection<Artifact> outputs)
+ throws InterruptedException {
PathFragment protoRootFragment = PathFragment.create(protoInfo.getDirectProtoSourceRoot());
String genfilesPath;
PathFragment genfilesFragment = ruleContext.getConfiguration().getGenfilesFragment();
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCommon.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCommon.java
index 1ddb0bb..9ebd86a 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCommon.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCommon.java
@@ -592,14 +592,10 @@
}
}
- public JavaTargetAttributes.Builder initCommon() {
+ public JavaTargetAttributes.Builder initCommon() throws InterruptedException {
return initCommon(ImmutableList.of(), getCompatibleJavacOptions());
}
- private ImmutableList<String> getCompatibleJavacOptions() {
- return semantics.getCompatibleJavacOptions(ruleContext, javaToolchain);
- }
-
/**
* Initialize the common actions and build various collections of artifacts for the
* initializationHook() methods of the subclasses.
@@ -609,7 +605,7 @@
* @return the processed attributes
*/
public JavaTargetAttributes.Builder initCommon(
- Collection<Artifact> extraSrcs, Iterable<String> extraJavacOpts) {
+ Collection<Artifact> extraSrcs, Iterable<String> extraJavacOpts) throws InterruptedException {
Preconditions.checkState(javacOpts == null);
javacOpts = computeJavacOpts(ImmutableList.copyOf(extraJavacOpts));
activePlugins = collectPlugins();
@@ -652,6 +648,10 @@
return javaTargetAttributes;
}
+ private ImmutableList<String> getCompatibleJavacOptions() {
+ return semantics.getCompatibleJavacOptions(ruleContext, javaToolchain);
+ }
+
private boolean disallowDepsWithoutSrcs(String ruleClass) {
return ruleClass.equals("java_library")
|| ruleClass.equals("java_binary")
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaHelper.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaHelper.java
index 6e2e7d8..0d440dc 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaHelper.java
@@ -23,6 +23,7 @@
import com.google.devtools.build.lib.packages.BuildType;
import com.google.devtools.build.lib.packages.Type;
import com.google.devtools.build.lib.shell.ShellUtils;
+import com.google.devtools.build.lib.syntax.StarlarkSemantics;
import com.google.devtools.build.lib.vfs.PathFragment;
import java.util.ArrayList;
import java.util.List;
@@ -108,11 +109,15 @@
}
public static PathFragment getJavaResourcePath(
- JavaSemantics semantics, RuleContext ruleContext, Artifact resource) {
+ JavaSemantics semantics, RuleContext ruleContext, Artifact resource)
+ throws InterruptedException {
PathFragment rootRelativePath = resource.getRootRelativePath();
+ StarlarkSemantics starlarkSemantics =
+ ruleContext.getAnalysisEnvironment().getSkylarkSemantics();
- if (!ruleContext.getLabel().getWorkspaceRoot().isEmpty()) {
- PathFragment workspace = PathFragment.create(ruleContext.getLabel().getWorkspaceRoot());
+ if (!ruleContext.getLabel().getWorkspaceRoot(starlarkSemantics).isEmpty()) {
+ PathFragment workspace =
+ PathFragment.create(ruleContext.getLabel().getWorkspaceRoot(starlarkSemantics));
rootRelativePath = rootRelativePath.relativeTo(workspace);
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaRuntime.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaRuntime.java
index 6925ef7..6a15ac4 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaRuntime.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaRuntime.java
@@ -47,7 +47,12 @@
JavaCommon.checkRuleLoadedThroughMacro(ruleContext);
NestedSetBuilder<Artifact> filesBuilder = NestedSetBuilder.stableOrder();
filesBuilder.addTransitive(PrerequisiteArtifacts.nestedSet(ruleContext, "srcs", Mode.TARGET));
- PathFragment javaHome = defaultJavaHome(ruleContext.getLabel());
+ boolean siblingRepositoryLayout =
+ ruleContext
+ .getAnalysisEnvironment()
+ .getSkylarkSemantics()
+ .experimentalSiblingRepositoryLayout();
+ PathFragment javaHome = defaultJavaHome(ruleContext.getLabel(), siblingRepositoryLayout);
if (ruleContext.attributes().isAttributeValueExplicitlySpecified("java_home")) {
PathFragment javaHomeAttribute =
PathFragment.create(ruleContext.getExpander().expand("java_home"));
@@ -124,11 +129,11 @@
|| path.endsWith(PathFragment.create("bin/java.exe"));
}
- static PathFragment defaultJavaHome(Label javabase) {
+ static PathFragment defaultJavaHome(Label javabase, boolean siblingRepositoryLayout) {
if (javabase.getPackageIdentifier().getRepository().isDefault()) {
return javabase.getPackageFragment();
}
- return javabase.getPackageIdentifier().getSourceRoot();
+ return javabase.getPackageIdentifier().getExecPath(siblingRepositoryLayout);
}
private static PathFragment getRunfilesJavaExecutable(PathFragment javaHome, Label javabase) {
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaLiteProtoAspect.java b/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaLiteProtoAspect.java
index bd866fa..8f917c3 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaLiteProtoAspect.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaLiteProtoAspect.java
@@ -243,7 +243,7 @@
aspectCommon.getProtoRuntimeDeps())));
}
- private void createProtoCompileAction(Artifact sourceJar) {
+ private void createProtoCompileAction(Artifact sourceJar) throws InterruptedException {
ProtoCompileActionBuilder.registerActions(
ruleContext,
ImmutableList.of(
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaProtoAspect.java b/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaProtoAspect.java
index 5e633ed..2077149 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaProtoAspect.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaProtoAspect.java
@@ -281,7 +281,7 @@
return protoBlackList.checkSrcs(protoInfo.getDirectProtoSources(), "java_proto_library");
}
- private void createProtoCompileAction(Artifact sourceJar) {
+ private void createProtoCompileAction(Artifact sourceJar) throws InterruptedException {
ImmutableList.Builder<ToolchainInvocation> invocations = ImmutableList.builder();
invocations.add(
new ToolchainInvocation(
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaProtoSkylarkCommon.java b/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaProtoSkylarkCommon.java
index f74dea9..db1c4c8 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaProtoSkylarkCommon.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaProtoSkylarkCommon.java
@@ -43,7 +43,7 @@
Artifact sourceJar,
String protoToolchainAttr,
String flavour)
- throws EvalException {
+ throws EvalException, InterruptedException {
ProtoInfo protoInfo = target.get(ProtoInfo.PROVIDER);
ProtoCompileActionBuilder.registerActions(
skylarkRuleContext.getRuleContext(),
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/J2ObjcAspect.java b/src/main/java/com/google/devtools/build/lib/rules/objc/J2ObjcAspect.java
index 7541582..777a2ad 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/J2ObjcAspect.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/J2ObjcAspect.java
@@ -616,7 +616,8 @@
ProtoLangToolchainProvider protoToolchain,
RuleContext ruleContext,
ImmutableList<Artifact> filteredProtoSources,
- J2ObjcSource j2ObjcSource) {
+ J2ObjcSource j2ObjcSource)
+ throws InterruptedException {
ImmutableList<Artifact> outputHeaderMappingFiles =
ProtoCommon.getGeneratedOutputs(ruleContext, filteredProtoSources, ".j2objc.mapping");
ImmutableList<Artifact> outputClassMappingFiles =
@@ -753,7 +754,7 @@
}
private static J2ObjcSource protoJ2ObjcSource(
- RuleContext ruleContext, ImmutableList<Artifact> protoSources) {
+ RuleContext ruleContext, ImmutableList<Artifact> protoSources) throws InterruptedException {
PathFragment objcFileRootExecPath = getProtoOutputRoot(ruleContext);
List<PathFragment> headerSearchPaths =
@@ -768,12 +769,21 @@
headerSearchPaths);
}
- private static PathFragment getProtoOutputRoot(RuleContext ruleContext) {
+ private static PathFragment getProtoOutputRoot(RuleContext ruleContext)
+ throws InterruptedException {
return ruleContext
.getConfiguration()
.getGenfilesFragment()
.getRelative(
- ruleContext.getLabel().getPackageIdentifier().getRepository().getPathUnderExecRoot());
+ ruleContext
+ .getLabel()
+ .getPackageIdentifier()
+ .getRepository()
+ .getExecPath(
+ ruleContext
+ .getAnalysisEnvironment()
+ .getSkylarkSemantics()
+ .experimentalSiblingRepositoryLayout()));
}
private static boolean isProtoRule(ConfiguredTarget base) {
diff --git a/src/main/java/com/google/devtools/build/lib/rules/proto/BazelProtoLibrary.java b/src/main/java/com/google/devtools/build/lib/rules/proto/BazelProtoLibrary.java
index cf1318f..3aed38127 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/proto/BazelProtoLibrary.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/proto/BazelProtoLibrary.java
@@ -31,7 +31,7 @@
@Override
public ConfiguredTarget create(RuleContext ruleContext)
- throws ActionConflictException, RuleErrorException {
+ throws ActionConflictException, RuleErrorException, InterruptedException {
ProtoCommon.checkRuleHasValidMigrationTag(ruleContext);
ProtoInfo protoInfo =
ProtoCommon.createProtoInfo(
diff --git a/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoCommon.java b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoCommon.java
index a0eb92a..81415b1 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoCommon.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoCommon.java
@@ -35,6 +35,7 @@
import com.google.devtools.build.lib.packages.BuildType;
import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException;
import com.google.devtools.build.lib.packages.Type;
+import com.google.devtools.build.lib.syntax.StarlarkSemantics;
import com.google.devtools.build.lib.util.Pair;
import com.google.devtools.build.lib.vfs.FileSystemUtils;
import com.google.devtools.build.lib.vfs.PathFragment;
@@ -214,13 +215,17 @@
// ProtoInfo so it's not an easy change :(
@Nullable
private static Library createLibraryWithoutVirtualSourceRoot(
- RuleContext ruleContext, ImmutableList<Artifact> directSources) {
+ RuleContext ruleContext, ImmutableList<Artifact> directSources) throws InterruptedException {
String protoSourceRoot =
ruleContext
.getLabel()
.getPackageIdentifier()
.getRepository()
- .getPathUnderExecRoot()
+ .getExecPath(
+ ruleContext
+ .getAnalysisEnvironment()
+ .getSkylarkSemantics()
+ .experimentalSiblingRepositoryLayout())
.getPathString();
ImmutableList.Builder<Pair<Artifact, String>> builder = ImmutableList.builder();
@@ -258,7 +263,8 @@
private static Library createLibraryWithVirtualSourceRootMaybe(
RuleContext ruleContext,
ImmutableList<Artifact> protoSources,
- boolean generatedProtosInVirtualImports) {
+ boolean generatedProtosInVirtualImports)
+ throws InterruptedException {
PathFragment importPrefixAttribute = getPathFragmentAttribute(ruleContext, "import_prefix");
PathFragment stripImportPrefixAttribute =
getPathFragmentAttribute(ruleContext, "strip_import_prefix");
@@ -283,20 +289,28 @@
PathFragment stripImportPrefix;
PathFragment importPrefix;
+ StarlarkSemantics starlarkSemantics =
+ ruleContext.getAnalysisEnvironment().getSkylarkSemantics();
+ boolean siblingRepositoryLayout = starlarkSemantics.experimentalSiblingRepositoryLayout();
if (stripImportPrefixAttribute != null || importPrefixAttribute != null) {
if (stripImportPrefixAttribute == null) {
- stripImportPrefix = PathFragment.create(ruleContext.getLabel().getWorkspaceRoot());
+ stripImportPrefix =
+ PathFragment.create(ruleContext.getLabel().getWorkspaceRoot(starlarkSemantics));
} else if (stripImportPrefixAttribute.isAbsolute()) {
stripImportPrefix =
ruleContext
.getLabel()
.getPackageIdentifier()
.getRepository()
- .getSourceRoot()
+ .getExecPath(siblingRepositoryLayout)
.getRelative(stripImportPrefixAttribute.toRelative());
} else {
stripImportPrefix =
- ruleContext.getPackageDirectory().getRelative(stripImportPrefixAttribute);
+ ruleContext
+ .getLabel()
+ .getPackageIdentifier()
+ .getExecPath(siblingRepositoryLayout)
+ .getRelative(stripImportPrefixAttribute);
}
if (importPrefixAttribute != null) {
@@ -312,7 +326,11 @@
} else {
// Has generated sources, but neither strip_import_prefix nor import_prefix
stripImportPrefix =
- ruleContext.getLabel().getPackageIdentifier().getRepository().getPathUnderExecRoot();
+ ruleContext
+ .getLabel()
+ .getPackageIdentifier()
+ .getRepository()
+ .getDerivedArtifactSourceRoot();
importPrefix = PathFragment.EMPTY_FRAGMENT;
}
@@ -323,7 +341,9 @@
PathFragment sourceRootPath = ruleContext.getUniqueDirectory("_virtual_imports");
for (Artifact realProtoSource : protoSources) {
- if (!realProtoSource.getRootRelativePath().startsWith(stripImportPrefix)) {
+ if (siblingRepositoryLayout && realProtoSource.isSourceArtifact()
+ ? !realProtoSource.getExecPath().startsWith(stripImportPrefix)
+ : !realProtoSource.getRootRelativePath().startsWith(stripImportPrefix)) {
ruleContext.ruleError(
String.format(
".proto file '%s' is not under the specified strip prefix '%s'",
@@ -332,7 +352,12 @@
}
Pair<PathFragment, Artifact> importsPair =
computeImports(
- ruleContext, realProtoSource, sourceRootPath, importPrefix, stripImportPrefix);
+ ruleContext,
+ realProtoSource,
+ sourceRootPath,
+ importPrefix,
+ stripImportPrefix,
+ starlarkSemantics.experimentalSiblingRepositoryLayout());
protoSourceImportPair.add(new Pair<>(realProtoSource, importsPair.first.toString()));
symlinks.add(importsPair.second);
}
@@ -351,14 +376,22 @@
Artifact realProtoSource,
PathFragment sourceRootPath,
PathFragment importPrefix,
- PathFragment stripImportPrefix) {
- PathFragment importPath =
- importPrefix.getRelative(
- realProtoSource.getRootRelativePath().relativeTo(stripImportPrefix));
+ PathFragment stripImportPrefix,
+ boolean siblingRepositoryLayout) {
+ PathFragment importPath;
- Artifact virtualProtoSource =
- ruleContext.getDerivedArtifact(
- sourceRootPath.getRelative(importPath), ruleContext.getBinOrGenfilesDirectory());
+ if (siblingRepositoryLayout && realProtoSource.isSourceArtifact()) {
+ importPath =
+ importPrefix.getRelative(realProtoSource.getExecPath().relativeTo(stripImportPrefix));
+ } else {
+ importPath =
+ importPrefix.getRelative(
+ realProtoSource.getRootRelativePath().relativeTo(stripImportPrefix));
+ }
+
+ Artifact virtualProtoSource =
+ ruleContext.getDerivedArtifact(
+ sourceRootPath.getRelative(importPath), ruleContext.getBinOrGenfilesDirectory());
ruleContext.registerAction(
SymlinkAction.toArtifact(
@@ -458,7 +491,8 @@
* ruleContext}.
*/
public static ProtoInfo createProtoInfo(
- RuleContext ruleContext, boolean generatedProtosInVirtualImports) {
+ RuleContext ruleContext, boolean generatedProtosInVirtualImports)
+ throws InterruptedException {
checkSourceFilesAreInSamePackage(ruleContext);
ImmutableList<Artifact> directProtoSources =
ruleContext.getPrerequisiteArtifacts("srcs", Mode.TARGET).list();
diff --git a/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoCompileActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoCompileActionBuilder.java
index b5d7881..f26499a 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoCompileActionBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoCompileActionBuilder.java
@@ -41,6 +41,7 @@
import com.google.devtools.build.lib.analysis.stringtemplate.TemplateContext;
import com.google.devtools.build.lib.analysis.stringtemplate.TemplateExpander;
import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.cmdline.LabelConstants;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
@@ -190,7 +191,7 @@
}
}
- public Action[] build() {
+ public Action[] build() throws InterruptedException {
if (isEmpty(outputs)) {
return NO_ACTIONS;
}
@@ -202,7 +203,8 @@
}
}
- private SpawnAction.Builder createAction() throws MissingPrerequisiteException {
+ private SpawnAction.Builder createAction()
+ throws MissingPrerequisiteException, InterruptedException {
SpawnAction.Builder result =
new SpawnAction.Builder().addTransitiveInputs(protoInfo.getTransitiveProtoSources());
@@ -244,7 +246,7 @@
/** Commandline generator for protoc invocations. */
@VisibleForTesting
- CustomCommandLine.Builder createProtoCompilerCommandLine() {
+ CustomCommandLine.Builder createProtoCompilerCommandLine() throws InterruptedException {
CustomCommandLine.Builder result = CustomCommandLine.builder();
if (langPlugin != null) {
@@ -262,13 +264,20 @@
boolean areDepsStrict = areDepsStrict(ruleContext);
+ boolean siblingRepositoryLayout =
+ ruleContext
+ .getAnalysisEnvironment()
+ .getSkylarkSemantics()
+ .experimentalSiblingRepositoryLayout();
+
// Add include maps
addIncludeMapArguments(
getOutputDirectory(ruleContext),
result,
areDepsStrict ? protoInfo.getStrictImportableProtoSourcesImportPaths() : null,
protoInfo.getStrictImportableProtoSourceRoots(),
- protoInfo.getTransitiveProtoSources());
+ protoInfo.getTransitiveProtoSources(),
+ siblingRepositoryLayout);
if (areDepsStrict) {
// Note: the %s in the line below is used by proto-compiler. That is, the string we create
@@ -297,7 +306,8 @@
.mapped(
new ExpandToPathFnWithImports(
getOutputDirectory(ruleContext),
- protoInfo.getTransitiveProtoSourceRoots())));
+ protoInfo.getTransitiveProtoSourceRoots(),
+ siblingRepositoryLayout)));
}
}
@@ -312,7 +322,8 @@
private static class MissingPrerequisiteException extends Exception {}
public static void writeDescriptorSet(
- RuleContext ruleContext, ProtoInfo protoInfo, Services allowServices) {
+ RuleContext ruleContext, ProtoInfo protoInfo, Services allowServices)
+ throws InterruptedException {
Artifact output = protoInfo.getDirectDescriptorSet();
NestedSet<Artifact> dependenciesDescriptorSets =
ProtoCommon.computeDependenciesDescriptorSets(ruleContext);
@@ -394,7 +405,8 @@
Iterable<Artifact> outputs,
String flavorName,
Exports useExports,
- Services allowServices) {
+ Services allowServices)
+ throws InterruptedException {
SpawnAction.Builder actions =
createActions(
ruleContext,
@@ -419,7 +431,8 @@
Iterable<Artifact> outputs,
String flavorName,
Exports useExports,
- Services allowServices) {
+ Services allowServices)
+ throws InterruptedException {
if (isEmpty(outputs)) {
return null;
@@ -441,6 +454,12 @@
return null;
}
+ boolean siblingRepositoryLayout =
+ ruleContext
+ .getAnalysisEnvironment()
+ .getSkylarkSemantics()
+ .experimentalSiblingRepositoryLayout();
+
result
.addOutputs(outputs)
.setResources(AbstractAction.DEFAULT_RESOURCE_SET)
@@ -455,7 +474,8 @@
areDepsStrict(ruleContext) ? Deps.STRICT : Deps.NON_STRICT,
arePublicImportsStrict(ruleContext) ? useExports : Exports.DO_NOT_USE,
allowServices,
- ruleContext.getFragment(ProtoConfiguration.class).protocOpts()),
+ ruleContext.getFragment(ProtoConfiguration.class).protocOpts(),
+ siblingRepositoryLayout),
ParamFileInfo.builder(ParameterFileType.UNQUOTED).build())
.setProgressMessage("Generating %s proto_library %s", flavorName, ruleContext.getLabel())
.setMnemonic(MNEMONIC);
@@ -496,7 +516,8 @@
Deps strictDeps,
Exports useExports,
Services allowServices,
- ImmutableList<String> protocOpts) {
+ ImmutableList<String> protocOpts,
+ boolean siblingRepositoryLayout) {
CustomCommandLine.Builder cmdLine = CustomCommandLine.builder();
cmdLine.addAll(
@@ -541,7 +562,8 @@
cmdLine,
strictDeps == Deps.STRICT ? protoInfo.getStrictImportableProtoSourcesImportPaths() : null,
protoInfo.getStrictImportableProtoSourceRoots(),
- protoInfo.getTransitiveProtoSources());
+ protoInfo.getTransitiveProtoSources(),
+ siblingRepositoryLayout);
if (strictDeps == Deps.STRICT) {
cmdLine.addFormatted(STRICT_DEPS_FLAG_TEMPLATE, ruleLabel);
@@ -559,7 +581,8 @@
.mapped(
new ExpandToPathFnWithImports(
outputDirectory,
- protoInfo.getExportedProtoSourceRoots())));
+ protoInfo.getExportedProtoSourceRoots(),
+ siblingRepositoryLayout)));
}
}
@@ -580,20 +603,25 @@
CustomCommandLine.Builder commandLine,
@Nullable NestedSet<Pair<Artifact, String>> protosInDirectDependencies,
NestedSet<String> directProtoSourceRoots,
- NestedSet<Artifact> transitiveImports) {
+ NestedSet<Artifact> transitiveImports,
+ boolean siblingRepositoryLayout) {
// For each import, include both the import as well as the import relativized against its
// protoSourceRoot. This ensures that protos can reference either the full path or the short
// path when including other protos.
commandLine.addAll(
VectorArg.of(transitiveImports)
- .mapped(new ExpandImportArgsFn(outputDirectory, directProtoSourceRoots)));
+ .mapped(
+ new ExpandImportArgsFn(
+ outputDirectory, directProtoSourceRoots, siblingRepositoryLayout)));
if (protosInDirectDependencies != null) {
if (!protosInDirectDependencies.isEmpty()) {
commandLine.addAll(
"--direct_dependencies",
VectorArg.join(":")
.each(protosInDirectDependencies)
- .mapped(new ExpandToPathFnWithImports(outputDirectory, directProtoSourceRoots)));
+ .mapped(
+ new ExpandToPathFnWithImports(
+ outputDirectory, directProtoSourceRoots, siblingRepositoryLayout)));
} else {
// The proto compiler requires an empty list to turn on strict deps checking
@@ -603,7 +631,10 @@
}
private static String guessProtoPathUnderRoot(
- String outputDirectory, PathFragment sourceRootPath, Artifact proto) {
+ String outputDirectory,
+ PathFragment sourceRootPath,
+ Artifact proto,
+ boolean siblingRepositoryLayout) {
// TODO(lberki): Instead of guesswork like this, we should track which proto belongs to
// which source root. Unfortunately, that's a non-trivial migration since
// ProtoInfo is on the Starlark API. Therefore, we hack:
@@ -617,8 +648,15 @@
return proto.getExecPath().relativeTo(sourceRootPath).getPathString();
}
} else {
+ PathFragment prefix =
+ siblingRepositoryLayout
+ ? LabelConstants.EXPERIMENTAL_EXTERNAL_PATH_PREFIX
+ : LabelConstants.EXTERNAL_PATH_PREFIX;
if (proto.getRootRelativePath().startsWith(sourceRootPath)) {
return proto.getRootRelativePath().relativeTo(sourceRootPath).getPathString();
+ } else if (proto.getExecPath().startsWith(prefix)
+ && proto.getExecPath().startsWith(sourceRootPath)) {
+ return proto.getExecPath().relativeTo(sourceRootPath).getPathString();
}
}
@@ -639,12 +677,15 @@
static final class ExpandImportArgsFn implements CapturingMapFn<Artifact> {
private final String outputDirectory;
private final NestedSet<String> directProtoSourceRoots;
+ private final boolean siblingRepositoryLayout;
public ExpandImportArgsFn(
String outputDirectory,
- NestedSet<String> directProtoSourceRoots) {
+ NestedSet<String> directProtoSourceRoots,
+ boolean siblingRepositoryLayout) {
this.outputDirectory = outputDirectory;
this.directProtoSourceRoots = directProtoSourceRoots;
+ this.siblingRepositoryLayout = siblingRepositoryLayout;
}
/**
@@ -656,7 +697,9 @@
public void expandToCommandLine(Artifact proto, Consumer<String> args) {
for (String directProtoSourceRoot : directProtoSourceRoots.toList()) {
PathFragment sourceRootPath = PathFragment.create(directProtoSourceRoot);
- String arg = guessProtoPathUnderRoot(outputDirectory, sourceRootPath, proto);
+ String arg =
+ guessProtoPathUnderRoot(
+ outputDirectory, sourceRootPath, proto, siblingRepositoryLayout);
if (arg != null) {
args.accept("-I" + arg + "=" + proto.getExecPathString());
}
@@ -669,12 +712,15 @@
static final class ExpandToPathFnWithImports implements CapturingMapFn<Pair<Artifact, String>> {
private final String outputDirectory;
private final NestedSet<String> directProtoSourceRoots;
+ private final boolean siblingRepositoryLayout;
public ExpandToPathFnWithImports(
String outputDirectory,
- NestedSet<String> directProtoSourceRoots) {
+ NestedSet<String> directProtoSourceRoots,
+ boolean siblingRepositoryLayout) {
this.outputDirectory = outputDirectory;
this.directProtoSourceRoots = directProtoSourceRoots;
+ this.siblingRepositoryLayout = siblingRepositoryLayout;
}
@Override
@@ -684,7 +730,9 @@
} else {
for (String directProtoSourceRoot : directProtoSourceRoots.toList()) {
PathFragment sourceRootPath = PathFragment.create(directProtoSourceRoot);
- String arg = guessProtoPathUnderRoot(outputDirectory, sourceRootPath, proto.first);
+ String arg =
+ guessProtoPathUnderRoot(
+ outputDirectory, sourceRootPath, proto.first, siblingRepositoryLayout);
if (arg != null) {
args.accept(arg);
}
diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/AbstractContainerizingSandboxedSpawn.java b/src/main/java/com/google/devtools/build/lib/sandbox/AbstractContainerizingSandboxedSpawn.java
index 0ed26fa..a886383 100644
--- a/src/main/java/com/google/devtools/build/lib/sandbox/AbstractContainerizingSandboxedSpawn.java
+++ b/src/main/java/com/google/devtools/build/lib/sandbox/AbstractContainerizingSandboxedSpawn.java
@@ -123,7 +123,14 @@
outputs.files(),
outputs.dirs())) {
Preconditions.checkArgument(!path.isAbsolute());
- Preconditions.checkArgument(!path.containsUplevelReferences());
+ if (path.segmentCount() > 1) {
+ // Allow a single up-level reference to allow inputs from the siblings of the main
+ // repository in the sandbox execution root.
+ Preconditions.checkArgument(
+ !path.subFragment(1).containsUplevelReferences(),
+ "%s escapes the sandbox exec root.",
+ path);
+ }
for (int i = 0; i < path.segmentCount(); i++) {
dirsToCreate.add(sandboxExecRoot.getRelative(path.subFragment(0, i)));
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java
index a51d334..11fe880 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java
@@ -53,7 +53,6 @@
import com.google.devtools.build.lib.causes.LabelCause;
import com.google.devtools.build.lib.clock.BlazeClock;
import com.google.devtools.build.lib.cmdline.Label;
-import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
import com.google.devtools.build.lib.cmdline.PackageIdentifier;
import com.google.devtools.build.lib.collect.compacthashset.CompactHashSet;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
@@ -69,6 +68,7 @@
import com.google.devtools.build.lib.skyframe.ActionRewindStrategy.RewindPlan;
import com.google.devtools.build.lib.skyframe.ArtifactFunction.MissingFileArtifactValue;
import com.google.devtools.build.lib.skyframe.SkyframeActionExecutor.ActionPostprocessing;
+import com.google.devtools.build.lib.syntax.StarlarkSemantics;
import com.google.devtools.build.lib.util.io.FileOutErr;
import com.google.devtools.build.lib.util.io.TimestampGranularityMonitor;
import com.google.devtools.build.lib.vfs.FileSystem;
@@ -667,6 +667,13 @@
"resolver should only be called once: %s %s",
keysRequested,
execPaths);
+ StarlarkSemantics starlarkSemantics = PrecomputedValue.STARLARK_SEMANTICS.get(env);
+ if (starlarkSemantics == null) {
+ return null;
+ }
+
+ boolean siblingRepositoryLayout = starlarkSemantics.experimentalSiblingRepositoryLayout();
+
// Create SkyKeys list based on execPaths.
Map<PathFragment, SkyKey> depKeys = new HashMap<>();
for (PathFragment path : execPaths) {
@@ -674,19 +681,11 @@
Preconditions.checkNotNull(
path.getParentDirectory(), "Must pass in files, not root directory");
Preconditions.checkArgument(!parent.isAbsolute(), path);
- try {
- SkyKey depKey =
- ContainingPackageLookupValue.key(PackageIdentifier.discoverFromExecPath(path, true));
- depKeys.put(path, depKey);
- keysRequested.add(depKey);
- } catch (LabelSyntaxException e) {
- // This code is only used to do action cache checks. If one of the file names we got from
- // the action cache is corrupted, or if the action cache is from a different Bazel
- // binary, then the path may not be valid for this Bazel binary, and trigger this
- // exception. In that case, it's acceptable for us to ignore the exception - we'll get an
- // action cache miss and re-execute the action, which is what we should do.
- continue;
- }
+ SkyKey depKey =
+ ContainingPackageLookupValue.key(
+ PackageIdentifier.discoverFromExecPath(path, true, siblingRepositoryLayout));
+ depKeys.put(path, depKey);
+ keysRequested.add(depKey);
}
Map<SkyKey, SkyValue> values = env.getValues(depKeys.values());
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 34708f9..c37f308 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
@@ -28,6 +28,7 @@
import com.google.devtools.build.lib.packages.RepositoryFetchException;
import com.google.devtools.build.lib.pkgcache.PathPackageLocator;
import com.google.devtools.build.lib.syntax.EvalException;
+import com.google.devtools.build.lib.syntax.StarlarkSemantics;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.lib.vfs.Root;
import com.google.devtools.build.lib.vfs.RootedPath;
@@ -252,12 +253,18 @@
Transience.PERSISTENT);
}
+ StarlarkSemantics starlarkSemantics = PrecomputedValue.STARLARK_SEMANTICS.get(env);
+ if (starlarkSemantics == null) {
+ return null;
+ }
+
if (localRepository.exists()
&& !localRepository.getRepository().equals(packageIdentifier.getRepository())) {
// There is a repository mismatch, this is an error.
// The correct package path is the one originally given, minus the part that is the local
// repository.
- PathFragment pathToRequestedPackage = packageIdentifier.getPathUnderExecRoot();
+ PathFragment pathToRequestedPackage =
+ packageIdentifier.getExecPath(starlarkSemantics.experimentalSiblingRepositoryLayout());
PathFragment localRepositoryPath = localRepository.getPath();
if (localRepositoryPath.isAbsolute()) {
// We need the package path to also be absolute.
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 9d09908..33e34fb 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
@@ -93,7 +93,6 @@
import com.google.devtools.build.lib.analysis.skylark.StarlarkTransition.TransitionException;
import com.google.devtools.build.lib.buildtool.BuildRequestOptions;
import com.google.devtools.build.lib.cmdline.Label;
-import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
import com.google.devtools.build.lib.cmdline.PackageIdentifier;
import com.google.devtools.build.lib.cmdline.RepositoryName;
import com.google.devtools.build.lib.cmdline.TargetParsingException;
@@ -335,6 +334,8 @@
private final TrimmedConfigurationProgressReceiver trimmingListener =
new TrimmedConfigurationProgressReceiver(trimmingCache);
+ private boolean siblingRepositoryLayout = false;
+
/** An {@link ArtifactResolverSupplier} that supports setting of an {@link ArtifactFactory}. */
public static class MutableArtifactFactorySupplier implements ArtifactResolverSupplier {
@@ -1169,13 +1170,9 @@
throws InterruptedException {
final Map<PathFragment, SkyKey> packageKeys = new HashMap<>();
for (PathFragment execPath : execPaths) {
- try {
- PackageIdentifier pkgIdentifier =
- PackageIdentifier.discoverFromExecPath(execPath, forFiles);
- packageKeys.put(execPath, ContainingPackageLookupValue.key(pkgIdentifier));
- } catch (LabelSyntaxException e) {
- continue;
- }
+ PackageIdentifier pkgIdentifier =
+ PackageIdentifier.discoverFromExecPath(execPath, forFiles, siblingRepositoryLayout);
+ packageKeys.put(execPath, ContainingPackageLookupValue.key(pkgIdentifier));
}
EvaluationResult<ContainingPackageLookupValue> result;
@@ -1185,6 +1182,7 @@
.setNumThreads(1)
.setEventHander(eventHandler)
.build();
+
synchronized (valueLookupLock) {
result = buildDriver.evaluate(packageKeys.values(), evaluationContext);
}
@@ -1371,7 +1369,10 @@
setShowLoadingProgress(packageCacheOptions.showLoadingProgress);
setDefaultVisibility(packageCacheOptions.defaultVisibility);
- setSkylarkSemantics(getEffectiveStarlarkSemantics(starlarkSemanticsOptions));
+
+ StarlarkSemantics starlarkSemantics = getEffectiveStarlarkSemantics(starlarkSemanticsOptions);
+ setSkylarkSemantics(starlarkSemantics);
+ setSiblingDirectoryLayout(starlarkSemantics.experimentalSiblingRepositoryLayout());
setPackageLocator(pkgLocator);
syscalls.set(getPerBuildSyscallCache(packageCacheOptions.globbingThreads));
@@ -1392,6 +1393,11 @@
cyclesReporter.set(createCyclesReporter());
}
+ private void setSiblingDirectoryLayout(boolean experimentalSiblingRepositoryLayout) {
+ this.siblingRepositoryLayout = experimentalSiblingRepositoryLayout;
+ this.artifactFactory.get().setSiblingRepositoryLayout(experimentalSiblingRepositoryLayout);
+ }
+
public StarlarkSemantics getEffectiveStarlarkSemantics(
StarlarkSemanticsOptions starlarkSemanticsOptions) {
return starlarkSemanticsOptions.toSkylarkSemantics();
diff --git a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/java/JavaProtoCommonApi.java b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/java/JavaProtoCommonApi.java
index af86838..0bf0b33 100644
--- a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/java/JavaProtoCommonApi.java
+++ b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/java/JavaProtoCommonApi.java
@@ -69,7 +69,7 @@
FileT sourceJar,
String protoToolchainAttr,
String flavour)
- throws EvalException;
+ throws EvalException, InterruptedException;
@SkylarkCallable(
name = "has_proto_sources",
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 89d44c1..26bb70c 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_SIBLING_REPOSITORY_LAYOUT =
+ "experimental_sibling_repository_layout";
public static final String EXPERIMENTAL_ASPECT_OUTPUT_PROPAGATION =
"experimental_aspect_output_propagation";
public static final String EXPERIMENTAL_ENABLE_ANDROID_MIGRATION_APIS =
@@ -94,6 +96,8 @@
return experimentalActionArgs();
case FlagIdentifier.EXPERIMENTAL_ALLOW_INCREMENTAL_REPOSITORY_UPDATES:
return experimentalAllowIncrementalRepositoryUpdates();
+ case FlagIdentifier.EXPERIMENTAL_SIBLING_REPOSITORY_LAYOUT:
+ return experimentalSiblingRepositoryLayout();
case FlagIdentifier.EXPERIMENTAL_ASPECT_OUTPUT_PROPAGATION:
return experimentalAspectOutputPropagation();
case FlagIdentifier.EXPERIMENTAL_ENABLE_ANDROID_MIGRATION_APIS:
@@ -199,6 +203,8 @@
public abstract boolean experimentalRepoRemoteExec();
+ public abstract boolean experimentalSiblingRepositoryLayout();
+
public abstract boolean incompatibleAlwaysCheckDepsetElements();
public abstract boolean incompatibleApplicableLicenses();
@@ -301,6 +307,7 @@
.experimentalStarlarkUnusedInputsList(true)
.experimentalCcSharedLibrary(false)
.experimentalRepoRemoteExec(false)
+ .experimentalSiblingRepositoryLayout(false)
.incompatibleAlwaysCheckDepsetElements(false)
.incompatibleApplicableLicenses(false)
.incompatibleBzlDisallowLoadAfterStatement(true)
@@ -364,6 +371,8 @@
public abstract Builder experimentalRepoRemoteExec(boolean value);
+ public abstract Builder experimentalSiblingRepositoryLayout(boolean value);
+
public abstract Builder incompatibleAlwaysCheckDepsetElements(boolean value);
public abstract Builder incompatibleApplicableLicenses(boolean value);
diff --git a/src/main/java/com/google/devtools/build/skydoc/SkydocMain.java b/src/main/java/com/google/devtools/build/skydoc/SkydocMain.java
index efb8ecc..c1f9f94 100644
--- a/src/main/java/com/google/devtools/build/skydoc/SkydocMain.java
+++ b/src/main/java/com/google/devtools/build/skydoc/SkydocMain.java
@@ -427,7 +427,7 @@
List<AspectInfoWrapper> aspectInfoList,
ImmutableMap.Builder<Label, String> moduleDocMap)
throws InterruptedException, IOException, LabelSyntaxException, StarlarkEvaluationException {
- Path path = pathOfLabel(label);
+ Path path = pathOfLabel(label, semantics);
if (pending.contains(path)) {
throw new StarlarkEvaluationException("cycle with " + path);
@@ -462,7 +462,7 @@
throw new StarlarkEvaluationException(
String.format(
"File %s imported '%s', yet %s was not found, even at roots %s.",
- path, module, pathOfLabel(relativeLabel), depRoots),
+ path, module, pathOfLabel(relativeLabel, semantics), depRoots),
noSuchFileException);
}
}
@@ -477,10 +477,11 @@
return thread;
}
- private Path pathOfLabel(Label label) {
+ private Path pathOfLabel(Label label, StarlarkSemantics semantics) {
String workspacePrefix = "";
- if (!label.getWorkspaceRoot().isEmpty() && !label.getWorkspaceName().equals(workspaceName)) {
- workspacePrefix = label.getWorkspaceRoot() + "/";
+ if (!label.getWorkspaceRoot(semantics).isEmpty()
+ && !label.getWorkspaceName().equals(workspaceName)) {
+ workspacePrefix = label.getWorkspaceRoot(semantics) + "/";
}
return Paths.get(workspacePrefix + label.toPathFragment());
diff --git a/src/test/java/com/google/devtools/build/lib/BUILD b/src/test/java/com/google/devtools/build/lib/BUILD
index 97365d9..697a66f 100644
--- a/src/test/java/com/google/devtools/build/lib/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/BUILD
@@ -186,6 +186,7 @@
":test_runner",
":testutil",
"//src/main/java/com/google/devtools/build/lib/cmdline",
+ "//src/main/java/com/google/devtools/build/lib/syntax:frontend",
"//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
],
)
diff --git a/src/test/java/com/google/devtools/build/lib/buildtool/SymlinkForestTest.java b/src/test/java/com/google/devtools/build/lib/buildtool/SymlinkForestTest.java
index 49278d9..ca54ed6 100644
--- a/src/test/java/com/google/devtools/build/lib/buildtool/SymlinkForestTest.java
+++ b/src/test/java/com/google/devtools/build/lib/buildtool/SymlinkForestTest.java
@@ -205,7 +205,8 @@
Path linkRoot = fileSystem.getPath("/linkRoot");
linkRoot.createDirectoryAndParents();
- new SymlinkForest(packageRootMap, linkRoot, TestConstants.PRODUCT_NAME, ImmutableSortedSet.of())
+ new SymlinkForest(
+ packageRootMap, linkRoot, TestConstants.PRODUCT_NAME, ImmutableSortedSet.of(), false)
.plantSymlinkForest();
assertLinksTo(linkRoot, rootA, "pkgA");
@@ -233,7 +234,8 @@
.put(createPkg(rootX, rootY, "foo"), rootX)
.build();
- new SymlinkForest(packageRootMap, linkRoot, TestConstants.PRODUCT_NAME, ImmutableSortedSet.of())
+ new SymlinkForest(
+ packageRootMap, linkRoot, TestConstants.PRODUCT_NAME, ImmutableSortedSet.of(), false)
.plantSymlinkForest();
assertLinksTo(linkRoot, rootX, "file");
}
@@ -261,7 +263,8 @@
.put(createExternalPkg(outputBase, "Z", ""), outputBase)
.build();
- new SymlinkForest(packageRootMap, linkRoot, TestConstants.PRODUCT_NAME, ImmutableSortedSet.of())
+ new SymlinkForest(
+ packageRootMap, linkRoot, TestConstants.PRODUCT_NAME, ImmutableSortedSet.of(), false)
.plantSymlinkForest();
assertLinksTo(linkRoot, mainRepo, "dir_main");
@@ -285,6 +288,62 @@
}
@Test
+ public void test_withSiblingRepoLayout_plantSymlinkForest() throws Exception {
+ Root outputBase = Root.fromPath(fileSystem.getPath("/ob"));
+ Root mainRepo = Root.fromPath(fileSystem.getPath("/my_repo"));
+ Path linkRoot = outputBase.getRelative("execroot/ws_name");
+
+ mainRepo.asPath().createDirectoryAndParents();
+ linkRoot.createDirectoryAndParents();
+
+ ImmutableMap<PackageIdentifier, Root> packageRootMap =
+ ImmutableMap.<PackageIdentifier, Root>builder()
+ .put(createMainPkg(mainRepo, "dir_main"), mainRepo)
+ .put(createMainPkg(mainRepo, "dir_lib/pkg"), mainRepo)
+ .put(createMainPkg(mainRepo, ""), mainRepo)
+ // Remote repo without top-level package.
+ .put(createExternalPkg(outputBase, "X", "dir_x/pkg"), outputBase)
+ // Remote repo with and without top-level package.
+ .put(createExternalPkg(outputBase, "Y", ""), outputBase)
+ .put(createExternalPkg(outputBase, "Y", "dir_y/pkg"), outputBase)
+ // Only top-level pkg.
+ .put(createExternalPkg(outputBase, "Z", ""), outputBase)
+ .build();
+
+ new SymlinkForest(
+ packageRootMap, linkRoot, TestConstants.PRODUCT_NAME, ImmutableSortedSet.of(), true)
+ .plantSymlinkForest();
+
+ // Expected sibling repository layout (X, Y and Z are siblings of ws_name):
+ //
+ // .
+ // ├── execroot
+ // │ ├── ws_name { ... }
+ // │ ├── X -> external/X
+ // │ ├── Y -> external/Y
+ // │ └── Z -> external/Z
+ // └── external
+ // ├── X
+ // ├── Y
+ // └── Z
+
+ assertLinksTo(linkRoot, mainRepo, "dir_main");
+ assertLinksTo(linkRoot, mainRepo, "dir_lib");
+ assertLinksTo(linkRoot, mainRepo, "file");
+ assertLinksTo(
+ linkRoot.getParentDirectory().getRelative("X"),
+ outputBase.getRelative(LabelConstants.EXTERNAL_PATH_PREFIX + "/X"));
+ assertLinksTo(
+ linkRoot.getParentDirectory().getRelative("Y"),
+ outputBase.getRelative(LabelConstants.EXTERNAL_PATH_PREFIX + "/Y"));
+ assertLinksTo(
+ linkRoot.getParentDirectory().getRelative("Z"),
+ outputBase.getRelative(LabelConstants.EXTERNAL_PATH_PREFIX + "/Z"));
+ assertThat(linkRoot.getParentDirectory().getRelative("Y/file").exists()).isTrue();
+ assertThat(linkRoot.getParentDirectory().getRelative("Z/file").exists()).isTrue();
+ }
+
+ @Test
public void testPlantSymlinkForestForMainRepo() throws Exception {
// For the main repo, plantSymlinkForest function should only link all files and dirs under
// main repo root that're presented in packageRootMap.
@@ -305,7 +364,8 @@
.put(createExternalPkg(outputBase, "X", "dir_x/pkg"), outputBase)
.build();
- new SymlinkForest(packageRootMap, linkRoot, TestConstants.PRODUCT_NAME, ImmutableSortedSet.of())
+ new SymlinkForest(
+ packageRootMap, linkRoot, TestConstants.PRODUCT_NAME, ImmutableSortedSet.of(), false)
.plantSymlinkForest();
assertLinksTo(linkRoot, mainRepo, "dir1");
@@ -319,7 +379,7 @@
}
@Test
- public void testTestExternalDirInMainRepoIsIgnored1() throws Exception {
+ public void test_withSubdirRepoLayout_TestExternalDirInMainRepoIsIgnored1() throws Exception {
// Test external/ is ignored even when packages like "//external/foo" is specified.
Root outputBase = Root.fromPath(fileSystem.getPath("/ob"));
Root mainRepo = Root.fromPath(fileSystem.getPath("/my_repo"));
@@ -338,7 +398,8 @@
.put(createExternalPkg(outputBase, "X", "dir_x/pkg"), outputBase)
.build();
- new SymlinkForest(packageRootMap, linkRoot, TestConstants.PRODUCT_NAME, ImmutableSortedSet.of())
+ new SymlinkForest(
+ packageRootMap, linkRoot, TestConstants.PRODUCT_NAME, ImmutableSortedSet.of(), false)
.plantSymlinkForest();
assertLinksTo(linkRoot, mainRepo, "dir1");
@@ -349,7 +410,7 @@
}
@Test
- public void testTestExternalDirInMainRepoIsIgnored2() throws Exception {
+ public void test_withSubDirRepoLayout_TestExternalDirInMainRepoIsIgnored2() throws Exception {
// Test external/ is ignored when root package "//:" is specified.
Root outputBase = Root.fromPath(fileSystem.getPath("/ob"));
Root mainRepo = Root.fromPath(fileSystem.getPath("/my_repo"));
@@ -369,7 +430,8 @@
.put(createExternalPkg(outputBase, "X", "dir_x/pkg"), outputBase)
.build();
- new SymlinkForest(packageRootMap, linkRoot, TestConstants.PRODUCT_NAME, ImmutableSortedSet.of())
+ new SymlinkForest(
+ packageRootMap, linkRoot, TestConstants.PRODUCT_NAME, ImmutableSortedSet.of(), false)
.plantSymlinkForest();
assertLinksTo(linkRoot, mainRepo, "dir1");
@@ -380,6 +442,109 @@
}
@Test
+ public void test_withSiblingRepoLayout_TestExternalDirInMainRepoExists() throws Exception {
+ // Test external/ is ignored even when packages like "//external/foo" is specified.
+ Root outputBase = Root.fromPath(fileSystem.getPath("/ob"));
+ Root mainRepo = Root.fromPath(fileSystem.getPath("/my_repo"));
+ Path linkRoot = outputBase.getRelative("execroot/ws_name");
+
+ linkRoot.createDirectoryAndParents();
+ mainRepo.asPath().createDirectoryAndParents();
+
+ ImmutableMap<PackageIdentifier, Root> packageRootMap =
+ ImmutableMap.<PackageIdentifier, Root>builder()
+ .put(createMainPkg(mainRepo, "dir1/pkg/foo"), mainRepo)
+ .put(createMainPkg(mainRepo, "dir2/pkg"), mainRepo)
+ .put(createMainPkg(mainRepo, "dir3"), mainRepo)
+ // external/ should not be linked even we have "//external/foo" package
+ .put(createMainPkg(mainRepo, "external/foo"), mainRepo)
+ .put(createExternalPkg(outputBase, "X", "dir_x/pkg"), outputBase)
+ .build();
+
+ new SymlinkForest(
+ packageRootMap, linkRoot, TestConstants.PRODUCT_NAME, ImmutableSortedSet.of(), true)
+ .plantSymlinkForest();
+
+ // Expected output base layout with sibling repositories in the execroot where
+ // ws_name and X are siblings:
+ //
+ // /ob
+ // ├── execroot
+ // │ ├── ws_name
+ // │ │ ├── dir1
+ // │ │ │ └── pkg
+ // │ │ │ └── foo -> /my_repo/dir1/pkg/foo
+ // │ │ ├── dir2
+ // │ │ │ └── pkg -> /my_repo/dir2/pkg
+ // │ │ ├── dir3 -> /my_repo/dir3
+ // │ │ └── external
+ // │ │ └── foo -> /my_repo/external/foo
+ // │ └── X -> /ob/external/X
+ // └── external
+ // └── X
+
+ assertLinksTo(linkRoot, mainRepo, "dir1");
+ assertLinksTo(linkRoot, mainRepo, "dir2");
+ assertLinksTo(linkRoot, mainRepo, "dir3");
+
+ assertThat(
+ outputBase.getRelative(LabelConstants.EXTERNAL_PATH_PREFIX).getRelative("X").exists())
+ .isTrue();
+ assertThat(outputBase.getRelative("execroot/X").exists()).isTrue();
+ assertLinksTo(
+ linkRoot.getParentDirectory().getRelative("X"), // Sibling of the main repo.
+ outputBase.getRelative(LabelConstants.EXTERNAL_PATH_PREFIX).getRelative("X"));
+
+ assertThat(linkRoot.getRelative("external/foo").exists()).isTrue();
+ }
+
+ @Test
+ public void test_withSiblingRepoLayoutAndRootPackageInRoots_TestExternalDirInMainRepoExists()
+ throws Exception {
+ // Test external/ is ignored when root package "//:" is specified.
+ Root outputBase = Root.fromPath(fileSystem.getPath("/ob"));
+ Root mainRepo = Root.fromPath(fileSystem.getPath("/my_repo"));
+ Path linkRoot = outputBase.getRelative("execroot/ws_name");
+
+ linkRoot.createDirectoryAndParents();
+ mainRepo.asPath().createDirectoryAndParents();
+ mainRepo.getRelative("external/foo").createDirectoryAndParents();
+
+ ImmutableMap<PackageIdentifier, Root> packageRootMap =
+ ImmutableMap.<PackageIdentifier, Root>builder()
+ // Empty package will cause every top-level files to be linked, except external/
+ .put(createMainPkg(mainRepo, ""), mainRepo)
+ .put(createExternalPkg(outputBase, "X", "dir_x/pkg"), outputBase)
+ .build();
+
+ new SymlinkForest(
+ packageRootMap, linkRoot, TestConstants.PRODUCT_NAME, ImmutableSortedSet.of(), true)
+ .plantSymlinkForest();
+
+ // Expected output base layout with sibling repositories in the execroot where
+ // ws_name and X are siblings:
+ //
+ // /ob
+ // ├── execroot
+ // │ ├── ws_name
+ // │ │ └── external
+ // │ │ └── foo -> /my_repo/external/foo
+ // │ └── X -> /ob/external/X
+ // └── external
+ // └── X
+
+ assertThat(
+ outputBase.getRelative(LabelConstants.EXTERNAL_PATH_PREFIX).getRelative("X").exists())
+ .isTrue();
+ assertThat(outputBase.getRelative("execroot/X").exists()).isTrue();
+ assertLinksTo(
+ linkRoot.getParentDirectory().getRelative("X"), // Sibling of the main repo.
+ outputBase.getRelative(LabelConstants.EXTERNAL_PATH_PREFIX).getRelative("X"));
+
+ assertThat(linkRoot.getRelative("external/foo").exists()).isTrue();
+ }
+
+ @Test
public void testExternalPackage() throws Exception {
Path linkRoot = fileSystem.getPath("/linkRoot");
linkRoot.createDirectoryAndParents();
@@ -391,7 +556,8 @@
.put(LabelConstants.EXTERNAL_PACKAGE_IDENTIFIER, root)
.build();
- new SymlinkForest(packageRootMap, linkRoot, TestConstants.PRODUCT_NAME, ImmutableSortedSet.of())
+ new SymlinkForest(
+ packageRootMap, linkRoot, TestConstants.PRODUCT_NAME, ImmutableSortedSet.of(), false)
.plantSymlinkForest();
assertThat(linkRoot.getRelative(LabelConstants.EXTERNAL_PATH_PREFIX).exists()).isFalse();
}
@@ -417,7 +583,11 @@
.build();
new SymlinkForest(
- packageRootMap, linkRoot, TestConstants.PRODUCT_NAME, ImmutableSortedSet.of("build"))
+ packageRootMap,
+ linkRoot,
+ TestConstants.PRODUCT_NAME,
+ ImmutableSortedSet.of("build"),
+ false)
.plantSymlinkForest();
assertLinksTo(linkRoot, mainRepo, "dir1");
@@ -446,7 +616,11 @@
SymlinkForest symlinkForest =
new SymlinkForest(
- packageRootMap, linkRoot, TestConstants.PRODUCT_NAME, ImmutableSortedSet.of("build"));
+ packageRootMap,
+ linkRoot,
+ TestConstants.PRODUCT_NAME,
+ ImmutableSortedSet.of("build"),
+ false);
symlinkForest.plantSymlinkForest();
assertLinksTo(linkRoot, mainRepo, "dir1");
@@ -491,7 +665,11 @@
.build();
new SymlinkForest(
- packageRootMap, linkRoot, TestConstants.PRODUCT_NAME, ImmutableSortedSet.of("build"))
+ packageRootMap,
+ linkRoot,
+ TestConstants.PRODUCT_NAME,
+ ImmutableSortedSet.of("build"),
+ false)
.plantSymlinkForest();
assertLinksTo(linkRoot, mainRepo, "dir1");
@@ -522,7 +700,8 @@
packageRootMap,
linkRoot,
TestConstants.PRODUCT_NAME,
- ImmutableSortedSet.of("build"))
+ ImmutableSortedSet.of("build"),
+ false)
.plantSymlinkForest());
assertThat(exception)
.hasMessageThat()
@@ -558,7 +737,8 @@
packageRootMap,
linkRoot,
TestConstants.PRODUCT_NAME,
- ImmutableSortedSet.of("build"))
+ ImmutableSortedSet.of("build"),
+ false)
.plantSymlinkForest());
assertThat(exception)
.hasMessageThat()
diff --git a/src/test/java/com/google/devtools/build/lib/cmdline/LabelTest.java b/src/test/java/com/google/devtools/build/lib/cmdline/LabelTest.java
index 7f9741f..bb50aa2 100644
--- a/src/test/java/com/google/devtools/build/lib/cmdline/LabelTest.java
+++ b/src/test/java/com/google/devtools/build/lib/cmdline/LabelTest.java
@@ -18,6 +18,7 @@
import com.google.common.collect.ImmutableMap;
import com.google.common.testing.EqualsTester;
+import com.google.devtools.build.lib.syntax.StarlarkSemantics;
import com.google.devtools.build.lib.testutil.TestUtils;
import com.google.devtools.build.lib.vfs.PathFragment;
import java.util.regex.Pattern;
@@ -465,9 +466,10 @@
@Test
public void testGetWorkspaceRoot() throws Exception {
Label label = Label.parseAbsolute("//bar/baz", ImmutableMap.of());
- assertThat(label.getWorkspaceRoot()).isEmpty();
+ assertThat(label.getWorkspaceRoot(StarlarkSemantics.DEFAULT_SEMANTICS)).isEmpty();
label = Label.parseAbsolute("@repo//bar/baz", ImmutableMap.of());
- assertThat(label.getWorkspaceRoot()).isEqualTo("external/repo");
+ assertThat(label.getWorkspaceRoot(StarlarkSemantics.DEFAULT_SEMANTICS))
+ .isEqualTo("external/repo");
}
@Test
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 af485de..b13f2dc 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_sibling_repository_layout=" + rand.nextBoolean(),
"--experimental_allow_incremental_repository_updates=" + rand.nextBoolean(),
"--experimental_aspect_output_propagation=" + rand.nextBoolean(),
"--experimental_build_setting_api=" + rand.nextBoolean(),
@@ -174,6 +175,7 @@
// <== Add new options here in alphabetic order ==>
.debugDepsetDepth(rand.nextBoolean())
.experimentalActionArgs(rand.nextBoolean())
+ .experimentalSiblingRepositoryLayout(rand.nextBoolean())
.experimentalAllowIncrementalRepositoryUpdates(rand.nextBoolean())
.experimentalAspectOutputPropagation(rand.nextBoolean())
.experimentalBuildSettingApi(rand.nextBoolean())
diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcLibraryConfiguredTargetTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcLibraryConfiguredTargetTest.java
index 954e538..9c995b6 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcLibraryConfiguredTargetTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcLibraryConfiguredTargetTest.java
@@ -1279,7 +1279,7 @@
"cc_library(name='a', srcs=['a.cc'], copts=['-Id/../../somewhere'])");
CppCompileAction compileAction = getCppCompileAction("//root:a");
try {
- compileAction.verifyActionIncludePaths(compileAction.getSystemIncludeDirs());
+ compileAction.verifyActionIncludePaths(compileAction.getSystemIncludeDirs(), false);
} catch (ActionExecutionException exception) {
assertThat(exception)
.hasMessageThat()
@@ -1296,7 +1296,7 @@
"cc_library(name='a', srcs=['a.cc'], copts=['-I/somewhere'])");
CppCompileAction compileAction = getCppCompileAction("//root:a");
try {
- compileAction.verifyActionIncludePaths(compileAction.getSystemIncludeDirs());
+ compileAction.verifyActionIncludePaths(compileAction.getSystemIncludeDirs(), false);
} catch (ActionExecutionException exception) {
assertThat(exception)
.hasMessageThat()
@@ -1313,7 +1313,7 @@
"cc_library(name='a', srcs=['a.cc'], copts=['-isystem../system'])");
CppCompileAction compileAction = getCppCompileAction("//root:a");
try {
- compileAction.verifyActionIncludePaths(compileAction.getSystemIncludeDirs());
+ compileAction.verifyActionIncludePaths(compileAction.getSystemIncludeDirs(), false);
} catch (ActionExecutionException exception) {
assertThat(exception)
.hasMessageThat()
@@ -1330,7 +1330,7 @@
"cc_library(name='a', srcs=['a.cc'], copts=['-isystem/system'])");
CppCompileAction compileAction = getCppCompileAction("//root:a");
try {
- compileAction.verifyActionIncludePaths(compileAction.getSystemIncludeDirs());
+ compileAction.verifyActionIncludePaths(compileAction.getSystemIncludeDirs(), false);
} catch (ActionExecutionException exception) {
assertThat(exception)
.hasMessageThat()
diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/HeaderDiscoveryTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/HeaderDiscoveryTest.java
index de3b621..e79f974 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/cpp/HeaderDiscoveryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/HeaderDiscoveryTest.java
@@ -73,7 +73,7 @@
.setDependencies(dependencies)
.setAllowedDerivedInputs(includedHeaders)
.build()
- .discoverInputsFromDependencies(execRoot, artifactResolver);
+ .discoverInputsFromDependencies(execRoot, artifactResolver, false);
}
private SpecialArtifact treeArtifact(Path path) {
diff --git a/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcLibraryTest.java b/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcLibraryTest.java
index 7b25ee7..095351f 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcLibraryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcLibraryTest.java
@@ -971,7 +971,7 @@
assertThat(
compileAction
.discoverInputsFromDotdFiles(
- new ActionExecutionContextBuilder().build(), null, null, null)
+ new ActionExecutionContextBuilder().build(), null, null, null, false)
.toList())
.isEmpty();
}
@@ -1629,7 +1629,7 @@
ActionExecutionException.class,
() ->
compileAction.discoverInputsFromDotdFiles(
- new ActionExecutionContextBuilder().build(), null, null, null));
+ new ActionExecutionContextBuilder().build(), null, null, null, false));
assertThat(expected).hasMessageThat().contains("error while parsing .d file");
}
diff --git a/src/test/java/com/google/devtools/build/lib/rules/proto/BazelProtoLibraryTest.java b/src/test/java/com/google/devtools/build/lib/rules/proto/BazelProtoLibraryTest.java
index 6b98c9d..a905862 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/proto/BazelProtoLibraryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/proto/BazelProtoLibraryTest.java
@@ -348,8 +348,7 @@
".");
}
- @Test
- public void testExternalRepoWithGeneratedProto() throws Exception {
+ private void testExternalRepoWithGeneratedProto(boolean siblingRepoLayout) throws Exception {
if (!isThisBazel()) {
return;
}
@@ -358,6 +357,10 @@
scratch.resolve("WORKSPACE"), "local_repository(name = 'foo', path = '/foo')");
invalidatePackages();
+ if (siblingRepoLayout) {
+ setSkylarkSemanticsOptions("--experimental_sibling_repository_layout");
+ }
+
scratch.file("/foo/WORKSPACE");
scratch.file(
"/foo/x/BUILD",
@@ -383,6 +386,16 @@
}
@Test
+ public void testExternalRepoWithGeneratedProto_withSubdirRepoLayout() throws Exception {
+ testExternalRepoWithGeneratedProto(/*siblingRepoLayout=*/ false);
+ }
+
+ @Test
+ public void test_siblingRepoLayout_ExternalRepoWithGeneratedProto() throws Exception {
+ testExternalRepoWithGeneratedProto(/*siblingRepoLayout=*/ true);
+ }
+
+ @Test
public void testExportedStrippedImportPrefixes() throws Exception {
if (!isThisBazel()) {
return;
@@ -441,8 +454,7 @@
.containsExactly(genfiles + "/a/_virtual_imports/a", genfiles + "/c/_virtual_imports/c");
}
- @Test
- public void testImportPrefixInExternalRepo() throws Exception {
+ private void testImportPrefixInExternalRepo(boolean siblingRepoLayout) throws Exception {
if (!isThisBazel()) {
return;
}
@@ -451,6 +463,10 @@
scratch.resolve("WORKSPACE"), "local_repository(name = 'yolo_repo', path = '/yolo_repo')");
invalidatePackages();
+ if (siblingRepoLayout) {
+ setSkylarkSemanticsOptions("--experimental_sibling_repository_layout");
+ }
+
scratch.file("/yolo_repo/WORKSPACE");
scratch.file("/yolo_repo/yolo_pkg/yolo.proto");
scratch.file(
@@ -483,7 +499,16 @@
}
@Test
- public void testImportPrefixAndStripInExternalRepo() throws Exception {
+ public void testImportPrefixInExternalRepo_withSubdirRepoLayout() throws Exception {
+ testImportPrefixInExternalRepo(/*siblingRepoLayout=*/ false);
+ }
+
+ @Test
+ public void testImportPrefixInExternalRepo_withSiblingRepoLayout() throws Exception {
+ testImportPrefixInExternalRepo(/*siblingRepoLayout=*/ true);
+ }
+
+ private void testImportPrefixAndStripInExternalRepo(boolean siblingRepoLayout) throws Exception {
if (!isThisBazel()) {
return;
}
@@ -492,6 +517,10 @@
scratch.resolve("WORKSPACE"), "local_repository(name = 'yolo_repo', path = '/yolo_repo')");
invalidatePackages();
+ if (siblingRepoLayout) {
+ setSkylarkSemanticsOptions("--experimental_sibling_repository_layout");
+ }
+
scratch.file("/yolo_repo/WORKSPACE");
scratch.file("/yolo_repo/yolo_pkg_to_be_stripped/yolo_pkg/yolo.proto");
scratch.file(
@@ -525,7 +554,16 @@
}
@Test
- public void testStripImportPrefixInExternalRepo() throws Exception {
+ public void testImportPrefixAndStripInExternalRepo_withSubdirRepoLayout() throws Exception {
+ testImportPrefixAndStripInExternalRepo(/*siblingRepoLayout=*/ false);
+ }
+
+ @Test
+ public void testImportPrefixAndStripInExternalRepo_withSiblingRepoLayout() throws Exception {
+ testImportPrefixAndStripInExternalRepo(/*siblingRepoLayout=*/ true);
+ }
+
+ private void testStripImportPrefixInExternalRepo(boolean siblingRepoLayout) throws Exception {
if (!isThisBazel()) {
return;
}
@@ -534,6 +572,10 @@
scratch.resolve("WORKSPACE"), "local_repository(name = 'yolo_repo', path = '/yolo_repo')");
invalidatePackages();
+ if (siblingRepoLayout) {
+ setSkylarkSemanticsOptions("--experimental_sibling_repository_layout");
+ }
+
scratch.file("/yolo_repo/WORKSPACE");
scratch.file("/yolo_repo/yolo_pkg_to_be_stripped/yolo_pkg/yolo.proto");
scratch.file(
@@ -565,7 +607,17 @@
}
@Test
- public void testRelativeStripImportPrefixInExternalRepo() throws Exception {
+ public void testStripImportPrefixInExternalRepo_withSubdirRepoLayout() throws Exception {
+ testStripImportPrefixInExternalRepo(/*siblingRepoLayout=*/ false);
+ }
+
+ @Test
+ public void testStripImportPrefixInExternalRepo_withSiblingRepoLayout() throws Exception {
+ testStripImportPrefixInExternalRepo(/*siblingRepoLayout=*/ true);
+ }
+
+ private void testRelativeStripImportPrefixInExternalRepo(boolean siblingRepoLayout)
+ throws Exception {
if (!isThisBazel()) {
return;
}
@@ -574,6 +626,10 @@
scratch.resolve("WORKSPACE"), "local_repository(name = 'yolo_repo', path = '/yolo_repo')");
invalidatePackages();
+ if (siblingRepoLayout) {
+ setSkylarkSemanticsOptions("--experimental_sibling_repository_layout");
+ }
+
scratch.file("/yolo_repo/WORKSPACE");
scratch.file("/yolo_repo/yolo_pkg_to_be_stripped/yolo_pkg/yolo.proto");
scratch.file(
@@ -605,6 +661,16 @@
}
@Test
+ public void testRelativeStripImportPrefixInExternalRepo_withSubdirRepoLayout() throws Exception {
+ testRelativeStripImportPrefixInExternalRepo(/*siblingRepoLayout=*/ false);
+ }
+
+ @Test
+ public void testRelativeStripImportPrefixInExternalRepo_withSiblingRepoLayout() throws Exception {
+ testRelativeStripImportPrefixInExternalRepo(/*siblingRepoLayout=*/ true);
+ }
+
+ @Test
public void testIllegalStripImportPrefix() throws Exception {
scratch.file(
"third_party/a/BUILD",
diff --git a/src/test/java/com/google/devtools/build/lib/rules/proto/ProtoCompileActionBuilderTest.java b/src/test/java/com/google/devtools/build/lib/rules/proto/ProtoCompileActionBuilderTest.java
index c37f408..8a465b5 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/proto/ProtoCompileActionBuilderTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/proto/ProtoCompileActionBuilderTest.java
@@ -126,7 +126,8 @@
Deps.NON_STRICT,
Exports.DO_NOT_USE,
Services.ALLOW,
- /* protocOpts= */ ImmutableList.of());
+ /* protocOpts= */ ImmutableList.of(),
+ false);
assertThat(cmdLine.arguments())
.containsExactly(
@@ -158,7 +159,8 @@
Deps.NON_STRICT,
Exports.DO_NOT_USE,
Services.ALLOW,
- /* protocOpts= */ ImmutableList.of());
+ /* protocOpts= */ ImmutableList.of(),
+ false);
assertThat(cmdLine.arguments()).containsExactly("out/source_file.proto");
}
@@ -193,7 +195,8 @@
Deps.STRICT,
Exports.DO_NOT_USE,
Services.ALLOW,
- /* protocOpts= */ ImmutableList.of());
+ /* protocOpts= */ ImmutableList.of(),
+ false);
assertThat(cmdLine.arguments())
.containsExactly(
@@ -237,7 +240,8 @@
Deps.NON_STRICT,
Exports.USE,
Services.ALLOW,
- /* protocOpts= */ ImmutableList.of());
+ /* protocOpts= */ ImmutableList.of(),
+ false);
assertThat(cmdLine.arguments())
.containsExactly(
@@ -267,7 +271,8 @@
Deps.STRICT,
Exports.DO_NOT_USE,
Services.DISALLOW,
- /* protocOpts= */ ImmutableList.of("--foo", "--bar"));
+ /* protocOpts= */ ImmutableList.of("--foo", "--bar"),
+ false);
assertThat(cmdLine.arguments()).containsAtLeast("--disallow_services", "--foo", "--bar");
}
@@ -308,7 +313,8 @@
Deps.STRICT,
Exports.DO_NOT_USE,
Services.ALLOW,
- /* protocOpts= */ ImmutableList.of());
+ /* protocOpts= */ ImmutableList.of(),
+ false);
assertThat(hasBeenCalled[0]).isFalse();
cmdLine.arguments();
@@ -356,7 +362,8 @@
Deps.STRICT,
Exports.DO_NOT_USE,
Services.ALLOW,
- /* protocOpts= */ ImmutableList.of()));
+ /* protocOpts= */ ImmutableList.of(),
+ false));
assertThat(e)
.hasMessageThat()
.isEqualTo(
@@ -460,7 +467,8 @@
commandLine,
protosInDirectDependenciesBuilder,
NestedSetBuilder.wrap(Order.STABLE_ORDER, protoSourceRoots),
- transitiveImportsNestedSet);
+ transitiveImportsNestedSet,
+ false);
return commandLine.build().arguments();
}
}
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/FilesetEntryFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/FilesetEntryFunctionTest.java
index 5cf4a41..60fd338 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/FilesetEntryFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/FilesetEntryFunctionTest.java
@@ -42,6 +42,7 @@
import com.google.devtools.build.lib.pkgcache.PathPackageLocator;
import com.google.devtools.build.lib.skyframe.ExternalFilesHelper.ExternalFileAction;
import com.google.devtools.build.lib.skyframe.PackageLookupFunction.CrossRepositoryLabelViolationStrategy;
+import com.google.devtools.build.lib.syntax.StarlarkSemantics;
import com.google.devtools.build.lib.testutil.FoundationTestCase;
import com.google.devtools.build.lib.testutil.TestConstants;
import com.google.devtools.build.lib.util.Fingerprint;
@@ -139,6 +140,7 @@
driver = new SequentialBuildDriver(evaluator);
PrecomputedValue.BUILD_ID.set(differencer, UUID.randomUUID());
PrecomputedValue.PATH_PACKAGE_LOCATOR.set(differencer, pkgLocator.get());
+ PrecomputedValue.STARLARK_SEMANTICS.set(differencer, StarlarkSemantics.DEFAULT_SEMANTICS);
}
private Artifact getSourceArtifact(String path) throws Exception {
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunctionTest.java
index 953d051..bd749af 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunctionTest.java
@@ -58,6 +58,7 @@
import com.google.devtools.build.lib.skyframe.RecursiveFilesystemTraversalFunction.FileOperationException;
import com.google.devtools.build.lib.skyframe.RecursiveFilesystemTraversalValue.ResolvedFile;
import com.google.devtools.build.lib.skyframe.RecursiveFilesystemTraversalValue.TraversalRequest;
+import com.google.devtools.build.lib.syntax.StarlarkSemantics;
import com.google.devtools.build.lib.testutil.FoundationTestCase;
import com.google.devtools.build.lib.testutil.TimestampGranularityUtils;
import com.google.devtools.build.lib.util.io.OutErr;
@@ -194,6 +195,7 @@
driver = new SequentialBuildDriver(evaluator);
PrecomputedValue.BUILD_ID.set(differencer, UUID.randomUUID());
PrecomputedValue.PATH_PACKAGE_LOCATOR.set(differencer, pkgLocator.get());
+ PrecomputedValue.STARLARK_SEMANTICS.set(differencer, StarlarkSemantics.DEFAULT_SEMANTICS);
}
private Artifact sourceArtifact(String path) {
diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkIntegrationTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkIntegrationTest.java
index d530a26..163e228 100644
--- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkIntegrationTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkIntegrationTest.java
@@ -133,7 +133,7 @@
}
@Test
- public void testExternalRepoLabelWorkspaceRoot() throws Exception {
+ public void testExternalRepoLabelWorkspaceRoot_subdirRepoLayout() throws Exception {
scratch.overwriteFile(
"WORKSPACE",
new ImmutableList.Builder<String>()
@@ -160,6 +160,35 @@
}
@Test
+ public void testExternalRepoLabelWorkspaceRoot_siblingRepoLayout() throws Exception {
+ scratch.overwriteFile(
+ "WORKSPACE",
+ new ImmutableList.Builder<String>()
+ .addAll(analysisMock.getWorkspaceContents(mockToolsConfig))
+ .add("local_repository(name='r', path='/r')")
+ .build());
+
+ scratch.file("/r/WORKSPACE");
+ scratch.file(
+ "/r/test/skylark/extension.bzl",
+ "load('@//myinfo:myinfo.bzl', 'MyInfo')",
+ "def _impl(ctx):",
+ " return [MyInfo(result = ctx.label.workspace_root)]",
+ "my_rule = rule(implementation = _impl, attrs = { })");
+ scratch.file(
+ "/r/BUILD", "load('//:test/skylark/extension.bzl', 'my_rule')", "my_rule(name='t')");
+
+ // Required since we have a new WORKSPACE file.
+ invalidatePackages(true);
+
+ setSkylarkSemanticsOptions("--experimental_sibling_repository_layout");
+
+ ConfiguredTarget myTarget = getConfiguredTarget("@r//:t");
+ String result = (String) getMyInfoFromTarget(myTarget).getValue("result");
+ assertThat(result).isEqualTo("../r");
+ }
+
+ @Test
public void testSameMethodNames() throws Exception {
// The alias feature of load() may hide the fact that two methods in the stack trace have the
// same name. This is perfectly legal as long as these two methods are actually distinct.
diff --git a/src/test/shell/bazel/cc_integration_test.sh b/src/test/shell/bazel/cc_integration_test.sh
index c703887..592b28a 100755
--- a/src/test/shell/bazel/cc_integration_test.sh
+++ b/src/test/shell/bazel/cc_integration_test.sh
@@ -932,4 +932,115 @@
expect_log "@local_config_cc//:empty"
}
+function setup_workspace_layout_with_external_directory() {
+ # Make the following layout to test builds in //external subpackages:
+ #
+ #├── baz
+ #│ ├── binary.cc
+ #│ └── BUILD
+ #└── external
+ # └── foo
+ # ├── BUILD
+ # ├── lib.cc
+ # └── lib.h
+ mkdir -p external/foo
+ cat > external/foo/BUILD <<EOF
+cc_library(
+ name = "lib",
+ srcs = ["lib.cc"],
+ hdrs = ["lib.h"],
+ visibility = ["//baz:__subpackages__"],
+)
+EOF
+ cat > external/foo/lib.cc <<EOF
+#include "external/foo/lib.h"
+#include <iostream>
+
+using std::cout;
+using std::endl;
+using std::string;
+
+HelloLib::HelloLib(const string& greeting) : greeting_(new string(greeting)) {
+}
+
+void HelloLib::greet(const string& thing) {
+ cout << *greeting_ << " " << thing << endl;
+}
+EOF
+
+ cat > external/foo/lib.h <<EOF
+#include <string>
+#include <memory>
+
+class HelloLib {
+ public:
+ explicit HelloLib(const std::string &greeting);
+ void greet(const std::string &thing);
+
+ private:
+ std::unique_ptr<const std::string> greeting_;
+};
+EOF
+
+ mkdir baz
+ cat > baz/BUILD <<EOF
+cc_binary(
+ name = "binary",
+ srcs = ["binary.cc"],
+ deps = ["//external/foo:lib"],
+)
+EOF
+ cat > baz/binary.cc <<EOF
+#include "external/foo/lib.h"
+#include <string>
+
+int main(int argc, char** argv) {
+ HelloLib lib("Hello");
+ std::string thing = "world";
+ if (argc > 1) {
+ thing = argv[1];
+ }
+ lib.greet(thing);
+ return 0;
+}
+EOF
+
+}
+
+function test_execroot_subdir_layout_fails_for_external_subpackages() {
+ setup_workspace_layout_with_external_directory
+
+ bazel build --experimental_sibling_repository_layout=false //baz:binary &> $TEST_log \
+ && fail "build should have failed with sources in the external directory" || true
+ expect_log "error:.*external/foo/lib.*"
+ expect_log "Target //baz:binary failed to build"
+}
+
+function test_execroot_sibling_layout_null_build_for_external_subpackages() {
+ setup_workspace_layout_with_external_directory
+ bazel build --experimental_sibling_repository_layout //baz:binary || fail "expected build success"
+
+ # Null build.
+ bazel build --experimental_sibling_repository_layout //baz:binary &> $TEST_log || fail "expected build success"
+ expect_log "INFO: 0 processes"
+}
+
+function test_execroot_sibling_layout_header_scanning_in_external_subpackage() {
+ setup_workspace_layout_with_external_directory
+ cat << 'EOF' > external/foo/BUILD
+cc_library(
+ name = "lib",
+ srcs = ["lib.cc"],
+ # missing header declaration
+ visibility = ["//baz:__subpackages__"],
+)
+EOF
+
+ bazel build --experimental_sibling_repository_layout --spawn_strategy=standalone //external/foo:lib &> $TEST_log \
+ && fail "build should not have succeeded with missing header file"
+
+ expect_log "undeclared inclusion(s) in rule '//external/foo:lib'" \
+ "could not find 'undeclared inclusion' error message in bazel output"
+}
+
run_suite "cc_integration_test"
diff --git a/src/test/shell/bazel/execroot_test.sh b/src/test/shell/bazel/execroot_test.sh
index cd89aee..f4ad5fc 100755
--- a/src/test/shell/bazel/execroot_test.sh
+++ b/src/test/shell/bazel/execroot_test.sh
@@ -44,4 +44,54 @@
assert_contains "$(dirname $execroot)/${ws_name}/bazel-out" out
}
+function test_sibling_repository_layout() {
+ touch WORKSPACE
+
+ mkdir -p external/foo
+ cat > external/foo/BUILD <<'EOF'
+genrule(
+ name = "use-srcs",
+ srcs = ["BUILD"],
+ cmd = "cp $< $@",
+ outs = ["used-srcs"],
+)
+EOF
+
+ bazel build --experimental_sibling_repository_layout //external/foo:use-srcs \
+ || fail "expected success"
+
+ execroot="$(bazel info execution_root)"
+
+ test -e "$execroot/external/foo/BUILD"
+
+ test -e "$execroot/../bazel_tools/tools/genrule/genrule-setup.sh"
+ test ! -e "$execroot/external/bazel_tools/tools/genrule/genrule-setup.sh"
+}
+
+function test_no_sibling_repository_layout() {
+ touch WORKSPACE
+
+ mkdir -p external/foo
+ cat > external/foo/BUILD <<'EOF'
+genrule(
+ name = "use-srcs",
+ srcs = ["BUILD"],
+ cmd = "cp $< $@",
+ outs = ["used-srcs"],
+)
+EOF
+
+ bazel build //external/foo:use-srcs --experimental_sibling_repository_layout=false \
+ &> $TEST_log && fail "should have failed" || true
+ expect_log "external/foo/BUILD.*: No such file or directory"
+
+ execroot="$(bazel info execution_root)"
+
+ test ! -e "$execroot/external/foo/BUILD"
+
+ test ! -e "$execroot/../bazel_tools/tools/genrule/genrule-setup.sh"
+ test -e "$execroot/external/bazel_tools/tools/genrule/genrule-setup.sh"
+
+}
+
run_suite "execution root tests"
