Use Artifact#getRunfilesPath to expand rootpath and rootpaths vars if --nolegacy_external_runfiles is specified.
PiperOrigin-RevId: 342051423
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/LocationExpander.java b/src/main/java/com/google/devtools/build/lib/analysis/LocationExpander.java
index e11c070..0b7c755 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/LocationExpander.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/LocationExpander.java
@@ -85,8 +85,12 @@
Label root,
Supplier<Map<Label, Collection<Artifact>>> locationMap,
boolean execPaths,
+ boolean legacyExternalRunfiles,
ImmutableMap<RepositoryName, RepositoryName> repositoryMapping) {
- this(ruleErrorConsumer, allLocationFunctions(root, locationMap, execPaths), repositoryMapping);
+ this(
+ ruleErrorConsumer,
+ allLocationFunctions(root, locationMap, execPaths, legacyExternalRunfiles),
+ repositoryMapping);
}
/**
@@ -111,6 +115,7 @@
Suppliers.memoize(
() -> LocationExpander.buildLocationMap(ruleContext, labelMap, allowData)),
execPaths,
+ ruleContext.getConfiguration().legacyExternalRunfiles(),
ruleContext.getRule().getPackage().getRepositoryMapping());
}
@@ -230,16 +235,19 @@
private final Label root;
private final Supplier<Map<Label, Collection<Artifact>>> locationMapSupplier;
private final boolean execPaths;
+ private final boolean legacyExternalRunfiles;
private final boolean multiple;
LocationFunction(
Label root,
Supplier<Map<Label, Collection<Artifact>>> locationMapSupplier,
boolean execPaths,
+ boolean legacyExternalRunfiles,
boolean multiple) {
this.root = root;
this.locationMapSupplier = locationMapSupplier;
this.execPaths = execPaths;
+ this.legacyExternalRunfiles = legacyExternalRunfiles;
this.multiple = multiple;
}
@@ -279,7 +287,7 @@
unresolved, functionName()));
}
- Set<String> paths = getPaths(artifacts, execPaths);
+ Set<String> paths = getPaths(artifacts);
if (paths.isEmpty()) {
throw new IllegalStateException(
String.format(
@@ -304,14 +312,17 @@
* Extracts list of all executables associated with given collection of label artifacts.
*
* @param artifacts to get the paths of
- * @param takeExecPath if false, the location path will be taken
* @return all associated executable paths
*/
- private Set<String> getPaths(Collection<Artifact> artifacts, boolean takeExecPath) {
+ private Set<String> getPaths(Collection<Artifact> artifacts) {
TreeSet<String> paths = Sets.newTreeSet();
for (Artifact artifact : artifacts) {
PathFragment execPath =
- takeExecPath ? artifact.getExecPath() : artifact.getPathForLocationExpansion();
+ execPaths
+ ? artifact.getExecPath()
+ : legacyExternalRunfiles
+ ? artifact.getPathForLocationExpansion()
+ : artifact.getRunfilesPath();
if (execPath != null) { // omit middlemen etc
paths.add(execPath.getCallablePathString());
}
@@ -329,16 +340,34 @@
}
static ImmutableMap<String, LocationFunction> allLocationFunctions(
- Label root, Supplier<Map<Label, Collection<Artifact>>> locationMap, boolean execPaths) {
+ Label root,
+ Supplier<Map<Label, Collection<Artifact>>> locationMap,
+ boolean execPaths,
+ boolean legacyExternalRunfiles) {
return new ImmutableMap.Builder<String, LocationFunction>()
- .put("location", new LocationFunction(root, locationMap, execPaths, EXACTLY_ONE))
- .put("locations", new LocationFunction(root, locationMap, execPaths, ALLOW_MULTIPLE))
- .put("rootpath", new LocationFunction(root, locationMap, USE_LOCATION_PATHS, EXACTLY_ONE))
+ .put(
+ "location",
+ new LocationFunction(root, locationMap, execPaths, legacyExternalRunfiles, EXACTLY_ONE))
+ .put(
+ "locations",
+ new LocationFunction(
+ root, locationMap, execPaths, legacyExternalRunfiles, ALLOW_MULTIPLE))
+ .put(
+ "rootpath",
+ new LocationFunction(
+ root, locationMap, USE_LOCATION_PATHS, legacyExternalRunfiles, EXACTLY_ONE))
.put(
"rootpaths",
- new LocationFunction(root, locationMap, USE_LOCATION_PATHS, ALLOW_MULTIPLE))
- .put("execpath", new LocationFunction(root, locationMap, USE_EXEC_PATHS, EXACTLY_ONE))
- .put("execpaths", new LocationFunction(root, locationMap, USE_EXEC_PATHS, ALLOW_MULTIPLE))
+ new LocationFunction(
+ root, locationMap, USE_LOCATION_PATHS, legacyExternalRunfiles, ALLOW_MULTIPLE))
+ .put(
+ "execpath",
+ new LocationFunction(
+ root, locationMap, USE_EXEC_PATHS, legacyExternalRunfiles, EXACTLY_ONE))
+ .put(
+ "execpaths",
+ new LocationFunction(
+ root, locationMap, USE_EXEC_PATHS, legacyExternalRunfiles, ALLOW_MULTIPLE))
.build();
}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/LocationTemplateContext.java b/src/main/java/com/google/devtools/build/lib/analysis/LocationTemplateContext.java
index ad25c18..494d39a 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/LocationTemplateContext.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/LocationTemplateContext.java
@@ -56,10 +56,12 @@
Label root,
Supplier<Map<Label, Collection<Artifact>>> locationMap,
boolean execPaths,
+ boolean legacyExternalRunfiles,
ImmutableMap<RepositoryName, RepositoryName> repositoryMapping,
boolean windowsPath) {
this.delegate = delegate;
- this.functions = LocationExpander.allLocationFunctions(root, locationMap, execPaths);
+ this.functions =
+ LocationExpander.allLocationFunctions(root, locationMap, execPaths, legacyExternalRunfiles);
this.repositoryMapping = repositoryMapping;
this.windowsPath = windowsPath;
}
@@ -78,6 +80,7 @@
Suppliers.memoize(
() -> LocationExpander.buildLocationMap(ruleContext, labelMap, allowData)),
execPaths,
+ ruleContext.getConfiguration().legacyExternalRunfiles(),
ruleContext.getRule().getPackage().getRepositoryMapping(),
windowsPath);
}
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/LocationExpanderIntegrationTest.java b/src/test/java/com/google/devtools/build/lib/analysis/LocationExpanderIntegrationTest.java
index 577a19c..4a22984 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/LocationExpanderIntegrationTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/LocationExpanderIntegrationTest.java
@@ -161,6 +161,31 @@
}
@Test
+ public void otherPathExternalExpansionNoLegacyExternalRunfiles() throws Exception {
+ scratch.file("expansion/BUILD", "sh_library(name='lib', srcs=['@r//p:foo'])");
+ FileSystemUtils.appendIsoLatin1(
+ scratch.resolve("WORKSPACE"), "local_repository(name='r', path='/r')");
+
+ // Invalidate WORKSPACE so @r can be resolved.
+ getSkyframeExecutor()
+ .invalidateFilesUnderPathForTesting(
+ reporter, ModifiedFileSet.EVERYTHING_MODIFIED, Root.fromPath(rootDirectory));
+
+ FileSystemUtils.createDirectoryAndParents(scratch.resolve("/foo/bar"));
+ scratch.file("/r/WORKSPACE", "workspace(name = 'r')");
+ scratch.file("/r/p/BUILD", "genrule(name='foo', outs=['foo.txt'], cmd='never executed')");
+
+ useConfiguration("--nolegacy_external_runfiles");
+ LocationExpander expander = makeExpander("//expansion:lib");
+ assertThat(expander.expand("foo $(execpath @r//p:foo) bar"))
+ .matches("foo .*-out/.*/external/r/p/foo\\.txt bar");
+ assertThat(expander.expand("foo $(execpaths @r//p:foo) bar"))
+ .matches("foo .*-out/.*/external/r/p/foo\\.txt bar");
+ assertThat(expander.expand("foo $(rootpath @r//p:foo) bar")).matches("foo ../r/p/foo.txt bar");
+ assertThat(expander.expand("foo $(rootpaths @r//p:foo) bar")).matches("foo ../r/p/foo.txt bar");
+ }
+
+ @Test
public void otherPathMultiExpansion() throws Exception {
scratch.file(
"expansion/BUILD",
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/LocationFunctionTest.java b/src/test/java/com/google/devtools/build/lib/analysis/LocationFunctionTest.java
index 8bb793a..7284784 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/LocationFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/LocationFunctionTest.java
@@ -164,6 +164,7 @@
private final Label root;
private final boolean multiple;
private boolean execPaths;
+ private boolean legacyExternalRunfiles;
private final Map<Label, Collection<Artifact>> labelMap = new HashMap<>();
LocationFunctionBuilder(String rootLabel, boolean multiple) {
@@ -172,7 +173,8 @@
}
public LocationFunction build() {
- return new LocationFunction(root, Suppliers.ofInstance(labelMap), execPaths, multiple);
+ return new LocationFunction(
+ root, Suppliers.ofInstance(labelMap), execPaths, legacyExternalRunfiles, multiple);
}
public LocationFunctionBuilder setExecPaths(boolean execPaths) {
@@ -180,6 +182,11 @@
return this;
}
+ public LocationFunctionBuilder setLegacyExternalRunfiles(boolean legacyExternalRunfiles) {
+ this.legacyExternalRunfiles = legacyExternalRunfiles;
+ return this;
+ }
+
public LocationFunctionBuilder add(String label, String... paths) {
labelMap.put(
Label.parseAbsoluteUnchecked(label),
