From cb3c0cba52023da70655602a5ebb43462a990c7a Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Fri, 7 Oct 2022 23:16:20 +0200 Subject: [PATCH] Add `$(rlocationpath(s) ...)` expansion The new location expansion pattern `rlocationpath` and its plural version `rlocationpaths` resolve to the path(s) suitable for the Rlocation function offered by runfiles libraries. Compared to the `rootpath` pattern, which can returns '../repo_name/pkg/file` for a file in an external repository and `pkg/file` for a file in the main repository, the path returned by `rlocationpath` is always of the form `repo_name/pkg/file`. Work towards #16124 Fixes #10923 --- .../docgen/templates/be/make-variables.vm | 79 ++++++++++---- .../build/lib/analysis/LocationExpander.java | 102 ++++++++++++------ .../lib/analysis/LocationTemplateContext.java | 10 +- .../LocationExpanderIntegrationTest.java | 33 ++++++ .../lib/analysis/LocationExpanderTest.java | 12 ++- .../lib/analysis/LocationFunctionTest.java | 48 +++++---- src/test/py/bazel/runfiles_test.py | 39 +++++++ 7 files changed, 243 insertions(+), 80 deletions(-) diff --git a/src/main/java/com/google/devtools/build/docgen/templates/be/make-variables.vm b/src/main/java/com/google/devtools/build/docgen/templates/be/make-variables.vm index 096a6eac0aea0d..f5ac7d1c84abcf 100644 --- a/src/main/java/com/google/devtools/build/docgen/templates/be/make-variables.vm +++ b/src/main/java/com/google/devtools/build/docgen/templates/be/make-variables.vm @@ -262,15 +262,15 @@

Output files are staged similarly, but are also prefixed with the subpath - bazel-out/cpu-compilation_mode/bin (or for certain outputs: - bazel-out/cpu-compilation_mode/genfiles, or for the outputs - of host tools: bazel-out/host/bin). In the above example, - //testapp:app is a host tool because it appears in + bazel-out/cpu-compilation_mode/bin (or for the outputs of + tools: bazel-out/cpu-opt-exec-hash/bin). In the above example, + //testapp:app is a tool because it appears in show_app_output's tools attribute. So its output file app is written to - bazel-myproject/bazel-out/host/bin/testapp/app. The exec path - is thus bazel-out/host/bin/testapp/app. This extra prefix + bazel-myproject/bazel-out/cpu-opt-exec-hash/bin/testapp/app. + The exec path is thus + bazel-out/cpu-opt-exec-hash/bin/testapp/app. This extra prefix makes it possible to build the same target for, say, two different CPUs in the same build without the results clobbering each other.

