diff options
4 files changed, 68 insertions, 59 deletions
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 01b97a6a0c..06842e5770 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 @@ -38,7 +38,6 @@ import java.util.List; import java.util.Map; import java.util.Set; import java.util.TreeSet; -import java.util.function.Function; import javax.annotation.Nullable; /** @@ -67,12 +66,12 @@ public final class LocationExpander { private static final boolean USE_EXEC_PATHS = true; private final RuleErrorConsumer ruleErrorConsumer; - private final ImmutableMap<String, Function<String, String>> functions; + private final ImmutableMap<String, LocationFunction> functions; @VisibleForTesting LocationExpander( RuleErrorConsumer ruleErrorConsumer, - Map<String, Function<String, String>> functions) { + Map<String, LocationFunction> functions) { this.ruleErrorConsumer = ruleErrorConsumer; this.functions = ImmutableMap.copyOf(functions); } @@ -223,7 +222,7 @@ public final class LocationExpander { } @VisibleForTesting - static final class LocationFunction implements Function<String, String> { + static final class LocationFunction { private static final int MAX_PATHS_SHOWN = 5; private final Label root; @@ -242,7 +241,12 @@ public final class LocationExpander { this.multiple = multiple; } - @Override + /** + * Looks up the label-like string in the locationMap and returns the resolved path string. + * + * @param arg The label-like string to be expanded, e.g. ":foo" or "//foo:bar" + * @return The expanded value + */ public String apply(String arg) { Label label; try { @@ -327,9 +331,9 @@ public final class LocationExpander { } } - static ImmutableMap<String, Function<String, String>> allLocationFunctions( + static ImmutableMap<String, LocationFunction> allLocationFunctions( Label root, Supplier<Map<Label, Collection<Artifact>>> locationMap, boolean execPaths) { - return new ImmutableMap.Builder<String, Function<String, String>>() + 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_ROOT_PATHS, EXACTLY_ONE)) 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 3deee31c50..faa5dff18e 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 @@ -19,12 +19,12 @@ import com.google.common.base.Suppliers; import com.google.common.collect.ImmutableCollection; import com.google.common.collect.ImmutableMap; import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.analysis.LocationExpander.LocationFunction; import com.google.devtools.build.lib.analysis.stringtemplate.ExpansionException; import com.google.devtools.build.lib.analysis.stringtemplate.TemplateContext; import com.google.devtools.build.lib.cmdline.Label; import java.util.Collection; import java.util.Map; -import java.util.function.Function; import javax.annotation.Nullable; /** @@ -46,7 +46,7 @@ import javax.annotation.Nullable; */ final class LocationTemplateContext implements TemplateContext { private final TemplateContext delegate; - private final ImmutableMap<String, Function<String, String>> functions; + private final ImmutableMap<String, LocationFunction> functions; private LocationTemplateContext( TemplateContext delegate, @@ -80,7 +80,7 @@ final class LocationTemplateContext implements TemplateContext { @Override public String lookupFunction(String name, String param) throws ExpansionException { try { - Function<String, String> f = functions.get(name); + LocationFunction f = functions.get(name); if (f != null) { return f.apply(param); } 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 f49cf5901a..c334cce1c7 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 @@ -17,11 +17,11 @@ package com.google.devtools.build.lib.analysis; import static com.google.common.truth.Truth.assertThat; import com.google.common.collect.ImmutableMap; +import com.google.devtools.build.lib.analysis.LocationExpander.LocationFunction; 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,11 +60,22 @@ public class LocationExpanderTest { } private LocationExpander makeExpander(RuleErrorConsumer ruleErrorConsumer) throws Exception { + + LocationFunction f1 = new LocationFunctionBuilder("//a", false) + .setExecPaths(false) + .add("//a", "/exec/src/a") + .build(); + + LocationFunction f2 = new LocationFunctionBuilder("//b", true) + .setExecPaths(false) + .add("//b", "/exec/src/b") + .build(); + return new LocationExpander( ruleErrorConsumer, - ImmutableMap.<String, Function<String, String>>of( - "location", (String s) -> "one(" + s + ")", - "locations", (String s) -> "more(" + s + ")")); + ImmutableMap.<String, LocationFunction>of( + "location", f1, + "locations", f2)); } private String expand(String input) throws Exception { @@ -78,14 +89,14 @@ public class LocationExpanderTest { @Test public void oneOrMore() throws Exception { - assertThat(expand("$(location a)")).isEqualTo("one(a)"); - assertThat(expand("$(locations b)")).isEqualTo("more(b)"); - assertThat(expand("---$(location a)---")).isEqualTo("---one(a)---"); + assertThat(expand("$(location a)")).isEqualTo("src/a"); + assertThat(expand("$(locations b)")).isEqualTo("src/b"); + assertThat(expand("---$(location a)---")).isEqualTo("---src/a---"); } @Test public void twoInOne() throws Exception { - assertThat(expand("$(location a) $(locations b)")).isEqualTo("one(a) more(b)"); + assertThat(expand("$(location a) $(locations b)")).isEqualTo("src/a src/b"); } @Test 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 491d54e1d7..c6ce084d7a 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 @@ -30,7 +30,6 @@ import java.util.Collection; import java.util.HashMap; import java.util.Map; import java.util.stream.Collectors; -import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -38,23 +37,6 @@ import org.junit.runners.JUnit4; /** Unit tests for {@link LocationExpander.LocationFunction}. */ @RunWith(JUnit4.class) public class LocationFunctionTest { - private FileSystem fs; - - @Before - public void createFileSystem() throws Exception { - fs = new InMemoryFileSystem(); - } - - private Artifact makeArtifact(String path) { - if (path.startsWith("/exec/out")) { - return new Artifact( - fs.getPath(path), - ArtifactRoot.asDerivedRoot(fs.getPath("/exec"), fs.getPath("/exec/out"))); - } else { - return new Artifact( - fs.getPath(path), ArtifactRoot.asSourceRoot(Root.fromPath(fs.getPath("/exec")))); - } - } @Test public void absoluteAndRelativeLabels() throws Exception { @@ -159,34 +141,46 @@ public class LocationFunctionTest { .build(); assertThat(func.apply("//foo")).isEqualTo("./bar out/foobar"); } +} - private final class LocationFunctionBuilder { - private final Label root; - private final boolean multiple; - private boolean execPaths; - private final Map<Label, Collection<Artifact>> labelMap = new HashMap<>(); +final class LocationFunctionBuilder { + private final Label root; + private final boolean multiple; + private boolean execPaths; + private final Map<Label, Collection<Artifact>> labelMap = new HashMap<>(); - LocationFunctionBuilder(String rootLabel, boolean multiple) { - this.root = Label.parseAbsoluteUnchecked(rootLabel); - this.multiple = multiple; - } + LocationFunctionBuilder(String rootLabel, boolean multiple) { + this.root = Label.parseAbsoluteUnchecked(rootLabel); + this.multiple = multiple; + } - public LocationFunction build() { - return new LocationFunction(root, Suppliers.ofInstance(labelMap), execPaths, multiple); - } + public LocationFunction build() { + return new LocationFunction(root, Suppliers.ofInstance(labelMap), execPaths, multiple); + } - public LocationFunctionBuilder setExecPaths(boolean execPaths) { - this.execPaths = execPaths; - return this; - } + public LocationFunctionBuilder setExecPaths(boolean execPaths) { + this.execPaths = execPaths; + return this; + } + + public LocationFunctionBuilder add(String label, String... paths) { + labelMap.put( + Label.parseAbsoluteUnchecked(label), + Arrays.stream(paths) + .map(LocationFunctionBuilder::makeArtifact) + .collect(Collectors.toList())); + return this; + } - public LocationFunctionBuilder add(String label, String... paths) { - labelMap.put( - Label.parseAbsoluteUnchecked(label), - Arrays.stream(paths) - .map(LocationFunctionTest.this::makeArtifact) - .collect(Collectors.toList())); - return this; + private static Artifact makeArtifact(String path) { + FileSystem fs = new InMemoryFileSystem(); + if (path.startsWith("/exec/out")) { + return new Artifact( + fs.getPath(path), + ArtifactRoot.asDerivedRoot(fs.getPath("/exec"), fs.getPath("/exec/out"))); + } else { + return new Artifact( + fs.getPath(path), ArtifactRoot.asSourceRoot(Root.fromPath(fs.getPath("/exec")))); } } } |