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);