@@ -284,15 +284,57 @@

  • - rootpath: Denotes the runfiles path that a built binary can - use to find its dependencies at runtime. -

    + rootpath: Denotes the path that a built binary can use to + find a dependency at runtime relative to the subdirectory of its runfiles + directory corresponding to the main repository. + Note: This only works if + --enable_runfiles is enabled, which is not the case on + Windows by default. Use rlocationpath instead for + cross-platform support.

    - This is the same as execpath but strips the output prefixes - described above. In the above example this means both + This is similar to execpath but strips the configuration + prefixes described above. In the example from above this means both empty.source and app use pure workspace-relative paths: testapp/empty.source and testapp/app.

    +

    + The rootpath of a file in an external repository + repo will start with ../repo/, followed by the + repository-relative path. +

    +

    + This has the same "one output only" requirements as execpath. +

    +
    • + +
  • +

    + rlocationpath: The path a built binary can pass to the + Rlocation function of a runfiles library to find a dependency at + runtime, either in the runfiles directory (if available) or using the + runfiles manifest. +

    +

    + This is similar to rootpath in that it does not contain + configuration prefixes, but differs in that it always starts with the + name of the repository. In the example from above this means that + empty.source and app result in the following + paths: myproject/testapp/empty.source and + myproject/testapp/app. +

    +

    + The rlocationpath of a file in an external repository + repo will start with repo/, followed by the + repository-relative path. +

    +

    + Passing this path to a binary and resolving it to a file system path using + the runfiles libraries is the preferred approach to find dependencies at + runtime. Compared to rootpath, it has the advantage that it + works on all platforms and even if the runfiles directory is not + available. +

    This has the same "one output only" requirements as execpath.

    @@ -303,24 +345,25 @@ rootpath, depending on the attribute being expanded. This is legacy pre-Starlark behavior and not recommended unless you really know what it does for a particular rule. See #2475 + href="https://github.com/bazelbuild/bazel/issues/2475#issuecomment-339318016">#2475 for details.

    • - execpaths, rootpaths, and locations are - the plural variations of execpath, rootpath, and - location, respectively. They support labels producing multiple - outputs, in which case each output is listed separated by a space. Zero-output - rules and malformed labels produce build errors. + execpaths, rootpaths, rlocationpaths, + and locations are the plural variations of execpath, + rootpath, rlocationpaths, andlocation, + respectively. They support labels producing multiple outputs, in which case + each output is listed separated by a space. Zero-output rules and malformed + labels produce build errors.

    All referenced labels must appear in the consuming target's srcs, output files, or deps. Otherwise the build fails. C++ targets can also reference labels in data. + href="$expander.expandRef("cc_binary.data")">data.

    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 d82d3de89a0f7d..bee7944c86835c 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 @@ -17,6 +17,7 @@ import static java.util.stream.Collectors.joining; import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Preconditions; import com.google.common.base.Supplier; import com.google.common.base.Suppliers; import com.google.common.collect.ImmutableCollection; @@ -26,7 +27,9 @@ import com.google.common.collect.Maps; import com.google.common.collect.Sets; import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.analysis.LocationExpander.LocationFunction.PathType; import com.google.devtools.build.lib.cmdline.Label; +import com.google.devtools.build.lib.cmdline.LabelConstants; import com.google.devtools.build.lib.cmdline.LabelSyntaxException; import com.google.devtools.build.lib.cmdline.RepositoryMapping; import com.google.devtools.build.lib.packages.BuildType; @@ -63,34 +66,35 @@ public final class LocationExpander { private static final boolean EXACTLY_ONE = false; private static final boolean ALLOW_MULTIPLE = true; - private static final boolean USE_LOCATION_PATHS = false; - private static final boolean USE_EXEC_PATHS = true; - private final RuleErrorConsumer ruleErrorConsumer; private final ImmutableMap functions; private final RepositoryMapping repositoryMapping; + private final String workspaceRunfilesDirectory; @VisibleForTesting LocationExpander( RuleErrorConsumer ruleErrorConsumer, Map functions, - RepositoryMapping repositoryMapping) { + RepositoryMapping repositoryMapping, + String workspaceRunfilesDirectory) { this.ruleErrorConsumer = ruleErrorConsumer; this.functions = ImmutableMap.copyOf(functions); this.repositoryMapping = repositoryMapping; + this.workspaceRunfilesDirectory = workspaceRunfilesDirectory; } private LocationExpander( - RuleErrorConsumer ruleErrorConsumer, + RuleContext ruleContext, Label root, Supplier>> locationMap, boolean execPaths, boolean legacyExternalRunfiles, RepositoryMapping repositoryMapping) { this( - ruleErrorConsumer, + ruleContext, allLocationFunctions(root, locationMap, execPaths, legacyExternalRunfiles), - repositoryMapping); + repositoryMapping, + ruleContext.getWorkspaceName()); } /** @@ -204,7 +208,7 @@ private String expand(String value, ErrorReporter reporter) { // (2) Call appropriate function to obtain string replacement. String functionValue = value.substring(nextWhitespace + 1, end).trim(); try { - String replacement = functions.get(fname).apply(functionValue, repositoryMapping); + String replacement = functions.get(fname).apply(functionValue, repositoryMapping, workspaceRunfilesDirectory); result.append(replacement); } catch (IllegalStateException ise) { reporter.report(ise.getMessage()); @@ -232,23 +236,29 @@ public String expandAttribute(String attrName, String attrValue) { @VisibleForTesting static final class LocationFunction { + enum PathType { + LOCATION, + EXEC, + RLOCATION, + } + private static final int MAX_PATHS_SHOWN = 5; private final Label root; private final Supplier>> locationMapSupplier; - private final boolean execPaths; + private final PathType pathType; private final boolean legacyExternalRunfiles; private final boolean multiple; LocationFunction( - Label root, - Supplier>> locationMapSupplier, - boolean execPaths, - boolean legacyExternalRunfiles, - boolean multiple) { + Label root, + Supplier>> locationMapSupplier, + PathType pathType, + boolean legacyExternalRunfiles, + boolean multiple) { this.root = root; this.locationMapSupplier = locationMapSupplier; - this.execPaths = execPaths; + this.pathType = Preconditions.checkNotNull(pathType); this.legacyExternalRunfiles = legacyExternalRunfiles; this.multiple = multiple; } @@ -259,10 +269,11 @@ static final class LocationFunction { * using the {@code repositoryMapping}. * * @param arg The label-like string to be expanded, e.g. ":foo" or "//foo:bar" - * @param repositoryMapping map of {@code RepositoryName}s defined in the main workspace + * @param repositoryMapping map of apparent repository names to {@code RepositoryName}s + * @param workspaceRunfilesDirectory name of the runfiles directory corresponding to the main repository * @return The expanded value */ - public String apply(String arg, RepositoryMapping repositoryMapping) { + public String apply(String arg, RepositoryMapping repositoryMapping, String workspaceRunfilesDirectory) { Label label; try { label = root.getRelativeWithRemapping(arg, repositoryMapping); @@ -271,14 +282,14 @@ public String apply(String arg, RepositoryMapping repositoryMapping) { String.format( "invalid label in %s expression: %s", functionName(), e.getMessage()), e); } - Collection paths = resolveLabel(label); + Collection paths = resolveLabel(label, workspaceRunfilesDirectory); return joinPaths(paths); } /** * Returns all target location(s) of the given label. */ - private Collection resolveLabel(Label unresolved) throws IllegalStateException { + private Collection resolveLabel(Label unresolved, String workspaceRunfilesDirectory) throws IllegalStateException { Collection artifacts = locationMapSupplier.get().get(unresolved); if (artifacts == null) { @@ -288,7 +299,7 @@ private Collection resolveLabel(Label unresolved) throws IllegalStateExc unresolved, functionName())); } - Set paths = getPaths(artifacts); + Set paths = getPaths(artifacts, workspaceRunfilesDirectory); if (paths.isEmpty()) { throw new IllegalStateException( String.format( @@ -313,24 +324,37 @@ private Collection resolveLabel(Label unresolved) throws IllegalStateExc * Extracts list of all executables associated with given collection of label artifacts. * * @param artifacts to get the paths of + * @param workspaceRunfilesDirectory name of the runfiles directory corresponding to the main repository * @return all associated executable paths */ - private Set getPaths(Collection artifacts) { + private Set getPaths(Collection artifacts, String workspaceRunfilesDirectory) { TreeSet paths = Sets.newTreeSet(); for (Artifact artifact : artifacts) { - PathFragment execPath = - execPaths - ? artifact.getExecPath() - : legacyExternalRunfiles - ? artifact.getPathForLocationExpansion() - : artifact.getRunfilesPath(); - if (execPath != null) { // omit middlemen etc - paths.add(execPath.getCallablePathString()); + PathFragment path = getPath(artifact, workspaceRunfilesDirectory); + if (path != null) { // omit middlemen etc + paths.add(path.getCallablePathString()); } } return paths; } + private PathFragment getPath(Artifact artifact, String workspaceRunfilesDirectory) { + switch (pathType) { + case LOCATION: + return legacyExternalRunfiles ? artifact.getPathForLocationExpansion() : artifact.getRunfilesPath(); + case EXEC: + return artifact.getExecPath(); + case RLOCATION: + PathFragment runfilesPath = artifact.getRunfilesPath(); + if (runfilesPath.startsWith(LabelConstants.EXTERNAL_RUNFILES_PATH_PREFIX)) { + return runfilesPath.relativeTo(LabelConstants.EXTERNAL_RUNFILES_PATH_PREFIX); + } else { + return PathFragment.create(workspaceRunfilesDirectory).getRelative(runfilesPath); + } + } + throw new IllegalStateException("Unexpected PathType: " + pathType); + } + private String joinPaths(Collection paths) { return paths.stream().map(ShellEscaper::escapeString).collect(joining(" ")); } @@ -348,27 +372,35 @@ static ImmutableMap allLocationFunctions( return new ImmutableMap.Builder() .put( "location", - new LocationFunction(root, locationMap, execPaths, legacyExternalRunfiles, EXACTLY_ONE)) + new LocationFunction(root, locationMap, execPaths ? PathType.EXEC : PathType.LOCATION, legacyExternalRunfiles, EXACTLY_ONE)) .put( "locations", new LocationFunction( - root, locationMap, execPaths, legacyExternalRunfiles, ALLOW_MULTIPLE)) + root, locationMap, execPaths ? PathType.EXEC : PathType.LOCATION, legacyExternalRunfiles, ALLOW_MULTIPLE)) .put( "rootpath", new LocationFunction( - root, locationMap, USE_LOCATION_PATHS, legacyExternalRunfiles, EXACTLY_ONE)) + root, locationMap, PathType.LOCATION, legacyExternalRunfiles, EXACTLY_ONE)) .put( "rootpaths", new LocationFunction( - root, locationMap, USE_LOCATION_PATHS, legacyExternalRunfiles, ALLOW_MULTIPLE)) + root, locationMap, PathType.LOCATION, legacyExternalRunfiles, ALLOW_MULTIPLE)) .put( "execpath", new LocationFunction( - root, locationMap, USE_EXEC_PATHS, legacyExternalRunfiles, EXACTLY_ONE)) + root, locationMap, PathType.EXEC, legacyExternalRunfiles, EXACTLY_ONE)) .put( "execpaths", new LocationFunction( - root, locationMap, USE_EXEC_PATHS, legacyExternalRunfiles, ALLOW_MULTIPLE)) + root, locationMap, PathType.EXEC, legacyExternalRunfiles, ALLOW_MULTIPLE)) + .put( + "rlocationpath", + new LocationFunction( + root, locationMap, PathType.RLOCATION, legacyExternalRunfiles, EXACTLY_ONE)) + .put( + "rlocationpaths", + new LocationFunction( + root, locationMap, PathType.RLOCATION, legacyExternalRunfiles, ALLOW_MULTIPLE)) .buildOrThrow(); } 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 f3e66781c7dec3..0de4a4e498c49b 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 @@ -50,6 +50,7 @@ final class LocationTemplateContext implements TemplateContext { private final ImmutableMap functions; private final RepositoryMapping repositoryMapping; private final boolean windowsPath; + private final String workspaceRunfilesDirectory; private LocationTemplateContext( TemplateContext delegate, @@ -58,12 +59,14 @@ private LocationTemplateContext( boolean execPaths, boolean legacyExternalRunfiles, RepositoryMapping repositoryMapping, - boolean windowsPath) { + boolean windowsPath, + String workspaceRunfilesDirectory) { this.delegate = delegate; this.functions = LocationExpander.allLocationFunctions(root, locationMap, execPaths, legacyExternalRunfiles); this.repositoryMapping = repositoryMapping; this.windowsPath = windowsPath; + this.workspaceRunfilesDirectory = workspaceRunfilesDirectory; } public LocationTemplateContext( @@ -83,7 +86,8 @@ public LocationTemplateContext( execPaths, ruleContext.getConfiguration().legacyExternalRunfiles(), ruleContext.getRule().getPackage().getRepositoryMapping(), - windowsPath); + windowsPath, + ruleContext.getWorkspaceName()); } @Override @@ -108,7 +112,7 @@ private String lookupFunctionImpl(String name, String param) throws ExpansionExc try { LocationFunction f = functions.get(name); if (f != null) { - return f.apply(param, repositoryMapping); + return f.apply(param, repositoryMapping, workspaceRunfilesDirectory); } } 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 4f40434ea75b74..25f04227f20921 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 @@ -121,6 +121,13 @@ public void otherPathExpansion() throws Exception { "genrule(name='foo', outs=['foo.txt'], cmd='never executed')", "sh_library(name='lib', srcs=[':foo'])"); + FileSystemUtils.appendIsoLatin1( + scratch.resolve("WORKSPACE"), "workspace(name='workspace')"); + // Invalidate WORKSPACE to pick up the name. + getSkyframeExecutor() + .invalidateFilesUnderPathForTesting( + reporter, ModifiedFileSet.EVERYTHING_MODIFIED, Root.fromPath(rootDirectory)); + LocationExpander expander = makeExpander("//expansion:lib"); assertThat(expander.expand("foo $(execpath :foo) bar")) .matches("foo .*-out/.*/expansion/foo\\.txt bar"); @@ -130,6 +137,22 @@ public void otherPathExpansion() throws Exception { .matches("foo expansion/foo.txt bar"); assertThat(expander.expand("foo $(rootpaths :foo) bar")) .matches("foo expansion/foo.txt bar"); + assertThat(expander.expand("foo $(rlocationpath :foo) bar")) + .isEqualTo("foo workspace/expansion/foo.txt bar"); + assertThat(expander.expand("foo $(rlocationpaths :foo) bar")) + .isEqualTo("foo workspace/expansion/foo.txt bar"); + assertThat(expander.expand("foo $(rlocationpath //expansion:foo) bar")) + .isEqualTo("foo workspace/expansion/foo.txt bar"); + assertThat(expander.expand("foo $(rlocationpaths //expansion:foo) bar")) + .isEqualTo("foo workspace/expansion/foo.txt bar"); + assertThat(expander.expand("foo $(rlocationpath @//expansion:foo) bar")) + .isEqualTo("foo workspace/expansion/foo.txt bar"); + assertThat(expander.expand("foo $(rlocationpaths @//expansion:foo) bar")) + .isEqualTo("foo workspace/expansion/foo.txt bar"); + assertThat(expander.expand("foo $(rlocationpath @workspace//expansion:foo) bar")) + .isEqualTo("foo workspace/expansion/foo.txt bar"); + assertThat(expander.expand("foo $(rlocationpaths @workspace//expansion:foo) bar")) + .isEqualTo("foo workspace/expansion/foo.txt bar"); } @Test @@ -158,6 +181,10 @@ public void otherPathExternalExpansion() throws Exception { .matches("foo external/r/p/foo.txt bar"); assertThat(expander.expand("foo $(rootpaths @r//p:foo) bar")) .matches("foo external/r/p/foo.txt bar"); + assertThat(expander.expand("foo $(rlocationpath @r//p:foo) bar")) + .isEqualTo("foo r/p/foo.txt bar"); + assertThat(expander.expand("foo $(rlocationpath @r//p:foo) bar")) + .isEqualTo("foo r/p/foo.txt bar"); } @Test @@ -183,6 +210,8 @@ public void otherPathExternalExpansionNoLegacyExternalRunfiles() throws Exceptio .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"); + assertThat(expander.expand("foo $(rlocationpath @r//p:foo) bar")).isEqualTo("foo r/p/foo.txt bar"); + assertThat(expander.expand("foo $(rlocationpath @r//p:foo) bar")).isEqualTo("foo r/p/foo.txt bar"); } @Test @@ -210,6 +239,8 @@ public void otherPathExternalExpansionNoLegacyExternalRunfilesSiblingRepositoryL .matches("foo .*-out/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"); + assertThat(expander.expand("foo $(rlocationpath @r//p:foo) bar")).isEqualTo("foo r/p/foo.txt bar"); + assertThat(expander.expand("foo $(rlocationpaths @r//p:foo) bar")).isEqualTo("foo r/p/foo.txt bar"); } @Test @@ -224,5 +255,7 @@ public void otherPathMultiExpansion() throws Exception { .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"); + assertThat(expander.expand("foo $(rlocationpaths :foo) bar")) + .isEqualTo("foo __main__/expansion/bar.txt __main__/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 abdeb1ed742df4..81fe420bc0b115 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 @@ -60,12 +60,12 @@ public boolean hasErrors() { private LocationExpander makeExpander(RuleErrorConsumer ruleErrorConsumer) throws Exception { LocationFunction f1 = new LocationFunctionBuilder("//a", false) - .setExecPaths(false) + .setPathType(LocationFunction.PathType.LOCATION) .add("//a", "/exec/src/a") .build(); LocationFunction f2 = new LocationFunctionBuilder("//b", true) - .setExecPaths(false) + .setPathType(LocationFunction.PathType.LOCATION) .add("//b", "/exec/src/b") .build(); @@ -74,7 +74,8 @@ private LocationExpander makeExpander(RuleErrorConsumer ruleErrorConsumer) throw ImmutableMap.of( "location", f1, "locations", f2), - RepositoryMapping.ALWAYS_FALLBACK); + RepositoryMapping.ALWAYS_FALLBACK, + "workspace"); } private String expand(String input) throws Exception { @@ -126,7 +127,7 @@ public void noExpansionOnError() throws Exception { @Test public void expansionWithRepositoryMapping() throws Exception { LocationFunction f1 = new LocationFunctionBuilder("//a", false) - .setExecPaths(false) + .setPathType(LocationFunction.PathType.LOCATION) .add("@bar//a", "/exec/src/a") .build(); @@ -137,7 +138,8 @@ public void expansionWithRepositoryMapping() throws Exception { new LocationExpander( new Capture(), ImmutableMap.of("location", f1), - RepositoryMapping.createAllowingFallback(repositoryMapping)); + RepositoryMapping.createAllowingFallback(repositoryMapping), + "workspace"); String value = locationExpander.expand("$(location @foo//a)"); assertThat(value).isEqualTo("src/a"); 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 38addac4979d01..e81d6a0fc155e8 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 @@ -49,16 +49,16 @@ public class LocationFunctionTest { public void absoluteAndRelativeLabels() throws Exception { LocationFunction func = new LocationFunctionBuilder("//foo", false).add("//foo", "/exec/src/bar").build(); - assertThat(func.apply("//foo", RepositoryMapping.ALWAYS_FALLBACK)).isEqualTo("src/bar"); - assertThat(func.apply(":foo", RepositoryMapping.ALWAYS_FALLBACK)).isEqualTo("src/bar"); - assertThat(func.apply("foo", RepositoryMapping.ALWAYS_FALLBACK)).isEqualTo("src/bar"); + assertThat(func.apply("//foo", RepositoryMapping.ALWAYS_FALLBACK, null)).isEqualTo("src/bar"); + assertThat(func.apply(":foo", RepositoryMapping.ALWAYS_FALLBACK, null)).isEqualTo("src/bar"); + assertThat(func.apply("foo", RepositoryMapping.ALWAYS_FALLBACK, null)).isEqualTo("src/bar"); } @Test public void pathUnderExecRootUsesDotSlash() throws Exception { LocationFunction func = new LocationFunctionBuilder("//foo", false).add("//foo", "/exec/bar").build(); - assertThat(func.apply("//foo", RepositoryMapping.ALWAYS_FALLBACK)).isEqualTo("./bar"); + assertThat(func.apply("//foo", RepositoryMapping.ALWAYS_FALLBACK, null)).isEqualTo("./bar"); } @Test @@ -67,7 +67,7 @@ public void noSuchLabel() throws Exception { IllegalStateException expected = assertThrows( IllegalStateException.class, - () -> func.apply("//bar", RepositoryMapping.ALWAYS_FALLBACK)); + () -> func.apply("//bar", RepositoryMapping.ALWAYS_FALLBACK, null)); assertThat(expected) .hasMessageThat() .isEqualTo( @@ -81,7 +81,7 @@ public void emptyList() throws Exception { IllegalStateException expected = assertThrows( IllegalStateException.class, - () -> func.apply("//foo", RepositoryMapping.ALWAYS_FALLBACK)); + () -> func.apply("//foo", RepositoryMapping.ALWAYS_FALLBACK, null)); assertThat(expected) .hasMessageThat() .isEqualTo("label '//foo:foo' in $(location) expression expands to no files"); @@ -94,7 +94,7 @@ public void tooMany() throws Exception { IllegalStateException expected = assertThrows( IllegalStateException.class, - () -> func.apply("//foo", RepositoryMapping.ALWAYS_FALLBACK)); + () -> func.apply("//foo", RepositoryMapping.ALWAYS_FALLBACK, null)); assertThat(expected) .hasMessageThat() .isEqualTo( @@ -109,7 +109,7 @@ public void noSuchLabelMultiple() throws Exception { IllegalStateException expected = assertThrows( IllegalStateException.class, - () -> func.apply("//bar", RepositoryMapping.ALWAYS_FALLBACK)); + () -> func.apply("//bar", RepositoryMapping.ALWAYS_FALLBACK, null)); assertThat(expected) .hasMessageThat() .isEqualTo( @@ -121,7 +121,7 @@ public void noSuchLabelMultiple() throws Exception { public void fileWithSpace() throws Exception { LocationFunction func = new LocationFunctionBuilder("//foo", false).add("//foo", "/exec/file/with space").build(); - assertThat(func.apply("//foo", RepositoryMapping.ALWAYS_FALLBACK)) + assertThat(func.apply("//foo", RepositoryMapping.ALWAYS_FALLBACK, null)) .isEqualTo("'file/with space'"); } @@ -130,7 +130,7 @@ public void multipleFiles() throws Exception { LocationFunction func = new LocationFunctionBuilder("//foo", true) .add("//foo", "/exec/foo/bar", "/exec/out/foo/foobar") .build(); - assertThat(func.apply("//foo", RepositoryMapping.ALWAYS_FALLBACK)) + assertThat(func.apply("//foo", RepositoryMapping.ALWAYS_FALLBACK, null)) .isEqualTo("foo/bar foo/foobar"); } @@ -139,27 +139,37 @@ public void filesWithSpace() throws Exception { LocationFunction func = new LocationFunctionBuilder("//foo", true) .add("//foo", "/exec/file/with space", "/exec/file/with spaces ") .build(); - assertThat(func.apply("//foo", RepositoryMapping.ALWAYS_FALLBACK)) + assertThat(func.apply("//foo", RepositoryMapping.ALWAYS_FALLBACK, null)) .isEqualTo("'file/with space' 'file/with spaces '"); } @Test public void execPath() throws Exception { LocationFunction func = new LocationFunctionBuilder("//foo", true) - .setExecPaths(true) + .setPathType(LocationFunction.PathType.EXEC) .add("//foo", "/exec/bar", "/exec/out/foobar") .build(); - assertThat(func.apply("//foo", RepositoryMapping.ALWAYS_FALLBACK)) + assertThat(func.apply("//foo", RepositoryMapping.ALWAYS_FALLBACK, null)) .isEqualTo("./bar out/foobar"); } + @Test + public void rlocationPath() throws Exception { + LocationFunction func = new LocationFunctionBuilder("//foo", true) + .setPathType(LocationFunction.PathType.RLOCATION) + .add("//foo", "/exec/bar", "/exec/out/foobar") + .build(); + assertThat(func.apply("//foo", RepositoryMapping.ALWAYS_FALLBACK, "workspace")) + .isEqualTo("workspace/bar workspace/foobar"); + } + @Test public void locationFunctionWithMappingReplace() throws Exception { RepositoryName b = RepositoryName.create("b"); ImmutableMap repositoryMapping = ImmutableMap.of("a", b); LocationFunction func = new LocationFunctionBuilder("//foo", false).add("@b//foo", "/exec/src/bar").build(); - assertThat(func.apply("@a//foo", RepositoryMapping.createAllowingFallback(repositoryMapping))) + assertThat(func.apply("@a//foo", RepositoryMapping.createAllowingFallback(repositoryMapping), null)) .isEqualTo("src/bar"); } @@ -170,7 +180,7 @@ public void locationFunctionWithMappingIgnoreRepo() throws Exception { LocationFunction func = new LocationFunctionBuilder("//foo", false).add("@potato//foo", "/exec/src/bar").build(); assertThat( - func.apply("@potato//foo", RepositoryMapping.createAllowingFallback(repositoryMapping))) + func.apply("@potato//foo", RepositoryMapping.createAllowingFallback(repositoryMapping), null)) .isEqualTo("src/bar"); } } @@ -178,7 +188,7 @@ public void locationFunctionWithMappingIgnoreRepo() throws Exception { final class LocationFunctionBuilder { private final Label root; private final boolean multiple; - private boolean execPaths; + private LocationFunction.PathType pathType = LocationFunction.PathType.LOCATION; private boolean legacyExternalRunfiles; private final Map> labelMap = new HashMap<>(); @@ -189,12 +199,12 @@ final class LocationFunctionBuilder { public LocationFunction build() { return new LocationFunction( - root, Suppliers.ofInstance(labelMap), execPaths, legacyExternalRunfiles, multiple); + root, Suppliers.ofInstance(labelMap), pathType, legacyExternalRunfiles, multiple); } @CanIgnoreReturnValue - public LocationFunctionBuilder setExecPaths(boolean execPaths) { - this.execPaths = execPaths; + public LocationFunctionBuilder setPathType(LocationFunction.PathType pathType) { + this.pathType = pathType; return this; } diff --git a/src/test/py/bazel/runfiles_test.py b/src/test/py/bazel/runfiles_test.py index cb37929531360e..c4c022e889b2de 100644 --- a/src/test/py/bazel/runfiles_test.py +++ b/src/test/py/bazel/runfiles_test.py @@ -312,6 +312,45 @@ def testLegacyExternalRunfilesOption(self): "bin/bin.runfiles_manifest") self.AssertFileContentNotContains(manifest_path, "__main__/external/A") + def testRunfilesLibrariesFindRlocationpathExpansion(self): + self.ScratchDir("A") + self.ScratchFile("A/WORKSPACE") + self.ScratchFile("A/p/BUILD", + ["exports_files(['foo.txt'])"]) + self.ScratchFile("A/p/foo.txt", + ["Hello, World!"]) + self.ScratchFile("WORKSPACE", + ["local_repository(name = 'A', path='A')"]) + self.ScratchFile("pkg/BUILD", [ + "py_binary(", + " name = 'bin',", + " srcs = ['bin.py'],", + " args = [", + " '$(rlocationpath bar.txt)',", + " '$(rlocationpath @A//p:foo.txt)',", + " ],", + " data = [", + " 'bar.txt',", + " '@A//p:foo.txt'", + " ],", + " deps = ['@bazel_tools//tools/python/runfiles'],", + ")", + ]) + self.ScratchFile("pkg/bar.txt", + ["Hello, Bazel!"]) + self.ScratchFile("pkg/bin.py", [ + "import sys", + "from tools.python.runfiles import runfiles", + "r = runfiles.Create()", + "for arg in sys.argv[1:]:", + " print(open(r.Rlocation(arg)).read().strip())", + ]) + _, stdout, _ = self.RunBazel(["run", "//pkg:bin"]) + if len(stdout) != 2: + self.fail("stdout: %s" % stdout) + self.assertEqual(stdout[0], "Hello, Bazel!") + self.assertEqual(stdout[1], "Hello, World!") + if __name__ == "__main__": unittest.main()