Make the code that is using execRoot paths use execRoot paths
Chipping away at making the big CL for #1262 smaller. Only runfiles paths
are different right now, so this makes getPathUnderExecRoot and getSourceRoot
return the same thing. Also corrected a couple places where
Label.EXTERNAL_PATH_PREFIX and Label.EXTERNAL_PACKAGE_NAME were being used
incorrectly.
--
MOS_MIGRATED_REVID=132671081
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPythonSemantics.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPythonSemantics.java
index 80f9a58..3d86e63 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPythonSemantics.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPythonSemantics.java
@@ -83,8 +83,7 @@
@Override
public List<PathFragment> getImports(RuleContext ruleContext) {
List<PathFragment> result = new ArrayList<>();
- PathFragment packageFragment = ruleContext.getLabel().getPackageIdentifier()
- .getPathUnderExecRoot();
+ PathFragment packageFragment = ruleContext.getLabel().getPackageIdentifier().getRunfilesPath();
// Python scripts start with x.runfiles/ as the module space, so everything must be manually
// adjusted to be relative to the workspace name.
packageFragment = new PathFragment(ruleContext.getWorkspaceName())
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 a1050ae..44cf5f1 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
@@ -134,13 +134,14 @@
PackageIdentifier dir = entry.getKey();
if (!dir.getRepository().isMain()) {
FileSystemUtils.createDirectoryAndParents(
- workspace.getRelative(dir.getRepository().getSourceRoot()));
+ workspace.getRelative(dir.getRepository().getPathUnderExecRoot()));
}
if (entry.getValue().size() > 1) {
if (LOG_FINER) {
- LOG.finer("mkdir " + workspace.getRelative(dir.getSourceRoot()));
+ LOG.finer("mkdir " + workspace.getRelative(dir.getPathUnderExecRoot()));
}
- FileSystemUtils.createDirectoryAndParents(workspace.getRelative(dir.getSourceRoot()));
+ FileSystemUtils.createDirectoryAndParents(
+ workspace.getRelative(dir.getPathUnderExecRoot()));
}
}
@@ -158,9 +159,9 @@
Path root = roots.iterator().next(); // lone root in set
if (LOG_FINER) {
LOG.finer("ln -s " + root.getRelative(dir.getSourceRoot()) + " "
- + workspace.getRelative(dir.getSourceRoot()));
+ + workspace.getRelative(dir.getPathUnderExecRoot()));
}
- workspace.getRelative(dir.getSourceRoot())
+ workspace.getRelative(dir.getPathUnderExecRoot())
.createSymbolicLink(root.getRelative(dir.getSourceRoot()));
}
}
@@ -202,7 +203,7 @@
if (!pkgId.getPackageFragment().equals(PathFragment.EMPTY_FRAGMENT)) {
continue;
}
- Path execrootDirectory = workspace.getRelative(pkgId.getSourceRoot());
+ Path execrootDirectory = workspace.getRelative(pkgId.getPathUnderExecRoot());
// If there were no subpackages, this directory might not exist yet.
if (!execrootDirectory.exists()) {
FileSystemUtils.createDirectoryAndParents(execrootDirectory);
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 a20ea18..905866f 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
@@ -123,12 +123,16 @@
return repository.getSourceRoot().getRelative(pkgName);
}
+ public PathFragment getPathUnderExecRoot() {
+ return getSourceRoot();
+ }
+
/**
* Returns the runfiles/execRoot path for this repository (relative to the x.runfiles/main-repo/
* directory).
*/
- public PathFragment getPathUnderExecRoot() {
- return getRepository().getPathUnderExecRoot().getRelative(getPackageFragment());
+ public PathFragment getRunfilesPath() {
+ return repository.getRunfilesPath().getRelative(pkgName);
}
public PackageIdentifier makeAbsolute() {
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 25332b3..ba0f6b4 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
@@ -210,16 +210,24 @@
*/
public PathFragment getSourceRoot() {
return isDefault() || isMain()
+ ? PathFragment.EMPTY_FRAGMENT : Label.EXTERNAL_PACKAGE_NAME.getRelative(strippedName());
+ }
+
+ /**
+ * 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.
+ */
+ public PathFragment getPathUnderExecRoot() {
+ return isDefault() || isMain()
? PathFragment.EMPTY_FRAGMENT
: new PathFragment(Label.EXTERNAL_PATH_PREFIX).getRelative(strippedName());
}
/**
- * Returns the runfiles/execRoot path for this repository (relative to the x.runfiles/main-repo/
- * directory). If we don't know the name of this repo (i.e., it is in the main repository),
- * return an empty path fragment.
+ * Returns the runfiles path relative to the x.runfiles/main-repo directory.
*/
- public PathFragment getPathUnderExecRoot() {
+ // TODO(kchodorow): remove once execroot is reorg-ed.
+ public PathFragment getRunfilesPath() {
return isDefault() || isMain()
? PathFragment.EMPTY_FRAGMENT : new PathFragment("..").getRelative(strippedName());
}
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 a02a003..c7f11d8 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
@@ -357,14 +357,15 @@
List<PathFragment> result = new ArrayList<>();
// The package directory of the rule contributes includes. Note that this also covers all
// non-subpackage sub-directories.
- PathFragment rulePackage = ruleContext.getLabel().getPackageIdentifier().getSourceRoot();
+ PathFragment rulePackage = ruleContext.getLabel().getPackageIdentifier()
+ .getPathUnderExecRoot();
result.add(rulePackage);
// Gather up all the dirs from the rule's srcs as well as any of the srcs outputs.
if (hasAttribute("srcs", BuildType.LABEL_LIST)) {
for (TransitiveInfoCollection src :
ruleContext.getPrerequisitesIf("srcs", Mode.TARGET, FileProvider.class)) {
- PathFragment packageDir = src.getLabel().getPackageIdentifier().getSourceRoot();
+ PathFragment packageDir = src.getLabel().getPackageIdentifier().getPathUnderExecRoot();
for (Artifact a : src.getProvider(FileProvider.class).getFilesToBuild()) {
result.add(packageDir);
// Attempt to gather subdirectories that might contain include files.
@@ -376,7 +377,7 @@
// Add in any 'includes' attribute values as relative path fragments
if (ruleContext.getRule().isAttributeValueExplicitlySpecified("includes")) {
PathFragment packageFragment = ruleContext.getLabel().getPackageIdentifier()
- .getSourceRoot();
+ .getPathUnderExecRoot();
// For now, anything with an 'includes' needs a blanket declaration
result.add(packageFragment.getRelative("**"));
}
@@ -386,7 +387,7 @@
List<PathFragment> getSystemIncludeDirs() {
List<PathFragment> result = new ArrayList<>();
PackageIdentifier packageIdentifier = ruleContext.getLabel().getPackageIdentifier();
- PathFragment packageFragment = packageIdentifier.getSourceRoot();
+ PathFragment packageFragment = packageIdentifier.getPathUnderExecRoot();
for (String includesAttr : ruleContext.attributes().get("includes", Type.STRING_LIST)) {
includesAttr = ruleContext.expandMakeVariables("includes", includesAttr);
if (includesAttr.startsWith("/")) {
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java
index 6de8ad1..70d74af 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java
@@ -1098,7 +1098,7 @@
// before the genfilesFragment to preferably pick up source files. Otherwise
// we might pick up stale generated files.
PathFragment repositoryPath =
- ruleContext.getLabel().getPackageIdentifier().getRepository().getSourceRoot();
+ ruleContext.getLabel().getPackageIdentifier().getRepository().getPathUnderExecRoot();
contextBuilder.addQuoteIncludeDir(repositoryPath);
contextBuilder.addQuoteIncludeDir(
ruleContext.getConfiguration().getGenfilesFragment().getRelative(repositoryPath));
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java
index d076040..471815a 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java
@@ -358,7 +358,7 @@
this.lipoContextCollector = cppOptions.lipoCollector;
- this.crosstoolTopPathFragment = crosstoolTop.getPackageIdentifier().getSourceRoot();
+ this.crosstoolTopPathFragment = crosstoolTop.getPackageIdentifier().getPathUnderExecRoot();
try {
this.staticRuntimeLibsLabel =
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java
index ee2b08d..49eefe9 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java
@@ -88,7 +88,7 @@
if (stl != null) {
// TODO(bazel-team): Clean this up.
contextBuilder.addSystemIncludeDir(
- stl.getLabel().getPackageIdentifier().getSourceRoot().getRelative("gcc3"));
+ stl.getLabel().getPackageIdentifier().getPathUnderExecRoot().getRelative("gcc3"));
contextBuilder.mergeDependentContext(stl.getProvider(CppCompilationContext.class));
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/Jvm.java b/src/main/java/com/google/devtools/build/lib/rules/java/Jvm.java
index 3331115..37271b8 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/Jvm.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/Jvm.java
@@ -111,7 +111,7 @@
if (jvmLabel == null || jvmLabel.getPackageIdentifier().getRepository().isMain()) {
return getJavaExecutable();
}
- return jvmLabel.getPackageIdentifier().getRepository().getPathUnderExecRoot().getRelative(BIN_JAVA);
+ return jvmLabel.getPackageIdentifier().getRepository().getRunfilesPath().getRelative(BIN_JAVA);
}
@Override
diff --git a/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryFunction.java b/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryFunction.java
index cb01f31..4e6628d 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryFunction.java
@@ -347,10 +347,8 @@
return value;
}
- public static Path getExternalRepositoryDirectory(BlazeDirectories directories) {
- return directories
- .getOutputBase()
- .getRelative(Label.EXTERNAL_PATH_PREFIX);
+ protected static Path getExternalRepositoryDirectory(BlazeDirectories directories) {
+ return directories.getOutputBase().getRelative(Label.EXTERNAL_PACKAGE_NAME);
}
/**
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ExternalFilesHelper.java b/src/main/java/com/google/devtools/build/lib/skyframe/ExternalFilesHelper.java
index b1b6259..664cf98 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ExternalFilesHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ExternalFilesHelper.java
@@ -137,7 +137,7 @@
return FileType.EXTERNAL_MUTABLE;
}
if (rootedPath.asPath().startsWith(outputBase)) {
- Path externalRepoDir = outputBase.getRelative(Label.EXTERNAL_PATH_PREFIX);
+ Path externalRepoDir = outputBase.getRelative(Label.EXTERNAL_PACKAGE_NAME);
if (rootedPath.asPath().startsWith(externalRepoDir)) {
anyNonOutputExternalFilesSeen = true;
return FileType.EXTERNAL_REPO;
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java
index 2708d97..aac6fe6 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java
@@ -778,7 +778,7 @@
protected Rule scratchRule(String packageName, String ruleName, String... lines)
throws Exception {
String buildFilePathString = packageName + "/BUILD";
- if (packageName.equals(Label.EXTERNAL_PATH_PREFIX)) {
+ if (packageName.equals(Label.EXTERNAL_PACKAGE_NAME.getPathString())) {
buildFilePathString = "WORKSPACE";
scratch.overwriteFile(buildFilePathString, lines);
} else {
diff --git a/src/test/java/com/google/devtools/build/lib/cmdline/PackageIdentifierTest.java b/src/test/java/com/google/devtools/build/lib/cmdline/PackageIdentifierTest.java
index e5a3ebb..3f5a0e0 100644
--- a/src/test/java/com/google/devtools/build/lib/cmdline/PackageIdentifierTest.java
+++ b/src/test/java/com/google/devtools/build/lib/cmdline/PackageIdentifierTest.java
@@ -102,9 +102,9 @@
@Test
public void testRunfilesDir() throws Exception {
- assertThat(PackageIdentifier.create("@foo", new PathFragment("bar/baz")).getPathUnderExecRoot())
+ assertThat(PackageIdentifier.create("@foo", new PathFragment("bar/baz")).getRunfilesPath())
.isEqualTo(new PathFragment("../foo/bar/baz"));
- assertThat(PackageIdentifier.create("@", new PathFragment("bar/baz")).getPathUnderExecRoot())
+ assertThat(PackageIdentifier.create("@", new PathFragment("bar/baz")).getRunfilesPath())
.isEqualTo(new PathFragment("bar/baz"));
}
}
diff --git a/src/test/java/com/google/devtools/build/lib/cmdline/RepositoryNameTest.java b/src/test/java/com/google/devtools/build/lib/cmdline/RepositoryNameTest.java
index 7d35861..9a39bbf 100644
--- a/src/test/java/com/google/devtools/build/lib/cmdline/RepositoryNameTest.java
+++ b/src/test/java/com/google/devtools/build/lib/cmdline/RepositoryNameTest.java
@@ -59,11 +59,11 @@
@Test
public void testRunfilesDir() throws Exception {
- assertThat(RepositoryName.create("@foo").getPathUnderExecRoot())
+ assertThat(RepositoryName.create("@foo").getRunfilesPath())
.isEqualTo(new PathFragment("../foo"));
- assertThat(RepositoryName.create("@").getPathUnderExecRoot())
+ assertThat(RepositoryName.create("@").getRunfilesPath())
.isEqualTo(PathFragment.EMPTY_FRAGMENT);
- assertThat(RepositoryName.create("").getPathUnderExecRoot())
+ assertThat(RepositoryName.create("").getRunfilesPath())
.isEqualTo(PathFragment.EMPTY_FRAGMENT);
}
diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CrosstoolConfigurationLoaderTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CrosstoolConfigurationLoaderTest.java
index b207a13..7eb0abb 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CrosstoolConfigurationLoaderTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CrosstoolConfigurationLoaderTest.java
@@ -677,7 +677,7 @@
TestConstants.TOOLS_REPOSITORY,
new PathFragment(
new PathFragment(TestConstants.TOOLS_REPOSITORY_PATH), new PathFragment(path)));
- return packageIdentifier.getSourceRoot();
+ return packageIdentifier.getPathUnderExecRoot();
}
private void checkToolchainB(CppConfigurationLoader loader, LipoMode lipoMode, String... args)
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/FileFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/FileFunctionTest.java
index 0269305..323fa84 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/FileFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/FileFunctionTest.java
@@ -319,7 +319,7 @@
@Test
public void testAbsoluteSymlinkToExternal() throws Exception {
String externalPath =
- outputBase.getRelative(Label.EXTERNAL_PATH_PREFIX).getRelative("a/b").getPathString();
+ outputBase.getRelative(Label.EXTERNAL_PACKAGE_NAME).getRelative("a/b").getPathString();
symlink("a", externalPath);
file("b");
file(externalPath);
