Add rootpath(s) and execpath(s) functions to template expansion
In addition to the $(location) function, we now also support $(rootpath) and
$(execpath) functions.
Unfortunately, we have to do this in two places since the Skylark API for expand_location has to continue calling into LocationExpander in order to preserve its semantic contract.
Progress on #2475.
RELNOTES[NEW]:
In addition to $(location), Bazel now also supports $(rootpath) to obtain
the root-relative path (i.e., for runfiles locations), and $(execpath) to
obtain the exec path (i.e., for build-time locations)
PiperOrigin-RevId: 174454119
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 2577ca4..bf8f10c 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
@@ -72,20 +72,21 @@
ALLOW_DATA,
}
- private static final String LOCATION = "$(location";
+ private static final boolean EXACTLY_ONE = false;
+ private static final boolean ALLOW_MULTIPLE = true;
+
+ private static final boolean USE_ROOT_PATHS = false;
+ private static final boolean USE_EXEC_PATHS = true;
private final RuleErrorConsumer ruleErrorConsumer;
- private final Function<String, String> locationFunction;
- private final Function<String, String> locationsFunction;
+ private final ImmutableMap<String, Function<String, String>> functions;
@VisibleForTesting
LocationExpander(
RuleErrorConsumer ruleErrorConsumer,
- Function<String, String> locationFunction,
- Function<String, String> locationsFunction) {
+ Map<String, Function<String, String>> functions) {
this.ruleErrorConsumer = ruleErrorConsumer;
- this.locationFunction = locationFunction;
- this.locationsFunction = locationsFunction;
+ this.functions = ImmutableMap.copyOf(functions);
}
private LocationExpander(
@@ -95,8 +96,7 @@
boolean execPaths) {
this(
ruleErrorConsumer,
- new LocationFunction(root, locationMap, execPaths, false),
- new LocationFunction(root, locationMap, execPaths, true));
+ allLocationFunctions(root, locationMap, execPaths));
}
/**
@@ -167,44 +167,40 @@
StringBuilder result = new StringBuilder(value.length());
while (true) {
- // (1) Find '$(location ' or '$(locations '.
- Function<String, String> func = locationFunction;
- int start = value.indexOf(LOCATION, restart);
- int scannedLength = LOCATION.length();
- if (start == -1 || start + scannedLength == attrLength) {
+ // (1) Find '$(<fname> '.
+ int start = value.indexOf("$(", restart);
+ if (start == -1) {
result.append(value.substring(restart));
break;
}
- if (value.charAt(start + scannedLength) == 's') {
- scannedLength++;
- if (start + scannedLength == attrLength) {
- result.append(value.substring(restart));
- break;
- }
- func = locationsFunction;
+ int nextWhitespace = value.indexOf(' ', start);
+ if (nextWhitespace == -1) {
+ result.append(value, restart, start + 2);
+ restart = start + 2;
+ continue;
}
- if (value.charAt(start + scannedLength) != ' ') {
- result.append(value, restart, start + scannedLength);
- restart = start + scannedLength;
+ String fname = value.substring(start + 2, nextWhitespace);
+ if (!functions.containsKey(fname)) {
+ result.append(value, restart, start + 2);
+ restart = start + 2;
continue;
}
result.append(value, restart, start);
- scannedLength++;
- int end = value.indexOf(')', start + scannedLength);
+ int end = value.indexOf(')', nextWhitespace);
if (end == -1) {
reporter.report(
String.format(
"unterminated $(%s) expression",
- value.substring(start + 2, start + scannedLength - 1)));
+ value.substring(start + 2, nextWhitespace)));
return value;
}
// (2) Call appropriate function to obtain string replacement.
- String functionValue = value.substring(start + scannedLength, end).trim();
+ String functionValue = value.substring(nextWhitespace + 1, end).trim();
try {
- String replacement = func.apply(functionValue);
+ String replacement = functions.get(fname).apply(functionValue);
result.append(replacement);
} catch (IllegalStateException ise) {
reporter.report(ise.getMessage());
@@ -322,6 +318,18 @@
}
}
+ static ImmutableMap<String, Function<String, String>> allLocationFunctions(
+ Label root, Supplier<Map<Label, Collection<Artifact>>> locationMap, boolean execPaths) {
+ return new ImmutableMap.Builder<String, Function<String, String>>()
+ .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_ROOT_PATHS, EXACTLY_ONE))
+ .put("rootpaths", new LocationFunction(root, locationMap, USE_ROOT_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))
+ .build();
+ }
+
/**
* Extracts all possible target locations from target specification.
*
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 d288820..96bb24f 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
@@ -20,7 +20,6 @@
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.actions.Artifact;
-import com.google.devtools.build.lib.analysis.LocationExpander.LocationFunction;
import com.google.devtools.build.lib.analysis.LocationExpander.Options;
import com.google.devtools.build.lib.analysis.stringtemplate.ExpansionException;
import com.google.devtools.build.lib.analysis.stringtemplate.TemplateContext;
@@ -49,8 +48,7 @@
*/
final class LocationTemplateContext implements TemplateContext {
private final TemplateContext delegate;
- private final Function<String, String> locationFunction;
- private final Function<String, String> locationsFunction;
+ private final ImmutableMap<String, Function<String, String>> functions;
private LocationTemplateContext(
TemplateContext delegate,
@@ -58,8 +56,7 @@
Supplier<Map<Label, Collection<Artifact>>> locationMap,
boolean execPaths) {
this.delegate = delegate;
- this.locationFunction = new LocationFunction(root, locationMap, execPaths, false);
- this.locationsFunction = new LocationFunction(root, locationMap, execPaths, true);
+ this.functions = LocationExpander.allLocationFunctions(root, locationMap, execPaths);
}
private LocationTemplateContext(
@@ -98,10 +95,9 @@
@Override
public String lookupFunction(String name, String param) throws ExpansionException {
try {
- if ("location".equals(name)) {
- return locationFunction.apply(param);
- } else if ("locations".equals(name)) {
- return locationsFunction.apply(param);
+ Function<String, String> f = functions.get(name);
+ if (f != null) {
+ return f.apply(param);
}
} catch (IllegalStateException e) {
throw new ExpansionException(e.getMessage(), e);
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 6dc4574..a1137cd 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
@@ -63,4 +63,36 @@
assertThat(result).isEqualTo("foo 'spaces/file with space A' 'spaces/file with space B' bar");
}
+
+ @Test
+ public void otherPathExpansion() throws Exception {
+ scratch.file(
+ "expansion/BUILD",
+ "genrule(name='foo', outs=['foo.txt'], cmd='never executed')",
+ "sh_library(name='lib', srcs=[':foo'])");
+
+ LocationExpander expander = makeExpander("//expansion:lib");
+ assertThat(expander.expand("foo $(execpath :foo) bar"))
+ .matches("foo .*-out/.*/expansion/foo\\.txt bar");
+ assertThat(expander.expand("foo $(execpaths :foo) bar"))
+ .matches("foo .*-out/.*/expansion/foo\\.txt bar");
+ assertThat(expander.expand("foo $(rootpath :foo) bar"))
+ .matches("foo expansion/foo.txt bar");
+ assertThat(expander.expand("foo $(rootpaths :foo) bar"))
+ .matches("foo expansion/foo.txt bar");
+ }
+
+ @Test
+ public void otherPathMultiExpansion() throws Exception {
+ scratch.file(
+ "expansion/BUILD",
+ "genrule(name='foo', outs=['foo.txt', 'bar.txt'], cmd='never executed')",
+ "sh_library(name='lib', srcs=[':foo'])");
+
+ LocationExpander expander = makeExpander("//expansion:lib");
+ assertThat(expander.expand("foo $(execpaths :foo) bar"))
+ .matches("foo .*-out/.*/expansion/bar\\.txt .*-out/.*/expansion/foo\\.txt bar");
+ assertThat(expander.expand("foo $(rootpaths :foo) bar"))
+ .matches("foo expansion/bar.txt expansion/foo.txt bar");
+ }
}
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/LocationExpanderTest.java b/src/test/java/com/google/devtools/build/lib/analysis/LocationExpanderTest.java
index 7fe4066..f49cf59 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/LocationExpanderTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/LocationExpanderTest.java
@@ -16,10 +16,12 @@
import static com.google.common.truth.Truth.assertThat;
+import com.google.common.collect.ImmutableMap;
import com.google.devtools.build.lib.packages.AbstractRuleErrorConsumer;
import com.google.devtools.build.lib.packages.RuleErrorConsumer;
import java.util.ArrayList;
import java.util.List;
+import java.util.function.Function;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
@@ -60,8 +62,9 @@
private LocationExpander makeExpander(RuleErrorConsumer ruleErrorConsumer) throws Exception {
return new LocationExpander(
ruleErrorConsumer,
- (String s) -> "one(" + s + ")",
- (String s) -> "more(" + s + ")");
+ ImmutableMap.<String, Function<String, String>>of(
+ "location", (String s) -> "one(" + s + ")",
+ "locations", (String s) -> "more(" + s + ")"));
}
private String expand(String input) throws Exception {
