From afeeb5da1edf7e3ac0e864bf7315980ea9df2542 Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Mon, 24 Oct 2022 22:56:27 +0200 Subject: [PATCH 1/9] Simplify tests --- tools/java/runfiles/testing/BUILD | 9 -- tools/java/runfiles/testing/MockFile.java | 45 ------- tools/java/runfiles/testing/RunfilesTest.java | 111 +++++++++--------- 3 files changed, 53 insertions(+), 112 deletions(-) delete mode 100644 tools/java/runfiles/testing/MockFile.java diff --git a/tools/java/runfiles/testing/BUILD b/tools/java/runfiles/testing/BUILD index 82d7836e16cde3..841724b36dc453 100644 --- a/tools/java/runfiles/testing/BUILD +++ b/tools/java/runfiles/testing/BUILD @@ -36,7 +36,6 @@ java_library( name = "test_deps", testonly = 1, exports = [ - ":mock-file", "//third_party:guava", "//third_party:guava-testlib", "//third_party:junit4", @@ -44,11 +43,3 @@ java_library( "//tools/java/runfiles", ], ) - -java_library( - name = "mock-file", - testonly = 1, - srcs = ["MockFile.java"], - exports = ["//third_party:guava"], - deps = ["//third_party:guava"], -) diff --git a/tools/java/runfiles/testing/MockFile.java b/tools/java/runfiles/testing/MockFile.java deleted file mode 100644 index 886bfcbcc82a38..00000000000000 --- a/tools/java/runfiles/testing/MockFile.java +++ /dev/null @@ -1,45 +0,0 @@ -// Copyright 2018 The Bazel Authors. All rights reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package com.google.devtools.build.runfiles; - -import com.google.common.base.Strings; -import com.google.common.collect.ImmutableList; -import java.io.Closeable; -import java.io.File; -import java.io.IOException; -import java.nio.charset.StandardCharsets; -import java.nio.file.Files; -import java.nio.file.Path; - -final class MockFile implements Closeable { - - public final Path path; - - public MockFile(ImmutableList lines) throws IOException { - String testTmpdir = System.getenv("TEST_TMPDIR"); - if (Strings.isNullOrEmpty(testTmpdir)) { - throw new IOException("$TEST_TMPDIR is empty or undefined"); - } - path = Files.createTempFile(new File(testTmpdir).toPath(), null, null); - Files.write(path, lines, StandardCharsets.UTF_8); - } - - @Override - public void close() throws IOException { - if (path != null) { - Files.delete(path); - } - } -} diff --git a/tools/java/runfiles/testing/RunfilesTest.java b/tools/java/runfiles/testing/RunfilesTest.java index e98ca554a8a27d..d8eb71988606e0 100644 --- a/tools/java/runfiles/testing/RunfilesTest.java +++ b/tools/java/runfiles/testing/RunfilesTest.java @@ -25,17 +25,23 @@ import java.nio.file.FileSystems; import java.nio.file.Files; import java.nio.file.Path; -import java.util.Collections; import java.util.Map; import javax.annotation.Nullable; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.TemporaryFolder; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; -/** Unit tests for {@link Runfiles}. */ +/** + * Unit tests for {@link Runfiles}. + */ @RunWith(JUnit4.class) public final class RunfilesTest { + @Rule + public TemporaryFolder tempDir = new TemporaryFolder(new File(System.getenv("TEST_TMPDIR"))); + private static boolean isWindows() { return File.separatorChar == '\\'; } @@ -44,7 +50,7 @@ private void assertRlocationArg(Runfiles runfiles, String path, @Nullable String IllegalArgumentException e = assertThrows(IllegalArgumentException.class, () -> runfiles.rlocation(path)); if (error != null) { - assertThat(e).hasMessageThat().contains(error); + assertThat(e).hasMessageThat().contains(error); } } @@ -71,24 +77,23 @@ public void testRlocationArgumentValidation() throws Exception { @Test public void testCreatesManifestBasedRunfiles() throws Exception { - try (MockFile mf = new MockFile(ImmutableList.of("a/b c/d"))) { - Runfiles r = - Runfiles.create( - ImmutableMap.of( - "RUNFILES_MANIFEST_ONLY", "1", - "RUNFILES_MANIFEST_FILE", mf.path.toString(), - "RUNFILES_DIR", "ignored when RUNFILES_MANIFEST_ONLY=1", - "JAVA_RUNFILES", "ignored when RUNFILES_DIR has a value", - "TEST_SRCDIR", "should always be ignored")); - assertThat(r.rlocation("a/b")).isEqualTo("c/d"); - assertThat(r.rlocation("foo")).isNull(); - - if (isWindows()) { - assertThat(r.rlocation("c:/foo")).isEqualTo("c:/foo"); - assertThat(r.rlocation("c:\\foo")).isEqualTo("c:\\foo"); - } else { - assertThat(r.rlocation("/foo")).isEqualTo("/foo"); - } + Path mf = tempFile("foo.runfiles_manifest", ImmutableList.of("a/b c/d")); + Runfiles r = + Runfiles.create( + ImmutableMap.of( + "RUNFILES_MANIFEST_ONLY", "1", + "RUNFILES_MANIFEST_FILE", mf.toString(), + "RUNFILES_DIR", "ignored when RUNFILES_MANIFEST_ONLY=1", + "JAVA_RUNFILES", "ignored when RUNFILES_DIR has a value", + "TEST_SRCDIR", "should always be ignored")); + assertThat(r.rlocation("a/b")).isEqualTo("c/d"); + assertThat(r.rlocation("foo")).isNull(); + + if (isWindows()) { + assertThat(r.rlocation("c:/foo")).isEqualTo("c:/foo"); + assertThat(r.rlocation("c:\\foo")).isEqualTo("c:\\foo"); + } else { + assertThat(r.rlocation("/foo")).isEqualTo("/foo"); } } @@ -146,7 +151,7 @@ public void testIgnoresTestSrcdirWhenJavaRunfilesIsUndefinedAndJustFails() throw "RUNFILES_DIR", "", "JAVA_RUNFILES", "", "RUNFILES_MANIFEST_FILE", - "ignored when RUNFILES_MANIFEST_ONLY is not set to 1", + "ignored when RUNFILES_MANIFEST_ONLY is not set to 1", "TEST_SRCDIR", "should always be ignored"))); assertThat(e).hasMessageThat().contains("$RUNFILES_DIR and $JAVA_RUNFILES"); } @@ -166,12 +171,7 @@ public void testFailsToCreateManifestBasedBecauseManifestDoesNotExist() { @Test public void testManifestBasedEnvVars() throws Exception { - Path dir = - Files.createTempDirectory( - FileSystems.getDefault().getPath(System.getenv("TEST_TMPDIR")), null); - - Path mf = dir.resolve("MANIFEST"); - Files.write(mf, Collections.emptyList(), StandardCharsets.UTF_8); + Path mf = tempFile("MANIFEST", ImmutableList.of()); Map envvars = Runfiles.create( ImmutableMap.of( @@ -186,13 +186,12 @@ public void testManifestBasedEnvVars() throws Exception { "RUNFILES_MANIFEST_ONLY", "RUNFILES_MANIFEST_FILE", "RUNFILES_DIR", "JAVA_RUNFILES"); assertThat(envvars.get("RUNFILES_MANIFEST_ONLY")).isEqualTo("1"); assertThat(envvars.get("RUNFILES_MANIFEST_FILE")).isEqualTo(mf.toString()); - assertThat(envvars.get("RUNFILES_DIR")).isEqualTo(dir.toString()); - assertThat(envvars.get("JAVA_RUNFILES")).isEqualTo(dir.toString()); + assertThat(envvars.get("RUNFILES_DIR")).isEqualTo(tempDir.getRoot().toString()); + assertThat(envvars.get("JAVA_RUNFILES")).isEqualTo(tempDir.getRoot().toString()); - Path rfDir = dir.resolve("foo.runfiles"); + Path rfDir = tempDir.getRoot().toPath().resolve("foo.runfiles"); Files.createDirectories(rfDir); - mf = dir.resolve("foo.runfiles_manifest"); - Files.write(mf, Collections.emptyList(), StandardCharsets.UTF_8); + mf = tempFile("foo.runfiles_manifest", ImmutableList.of()); envvars = Runfiles.create( ImmutableMap.of( @@ -210,25 +209,21 @@ public void testManifestBasedEnvVars() throws Exception { @Test public void testDirectoryBasedEnvVars() throws Exception { - Path dir = - Files.createTempDirectory( - FileSystems.getDefault().getPath(System.getenv("TEST_TMPDIR")), null); - Map envvars = Runfiles.create( ImmutableMap.of( "RUNFILES_MANIFEST_FILE", "ignored when RUNFILES_MANIFEST_ONLY is not set to 1", "RUNFILES_DIR", - dir.toString(), + tempDir.getRoot().toString(), "JAVA_RUNFILES", "ignored when RUNFILES_DIR has a value", "TEST_SRCDIR", "should always be ignored")) .getEnvVars(); assertThat(envvars.keySet()).containsExactly("RUNFILES_DIR", "JAVA_RUNFILES"); - assertThat(envvars.get("RUNFILES_DIR")).isEqualTo(dir.toString()); - assertThat(envvars.get("JAVA_RUNFILES")).isEqualTo(dir.toString()); + assertThat(envvars.get("RUNFILES_DIR")).isEqualTo(tempDir.getRoot().toString()); + assertThat(envvars.get("JAVA_RUNFILES")).isEqualTo(tempDir.getRoot().toString()); } @Test @@ -245,21 +240,18 @@ public void testDirectoryBasedRlocation() { @Test public void testManifestBasedRlocation() throws Exception { - try (MockFile mf = - new MockFile( - ImmutableList.of( - "Foo/runfile1 C:/Actual Path\\runfile1", - "Foo/Bar/runfile2 D:\\the path\\run file 2.txt", - "Foo/Bar/Dir E:\\Actual Path\\Directory"))) { - Runfiles r = Runfiles.createManifestBasedForTesting(mf.path.toString()); - assertThat(r.rlocation("Foo/runfile1")).isEqualTo("C:/Actual Path\\runfile1"); - assertThat(r.rlocation("Foo/Bar/runfile2")).isEqualTo("D:\\the path\\run file 2.txt"); - assertThat(r.rlocation("Foo/Bar/Dir")).isEqualTo("E:\\Actual Path\\Directory"); - assertThat(r.rlocation("Foo/Bar/Dir/File")).isEqualTo("E:\\Actual Path\\Directory/File"); - assertThat(r.rlocation("Foo/Bar/Dir/Deeply/Nested/File")) - .isEqualTo("E:\\Actual Path\\Directory/Deeply/Nested/File"); - assertThat(r.rlocation("unknown")).isNull(); - } + Path mf = tempFile("MANIFEST", ImmutableList.of( + "Foo/runfile1 C:/Actual Path\\runfile1", + "Foo/Bar/runfile2 D:\\the path\\run file 2.txt", + "Foo/Bar/Dir E:\\Actual Path\\Directory")); + Runfiles r = Runfiles.createManifestBasedForTesting(mf.toString()); + assertThat(r.rlocation("Foo/runfile1")).isEqualTo("C:/Actual Path\\runfile1"); + assertThat(r.rlocation("Foo/Bar/runfile2")).isEqualTo("D:\\the path\\run file 2.txt"); + assertThat(r.rlocation("Foo/Bar/Dir")).isEqualTo("E:\\Actual Path\\Directory"); + assertThat(r.rlocation("Foo/Bar/Dir/File")).isEqualTo("E:\\Actual Path\\Directory/File"); + assertThat(r.rlocation("Foo/Bar/Dir/Deeply/Nested/File")) + .isEqualTo("E:\\Actual Path\\Directory/Deeply/Nested/File"); + assertThat(r.rlocation("unknown")).isNull(); } @Test @@ -283,8 +275,11 @@ public void testManifestBasedCtorArgumentValidation() throws Exception { assertThrows(IllegalArgumentException.class, () -> Runfiles.createManifestBasedForTesting("")); - try (MockFile mf = new MockFile(ImmutableList.of("a b"))) { - Runfiles.createManifestBasedForTesting(mf.path.toString()); - } + Path mf = tempFile("foobar", ImmutableList.of("a b")); + Runfiles.createManifestBasedForTesting(mf.toString()); + } + + private Path tempFile(String name, ImmutableList lines) throws IOException { + return Files.write(tempDir.getRoot().toPath().resolve(name), lines, StandardCharsets.UTF_8); } } From c81b37ddb8519a6cf755d1558aecb6bc70d49312 Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Mon, 24 Oct 2022 11:29:09 +0200 Subject: [PATCH 2/9] Make Java runfiles library repo mapping aware --- tools/java/runfiles/Runfiles.java | 233 ++++++++++++++++-- tools/java/runfiles/testing/RunfilesTest.java | 200 ++++++++++++++- 2 files changed, 399 insertions(+), 34 deletions(-) diff --git a/tools/java/runfiles/Runfiles.java b/tools/java/runfiles/Runfiles.java index 9a3aa26c4517a9..052fdf03a2f8fd 100644 --- a/tools/java/runfiles/Runfiles.java +++ b/tools/java/runfiles/Runfiles.java @@ -17,12 +17,15 @@ import java.io.BufferedReader; import java.io.File; import java.io.FileInputStream; +import java.io.FileReader; import java.io.IOException; import java.io.InputStreamReader; import java.nio.charset.StandardCharsets; import java.util.Collections; import java.util.HashMap; import java.util.Map; +import java.util.Objects; +import java.util.stream.Collectors; /** * Runfiles lookup library for Bazel-built Java binaries and tests. @@ -39,17 +42,27 @@ * ) * * - *

2. Import the runfiles library. + *

2. Import the runfiles library and the {@code AutoBazelRepository} annotation. * * 

+ *   import com.google.devtools.build.runfiles.AutoBazelRepository;
  *   import com.google.devtools.build.runfiles.Runfiles;
  *
* - *

3. Create a Runfiles object and use rlocation to look up runfile paths: + *

3. Annotate the class in which a {@code Runfiles} object is created with + * {@link AutoBazelRepository}: + * + * 

+ *   @AutoBazelRepository
+ *   public class MyClass {
+ *     ...
+ *
+ * + *

4. Create a Runfiles object and use rlocation to look up runfile paths: * * 

  *   public void myFunction() {
- *     Runfiles runfiles = Runfiles.create();
+ *     Runfiles runfiles = Runfiles.create(AutoBazelRepository_MyClass.BAZEL_REPOSITORY);
  *     String path = runfiles.rlocation("my_workspace/path/to/my/data.txt");
  *     ...
  *
@@ -67,14 +80,55 @@ */ public abstract class Runfiles { + private static final String MAIN_REPOSITORY_NAME = ""; + + private static class RepoMappingKey { + + public final String sourceCanonical; + public final String targetApparent; + + public RepoMappingKey(String sourceCanonical, String targetApparent) { + this.sourceCanonical = sourceCanonical; + this.targetApparent = targetApparent; + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + RepoMappingKey that = (RepoMappingKey) o; + return sourceCanonical.equals(that.sourceCanonical) && targetApparent.equals( + that.targetApparent); + } + + @Override + public int hashCode() { + return Objects.hash(sourceCanonical, targetApparent); + } + } + + private final Map repoMapping; + private final String sourceCanonical; + // Package-private constructor, so only package-private classes may extend it. - private Runfiles() {} + private Runfiles(String repoMappingPath, String sourceRepository) throws IOException { + this.repoMapping = loadRepositoryMapping(repoMappingPath); + this.sourceCanonical = sourceRepository; + } /** * Returns a new {@link Runfiles} instance. * + *

Deprecated: With {@code --enable_bzlmod}, this function can only resolve runfiles + * correctly if called from the main repository. Use {@link #create(String)} instead. + * *

This method passes the JVM's environment variable map to {@link #create(Map)}. */ + @Deprecated public static Runfiles create() throws IOException { return create(System.getenv()); } @@ -82,6 +136,23 @@ public static Runfiles create() throws IOException { /** * Returns a new {@link Runfiles} instance. * + *

This method passes the JVM's environment variable map to {@link #create(Map)}. + * + * @param sourceRepository the canonical name of the Bazel repository relative to which runfiles + * lookups should be performed. This can be obtained using + * {@link AutoBazelRepository} (see class documentation). + */ + public static Runfiles create(String sourceRepository) throws IOException { + return create(System.getenv(), sourceRepository); + } + + /** + * Returns a new {@link Runfiles} instance. + * + *

Deprecated: With {@code --enable_bzlmod}, this function can only resolve runfiles + * correctly if called from the main repository. Use {@link #create(Map, String)} instead. + * + * *

The returned object is either: * *

    @@ -101,34 +172,89 @@ public static Runfiles create() throws IOException { * manifest file upon instantiation. * * @throws IOException if RUNFILES_MANIFEST_ONLY=1 is in {@code env} but there's no - * "RUNFILES_MANIFEST_FILE", "RUNFILES_DIR", or "JAVA_RUNFILES" key in {@code env} or their - * values are empty, or some IO error occurs + * "RUNFILES_MANIFEST_FILE", "RUNFILES_DIR", or "JAVA_RUNFILES" key in + * {@code env} or their values are empty, or some IO error occurs */ + @Deprecated public static Runfiles create(Map env) throws IOException { + return create(env, MAIN_REPOSITORY_NAME); + } + + /** + * Returns a new {@link Runfiles} instance. + * + *

    The returned object is either: + * + *

      + *
    • manifest-based, meaning it looks up runfile paths from a manifest file, or + *
    • directory-based, meaning it looks up runfile paths under a given directory path + *
    + * + *

    If {@code env} contains "RUNFILES_MANIFEST_ONLY" with value "1", this method returns a + * manifest-based implementation. The manifest's path is defined by the "RUNFILES_MANIFEST_FILE" + * key's value in {@code env}. + * + *

    Otherwise this method returns a directory-based implementation. The directory's path is + * defined by the value in {@code env} under the "RUNFILES_DIR" key, or if absent, then under the + * "JAVA_RUNFILES" key. + * + *

    Note about performance: the manifest-based implementation eagerly reads and caches the whole + * manifest file upon instantiation. + * + * @param sourceRepository the canonical name of the Bazel repository relative to which apparent + * repository names in runfiles paths should be resolved. This can be + * obtained using {@link AutoBazelRepository} (see class documentation). + * + * @throws IOException if RUNFILES_MANIFEST_ONLY=1 is in {@code env} but there's no + * "RUNFILES_MANIFEST_FILE", "RUNFILES_DIR", or "JAVA_RUNFILES" key in + * {@code env} or their values are empty, or some IO error occurs + */ + public static Runfiles create(Map env, String sourceRepository) + throws IOException { if (isManifestOnly(env)) { // On Windows, Bazel sets RUNFILES_MANIFEST_ONLY=1. // On every platform, Bazel also sets RUNFILES_MANIFEST_FILE, but on Linux and macOS it's // faster to use RUNFILES_DIR. - return new ManifestBased(getManifestPath(env)); + return new ManifestBased(getManifestPath(env), sourceRepository); } else { - return new DirectoryBased(getRunfilesDir(env)); + return new DirectoryBased(getRunfilesDir(env), sourceRepository); } } /** * Returns the runtime path of a runfile (a Bazel-built binary's/test's data-dependency). * - *

    The returned path may not be valid. The caller should check the path's validity and that the - * path exists. + *

    The returned path may not be valid. The caller should check the path's validity and that + * the path exists. * *

    The function may return null. In that case the caller can be sure that the rule does not * know about this data-dependency. * * @param path runfiles-root-relative path of the runfile * @throws IllegalArgumentException if {@code path} fails validation, for example if it's null or - * empty, or not normalized (contains "./", "../", or "//") + * empty, or not normalized (contains "./", "../", or "//") */ public final String rlocation(String path) { + return rlocation(path, this.sourceCanonical); + } + + /** + * Returns the runtime path of a runfile (a Bazel-built binary's/test's data-dependency). + * + *

    The returned path may not be valid. The caller should check the path's validity and that + * the path exists. + * + *

    The function may return null. In that case the caller can be sure that the rule does not + * know about this data-dependency. + * + * @param path runfiles-root-relative path of the runfile + * @param sourceRepository the canonical name of the Bazel repository relative to which apparent + * repository names in runfiles paths should be resolved. Overrides the + * repository set when creating this {@link Runfiles} instance. + * @throws IllegalArgumentException if {@code path} fails validation, for example if it's null or + * empty, or not normalized (contains "./", "../", or "//") + */ + public final String rlocation(String path, String sourceRepository) { Util.checkArgument(path != null); Util.checkArgument(!path.isEmpty()); Util.checkArgument( @@ -145,7 +271,15 @@ public final String rlocation(String path) { if (new File(path).isAbsolute()) { return path; } - return rlocationChecked(path); + + String[] apparentTargetAndRemainder = path.split("/", 2); + if (apparentTargetAndRemainder.length < 2) { + return rlocationChecked(path); + } + String targetCanonical = repoMapping.getOrDefault( + new RepoMappingKey(sourceRepository, apparentTargetAndRemainder[0]), + apparentTargetAndRemainder[0]); + return rlocationChecked(targetCanonical + "/" + apparentTargetAndRemainder[1]); } /** @@ -156,7 +290,9 @@ public final String rlocation(String path) { */ public abstract Map getEnvVars(); - /** Returns true if the platform supports runfiles only via manifests. */ + /** + * Returns true if the platform supports runfiles only via manifests. + */ private static boolean isManifestOnly(Map env) { return "1".equals(env.get("RUNFILES_MANIFEST_ONLY")); } @@ -183,14 +319,41 @@ private static String getRunfilesDir(Map env) throws IOException return value; } + private static Map loadRepositoryMapping(String path) + throws IOException { + if (path == null) { + return Collections.emptyMap(); + } + + try (BufferedReader r = new BufferedReader(new FileReader(path, StandardCharsets.UTF_8))) { + return Collections.unmodifiableMap(r.lines() + .filter(line -> !line.isEmpty()) + .map(line -> { + String[] split = line.split(","); + if (split.length != 3) { + throw new IllegalArgumentException( + "Invalid line in repository mapping: '" + line + "'"); + } + return split; + }) + .collect(Collectors.toMap( + split -> new RepoMappingKey(split[0], split[1]), + split -> split[2]))); + } + } + abstract String rlocationChecked(String path); - /** {@link Runfiles} implementation that parses a runfiles-manifest file to look up runfiles. */ + /** + * {@link Runfiles} implementation that parses a runfiles-manifest file to look up runfiles. + */ private static final class ManifestBased extends Runfiles { + private final Map runfiles; private final String manifestPath; - ManifestBased(String manifestPath) throws IOException { + ManifestBased(String manifestPath, String sourceRepository) throws IOException { + super(findRepositoryMapping(manifestPath), sourceRepository); Util.checkArgument(manifestPath != null); Util.checkArgument(!manifestPath.isEmpty()); this.manifestPath = manifestPath; @@ -225,6 +388,18 @@ private static String findRunfilesDir(String manifest) { return ""; } + private static String findRepositoryMapping(String manifestPath) { + if (manifestPath != null && (manifestPath.endsWith(".runfiles/MANIFEST") + || manifestPath.endsWith(".runfiles\\MANIFEST") + || manifestPath.endsWith(".runfiles_manifest"))) { + String path = manifestPath.substring(0, manifestPath.length() - 18) + ".repo_mapping"; + if (new File(path).isFile()) { + return path; + } + } + return null; + } + @Override public String rlocationChecked(String path) { String exactMatch = runfiles.get(path); @@ -257,16 +432,30 @@ public Map getEnvVars() { } } - /** {@link Runfiles} implementation that appends runfiles paths to the runfiles root. */ + /** + * {@link Runfiles} implementation that appends runfiles paths to the runfiles root. + */ private static final class DirectoryBased extends Runfiles { + private final String runfilesRoot; - DirectoryBased(String runfilesDir) { + DirectoryBased(String runfilesDir, String sourceRepository) throws IOException { + super(findRepositoryMapping(runfilesDir), sourceRepository); Util.checkArgument(!Util.isNullOrEmpty(runfilesDir)); Util.checkArgument(new File(runfilesDir).isDirectory()); this.runfilesRoot = runfilesDir; } + private static String findRepositoryMapping(String runfilesRoot) { + if (runfilesRoot != null && runfilesRoot.endsWith(".runfiles")) { + String path = runfilesRoot.substring(0, runfilesRoot.length() - 9) + ".repo_mapping"; + if (new File(path).isFile()) { + return path; + } + } + return null; + } + @Override String rlocationChecked(String path) { return runfilesRoot + "/" + path; @@ -282,11 +471,13 @@ public Map getEnvVars() { } } - static Runfiles createManifestBasedForTesting(String manifestPath) throws IOException { - return new ManifestBased(manifestPath); + static Runfiles createManifestBasedForTesting(String manifestPath, String sourceRepository) + throws IOException { + return new ManifestBased(manifestPath, sourceRepository); } - static Runfiles createDirectoryBasedForTesting(String runfilesDir) { - return new DirectoryBased(runfilesDir); + static Runfiles createDirectoryBasedForTesting(String runfilesDir, String sourceRepository) + throws IOException { + return new DirectoryBased(runfilesDir, sourceRepository); } } diff --git a/tools/java/runfiles/testing/RunfilesTest.java b/tools/java/runfiles/testing/RunfilesTest.java index d8eb71988606e0..6505996804f8af 100644 --- a/tools/java/runfiles/testing/RunfilesTest.java +++ b/tools/java/runfiles/testing/RunfilesTest.java @@ -227,13 +227,13 @@ public void testDirectoryBasedEnvVars() throws Exception { } @Test - public void testDirectoryBasedRlocation() { + public void testDirectoryBasedRlocation() throws IOException { // The DirectoryBased implementation simply joins the runfiles directory and the runfile's path // on a "/". DirectoryBased does not perform any normalization, nor does it check that the path // exists. File dir = new File(System.getenv("TEST_TMPDIR"), "mock/runfiles"); assertThat(dir.mkdirs()).isTrue(); - Runfiles r = Runfiles.createDirectoryBasedForTesting(dir.toString()); + Runfiles r = Runfiles.createDirectoryBasedForTesting(dir.toString(), ""); // Escaping for "\": once for string and once for regex. assertThat(r.rlocation("arg")).matches(".*[/\\\\]mock[/\\\\]runfiles[/\\\\]arg"); } @@ -244,7 +244,7 @@ public void testManifestBasedRlocation() throws Exception { "Foo/runfile1 C:/Actual Path\\runfile1", "Foo/Bar/runfile2 D:\\the path\\run file 2.txt", "Foo/Bar/Dir E:\\Actual Path\\Directory")); - Runfiles r = Runfiles.createManifestBasedForTesting(mf.toString()); + Runfiles r = Runfiles.createManifestBasedForTesting(mf.toString(), ""); assertThat(r.rlocation("Foo/runfile1")).isEqualTo("C:/Actual Path\\runfile1"); assertThat(r.rlocation("Foo/Bar/runfile2")).isEqualTo("D:\\the path\\run file 2.txt"); assertThat(r.rlocation("Foo/Bar/Dir")).isEqualTo("E:\\Actual Path\\Directory"); @@ -255,31 +255,205 @@ public void testManifestBasedRlocation() throws Exception { } @Test - public void testDirectoryBasedCtorArgumentValidation() { + public void testManifestBasedRlocationWithRepoMapping_fromMain() throws Exception { + Path mf = tempFile("foo.runfiles_manifest", ImmutableList.of( + "config.json /etc/config.json", + "protobuf~3.19.2/foo/runfile C:/Actual Path\\protobuf\\runfile", + "_main/bar/runfile /the/path/./to/other//other runfile.txt", + "protobuf~3.19.2/bar/dir E:\\Actual Path\\Directory")); + tempFile("foo.repo_mapping", ImmutableList.of( + ",my_module,_main", + ",my_protobuf,protobuf~3.19.2", + ",my_workspace,_main", + "protobuf~3.19.2,protobuf,protobuf~3.19.2")); + Runfiles r = Runfiles.createManifestBasedForTesting(mf.toString(), ""); + + assertThat(r.rlocation("my_module/bar/runfile")).isEqualTo( + "/the/path/./to/other//other runfile.txt"); + assertThat(r.rlocation("my_workspace/bar/runfile")).isEqualTo( + "/the/path/./to/other//other runfile.txt"); + assertThat(r.rlocation("my_protobuf/foo/runfile")).isEqualTo( + "C:/Actual Path\\protobuf\\runfile"); + assertThat(r.rlocation("my_protobuf/bar/dir")).isEqualTo("E:\\Actual Path\\Directory"); + assertThat(r.rlocation("my_protobuf/bar/dir/file")).isEqualTo( + "E:\\Actual Path\\Directory/file"); + assertThat(r.rlocation("my_protobuf/bar/dir/de eply/nes ted/fi~le")).isEqualTo( + "E:\\Actual Path\\Directory/de eply/nes ted/fi~le"); + + assertThat(r.rlocation("protobuf/foo/runfile")).isNull(); + assertThat(r.rlocation("protobuf/bar/dir")).isNull(); + assertThat(r.rlocation("protobuf/bar/dir/file")).isNull(); + assertThat(r.rlocation("protobuf/bar/dir/dir/de eply/nes ted/fi~le")).isNull(); + + assertThat(r.rlocation("_main/bar/runfile")).isEqualTo( + "/the/path/./to/other//other runfile.txt"); + assertThat(r.rlocation("protobuf~3.19.2/foo/runfile")).isEqualTo( + "C:/Actual Path\\protobuf\\runfile"); + assertThat(r.rlocation("protobuf~3.19.2/bar/dir")).isEqualTo("E:\\Actual Path\\Directory"); + assertThat(r.rlocation("protobuf~3.19.2/bar/dir/file")).isEqualTo( + "E:\\Actual Path\\Directory/file"); + assertThat(r.rlocation("protobuf~3.19.2/bar/dir/de eply/nes ted/fi~le")).isEqualTo( + "E:\\Actual Path\\Directory/de eply/nes ted/fi~le"); + + assertThat(r.rlocation("config.json")).isEqualTo("/etc/config.json"); + assertThat(r.rlocation("_main")).isNull(); + assertThat(r.rlocation("my_module")).isNull(); + assertThat(r.rlocation("protobuf")).isNull(); + } + + @Test + public void testManifestBasedRlocationWithRepoMapping_fromOtherRepo() throws Exception { + Path mf = tempFile("foo.runfiles/MANIFEST", ImmutableList.of( + "config.json /etc/config.json", + "protobuf~3.19.2/foo/runfile C:/Actual Path\\protobuf\\runfile", + "_main/bar/runfile /the/path/./to/other//other runfile.txt", + "protobuf~3.19.2/bar/dir E:\\Actual Path\\Directory")); + tempFile("foo.repo_mapping", ImmutableList.of( + ",my_module,_main", + ",my_protobuf,protobuf~3.19.2", + ",my_workspace,_main", + "protobuf~3.19.2,protobuf,protobuf~3.19.2")); + Runfiles r = Runfiles.createManifestBasedForTesting(mf.toString(), "protobuf~3.19.2"); + + assertThat(r.rlocation("protobuf/foo/runfile")).isEqualTo("C:/Actual Path\\protobuf\\runfile"); + assertThat(r.rlocation("protobuf/bar/dir")).isEqualTo("E:\\Actual Path\\Directory"); + assertThat(r.rlocation("protobuf/bar/dir/file")).isEqualTo("E:\\Actual Path\\Directory/file"); + assertThat(r.rlocation("protobuf/bar/dir/de eply/nes ted/fi~le")).isEqualTo( + "E:\\Actual Path\\Directory/de eply/nes ted/fi~le"); + + assertThat(r.rlocation("my_module/bar/runfile")).isNull(); + assertThat(r.rlocation("my_protobuf/foo/runfile")).isNull(); + assertThat(r.rlocation("my_protobuf/bar/dir")).isNull(); + assertThat(r.rlocation("my_protobuf/bar/dir/file")).isNull(); + assertThat(r.rlocation("my_protobuf/bar/dir/de eply/nes ted/fi~le")).isNull(); + + assertThat(r.rlocation("_main/bar/runfile")).isEqualTo( + "/the/path/./to/other//other runfile.txt"); + assertThat(r.rlocation("protobuf~3.19.2/foo/runfile")).isEqualTo( + "C:/Actual Path\\protobuf\\runfile"); + assertThat(r.rlocation("protobuf~3.19.2/bar/dir")).isEqualTo("E:\\Actual Path\\Directory"); + assertThat(r.rlocation("protobuf~3.19.2/bar/dir/file")).isEqualTo( + "E:\\Actual Path\\Directory/file"); + assertThat(r.rlocation("protobuf~3.19.2/bar/dir/de eply/nes ted/fi~le")).isEqualTo( + "E:\\Actual Path\\Directory/de eply/nes ted/fi~le"); + + assertThat(r.rlocation("config.json")).isEqualTo("/etc/config.json"); + assertThat(r.rlocation("_main")).isNull(); + assertThat(r.rlocation("my_module")).isNull(); + assertThat(r.rlocation("protobuf")).isNull(); + } + + @Test + public void testDirectoryBasedRlocationWithRepoMapping_fromMain() throws Exception { + Path dir = tempDir.newFolder("foo.runfiles").toPath(); + tempFile(dir.getParent().resolve("foo.repo_mapping").toString(), ImmutableList.of( + ",my_module,_main", + ",my_protobuf,protobuf~3.19.2", + ",my_workspace,_main", + "protobuf~3.19.2,protobuf,protobuf~3.19.2")); + Runfiles r = Runfiles.createDirectoryBasedForTesting(dir.toString(), ""); + + assertThat(r.rlocation("my_module/bar/runfile")).isEqualTo( + dir + "/_main/bar/runfile"); + assertThat(r.rlocation("my_workspace/bar/runfile")).isEqualTo( + dir + "/_main/bar/runfile"); + assertThat(r.rlocation("my_protobuf/foo/runfile")).isEqualTo( + dir + "/protobuf~3.19.2/foo/runfile"); + assertThat(r.rlocation("my_protobuf/bar/dir")).isEqualTo(dir + "/protobuf~3.19.2/bar/dir"); + assertThat(r.rlocation("my_protobuf/bar/dir/file")).isEqualTo( + dir + "/protobuf~3.19.2/bar/dir/file"); + assertThat(r.rlocation("my_protobuf/bar/dir/de eply/nes ted/fi~le")).isEqualTo( + dir + "/protobuf~3.19.2/bar/dir/de eply/nes ted/fi~le"); + + assertThat(r.rlocation("protobuf/foo/runfile")).isEqualTo(dir + "/protobuf/foo/runfile"); + assertThat(r.rlocation("protobuf/bar/dir/dir/de eply/nes ted/fi~le")).isEqualTo( + dir + "/protobuf/bar/dir/dir/de eply/nes ted/fi~le"); + + assertThat(r.rlocation("_main/bar/runfile")).isEqualTo( + dir + "/_main/bar/runfile"); + assertThat(r.rlocation("protobuf~3.19.2/foo/runfile")).isEqualTo( + dir + "/protobuf~3.19.2/foo/runfile"); + assertThat(r.rlocation("protobuf~3.19.2/bar/dir")).isEqualTo(dir + "/protobuf~3.19.2/bar/dir"); + assertThat(r.rlocation("protobuf~3.19.2/bar/dir/file")).isEqualTo( + dir + "/protobuf~3.19.2/bar/dir/file"); + assertThat(r.rlocation("protobuf~3.19.2/bar/dir/de eply/nes ted/fi~le")).isEqualTo( + dir + "/protobuf~3.19.2/bar/dir/de eply/nes ted/fi~le"); + + assertThat(r.rlocation("config.json")).isEqualTo(dir + "/config.json"); + } + + @Test + public void testDirectoryBasedRlocationWithRepoMapping_fromOtherRepo() throws Exception { + Path dir = tempDir.newFolder("foo.runfiles").toPath(); + Path rm = tempFile(dir.getParent().resolve("foo.repo_mapping").toString(), ImmutableList.of( + ",my_module,_main", + ",my_protobuf,protobuf~3.19.2", + ",my_workspace,_main", + "protobuf~3.19.2,protobuf,protobuf~3.19.2")); + Runfiles r = Runfiles.createDirectoryBasedForTesting(dir.toString(), "protobuf~3.19.2"); + + assertThat(r.rlocation("protobuf/foo/runfile")).isEqualTo(dir + "/protobuf~3.19.2/foo/runfile"); + assertThat(r.rlocation("protobuf/bar/dir")).isEqualTo(dir + "/protobuf~3.19.2/bar/dir"); + assertThat(r.rlocation("protobuf/bar/dir/file")).isEqualTo( + dir + "/protobuf~3.19.2/bar/dir/file"); + assertThat(r.rlocation("protobuf/bar/dir/de eply/nes ted/fi~le")).isEqualTo( + dir + "/protobuf~3.19.2/bar/dir/de eply/nes ted/fi~le"); + + assertThat(r.rlocation("my_module/bar/runfile")).isEqualTo(dir + "/my_module/bar/runfile"); + assertThat(r.rlocation("my_protobuf/bar/dir/de eply/nes ted/fi~le")).isEqualTo( + dir + "/my_protobuf/bar/dir/de eply/nes ted/fi~le"); + + assertThat(r.rlocation("_main/bar/runfile")).isEqualTo( + dir + "/_main/bar/runfile"); + assertThat(r.rlocation("protobuf~3.19.2/foo/runfile")).isEqualTo( + dir + "/protobuf~3.19.2/foo/runfile"); + assertThat(r.rlocation("protobuf~3.19.2/bar/dir")).isEqualTo(dir + "/protobuf~3.19.2/bar/dir"); + assertThat(r.rlocation("protobuf~3.19.2/bar/dir/file")).isEqualTo( + dir + "/protobuf~3.19.2/bar/dir/file"); + assertThat(r.rlocation("protobuf~3.19.2/bar/dir/de eply/nes ted/fi~le")).isEqualTo( + dir + "/protobuf~3.19.2/bar/dir/de eply/nes ted/fi~le"); + + assertThat(r.rlocation("config.json")).isEqualTo(dir + "/config.json"); + } + + @Test + public void testDirectoryBasedCtorArgumentValidation() throws IOException { assertThrows( - IllegalArgumentException.class, () -> Runfiles.createDirectoryBasedForTesting(null)); + IllegalArgumentException.class, () -> Runfiles.createDirectoryBasedForTesting(null, "")); - assertThrows(IllegalArgumentException.class, () -> Runfiles.createDirectoryBasedForTesting("")); + assertThrows(IllegalArgumentException.class, () -> Runfiles.createDirectoryBasedForTesting("", + "")); assertThrows( IllegalArgumentException.class, - () -> Runfiles.createDirectoryBasedForTesting("non-existent directory is bad")); + () -> Runfiles.createDirectoryBasedForTesting("non-existent directory is bad", "")); - Runfiles.createDirectoryBasedForTesting(System.getenv("TEST_TMPDIR")); + Runfiles.createDirectoryBasedForTesting(System.getenv("TEST_TMPDIR"), ""); } @Test public void testManifestBasedCtorArgumentValidation() throws Exception { assertThrows( - IllegalArgumentException.class, () -> Runfiles.createManifestBasedForTesting(null)); + IllegalArgumentException.class, () -> Runfiles.createManifestBasedForTesting(null, "")); - assertThrows(IllegalArgumentException.class, () -> Runfiles.createManifestBasedForTesting("")); + assertThrows(IllegalArgumentException.class, () -> Runfiles.createManifestBasedForTesting("", + "")); Path mf = tempFile("foobar", ImmutableList.of("a b")); - Runfiles.createManifestBasedForTesting(mf.toString()); + Runfiles.createManifestBasedForTesting(mf.toString(), ""); + } + + @Test + public void testInvalidRepoMapping() throws Exception { + Path mf = tempFile("foo.runfiles/MANIFEST", ImmutableList.of()); + tempFile("foo.repo_mapping", ImmutableList.of("a,b,c,d")); + assertThrows(IllegalArgumentException.class, + () -> Runfiles.createManifestBasedForTesting(mf.toString(), "")); } - private Path tempFile(String name, ImmutableList lines) throws IOException { - return Files.write(tempDir.getRoot().toPath().resolve(name), lines, StandardCharsets.UTF_8); + private Path tempFile(String path, ImmutableList lines) throws IOException { + Path file = tempDir.getRoot().toPath().resolve(path.replace('/', File.separatorChar)); + Files.createDirectories(file.getParent()); + return Files.write(file, lines, StandardCharsets.UTF_8); } } From 19569858588201cedadbd2864638a0ba163cf0b3 Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Wed, 26 Oct 2022 12:11:51 +0200 Subject: [PATCH 3/9] Add withSourceRepository --- tools/java/runfiles/Runfiles.java | 42 ++++++++++ tools/java/runfiles/testing/RunfilesTest.java | 80 +++++++++++++++++++ 2 files changed, 122 insertions(+) diff --git a/tools/java/runfiles/Runfiles.java b/tools/java/runfiles/Runfiles.java index 052fdf03a2f8fd..85dbb22fde6b39 100644 --- a/tools/java/runfiles/Runfiles.java +++ b/tools/java/runfiles/Runfiles.java @@ -120,6 +120,11 @@ private Runfiles(String repoMappingPath, String sourceRepository) throws IOExcep this.sourceCanonical = sourceRepository; } + protected Runfiles(Runfiles runfiles, String sourceRepository) { + this.repoMapping = runfiles.repoMapping; + this.sourceCanonical = sourceRepository; + } + /** * Returns a new {@link Runfiles} instance. * @@ -221,6 +226,22 @@ public static Runfiles create(Map env, String sourceRepository) } } + /** + * Returns a new instance that uses the provided source repository as a default for all calls to + * {@link #rlocation(String)}. + * + *

    This is useful when receiving a {@link Runfiles} instance from a different Bazel + * repository. In this case, while the runfiles manifest or directory encoded in the instance + * should be used for runfiles lookups, the repository from which apparent repository names should + * be resolved needs to change. + * + * @param sourceRepository the canonical name of the Bazel repository relative to which apparent + * repository names should be resolved + * @return a new {@link Runfiles} instance identical to this one, except that calls to + * {@link #rlocation(String)} use the provided source repository. + */ + public abstract Runfiles withSourceRepository(String sourceRepository); + /** * Returns the runtime path of a runfile (a Bazel-built binary's/test's data-dependency). * @@ -360,6 +381,17 @@ private static final class ManifestBased extends Runfiles { this.runfiles = loadRunfiles(manifestPath); } + private ManifestBased(ManifestBased existing, String sourceRepository) { + super(existing, sourceRepository); + this.manifestPath = existing.manifestPath; + this.runfiles = existing.runfiles; + } + + @Override + public Runfiles withSourceRepository(String sourceRepository) { + return new ManifestBased(this, sourceRepository); + } + private static Map loadRunfiles(String path) throws IOException { HashMap result = new HashMap<>(); try (BufferedReader r = @@ -446,6 +478,16 @@ private static final class DirectoryBased extends Runfiles { this.runfilesRoot = runfilesDir; } + private DirectoryBased(DirectoryBased existing, String sourceRepository) { + super(existing, sourceRepository); + this.runfilesRoot = existing.runfilesRoot; + } + + @Override + public Runfiles withSourceRepository(String sourceRepository) { + return new DirectoryBased(this, sourceRepository); + } + private static String findRepositoryMapping(String runfilesRoot) { if (runfilesRoot != null && runfilesRoot.endsWith(".runfiles")) { String path = runfilesRoot.substring(0, runfilesRoot.length() - 9) + ".repo_mapping"; diff --git a/tools/java/runfiles/testing/RunfilesTest.java b/tools/java/runfiles/testing/RunfilesTest.java index 6505996804f8af..10cb1d644c2061 100644 --- a/tools/java/runfiles/testing/RunfilesTest.java +++ b/tools/java/runfiles/testing/RunfilesTest.java @@ -343,6 +343,50 @@ public void testManifestBasedRlocationWithRepoMapping_fromOtherRepo() throws Exc assertThat(r.rlocation("protobuf")).isNull(); } + @Test + public void testManifestBasedRlocationWithRepoMapping_fromOtherRepo_withSourceRepository() + throws Exception { + Path mf = tempFile("foo.runfiles/MANIFEST", ImmutableList.of( + "config.json /etc/config.json", + "protobuf~3.19.2/foo/runfile C:/Actual Path\\protobuf\\runfile", + "_main/bar/runfile /the/path/./to/other//other runfile.txt", + "protobuf~3.19.2/bar/dir E:\\Actual Path\\Directory")); + tempFile("foo.repo_mapping", ImmutableList.of( + ",my_module,_main", + ",my_protobuf,protobuf~3.19.2", + ",my_workspace,_main", + "protobuf~3.19.2,protobuf,protobuf~3.19.2")); + Runfiles r = Runfiles.createManifestBasedForTesting(mf.toString(), "") + .withSourceRepository("protobuf~3.19.2"); + + assertThat(r.rlocation("protobuf/foo/runfile")).isEqualTo("C:/Actual Path\\protobuf\\runfile"); + assertThat(r.rlocation("protobuf/bar/dir")).isEqualTo("E:\\Actual Path\\Directory"); + assertThat(r.rlocation("protobuf/bar/dir/file")).isEqualTo("E:\\Actual Path\\Directory/file"); + assertThat(r.rlocation("protobuf/bar/dir/de eply/nes ted/fi~le")).isEqualTo( + "E:\\Actual Path\\Directory/de eply/nes ted/fi~le"); + + assertThat(r.rlocation("my_module/bar/runfile")).isNull(); + assertThat(r.rlocation("my_protobuf/foo/runfile")).isNull(); + assertThat(r.rlocation("my_protobuf/bar/dir")).isNull(); + assertThat(r.rlocation("my_protobuf/bar/dir/file")).isNull(); + assertThat(r.rlocation("my_protobuf/bar/dir/de eply/nes ted/fi~le")).isNull(); + + assertThat(r.rlocation("_main/bar/runfile")).isEqualTo( + "/the/path/./to/other//other runfile.txt"); + assertThat(r.rlocation("protobuf~3.19.2/foo/runfile")).isEqualTo( + "C:/Actual Path\\protobuf\\runfile"); + assertThat(r.rlocation("protobuf~3.19.2/bar/dir")).isEqualTo("E:\\Actual Path\\Directory"); + assertThat(r.rlocation("protobuf~3.19.2/bar/dir/file")).isEqualTo( + "E:\\Actual Path\\Directory/file"); + assertThat(r.rlocation("protobuf~3.19.2/bar/dir/de eply/nes ted/fi~le")).isEqualTo( + "E:\\Actual Path\\Directory/de eply/nes ted/fi~le"); + + assertThat(r.rlocation("config.json")).isEqualTo("/etc/config.json"); + assertThat(r.rlocation("_main")).isNull(); + assertThat(r.rlocation("my_module")).isNull(); + assertThat(r.rlocation("protobuf")).isNull(); + } + @Test public void testDirectoryBasedRlocationWithRepoMapping_fromMain() throws Exception { Path dir = tempDir.newFolder("foo.runfiles").toPath(); @@ -416,6 +460,42 @@ public void testDirectoryBasedRlocationWithRepoMapping_fromOtherRepo() throws Ex assertThat(r.rlocation("config.json")).isEqualTo(dir + "/config.json"); } + @Test + public void testDirectoryBasedRlocationWithRepoMapping_fromOtherRepo_withSourceRepository() + throws Exception { + Path dir = tempDir.newFolder("foo.runfiles").toPath(); + Path rm = tempFile(dir.getParent().resolve("foo.repo_mapping").toString(), ImmutableList.of( + ",my_module,_main", + ",my_protobuf,protobuf~3.19.2", + ",my_workspace,_main", + "protobuf~3.19.2,protobuf,protobuf~3.19.2")); + Runfiles r = Runfiles.createDirectoryBasedForTesting(dir.toString(), "") + .withSourceRepository("protobuf~3.19.2"); + + assertThat(r.rlocation("protobuf/foo/runfile")).isEqualTo(dir + "/protobuf~3.19.2/foo/runfile"); + assertThat(r.rlocation("protobuf/bar/dir")).isEqualTo(dir + "/protobuf~3.19.2/bar/dir"); + assertThat(r.rlocation("protobuf/bar/dir/file")).isEqualTo( + dir + "/protobuf~3.19.2/bar/dir/file"); + assertThat(r.rlocation("protobuf/bar/dir/de eply/nes ted/fi~le")).isEqualTo( + dir + "/protobuf~3.19.2/bar/dir/de eply/nes ted/fi~le"); + + assertThat(r.rlocation("my_module/bar/runfile")).isEqualTo(dir + "/my_module/bar/runfile"); + assertThat(r.rlocation("my_protobuf/bar/dir/de eply/nes ted/fi~le")).isEqualTo( + dir + "/my_protobuf/bar/dir/de eply/nes ted/fi~le"); + + assertThat(r.rlocation("_main/bar/runfile")).isEqualTo( + dir + "/_main/bar/runfile"); + assertThat(r.rlocation("protobuf~3.19.2/foo/runfile")).isEqualTo( + dir + "/protobuf~3.19.2/foo/runfile"); + assertThat(r.rlocation("protobuf~3.19.2/bar/dir")).isEqualTo(dir + "/protobuf~3.19.2/bar/dir"); + assertThat(r.rlocation("protobuf~3.19.2/bar/dir/file")).isEqualTo( + dir + "/protobuf~3.19.2/bar/dir/file"); + assertThat(r.rlocation("protobuf~3.19.2/bar/dir/de eply/nes ted/fi~le")).isEqualTo( + dir + "/protobuf~3.19.2/bar/dir/de eply/nes ted/fi~le"); + + assertThat(r.rlocation("config.json")).isEqualTo(dir + "/config.json"); + } + @Test public void testDirectoryBasedCtorArgumentValidation() throws IOException { assertThrows( From e5c22db2327c0574a1154ab47daea0088fc49a66 Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Fri, 4 Nov 2022 12:20:14 +0100 Subject: [PATCH 4/9] Rename fields --- tools/java/runfiles/Runfiles.java | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/tools/java/runfiles/Runfiles.java b/tools/java/runfiles/Runfiles.java index 85dbb22fde6b39..81fa0b37e3481f 100644 --- a/tools/java/runfiles/Runfiles.java +++ b/tools/java/runfiles/Runfiles.java @@ -82,14 +82,17 @@ public abstract class Runfiles { private static final String MAIN_REPOSITORY_NAME = ""; + /** + * See {@link com.google.devtools.build.lib.analysis.RepoMappingManifestAction.Entry}. + */ private static class RepoMappingKey { - public final String sourceCanonical; - public final String targetApparent; + public final String sourceRepo; + public final String targetRepoApparentName; - public RepoMappingKey(String sourceCanonical, String targetApparent) { - this.sourceCanonical = sourceCanonical; - this.targetApparent = targetApparent; + public RepoMappingKey(String sourceRepo, String targetRepoApparentName) { + this.sourceRepo = sourceRepo; + this.targetRepoApparentName = targetRepoApparentName; } @Override @@ -101,28 +104,28 @@ public boolean equals(Object o) { return false; } RepoMappingKey that = (RepoMappingKey) o; - return sourceCanonical.equals(that.sourceCanonical) && targetApparent.equals( - that.targetApparent); + return sourceRepo.equals(that.sourceRepo) && targetRepoApparentName.equals( + that.targetRepoApparentName); } @Override public int hashCode() { - return Objects.hash(sourceCanonical, targetApparent); + return Objects.hash(sourceRepo, targetRepoApparentName); } } private final Map repoMapping; - private final String sourceCanonical; + private final String sourceRepository; // Package-private constructor, so only package-private classes may extend it. private Runfiles(String repoMappingPath, String sourceRepository) throws IOException { this.repoMapping = loadRepositoryMapping(repoMappingPath); - this.sourceCanonical = sourceRepository; + this.sourceRepository = sourceRepository; } protected Runfiles(Runfiles runfiles, String sourceRepository) { this.repoMapping = runfiles.repoMapping; - this.sourceCanonical = sourceRepository; + this.sourceRepository = sourceRepository; } /** @@ -256,7 +259,7 @@ public static Runfiles create(Map env, String sourceRepository) * empty, or not normalized (contains "./", "../", or "//") */ public final String rlocation(String path) { - return rlocation(path, this.sourceCanonical); + return rlocation(path, this.sourceRepository); } /** From af03035279ac8e43dceb33a29718e913d4a42a5e Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Fri, 4 Nov 2022 13:33:28 +0100 Subject: [PATCH 5/9] Use .preload().withSourceRepository(...) pattern --- tools/java/runfiles/Runfiles.java | 351 +++++++++--------- tools/java/runfiles/testing/RunfilesTest.java | 91 ++--- 2 files changed, 217 insertions(+), 225 deletions(-) diff --git a/tools/java/runfiles/Runfiles.java b/tools/java/runfiles/Runfiles.java index 81fa0b37e3481f..b704847b5a784e 100644 --- a/tools/java/runfiles/Runfiles.java +++ b/tools/java/runfiles/Runfiles.java @@ -78,67 +78,84 @@ * Process p = pb.start(); * */ -public abstract class Runfiles { +public final class Runfiles { - private static final String MAIN_REPOSITORY_NAME = ""; + public abstract static class Preloaded { - /** - * See {@link com.google.devtools.build.lib.analysis.RepoMappingManifestAction.Entry}. - */ - private static class RepoMappingKey { + /** + * See {@link com.google.devtools.build.lib.analysis.RepoMappingManifestAction.Entry}. + */ + static class RepoMappingKey { - public final String sourceRepo; - public final String targetRepoApparentName; + public final String sourceRepo; + public final String targetRepoApparentName; - public RepoMappingKey(String sourceRepo, String targetRepoApparentName) { - this.sourceRepo = sourceRepo; - this.targetRepoApparentName = targetRepoApparentName; - } + public RepoMappingKey(String sourceRepo, String targetRepoApparentName) { + this.sourceRepo = sourceRepo; + this.targetRepoApparentName = targetRepoApparentName; + } - @Override - public boolean equals(Object o) { - if (this == o) { - return true; + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + RepoMappingKey that = (RepoMappingKey) o; + return sourceRepo.equals(that.sourceRepo) && targetRepoApparentName.equals( + that.targetRepoApparentName); } - if (o == null || getClass() != o.getClass()) { - return false; + + @Override + public int hashCode() { + return Objects.hash(sourceRepo, targetRepoApparentName); } - RepoMappingKey that = (RepoMappingKey) o; - return sourceRepo.equals(that.sourceRepo) && targetRepoApparentName.equals( - that.targetRepoApparentName); } - @Override - public int hashCode() { - return Objects.hash(sourceRepo, targetRepoApparentName); + /** + * Returns a new instance that uses the provided source repository as a default for all calls to + * {@link #rlocation(String)}. + * + *

    This is useful when receiving a {@link Runfiles} instance from a different Bazel + * repository. In this case, while the runfiles manifest or directory encoded in the instance + * should be used for runfiles lookups, the repository from which apparent repository names + * should be resolved needs to change. + * + * @param sourceRepository the canonical name of the Bazel repository relative to which apparent + * repository names should be resolved + * @return a new {@link Runfiles} instance identical to this one, except that calls to + * {@link #rlocation(String)} use the provided source repository. + */ + public final Runfiles withSourceRepository(String sourceRepository) { + Util.checkArgument(sourceRepository != null); + return new Runfiles(this, sourceRepository); } - } - private final Map repoMapping; - private final String sourceRepository; + public final Runfiles unmapped() { + return new Runfiles(this, null); + } - // Package-private constructor, so only package-private classes may extend it. - private Runfiles(String repoMappingPath, String sourceRepository) throws IOException { - this.repoMapping = loadRepositoryMapping(repoMappingPath); - this.sourceRepository = sourceRepository; - } + protected abstract Map getEnvVars(); - protected Runfiles(Runfiles runfiles, String sourceRepository) { - this.repoMapping = runfiles.repoMapping; - this.sourceRepository = sourceRepository; + protected abstract String rlocationChecked(String path); + + protected abstract Map getRepoMapping(); + + // Private constructor, so only nested classes may extend it. + private Preloaded() { + } } - /** - * Returns a new {@link Runfiles} instance. - * - *

    Deprecated: With {@code --enable_bzlmod}, this function can only resolve runfiles - * correctly if called from the main repository. Use {@link #create(String)} instead. - * - *

    This method passes the JVM's environment variable map to {@link #create(Map)}. - */ - @Deprecated - public static Runfiles create() throws IOException { - return create(System.getenv()); + private static final String MAIN_REPOSITORY = ""; + + private final Preloaded preloadedRunfiles; + private final String sourceRepository; + + private Runfiles(Preloaded preloadedRunfiles, String sourceRepository) { + this.preloadedRunfiles = preloadedRunfiles; + this.sourceRepository = sourceRepository; } /** @@ -150,17 +167,13 @@ public static Runfiles create() throws IOException { * lookups should be performed. This can be obtained using * {@link AutoBazelRepository} (see class documentation). */ - public static Runfiles create(String sourceRepository) throws IOException { - return create(System.getenv(), sourceRepository); + public static Preloaded preload() throws IOException { + return preload(System.getenv()); } /** * Returns a new {@link Runfiles} instance. * - *

    Deprecated: With {@code --enable_bzlmod}, this function can only resolve runfiles - * correctly if called from the main repository. Use {@link #create(Map, String)} instead. - * - * *

    The returned object is either: * *

      @@ -179,18 +192,46 @@ public static Runfiles create(String sourceRepository) throws IOException { *

      Note about performance: the manifest-based implementation eagerly reads and caches the whole * manifest file upon instantiation. * + * @param sourceRepository the canonical name of the Bazel repository relative to which apparent + * repository names in runfiles paths should be resolved. This can be + * obtained using {@link AutoBazelRepository} (see class documentation). + * * @throws IOException if RUNFILES_MANIFEST_ONLY=1 is in {@code env} but there's no * "RUNFILES_MANIFEST_FILE", "RUNFILES_DIR", or "JAVA_RUNFILES" key in * {@code env} or their values are empty, or some IO error occurs */ + public static Preloaded preload(Map env) throws IOException { + if (isManifestOnly(env)) { + // On Windows, Bazel sets RUNFILES_MANIFEST_ONLY=1. + // On every platform, Bazel also sets RUNFILES_MANIFEST_FILE, but on Linux and macOS it's + // faster to use RUNFILES_DIR. + return new ManifestBased(getManifestPath(env)); + } else { + return new DirectoryBased(getRunfilesDir(env)); + } + } + + /** + * Returns a new {@link Runfiles} instance. + * + *

      Deprecated: With {@code --enable_bzlmod}, this function can only resolve runfiles + * correctly if called from the main repository. Use {@link #preload()}} + * instead. + * + *

      This method passes the JVM's environment variable map to {@link #create(Map)}. + */ @Deprecated - public static Runfiles create(Map env) throws IOException { - return create(env, MAIN_REPOSITORY_NAME); + public static Runfiles create() throws IOException { + return preload().withSourceRepository(MAIN_REPOSITORY); } /** * Returns a new {@link Runfiles} instance. * + *

      Deprecated: With {@code --enable_bzlmod}, this function can only resolve runfiles + * correctly if called from the main repository. Use {@link #preload(Map)} instead. + * + * *

      The returned object is either: * *

        @@ -209,42 +250,15 @@ public static Runfiles create(Map env) throws IOException { *

        Note about performance: the manifest-based implementation eagerly reads and caches the whole * manifest file upon instantiation. * - * @param sourceRepository the canonical name of the Bazel repository relative to which apparent - * repository names in runfiles paths should be resolved. This can be - * obtained using {@link AutoBazelRepository} (see class documentation). - * * @throws IOException if RUNFILES_MANIFEST_ONLY=1 is in {@code env} but there's no * "RUNFILES_MANIFEST_FILE", "RUNFILES_DIR", or "JAVA_RUNFILES" key in * {@code env} or their values are empty, or some IO error occurs */ - public static Runfiles create(Map env, String sourceRepository) - throws IOException { - if (isManifestOnly(env)) { - // On Windows, Bazel sets RUNFILES_MANIFEST_ONLY=1. - // On every platform, Bazel also sets RUNFILES_MANIFEST_FILE, but on Linux and macOS it's - // faster to use RUNFILES_DIR. - return new ManifestBased(getManifestPath(env), sourceRepository); - } else { - return new DirectoryBased(getRunfilesDir(env), sourceRepository); - } + @Deprecated + public static Runfiles create(Map env) throws IOException { + return preload(env).withSourceRepository(MAIN_REPOSITORY); } - /** - * Returns a new instance that uses the provided source repository as a default for all calls to - * {@link #rlocation(String)}. - * - *

        This is useful when receiving a {@link Runfiles} instance from a different Bazel - * repository. In this case, while the runfiles manifest or directory encoded in the instance - * should be used for runfiles lookups, the repository from which apparent repository names should - * be resolved needs to change. - * - * @param sourceRepository the canonical name of the Bazel repository relative to which apparent - * repository names should be resolved - * @return a new {@link Runfiles} instance identical to this one, except that calls to - * {@link #rlocation(String)} use the provided source repository. - */ - public abstract Runfiles withSourceRepository(String sourceRepository); - /** * Returns the runtime path of a runfile (a Bazel-built binary's/test's data-dependency). * @@ -258,27 +272,7 @@ public static Runfiles create(Map env, String sourceRepository) * @throws IllegalArgumentException if {@code path} fails validation, for example if it's null or * empty, or not normalized (contains "./", "../", or "//") */ - public final String rlocation(String path) { - return rlocation(path, this.sourceRepository); - } - - /** - * Returns the runtime path of a runfile (a Bazel-built binary's/test's data-dependency). - * - *

        The returned path may not be valid. The caller should check the path's validity and that - * the path exists. - * - *

        The function may return null. In that case the caller can be sure that the rule does not - * know about this data-dependency. - * - * @param path runfiles-root-relative path of the runfile - * @param sourceRepository the canonical name of the Bazel repository relative to which apparent - * repository names in runfiles paths should be resolved. Overrides the - * repository set when creating this {@link Runfiles} instance. - * @throws IllegalArgumentException if {@code path} fails validation, for example if it's null or - * empty, or not normalized (contains "./", "../", or "//") - */ - public final String rlocation(String path, String sourceRepository) { + public String rlocation(String path) { Util.checkArgument(path != null); Util.checkArgument(!path.isEmpty()); Util.checkArgument( @@ -296,14 +290,18 @@ public final String rlocation(String path, String sourceRepository) { return path; } + if (sourceRepository == null) { + return preloadedRunfiles.rlocationChecked(path); + } String[] apparentTargetAndRemainder = path.split("/", 2); if (apparentTargetAndRemainder.length < 2) { - return rlocationChecked(path); + return preloadedRunfiles.rlocationChecked(path); } - String targetCanonical = repoMapping.getOrDefault( - new RepoMappingKey(sourceRepository, apparentTargetAndRemainder[0]), + String targetCanonical = preloadedRunfiles.getRepoMapping().getOrDefault( + new Preloaded.RepoMappingKey(sourceRepository, apparentTargetAndRemainder[0]), apparentTargetAndRemainder[0]); - return rlocationChecked(targetCanonical + "/" + apparentTargetAndRemainder[1]); + return preloadedRunfiles.rlocationChecked( + targetCanonical + "/" + apparentTargetAndRemainder[1]); } /** @@ -312,7 +310,9 @@ public final String rlocation(String path, String sourceRepository) { *

        The caller should add the returned key-value pairs to the environment of subprocesses in * case those subprocesses are also Bazel-built binaries that need to use runfiles. */ - public abstract Map getEnvVars(); + public Map getEnvVars() { + return preloadedRunfiles.getEnvVars(); + } /** * Returns true if the platform supports runfiles only via manifests. @@ -343,7 +343,7 @@ private static String getRunfilesDir(Map env) throws IOException return value; } - private static Map loadRepositoryMapping(String path) + private static Map loadRepositoryMapping(String path) throws IOException { if (path == null) { return Collections.emptyMap(); @@ -361,38 +361,62 @@ private static Map loadRepositoryMapping(String path) return split; }) .collect(Collectors.toMap( - split -> new RepoMappingKey(split[0], split[1]), + split -> new Preloaded.RepoMappingKey(split[0], split[1]), split -> split[2]))); } } - abstract String rlocationChecked(String path); - /** * {@link Runfiles} implementation that parses a runfiles-manifest file to look up runfiles. */ - private static final class ManifestBased extends Runfiles { + private static final class ManifestBased extends Runfiles.Preloaded { private final Map runfiles; private final String manifestPath; + private final Map repoMapping; - ManifestBased(String manifestPath, String sourceRepository) throws IOException { - super(findRepositoryMapping(manifestPath), sourceRepository); + ManifestBased(String manifestPath) throws IOException { Util.checkArgument(manifestPath != null); Util.checkArgument(!manifestPath.isEmpty()); this.manifestPath = manifestPath; this.runfiles = loadRunfiles(manifestPath); + this.repoMapping = loadRepositoryMapping(findRepositoryMapping(manifestPath)); + } + + @Override + protected String rlocationChecked(String path) { + String exactMatch = runfiles.get(path); + if (exactMatch != null) { + return exactMatch; + } + // If path references a runfile that lies under a directory that itself is a runfile, then + // only the directory is listed in the manifest. Look up all prefixes of path in the manifest + // and append the relative path from the prefix if there is a match. + int prefixEnd = path.length(); + while ((prefixEnd = path.lastIndexOf('/', prefixEnd - 1)) != -1) { + String prefixMatch = runfiles.get(path.substring(0, prefixEnd)); + if (prefixMatch != null) { + return prefixMatch + '/' + path.substring(prefixEnd + 1); + } + } + return null; } - private ManifestBased(ManifestBased existing, String sourceRepository) { - super(existing, sourceRepository); - this.manifestPath = existing.manifestPath; - this.runfiles = existing.runfiles; + @Override + protected Map getEnvVars() { + HashMap result = new HashMap<>(4); + result.put("RUNFILES_MANIFEST_ONLY", "1"); + result.put("RUNFILES_MANIFEST_FILE", manifestPath); + String runfilesDir = findRunfilesDir(manifestPath); + result.put("RUNFILES_DIR", runfilesDir); + // TODO(laszlocsomor): remove JAVA_RUNFILES once the Java launcher can pick up RUNFILES_DIR. + result.put("JAVA_RUNFILES", runfilesDir); + return result; } @Override - public Runfiles withSourceRepository(String sourceRepository) { - return new ManifestBased(this, sourceRepository); + protected Map getRepoMapping() { + return repoMapping; } private static Map loadRunfiles(String path) throws IOException { @@ -434,61 +458,40 @@ private static String findRepositoryMapping(String manifestPath) { } return null; } - - @Override - public String rlocationChecked(String path) { - String exactMatch = runfiles.get(path); - if (exactMatch != null) { - return exactMatch; - } - // If path references a runfile that lies under a directory that itself is a runfile, then - // only the directory is listed in the manifest. Look up all prefixes of path in the manifest - // and append the relative path from the prefix if there is a match. - int prefixEnd = path.length(); - while ((prefixEnd = path.lastIndexOf('/', prefixEnd - 1)) != -1) { - String prefixMatch = runfiles.get(path.substring(0, prefixEnd)); - if (prefixMatch != null) { - return prefixMatch + '/' + path.substring(prefixEnd + 1); - } - } - return null; - } - - @Override - public Map getEnvVars() { - HashMap result = new HashMap<>(4); - result.put("RUNFILES_MANIFEST_ONLY", "1"); - result.put("RUNFILES_MANIFEST_FILE", manifestPath); - String runfilesDir = findRunfilesDir(manifestPath); - result.put("RUNFILES_DIR", runfilesDir); - // TODO(laszlocsomor): remove JAVA_RUNFILES once the Java launcher can pick up RUNFILES_DIR. - result.put("JAVA_RUNFILES", runfilesDir); - return result; - } } /** * {@link Runfiles} implementation that appends runfiles paths to the runfiles root. */ - private static final class DirectoryBased extends Runfiles { + private static final class DirectoryBased extends Preloaded { private final String runfilesRoot; + private final Map repoMapping; - DirectoryBased(String runfilesDir, String sourceRepository) throws IOException { - super(findRepositoryMapping(runfilesDir), sourceRepository); + DirectoryBased(String runfilesDir) throws IOException { Util.checkArgument(!Util.isNullOrEmpty(runfilesDir)); Util.checkArgument(new File(runfilesDir).isDirectory()); this.runfilesRoot = runfilesDir; + this.repoMapping = loadRepositoryMapping(findRepositoryMapping(runfilesRoot)); } - private DirectoryBased(DirectoryBased existing, String sourceRepository) { - super(existing, sourceRepository); - this.runfilesRoot = existing.runfilesRoot; + @Override + protected String rlocationChecked(String path) { + return runfilesRoot + "/" + path; } @Override - public Runfiles withSourceRepository(String sourceRepository) { - return new DirectoryBased(this, sourceRepository); + protected Map getRepoMapping() { + return repoMapping; + } + + @Override + protected Map getEnvVars() { + HashMap result = new HashMap<>(2); + result.put("RUNFILES_DIR", runfilesRoot); + // TODO(laszlocsomor): remove JAVA_RUNFILES once the Java launcher can pick up RUNFILES_DIR. + result.put("JAVA_RUNFILES", runfilesRoot); + return result; } private static String findRepositoryMapping(String runfilesRoot) { @@ -500,29 +503,13 @@ private static String findRepositoryMapping(String runfilesRoot) { } return null; } - - @Override - String rlocationChecked(String path) { - return runfilesRoot + "/" + path; - } - - @Override - public Map getEnvVars() { - HashMap result = new HashMap<>(2); - result.put("RUNFILES_DIR", runfilesRoot); - // TODO(laszlocsomor): remove JAVA_RUNFILES once the Java launcher can pick up RUNFILES_DIR. - result.put("JAVA_RUNFILES", runfilesRoot); - return result; - } } - static Runfiles createManifestBasedForTesting(String manifestPath, String sourceRepository) - throws IOException { - return new ManifestBased(manifestPath, sourceRepository); + static Preloaded createManifestBasedForTesting(String manifestPath) throws IOException { + return new ManifestBased(manifestPath); } - static Runfiles createDirectoryBasedForTesting(String runfilesDir, String sourceRepository) - throws IOException { - return new DirectoryBased(runfilesDir, sourceRepository); + static Preloaded createDirectoryBasedForTesting(String runfilesDir) throws IOException { + return new DirectoryBased(runfilesDir); } } diff --git a/tools/java/runfiles/testing/RunfilesTest.java b/tools/java/runfiles/testing/RunfilesTest.java index 10cb1d644c2061..c645efbb1ee8ee 100644 --- a/tools/java/runfiles/testing/RunfilesTest.java +++ b/tools/java/runfiles/testing/RunfilesTest.java @@ -233,7 +233,7 @@ public void testDirectoryBasedRlocation() throws IOException { // exists. File dir = new File(System.getenv("TEST_TMPDIR"), "mock/runfiles"); assertThat(dir.mkdirs()).isTrue(); - Runfiles r = Runfiles.createDirectoryBasedForTesting(dir.toString(), ""); + Runfiles r = Runfiles.createDirectoryBasedForTesting(dir.toString()).withSourceRepository(""); // Escaping for "\": once for string and once for regex. assertThat(r.rlocation("arg")).matches(".*[/\\\\]mock[/\\\\]runfiles[/\\\\]arg"); } @@ -244,7 +244,7 @@ public void testManifestBasedRlocation() throws Exception { "Foo/runfile1 C:/Actual Path\\runfile1", "Foo/Bar/runfile2 D:\\the path\\run file 2.txt", "Foo/Bar/Dir E:\\Actual Path\\Directory")); - Runfiles r = Runfiles.createManifestBasedForTesting(mf.toString(), ""); + Runfiles r = Runfiles.createManifestBasedForTesting(mf.toString()).withSourceRepository(""); assertThat(r.rlocation("Foo/runfile1")).isEqualTo("C:/Actual Path\\runfile1"); assertThat(r.rlocation("Foo/Bar/runfile2")).isEqualTo("D:\\the path\\run file 2.txt"); assertThat(r.rlocation("Foo/Bar/Dir")).isEqualTo("E:\\Actual Path\\Directory"); @@ -266,7 +266,7 @@ public void testManifestBasedRlocationWithRepoMapping_fromMain() throws Exceptio ",my_protobuf,protobuf~3.19.2", ",my_workspace,_main", "protobuf~3.19.2,protobuf,protobuf~3.19.2")); - Runfiles r = Runfiles.createManifestBasedForTesting(mf.toString(), ""); + Runfiles r = Runfiles.createManifestBasedForTesting(mf.toString()).withSourceRepository(""); assertThat(r.rlocation("my_module/bar/runfile")).isEqualTo( "/the/path/./to/other//other runfile.txt"); @@ -302,8 +302,8 @@ public void testManifestBasedRlocationWithRepoMapping_fromMain() throws Exceptio } @Test - public void testManifestBasedRlocationWithRepoMapping_fromOtherRepo() throws Exception { - Path mf = tempFile("foo.runfiles/MANIFEST", ImmutableList.of( + public void testManifestBasedRlocationUnmapped() throws Exception { + Path mf = tempFile("foo.runfiles_manifest", ImmutableList.of( "config.json /etc/config.json", "protobuf~3.19.2/foo/runfile C:/Actual Path\\protobuf\\runfile", "_main/bar/runfile /the/path/./to/other//other runfile.txt", @@ -313,19 +313,19 @@ public void testManifestBasedRlocationWithRepoMapping_fromOtherRepo() throws Exc ",my_protobuf,protobuf~3.19.2", ",my_workspace,_main", "protobuf~3.19.2,protobuf,protobuf~3.19.2")); - Runfiles r = Runfiles.createManifestBasedForTesting(mf.toString(), "protobuf~3.19.2"); - - assertThat(r.rlocation("protobuf/foo/runfile")).isEqualTo("C:/Actual Path\\protobuf\\runfile"); - assertThat(r.rlocation("protobuf/bar/dir")).isEqualTo("E:\\Actual Path\\Directory"); - assertThat(r.rlocation("protobuf/bar/dir/file")).isEqualTo("E:\\Actual Path\\Directory/file"); - assertThat(r.rlocation("protobuf/bar/dir/de eply/nes ted/fi~le")).isEqualTo( - "E:\\Actual Path\\Directory/de eply/nes ted/fi~le"); + Runfiles r = Runfiles.createManifestBasedForTesting(mf.toString()).unmapped(); assertThat(r.rlocation("my_module/bar/runfile")).isNull(); + assertThat(r.rlocation("my_workspace/bar/runfile")).isNull(); assertThat(r.rlocation("my_protobuf/foo/runfile")).isNull(); assertThat(r.rlocation("my_protobuf/bar/dir")).isNull(); assertThat(r.rlocation("my_protobuf/bar/dir/file")).isNull(); - assertThat(r.rlocation("my_protobuf/bar/dir/de eply/nes ted/fi~le")).isNull(); + assertThat(r.rlocation("my_protobuf/bar/dir/de eply/nes ted/fi~le")).isNull(); + + assertThat(r.rlocation("protobuf/foo/runfile")).isNull(); + assertThat(r.rlocation("protobuf/bar/dir")).isNull(); + assertThat(r.rlocation("protobuf/bar/dir/file")).isNull(); + assertThat(r.rlocation("protobuf/bar/dir/dir/de eply/nes ted/fi~le")).isNull(); assertThat(r.rlocation("_main/bar/runfile")).isEqualTo( "/the/path/./to/other//other runfile.txt"); @@ -344,8 +344,7 @@ public void testManifestBasedRlocationWithRepoMapping_fromOtherRepo() throws Exc } @Test - public void testManifestBasedRlocationWithRepoMapping_fromOtherRepo_withSourceRepository() - throws Exception { + public void testManifestBasedRlocationWithRepoMapping_fromOtherRepo() throws Exception { Path mf = tempFile("foo.runfiles/MANIFEST", ImmutableList.of( "config.json /etc/config.json", "protobuf~3.19.2/foo/runfile C:/Actual Path\\protobuf\\runfile", @@ -356,7 +355,7 @@ public void testManifestBasedRlocationWithRepoMapping_fromOtherRepo_withSourceRe ",my_protobuf,protobuf~3.19.2", ",my_workspace,_main", "protobuf~3.19.2,protobuf,protobuf~3.19.2")); - Runfiles r = Runfiles.createManifestBasedForTesting(mf.toString(), "") + Runfiles r = Runfiles.createManifestBasedForTesting(mf.toString()) .withSourceRepository("protobuf~3.19.2"); assertThat(r.rlocation("protobuf/foo/runfile")).isEqualTo("C:/Actual Path\\protobuf\\runfile"); @@ -395,7 +394,7 @@ public void testDirectoryBasedRlocationWithRepoMapping_fromMain() throws Excepti ",my_protobuf,protobuf~3.19.2", ",my_workspace,_main", "protobuf~3.19.2,protobuf,protobuf~3.19.2")); - Runfiles r = Runfiles.createDirectoryBasedForTesting(dir.toString(), ""); + Runfiles r = Runfiles.createDirectoryBasedForTesting(dir.toString()).withSourceRepository(""); assertThat(r.rlocation("my_module/bar/runfile")).isEqualTo( dir + "/_main/bar/runfile"); @@ -427,25 +426,30 @@ public void testDirectoryBasedRlocationWithRepoMapping_fromMain() throws Excepti } @Test - public void testDirectoryBasedRlocationWithRepoMapping_fromOtherRepo() throws Exception { + public void testDirectoryBasedRlocationUnmapped() throws Exception { Path dir = tempDir.newFolder("foo.runfiles").toPath(); - Path rm = tempFile(dir.getParent().resolve("foo.repo_mapping").toString(), ImmutableList.of( + tempFile(dir.getParent().resolve("foo.repo_mapping").toString(), ImmutableList.of( ",my_module,_main", ",my_protobuf,protobuf~3.19.2", ",my_workspace,_main", "protobuf~3.19.2,protobuf,protobuf~3.19.2")); - Runfiles r = Runfiles.createDirectoryBasedForTesting(dir.toString(), "protobuf~3.19.2"); + Runfiles r = Runfiles.createDirectoryBasedForTesting(dir.toString()).unmapped(); - assertThat(r.rlocation("protobuf/foo/runfile")).isEqualTo(dir + "/protobuf~3.19.2/foo/runfile"); - assertThat(r.rlocation("protobuf/bar/dir")).isEqualTo(dir + "/protobuf~3.19.2/bar/dir"); - assertThat(r.rlocation("protobuf/bar/dir/file")).isEqualTo( - dir + "/protobuf~3.19.2/bar/dir/file"); - assertThat(r.rlocation("protobuf/bar/dir/de eply/nes ted/fi~le")).isEqualTo( - dir + "/protobuf~3.19.2/bar/dir/de eply/nes ted/fi~le"); + assertThat(r.rlocation("my_module/bar/runfile")).isEqualTo( + dir + "/my_module/bar/runfile"); + assertThat(r.rlocation("my_workspace/bar/runfile")).isEqualTo( + dir + "/my_workspace/bar/runfile"); + assertThat(r.rlocation("my_protobuf/foo/runfile")).isEqualTo( + dir + "/my_protobuf/foo/runfile"); + assertThat(r.rlocation("my_protobuf/bar/dir")).isEqualTo(dir + "/my_protobuf/bar/dir"); + assertThat(r.rlocation("my_protobuf/bar/dir/file")).isEqualTo( + dir + "/my_protobuf/bar/dir/file"); + assertThat(r.rlocation("my_protobuf/bar/dir/de eply/nes ted/fi~le")).isEqualTo( + dir + "/my_protobuf/bar/dir/de eply/nes ted/fi~le"); - assertThat(r.rlocation("my_module/bar/runfile")).isEqualTo(dir + "/my_module/bar/runfile"); - assertThat(r.rlocation("my_protobuf/bar/dir/de eply/nes ted/fi~le")).isEqualTo( - dir + "/my_protobuf/bar/dir/de eply/nes ted/fi~le"); + assertThat(r.rlocation("protobuf/foo/runfile")).isEqualTo(dir + "/protobuf/foo/runfile"); + assertThat(r.rlocation("protobuf/bar/dir/dir/de eply/nes ted/fi~le")).isEqualTo( + dir + "/protobuf/bar/dir/dir/de eply/nes ted/fi~le"); assertThat(r.rlocation("_main/bar/runfile")).isEqualTo( dir + "/_main/bar/runfile"); @@ -461,15 +465,14 @@ public void testDirectoryBasedRlocationWithRepoMapping_fromOtherRepo() throws Ex } @Test - public void testDirectoryBasedRlocationWithRepoMapping_fromOtherRepo_withSourceRepository() - throws Exception { + public void testDirectoryBasedRlocationWithRepoMapping_fromOtherRepo() throws Exception { Path dir = tempDir.newFolder("foo.runfiles").toPath(); Path rm = tempFile(dir.getParent().resolve("foo.repo_mapping").toString(), ImmutableList.of( ",my_module,_main", ",my_protobuf,protobuf~3.19.2", ",my_workspace,_main", "protobuf~3.19.2,protobuf,protobuf~3.19.2")); - Runfiles r = Runfiles.createDirectoryBasedForTesting(dir.toString(), "") + Runfiles r = Runfiles.createDirectoryBasedForTesting(dir.toString()) .withSourceRepository("protobuf~3.19.2"); assertThat(r.rlocation("protobuf/foo/runfile")).isEqualTo(dir + "/protobuf~3.19.2/foo/runfile"); @@ -498,29 +501,31 @@ public void testDirectoryBasedRlocationWithRepoMapping_fromOtherRepo_withSourceR @Test public void testDirectoryBasedCtorArgumentValidation() throws IOException { - assertThrows( - IllegalArgumentException.class, () -> Runfiles.createDirectoryBasedForTesting(null, "")); + assertThrows(IllegalArgumentException.class, + () -> Runfiles.createDirectoryBasedForTesting(null).withSourceRepository("")); - assertThrows(IllegalArgumentException.class, () -> Runfiles.createDirectoryBasedForTesting("", - "")); + assertThrows(IllegalArgumentException.class, + () -> Runfiles.createDirectoryBasedForTesting("").withSourceRepository("")); assertThrows( IllegalArgumentException.class, - () -> Runfiles.createDirectoryBasedForTesting("non-existent directory is bad", "")); + () -> Runfiles.createDirectoryBasedForTesting("non-existent directory is bad") + .withSourceRepository("")); - Runfiles.createDirectoryBasedForTesting(System.getenv("TEST_TMPDIR"), ""); + Runfiles.createDirectoryBasedForTesting(System.getenv("TEST_TMPDIR")).withSourceRepository(""); } @Test public void testManifestBasedCtorArgumentValidation() throws Exception { assertThrows( - IllegalArgumentException.class, () -> Runfiles.createManifestBasedForTesting(null, "")); + IllegalArgumentException.class, + () -> Runfiles.createManifestBasedForTesting(null).withSourceRepository("")); - assertThrows(IllegalArgumentException.class, () -> Runfiles.createManifestBasedForTesting("", - "")); + assertThrows(IllegalArgumentException.class, + () -> Runfiles.createManifestBasedForTesting("").withSourceRepository("")); Path mf = tempFile("foobar", ImmutableList.of("a b")); - Runfiles.createManifestBasedForTesting(mf.toString(), ""); + Runfiles.createManifestBasedForTesting(mf.toString()).withSourceRepository(""); } @Test @@ -528,7 +533,7 @@ public void testInvalidRepoMapping() throws Exception { Path mf = tempFile("foo.runfiles/MANIFEST", ImmutableList.of()); tempFile("foo.repo_mapping", ImmutableList.of("a,b,c,d")); assertThrows(IllegalArgumentException.class, - () -> Runfiles.createManifestBasedForTesting(mf.toString(), "")); + () -> Runfiles.createManifestBasedForTesting(mf.toString()).withSourceRepository("")); } private Path tempFile(String path, ImmutableList lines) throws IOException { From ca5e9a9c8463654d71792408f0c7a6b4ab260774 Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Thu, 3 Nov 2022 22:04:50 +0100 Subject: [PATCH 6/9] Load repo mapping from `_repo_mapping` --- tools/java/runfiles/Runfiles.java | 28 ++---------- tools/java/runfiles/testing/RunfilesTest.java | 43 ++++++++++--------- 2 files changed, 26 insertions(+), 45 deletions(-) diff --git a/tools/java/runfiles/Runfiles.java b/tools/java/runfiles/Runfiles.java index b704847b5a784e..40edb48bbe442d 100644 --- a/tools/java/runfiles/Runfiles.java +++ b/tools/java/runfiles/Runfiles.java @@ -345,7 +345,7 @@ private static String getRunfilesDir(Map env) throws IOException private static Map loadRepositoryMapping(String path) throws IOException { - if (path == null) { + if (path == null || !new File(path).exists()) { return Collections.emptyMap(); } @@ -380,7 +380,7 @@ private static final class ManifestBased extends Runfiles.Preloaded { Util.checkArgument(!manifestPath.isEmpty()); this.manifestPath = manifestPath; this.runfiles = loadRunfiles(manifestPath); - this.repoMapping = loadRepositoryMapping(findRepositoryMapping(manifestPath)); + this.repoMapping = loadRepositoryMapping(rlocationChecked("_repo_mapping")); } @Override @@ -446,18 +446,6 @@ private static String findRunfilesDir(String manifest) { } return ""; } - - private static String findRepositoryMapping(String manifestPath) { - if (manifestPath != null && (manifestPath.endsWith(".runfiles/MANIFEST") - || manifestPath.endsWith(".runfiles\\MANIFEST") - || manifestPath.endsWith(".runfiles_manifest"))) { - String path = manifestPath.substring(0, manifestPath.length() - 18) + ".repo_mapping"; - if (new File(path).isFile()) { - return path; - } - } - return null; - } } /** @@ -472,7 +460,7 @@ private static final class DirectoryBased extends Preloaded { Util.checkArgument(!Util.isNullOrEmpty(runfilesDir)); Util.checkArgument(new File(runfilesDir).isDirectory()); this.runfilesRoot = runfilesDir; - this.repoMapping = loadRepositoryMapping(findRepositoryMapping(runfilesRoot)); + this.repoMapping = loadRepositoryMapping(rlocationChecked("_repo_mapping")); } @Override @@ -493,16 +481,6 @@ protected Map getEnvVars() { result.put("JAVA_RUNFILES", runfilesRoot); return result; } - - private static String findRepositoryMapping(String runfilesRoot) { - if (runfilesRoot != null && runfilesRoot.endsWith(".runfiles")) { - String path = runfilesRoot.substring(0, runfilesRoot.length() - 9) + ".repo_mapping"; - if (new File(path).isFile()) { - return path; - } - } - return null; - } } static Preloaded createManifestBasedForTesting(String manifestPath) throws IOException { diff --git a/tools/java/runfiles/testing/RunfilesTest.java b/tools/java/runfiles/testing/RunfilesTest.java index c645efbb1ee8ee..22605c3bf0952c 100644 --- a/tools/java/runfiles/testing/RunfilesTest.java +++ b/tools/java/runfiles/testing/RunfilesTest.java @@ -256,16 +256,17 @@ public void testManifestBasedRlocation() throws Exception { @Test public void testManifestBasedRlocationWithRepoMapping_fromMain() throws Exception { + Path rm = tempFile("foo.repo_mapping", ImmutableList.of( + ",my_module,_main", + ",my_protobuf,protobuf~3.19.2", + ",my_workspace,_main", + "protobuf~3.19.2,protobuf,protobuf~3.19.2")); Path mf = tempFile("foo.runfiles_manifest", ImmutableList.of( + "_repo_mapping " + rm, "config.json /etc/config.json", "protobuf~3.19.2/foo/runfile C:/Actual Path\\protobuf\\runfile", "_main/bar/runfile /the/path/./to/other//other runfile.txt", "protobuf~3.19.2/bar/dir E:\\Actual Path\\Directory")); - tempFile("foo.repo_mapping", ImmutableList.of( - ",my_module,_main", - ",my_protobuf,protobuf~3.19.2", - ",my_workspace,_main", - "protobuf~3.19.2,protobuf,protobuf~3.19.2")); Runfiles r = Runfiles.createManifestBasedForTesting(mf.toString()).withSourceRepository(""); assertThat(r.rlocation("my_module/bar/runfile")).isEqualTo( @@ -303,16 +304,17 @@ public void testManifestBasedRlocationWithRepoMapping_fromMain() throws Exceptio @Test public void testManifestBasedRlocationUnmapped() throws Exception { + Path rm = tempFile("foo.repo_mapping", ImmutableList.of( + ",my_module,_main", + ",my_protobuf,protobuf~3.19.2", + ",my_workspace,_main", + "protobuf~3.19.2,protobuf,protobuf~3.19.2")); Path mf = tempFile("foo.runfiles_manifest", ImmutableList.of( + "_repo_mapping " + rm, "config.json /etc/config.json", "protobuf~3.19.2/foo/runfile C:/Actual Path\\protobuf\\runfile", "_main/bar/runfile /the/path/./to/other//other runfile.txt", "protobuf~3.19.2/bar/dir E:\\Actual Path\\Directory")); - tempFile("foo.repo_mapping", ImmutableList.of( - ",my_module,_main", - ",my_protobuf,protobuf~3.19.2", - ",my_workspace,_main", - "protobuf~3.19.2,protobuf,protobuf~3.19.2")); Runfiles r = Runfiles.createManifestBasedForTesting(mf.toString()).unmapped(); assertThat(r.rlocation("my_module/bar/runfile")).isNull(); @@ -345,16 +347,17 @@ public void testManifestBasedRlocationUnmapped() throws Exception { @Test public void testManifestBasedRlocationWithRepoMapping_fromOtherRepo() throws Exception { + Path rm = tempFile("foo.repo_mapping", ImmutableList.of( + ",my_module,_main", + ",my_protobuf,protobuf~3.19.2", + ",my_workspace,_main", + "protobuf~3.19.2,protobuf,protobuf~3.19.2")); Path mf = tempFile("foo.runfiles/MANIFEST", ImmutableList.of( + "_repo_mapping " + rm, "config.json /etc/config.json", "protobuf~3.19.2/foo/runfile C:/Actual Path\\protobuf\\runfile", "_main/bar/runfile /the/path/./to/other//other runfile.txt", "protobuf~3.19.2/bar/dir E:\\Actual Path\\Directory")); - tempFile("foo.repo_mapping", ImmutableList.of( - ",my_module,_main", - ",my_protobuf,protobuf~3.19.2", - ",my_workspace,_main", - "protobuf~3.19.2,protobuf,protobuf~3.19.2")); Runfiles r = Runfiles.createManifestBasedForTesting(mf.toString()) .withSourceRepository("protobuf~3.19.2"); @@ -389,7 +392,7 @@ public void testManifestBasedRlocationWithRepoMapping_fromOtherRepo() throws Exc @Test public void testDirectoryBasedRlocationWithRepoMapping_fromMain() throws Exception { Path dir = tempDir.newFolder("foo.runfiles").toPath(); - tempFile(dir.getParent().resolve("foo.repo_mapping").toString(), ImmutableList.of( + tempFile(dir.resolve("_repo_mapping").toString(), ImmutableList.of( ",my_module,_main", ",my_protobuf,protobuf~3.19.2", ",my_workspace,_main", @@ -428,7 +431,7 @@ public void testDirectoryBasedRlocationWithRepoMapping_fromMain() throws Excepti @Test public void testDirectoryBasedRlocationUnmapped() throws Exception { Path dir = tempDir.newFolder("foo.runfiles").toPath(); - tempFile(dir.getParent().resolve("foo.repo_mapping").toString(), ImmutableList.of( + Path rm = tempFile(dir.resolve("_repo_mapping").toString(), ImmutableList.of( ",my_module,_main", ",my_protobuf,protobuf~3.19.2", ",my_workspace,_main", @@ -467,7 +470,7 @@ public void testDirectoryBasedRlocationUnmapped() throws Exception { @Test public void testDirectoryBasedRlocationWithRepoMapping_fromOtherRepo() throws Exception { Path dir = tempDir.newFolder("foo.runfiles").toPath(); - Path rm = tempFile(dir.getParent().resolve("foo.repo_mapping").toString(), ImmutableList.of( + Path rm = tempFile(dir.resolve("_repo_mapping").toString(), ImmutableList.of( ",my_module,_main", ",my_protobuf,protobuf~3.19.2", ",my_workspace,_main", @@ -530,8 +533,8 @@ public void testManifestBasedCtorArgumentValidation() throws Exception { @Test public void testInvalidRepoMapping() throws Exception { - Path mf = tempFile("foo.runfiles/MANIFEST", ImmutableList.of()); - tempFile("foo.repo_mapping", ImmutableList.of("a,b,c,d")); + Path rm = tempFile("foo.repo_mapping", ImmutableList.of("a,b,c,d")); + Path mf = tempFile("foo.runfiles/MANIFEST", ImmutableList.of("_repo_mapping " + rm)); assertThrows(IllegalArgumentException.class, () -> Runfiles.createManifestBasedForTesting(mf.toString()).withSourceRepository("")); } From 13f67dcae40ca5785f878cfb83417f670eaea39e Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Fri, 4 Nov 2022 22:53:46 +0100 Subject: [PATCH 7/9] Add integration test --- src/test/py/bazel/bzlmod/bazel_module_test.py | 63 +++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/src/test/py/bazel/bzlmod/bazel_module_test.py b/src/test/py/bazel/bzlmod/bazel_module_test.py index 301477560bb224..ddc3b59364b687 100644 --- a/src/test/py/bazel/bzlmod/bazel_module_test.py +++ b/src/test/py/bazel/bzlmod/bazel_module_test.py @@ -694,5 +694,68 @@ def testRunfilesRepoMappingManifest(self): self.Path('bazel-bin/external/bar~2.0/bar.runfiles_manifest')) as f: self.assertIn('_repo_mapping ', f.read()) + def testJavaRunfilesLibraryRepoMapping(self): + self.main_registry.setModuleBasePath('projects') + projects_dir = self.main_registry.projects + + self.main_registry.createLocalPathModule('data', '1.0', 'data') + projects_dir.joinpath('data').mkdir(exist_ok=True) + scratchFile(projects_dir.joinpath('data', 'WORKSPACE')) + scratchFile(projects_dir.joinpath('data', 'foo.txt'), ['hello']) + scratchFile(projects_dir.joinpath('data', 'BUILD'), + ['exports_files(["foo.txt"])']) + + self.main_registry.createLocalPathModule('test', '1.0', 'test', + {'data': '1.0'}) + projects_dir.joinpath('test').mkdir(exist_ok=True) + scratchFile(projects_dir.joinpath('test', 'WORKSPACE')) + scratchFile(projects_dir.joinpath('test', 'BUILD'), [ + 'java_test(', + ' name = "test",' + ' srcs = ["Test.java"],', + ' main_class = "com.example.Test",', + ' use_testrunner = False,', + ' data = ["@data//:foo.txt"],', + ' args = ["$(rlocationpath @data//:foo.txt)"],', + ' deps = ["@bazel_tools//tools/java/runfiles"],', + ')' + ]) + scratchFile(projects_dir.joinpath('test', 'Test.java'), [ + 'package com.example;', + '', + 'import com.google.devtools.build.runfiles.AutoBazelRepository;', + 'import com.google.devtools.build.runfiles.Runfiles;', + '', + 'import java.io.File;', + 'import java.io.IOException;', + '', + '@AutoBazelRepository', + 'public class Test {', + ' public static void main(String[] args) throws IOException {', + ' Runfiles.Preloaded rp = Runfiles.preload();', + ' if (!new File(rp.unmapped().rlocation(args[0])).exists()) {', + ' System.exit(1);', + ' }', + ' if (!new File(rp.withSourceRepository(AutoBazelRepository_Test.NAME).rlocation("data/foo.txt")).exists()) {', + ' System.exit(1);', + ' }', + ' }', + '}', + ]) + + self.ScratchFile('MODULE.bazel', ['bazel_dep(name="test",version="1.0")']) + self.ScratchFile('WORKSPACE') + + # Run sandboxed on Linux and macOS. + exit_code, stderr, stdout = self.RunBazel( + ['test', '@test//:test', '--test_output=errors', + '--test_env=RUNFILES_LIB_DEBUG=1'], allow_failure=True) + self.AssertExitCode(exit_code, 0, stderr, stdout) + # Run unsandboxed on all platforms. + exit_code, stderr, stdout = self.RunBazel( + ['run', '@test//:test'], allow_failure=True, + env_add={"RUNFILES_LIB_DEBUG": "1"}) + self.AssertExitCode(exit_code, 0, stderr, stdout) + if __name__ == '__main__': unittest.main() From bea18a451322c342beff62a370c9cd78c6c61a77 Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Tue, 8 Nov 2022 22:17:46 +0100 Subject: [PATCH 8/9] Improve documentation --- tools/java/runfiles/Runfiles.java | 121 +++++++++++++++++++++--------- 1 file changed, 86 insertions(+), 35 deletions(-) diff --git a/tools/java/runfiles/Runfiles.java b/tools/java/runfiles/Runfiles.java index 40edb48bbe442d..464f4a2980e1f2 100644 --- a/tools/java/runfiles/Runfiles.java +++ b/tools/java/runfiles/Runfiles.java @@ -42,31 +42,64 @@ * ) * * - *

        2. Import the runfiles library and the {@code AutoBazelRepository} annotation. + *

        2. Import the runfiles library. * * 

        - *   import com.google.devtools.build.runfiles.AutoBazelRepository;
  *   import com.google.devtools.build.runfiles.Runfiles;
  *
        * - *

        3. Annotate the class in which a {@code Runfiles} object is created with - * {@link AutoBazelRepository}: + *

        3. Create a {@link Preloaded} object: + * + * 

        + *   public void myFunction() {
+ *     Runfiles.Preloaded runfiles = Runfiles.preload();
+ *     ...
+ *
        + * + *

        4. To look up a runfile, use either of the following approaches: + * + *

        4a. Annotate the class from which runfiles should be looked up with + * {@link AutoBazelRepository} and obtain the name of the Bazel repository containing the class from + * a constant generated by this annotation: * * 

        + *   import com.google.devtools.build.runfiles.AutoBazelRepository;
  *   @AutoBazelRepository
  *   public class MyClass {
- *     ...
+ *     public void myFunction() {
+ *       Runfiles.Preloaded runfiles = Runfiles.preload();
+ *       String path = runfiles.withSourceRepository(AutoBazelRepository_MyClass.NAME)
+ *                             .rlocation("my_workspace/path/to/my/data.txt");
+ *       ...
+ *
  *
        * - *

        4. Create a Runfiles object and use rlocation to look up runfile paths: + *

        4b. Let Bazel compute the path passed to rlocation and pass it into a java_binary + * via an argument or an environment variable: * * 

        - *   public void myFunction() {
- *     Runfiles runfiles = Runfiles.create(AutoBazelRepository_MyClass.BAZEL_REPOSITORY);
- *     String path = runfiles.rlocation("my_workspace/path/to/my/data.txt");
- *     ...
+ *   java_binary(
+ *       name = "my_binary",
+ *       srcs = ["MyClass.java"],
+ *       data = ["@my_workspace//path/to/my:data.txt"],
+ *       env = {"MY_RUNFILE": "$(rlocationpath @my_workspace//path/to/my:data.txt)"},
+ *   )
  *
        * + * 
        + *   public class MyClass {
+ *     public void myFunction() {
+ *       Runfiles.Preloaded runfiles = Runfiles.preload();
+ *       String path = runfiles.unmapped().rlocation(System.getenv("MY_RUNFILE"));
+ *       ...
+ *
+ *
        + * + * For more details on why it is required to pass in the current repository name, see + * {@see https://bazel.build/build/bzlmod#repository-names}. + * + *

        Subprocesses

        + * *

        If you want to start subprocesses that also need runfiles, you need to set the right * environment variables for them: * @@ -77,9 +110,31 @@ * ... * Process p = pb.start(); * + * + *

        {@link Preloaded} vs. {@link Runfiles}

        + * + *

        Instances of {@link Preloaded} are meant to be stored and passed around to other components + * that need to access runfiles. They are created by calling {@link Runfiles#preload()} + * {@link Runfiles#preload(Map)} and immutably encapsulate all data required to look up runfiles + * with the repository mapping of any Bazel repository specified at a later time. + * + *

        Creating {@link Runfiles} instances can be costly, so applications should try to create as few + * instances as possible. + * + *

        Instance of {@link Runfiles} are only meant to be used by code located in a single Bazel + * repository and should not be passed around. They are created by calling + * {@link Preloaded#withSourceRepository(String)} or {@link Preloaded#unmapped()} and in addition to + * the data in {@link Preloaded} also fix a source repository relative to which apparent repository + * names are resolved. + * + *

        Creating {@link Preloaded} instances is cheap. */ public final class Runfiles { + /** + * A class that encapsulates all data required to look up runfiles relative to any Bazel + * repository fixed at a later time. + */ public abstract static class Preloaded { /** @@ -115,24 +170,30 @@ public int hashCode() { } /** - * Returns a new instance that uses the provided source repository as a default for all calls to - * {@link #rlocation(String)}. + * Returns a {@link Runfiles} instance that uses the provided source repository's repository + * mapping to translate apparent into canonical repository names. * - *

        This is useful when receiving a {@link Runfiles} instance from a different Bazel - * repository. In this case, while the runfiles manifest or directory encoded in the instance - * should be used for runfiles lookups, the repository from which apparent repository names - * should be resolved needs to change. + *

        {@see https://bazel.build/build/bzlmod#repository-names} * * @param sourceRepository the canonical name of the Bazel repository relative to which apparent - * repository names should be resolved - * @return a new {@link Runfiles} instance identical to this one, except that calls to - * {@link #rlocation(String)} use the provided source repository. + * repository names should be resolved. Should generally coincide with + * the Bazel repository that contains the caller of this method, which + * can be obtained via {@link AutoBazelRepository}. + * @return a {@link Runfiles} instance that looks up runfiles relative to the provided source + * repository and shares all other data with this {@link Preloaded} instance. */ public final Runfiles withSourceRepository(String sourceRepository) { Util.checkArgument(sourceRepository != null); return new Runfiles(this, sourceRepository); } + /** + * Returns a {@link Runfiles} instance backed by the preloaded runfiles data that can be used to + * look up runfiles paths with canonical repository names only. + * + * @return a {@link Runfiles} instance that can only look up paths with canonical repository + * names and shared all data with this {@link Preloaded} instance. + */ public final Runfiles unmapped() { return new Runfiles(this, null); } @@ -159,20 +220,16 @@ private Runfiles(Preloaded preloadedRunfiles, String sourceRepository) { } /** - * Returns a new {@link Runfiles} instance. + * Returns a new {@link Runfiles.Preloaded} instance. * *

        This method passes the JVM's environment variable map to {@link #create(Map)}. - * - * @param sourceRepository the canonical name of the Bazel repository relative to which runfiles - * lookups should be performed. This can be obtained using - * {@link AutoBazelRepository} (see class documentation). */ public static Preloaded preload() throws IOException { return preload(System.getenv()); } /** - * Returns a new {@link Runfiles} instance. + * Returns a new {@link Runfiles.Preloaded} instance. * *

        The returned object is either: * @@ -192,10 +249,6 @@ public static Preloaded preload() throws IOException { *

        Note about performance: the manifest-based implementation eagerly reads and caches the whole * manifest file upon instantiation. * - * @param sourceRepository the canonical name of the Bazel repository relative to which apparent - * repository names in runfiles paths should be resolved. This can be - * obtained using {@link AutoBazelRepository} (see class documentation). - * * @throws IOException if RUNFILES_MANIFEST_ONLY=1 is in {@code env} but there's no * "RUNFILES_MANIFEST_FILE", "RUNFILES_DIR", or "JAVA_RUNFILES" key in * {@code env} or their values are empty, or some IO error occurs @@ -214,9 +267,8 @@ public static Preloaded preload(Map env) throws IOException { /** * Returns a new {@link Runfiles} instance. * - *

        Deprecated: With {@code --enable_bzlmod}, this function can only resolve runfiles - * correctly if called from the main repository. Use {@link #preload()}} - * instead. + *

        Deprecated: Use {@link #preload()} instead. With {@code --enable_bzlmod}, this + * function does not work correctly. * *

        This method passes the JVM's environment variable map to {@link #create(Map)}. */ @@ -228,9 +280,8 @@ public static Runfiles create() throws IOException { /** * Returns a new {@link Runfiles} instance. * - *

        Deprecated: With {@code --enable_bzlmod}, this function can only resolve runfiles - * correctly if called from the main repository. Use {@link #preload(Map)} instead. - * + *

        Deprecated: Use {@link #preload(Map)} )} instead. With {@code --enable_bzlmod}, this + * function does not work correctly. * *

        The returned object is either: * From b93bc8dfc4cfdee7d053a29656642020a356ba38 Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Tue, 8 Nov 2022 23:00:24 +0100 Subject: [PATCH 9/9] Softly cache a global `Preloaded` instance in `preload()` --- tools/java/runfiles/Runfiles.java | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/tools/java/runfiles/Runfiles.java b/tools/java/runfiles/Runfiles.java index 464f4a2980e1f2..5d75e093a78ae2 100644 --- a/tools/java/runfiles/Runfiles.java +++ b/tools/java/runfiles/Runfiles.java @@ -20,6 +20,7 @@ import java.io.FileReader; import java.io.IOException; import java.io.InputStreamReader; +import java.lang.ref.SoftReference; import java.nio.charset.StandardCharsets; import java.util.Collections; import java.util.HashMap; @@ -119,7 +120,9 @@ * with the repository mapping of any Bazel repository specified at a later time. * *

        Creating {@link Runfiles} instances can be costly, so applications should try to create as few - * instances as possible. + * instances as possible. {@link Runfiles#preload()}, but not {@link Runfiles#preload(Map)}, returns + * a single global, softly cached instance of {@link Preloaded} that is constructed based on the + * JVM's environment variables. * *

        Instance of {@link Runfiles} are only meant to be used by code located in a single Bazel * repository and should not be passed around. They are created by calling @@ -134,6 +137,8 @@ public final class Runfiles { /** * A class that encapsulates all data required to look up runfiles relative to any Bazel * repository fixed at a later time. + * + *

        This class is immutable. */ public abstract static class Preloaded { @@ -211,6 +216,8 @@ private Preloaded() { private static final String MAIN_REPOSITORY = ""; + private static SoftReference DEFAULT_INSTANCE = new SoftReference<>(null); + private final Preloaded preloadedRunfiles; private final String sourceRepository; @@ -220,12 +227,18 @@ private Runfiles(Preloaded preloadedRunfiles, String sourceRepository) { } /** - * Returns a new {@link Runfiles.Preloaded} instance. + * Returns the softly cached global {@link Runfiles.Preloaded} instance, creating it if needed. * *

        This method passes the JVM's environment variable map to {@link #create(Map)}. */ - public static Preloaded preload() throws IOException { - return preload(System.getenv()); + public static synchronized Preloaded preload() throws IOException { + Preloaded instance = DEFAULT_INSTANCE.get(); + if (instance != null) { + return instance; + } + instance = preload(System.getenv()); + DEFAULT_INSTANCE = new SoftReference<>(instance); + return instance; } /**