Java,runfiles: add Runfiles.getEnvVars()

The new method lets Java programs export the
RUNFILES_* environment variables for subprocesses,
in case those subprocesses are other Bazel-built
binaries that also need runfiles.

See https://github.com/bazelbuild/bazel/issues/4460

Change-Id: I05c0b84db357128bc15f21a167a0d92e8d17599c

Closes #5016.

Change-Id: I66ca5c44067a7353b8de180a64f20c8352e3c9ec
PiperOrigin-RevId: 193013289

8 files changed, 235 insertions, 59 deletions

diff --git a/src/test/py/bazel/runfiles_test.py b/src/test/py/bazel/runfiles_test.py
index 21ed5fd855..fb2b0e180f 100644
--- a/src/test/py/bazel/runfiles_test.py
+++ b/src/test/py/bazel/runfiles_test.py
@@ -31,50 +31,12 @@ class RunfilesTest(test_base.TestBase):
     self.assertIn("building runfiles is not supported on Windows",
                   "\n".join(stderr))
 
-  def testJavaRunfilesLibraryInBazelToolsRepo(self):
-    for s, t in [
-        ("WORKSPACE.mock", "WORKSPACE"),
-        ("foo/BUILD.mock", "foo/BUILD"),
-        ("foo/Foo.java", "foo/Foo.java"),
-        ("foo/datadep/hello.txt", "foo/datadep/hello.txt"),
-    ]:
-      self.CopyFile(
-          self.Rlocation(
-              "io_bazel/src/test/py/bazel/testdata/runfiles_test/" + s), t)
-
-    exit_code, stdout, stderr = self.RunBazel(["info", "bazel-bin"])
-    self.AssertExitCode(exit_code, 0, stderr)
-    bazel_bin = stdout[0]
-
-    exit_code, _, stderr = self.RunBazel(["build", "//foo:runfiles-java"])
-    self.AssertExitCode(exit_code, 0, stderr)
-
-    if test_base.TestBase.IsWindows():
-      bin_path = os.path.join(bazel_bin, "foo/runfiles-java.exe")
-    else:
-      bin_path = os.path.join(bazel_bin, "foo/runfiles-java")
-
-    self.assertTrue(os.path.exists(bin_path))
-
-    exit_code, stdout, stderr = self.RunProgram(
-        [bin_path], env_add={"TEST_SRCDIR": "__ignore_me__"})
-    self.AssertExitCode(exit_code, 0, stderr)
-    if len(stdout) != 2:
-      self.fail("stdout: %s" % stdout)
-    self.assertEqual(stdout[0], "Hello Java Foo!")
-    six.assertRegex(self, stdout[1], "^rloc=.*/foo/datadep/hello.txt")
-    self.assertNotIn("__ignore_me__", stdout[1])
-    with open(stdout[1].split("=", 1)[1], "r") as f:
-      lines = [l.strip() for l in f.readlines()]
-    if len(lines) != 1:
-      self.fail("lines: %s" % lines)
-    self.assertEqual(lines[0], "world")
-
-  def _AssertPythonRunfilesLibraryInBazelToolsRepo(self, family, lang_name):
+  def _AssertRunfilesLibraryInBazelToolsRepo(self, family, lang_name):
     for s, t in [
         ("WORKSPACE.mock", "WORKSPACE"),
         ("foo/BUILD.mock", "foo/BUILD"),
         ("foo/foo.py", "foo/foo.py"),
+        ("foo/Foo.java", "foo/Foo.java"),
         ("foo/datadep/hello.txt", "foo/datadep/hello.txt"),
         ("bar/BUILD.mock", "bar/BUILD"),
         ("bar/bar.py", "bar/bar.py"),
@@ -132,7 +94,10 @@ class RunfilesTest(test_base.TestBase):
       i += 2
 
   def testPythonRunfilesLibraryInBazelToolsRepo(self):
-    self._AssertPythonRunfilesLibraryInBazelToolsRepo("py", "Python")
+    self._AssertRunfilesLibraryInBazelToolsRepo("py", "Python")
+
+  def testJavaRunfilesLibraryInBazelToolsRepo(self):
+    self._AssertRunfilesLibraryInBazelToolsRepo("java", "Java")
 
   def testRunfilesLibrariesFindRunfilesWithoutEnvvars(self):
     for s, t in [
diff --git a/src/test/py/bazel/testdata/runfiles_test/foo/BUILD.mock b/src/test/py/bazel/testdata/runfiles_test/foo/BUILD.mock
index 04fc23ebac..48d3108f67 100644
--- a/src/test/py/bazel/testdata/runfiles_test/foo/BUILD.mock
+++ b/src/test/py/bazel/testdata/runfiles_test/foo/BUILD.mock
@@ -15,6 +15,8 @@ java_binary(
     srcs = ["Foo.java"],
     data = [
         "datadep/hello.txt",
+        "//bar:bar-py",
+        "//bar:bar-java",
     ],
     main_class = "Foo",
     deps = ["@bazel_tools//tools/runfiles:java-runfiles"],
diff --git a/src/test/py/bazel/testdata/runfiles_test/foo/Foo.java b/src/test/py/bazel/testdata/runfiles_test/foo/Foo.java
index 0e9f5cfcd0..44a01847ce 100644
--- a/src/test/py/bazel/testdata/runfiles_test/foo/Foo.java
+++ b/src/test/py/bazel/testdata/runfiles_test/foo/Foo.java
@@ -13,13 +13,66 @@
 // limitations under the License.
 
 import com.google.devtools.build.runfiles.Runfiles;
+import java.io.BufferedReader;
+import java.io.File;
 import java.io.IOException;
+import java.io.InputStream;
+import java.io.InputStreamReader;
+import java.nio.charset.StandardCharsets;
+import java.util.concurrent.TimeUnit;
 
 /** A mock Java binary only used in tests, to exercise the Java Runfiles library. */
 public class Foo {
-  public static void main(String[] args) throws IOException {
+  public static void main(String[] args) throws IOException, InterruptedException {
     System.out.println("Hello Java Foo!");
     Runfiles r = Runfiles.create();
     System.out.println("rloc=" + r.rlocation("foo_ws/foo/datadep/hello.txt"));
+
+    for (String lang : new String[] {"py", "java"}) {
+      String path = r.rlocation(childBinaryName(lang));
+      if (path == null || path.isEmpty()) {
+        throw new IOException("cannot find child binary for " + lang);
+      }
+
+      ProcessBuilder pb = new ProcessBuilder(path);
+      pb.environment().putAll(r.getEnvVars());
+      if (isWindows()) {
+        pb.environment().put("SYSTEMROOT", System.getenv("SYSTEMROOT"));
+      }
+      Process p = pb.start();
+      if (!p.waitFor(3, TimeUnit.SECONDS)) {
+        throw new IOException("child process for " + lang + " timed out");
+      }
+      if (p.exitValue() != 0) {
+        throw new IOException(
+            "child process for " + lang + " failed: " + readStream(p.getErrorStream()));
+      }
+      System.out.printf(readStream(p.getInputStream()));
+    }
+  }
+
+  private static boolean isWindows() {
+    return File.separatorChar == '\\';
+  }
+
+  private static String childBinaryName(String lang) {
+    if (isWindows()) {
+      return "foo_ws/bar/bar-" + lang + ".exe";
+    } else {
+      return "foo_ws/bar/bar-" + lang;
+    }
+  }
+
+  private static String readStream(InputStream stm) throws IOException {
+    StringBuilder result = new StringBuilder();
+    try (BufferedReader r =
+        new BufferedReader(new InputStreamReader(stm, StandardCharsets.UTF_8))) {
+      String line = null;
+      while ((line = r.readLine()) != null) {
+        line = line.trim(); // trim CRLF on Windows, LF on Linux
+        result.append(line).append("\n"); // ensure uniform line ending
+      }
+    }
+    return result.toString();
   }
 }
diff --git a/src/tools/runfiles/java/com/google/devtools/build/runfiles/DirectoryBased.java b/src/tools/runfiles/java/com/google/devtools/build/runfiles/DirectoryBased.java
index 4376810a4c..0215a7c26b 100644
--- a/src/tools/runfiles/java/com/google/devtools/build/runfiles/DirectoryBased.java
+++ b/src/tools/runfiles/java/com/google/devtools/build/runfiles/DirectoryBased.java
@@ -14,13 +14,18 @@
 
 package com.google.devtools.build.runfiles;
 
+import java.io.File;
+import java.io.IOException;
+import java.util.HashMap;
+import java.util.Map;
+
 /** {@link Runfiles} implementation that appends runfiles paths to the runfiles root. */
-class DirectoryBased extends Runfiles {
+final class DirectoryBased extends Runfiles {
   private final String runfilesRoot;
 
-  DirectoryBased(String runfilesDir) {
-    Util.checkArgument(runfilesDir != null);
-    Util.checkArgument(!runfilesDir.isEmpty());
+  DirectoryBased(String runfilesDir) throws IOException {
+    Util.checkArgument(!Util.isNullOrEmpty(runfilesDir));
+    Util.checkArgument(new File(runfilesDir).isDirectory());
     this.runfilesRoot = runfilesDir;
   }
 
@@ -28,4 +33,13 @@ class DirectoryBased extends Runfiles {
   String rlocationChecked(String path) {
     return runfilesRoot + "/" + path;
   }
+
+  @Override
+  public Map<String, String> getEnvVars() {
+    HashMap<String, String> 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;
+  }
 }
diff --git a/src/tools/runfiles/java/com/google/devtools/build/runfiles/DirectoryBasedTest.java b/src/tools/runfiles/java/com/google/devtools/build/runfiles/DirectoryBasedTest.java
index b15f12a58d..abd9073179 100644
--- a/src/tools/runfiles/java/com/google/devtools/build/runfiles/DirectoryBasedTest.java
+++ b/src/tools/runfiles/java/com/google/devtools/build/runfiles/DirectoryBasedTest.java
@@ -17,6 +17,7 @@ package com.google.devtools.build.runfiles;
 import static com.google.common.truth.Truth.assertThat;
 import static org.junit.Assert.fail;
 
+import java.io.File;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.JUnit4;
@@ -30,8 +31,11 @@ public final class DirectoryBasedTest {
     // 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.
-    DirectoryBased r = new DirectoryBased("foo/bar baz//qux/");
-    assertThat(r.rlocation("arg")).isEqualTo("foo/bar baz//qux//arg");
+    File dir = new File(System.getenv("TEST_TMPDIR"), "mock/runfiles");
+    assertThat(dir.mkdirs()).isTrue();
+    DirectoryBased r = new DirectoryBased(dir.toString());
+    // Escaping for "\": once for string and once for regex.
+    assertThat(r.rlocation("arg")).matches(".*[/\\\\]mock[/\\\\]runfiles[/\\\\]arg");
   }
 
   @Test
@@ -50,6 +54,13 @@ public final class DirectoryBasedTest {
       // expected
     }
 
-    new DirectoryBased("non-empty value is fine");
+    try {
+      new DirectoryBased("non-existent directory is bad");
+      fail();
+    } catch (IllegalArgumentException e) {
+      // expected
+    }
+
+    new DirectoryBased(System.getenv("TEST_TMPDIR"));
   }
 }
diff --git a/src/tools/runfiles/java/com/google/devtools/build/runfiles/ManifestBased.java b/src/tools/runfiles/java/com/google/devtools/build/runfiles/ManifestBased.java
index 7c47b60aaa..30b3bd2496 100644
--- a/src/tools/runfiles/java/com/google/devtools/build/runfiles/ManifestBased.java
+++ b/src/tools/runfiles/java/com/google/devtools/build/runfiles/ManifestBased.java
@@ -15,6 +15,7 @@
 package com.google.devtools.build.runfiles;
 
 import java.io.BufferedReader;
+import java.io.File;
 import java.io.FileInputStream;
 import java.io.IOException;
 import java.io.InputStreamReader;
@@ -24,12 +25,14 @@ import java.util.HashMap;
 import java.util.Map;
 
 /** {@link Runfiles} implementation that parses a runfiles-manifest file to look up runfiles. */
-class ManifestBased extends Runfiles {
+final class ManifestBased extends Runfiles {
   private final Map<String, String> runfiles;
+  private final String manifestPath;
 
   ManifestBased(String manifestPath) throws IOException {
     Util.checkArgument(manifestPath != null);
     Util.checkArgument(!manifestPath.isEmpty());
+    this.manifestPath = manifestPath;
     this.runfiles = loadRunfiles(manifestPath);
   }
 
@@ -49,8 +52,32 @@ class ManifestBased extends Runfiles {
     return Collections.unmodifiableMap(result);
   }
 
+  private static String findRunfilesDir(String manifest) {
+    if (manifest.endsWith("/MANIFEST")
+        || manifest.endsWith("\\MANIFEST")
+        || manifest.endsWith(".runfiles_manifest")) {
+      String path = manifest.substring(0, manifest.length() - 9);
+      if (new File(path).isDirectory()) {
+        return path;
+      }
+    }
+    return "";
+  }
+
   @Override
   public String rlocationChecked(String path) {
     return runfiles.get(path);
   }
+
+  @Override
+  public Map<String, String> getEnvVars() {
+    HashMap<String, String> 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;
+  }
 }
diff --git a/src/tools/runfiles/java/com/google/devtools/build/runfiles/Runfiles.java b/src/tools/runfiles/java/com/google/devtools/build/runfiles/Runfiles.java
index bfe571c8ec..c342985500 100644
--- a/src/tools/runfiles/java/com/google/devtools/build/runfiles/Runfiles.java
+++ b/src/tools/runfiles/java/com/google/devtools/build/runfiles/Runfiles.java
@@ -27,6 +27,17 @@ import java.util.Map;
  *   Runfiles runfiles = Runfiles.create();
  *   File p = new File(runfiles.rlocation("io_bazel/src/bazel"));
  * </pre>
+ *
+ * <p>If you want to start subprocesses that also need runfiles, you need to set the right
+ * environment variables for them:
+ *
+ * <pre>
+ *   String path = r.rlocation("path/to/binary");
+ *   ProcessBuilder pb = new ProcessBuilder(path);
+ *   pb.environment().putAll(r.getEnvVars());
+ *   ...
+ *   Process p = pb.start();
+ * </pre>
  */
 public abstract class Runfiles {
 
@@ -103,6 +114,14 @@ public abstract class Runfiles {
     return rlocationChecked(path);
   }
 
+  /**
+   * Returns environment variables for subprocesses.
+   *
+   * <p>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<String, String> getEnvVars();
+
   /** Returns true if the platform supports runfiles only via manifests. */
   private static boolean isManifestOnly(Map<String, String> env) {
     return "1".equals(env.get("RUNFILES_MANIFEST_ONLY"));
diff --git a/src/tools/runfiles/java/com/google/devtools/build/runfiles/RunfilesTest.java b/src/tools/runfiles/java/com/google/devtools/build/runfiles/RunfilesTest.java
index 0d1904aff0..af9e8d6ced 100644
--- a/src/tools/runfiles/java/com/google/devtools/build/runfiles/RunfilesTest.java
+++ b/src/tools/runfiles/java/com/google/devtools/build/runfiles/RunfilesTest.java
@@ -21,6 +21,12 @@ import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import java.io.File;
 import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+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.Test;
 import org.junit.runner.RunWith;
@@ -48,7 +54,11 @@ public final class RunfilesTest {
 
   @Test
   public void testRlocationArgumentValidation() throws Exception {
-    Runfiles r = Runfiles.create(ImmutableMap.of("RUNFILES_DIR", "whatever"));
+    Path dir =
+        Files.createTempDirectory(
+            FileSystems.getDefault().getPath(System.getenv("TEST_TMPDIR")), null);
+
+    Runfiles r = Runfiles.create(ImmutableMap.of("RUNFILES_DIR", dir.toString()));
     assertRlocationArg(r, null, null);
     assertRlocationArg(r, "", null);
     assertRlocationArg(r, "foo/..", "contains uplevel");
@@ -80,38 +90,46 @@ public final class RunfilesTest {
 
   @Test
   public void testCreatesDirectoryBasedRunfiles() throws Exception {
+    Path dir =
+        Files.createTempDirectory(
+            FileSystems.getDefault().getPath(System.getenv("TEST_TMPDIR")), null);
+
     Runfiles r =
         Runfiles.create(
             ImmutableMap.of(
                 "RUNFILES_MANIFEST_FILE", "ignored when RUNFILES_MANIFEST_ONLY is not set to 1",
-                "RUNFILES_DIR", "runfiles/dir",
+                "RUNFILES_DIR", dir.toString(),
                 "JAVA_RUNFILES", "ignored when RUNFILES_DIR has a value",
                 "TEST_SRCDIR", "should always be ignored"));
-    assertThat(r.rlocation("a/b")).isEqualTo("runfiles/dir/a/b");
-    assertThat(r.rlocation("foo")).isEqualTo("runfiles/dir/foo");
+    assertThat(r.rlocation("a/b")).endsWith("/a/b");
+    assertThat(r.rlocation("foo")).endsWith("/foo");
 
     r =
         Runfiles.create(
             ImmutableMap.of(
                 "RUNFILES_MANIFEST_FILE", "ignored when RUNFILES_MANIFEST_ONLY is not set to 1",
                 "RUNFILES_DIR", "",
-                "JAVA_RUNFILES", "runfiles/dir",
+                "JAVA_RUNFILES", dir.toString(),
                 "TEST_SRCDIR", "should always be ignored"));
-    assertThat(r.rlocation("a/b")).isEqualTo("runfiles/dir/a/b");
-    assertThat(r.rlocation("foo")).isEqualTo("runfiles/dir/foo");
+    assertThat(r.rlocation("a/b")).endsWith("/a/b");
+    assertThat(r.rlocation("foo")).endsWith("/foo");
   }
 
   @Test
   public void testIgnoresTestSrcdirWhenJavaRunfilesIsUndefinedAndJustFails() throws Exception {
+    Path dir =
+        Files.createTempDirectory(
+            FileSystems.getDefault().getPath(System.getenv("TEST_TMPDIR")), null);
+
     Runfiles.create(
         ImmutableMap.of(
-            "RUNFILES_DIR", "non-empty",
+            "RUNFILES_DIR", dir.toString(),
             "RUNFILES_MANIFEST_FILE", "ignored when RUNFILES_MANIFEST_ONLY is not set to 1",
             "TEST_SRCDIR", "should always be ignored"));
 
     Runfiles.create(
         ImmutableMap.of(
-            "JAVA_RUNFILES", "non-empty",
+            "JAVA_RUNFILES", dir.toString(),
             "RUNFILES_MANIFEST_FILE", "ignored when RUNFILES_MANIFEST_ONLY is not set to 1",
             "TEST_SRCDIR", "should always be ignored"));
 
@@ -143,4 +161,71 @@ public final class RunfilesTest {
       assertThat(e).hasMessageThat().contains("non-existing path");
     }
   }
+
+  @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);
+    Map<String, String> envvars =
+        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"))
+            .getEnvVars();
+    assertThat(envvars.keySet())
+        .containsExactly(
+            "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());
+
+    Path rfDir = dir.resolve("foo.runfiles");
+    Files.createDirectories(rfDir);
+    mf = dir.resolve("foo.runfiles_manifest");
+    Files.write(mf, Collections.emptyList(), StandardCharsets.UTF_8);
+    envvars =
+        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"))
+            .getEnvVars();
+    assertThat(envvars.get("RUNFILES_MANIFEST_ONLY")).isEqualTo("1");
+    assertThat(envvars.get("RUNFILES_MANIFEST_FILE")).isEqualTo(mf.toString());
+    assertThat(envvars.get("RUNFILES_DIR")).isEqualTo(rfDir.toString());
+    assertThat(envvars.get("JAVA_RUNFILES")).isEqualTo(rfDir.toString());
+  }
+
+  @Test
+  public void testDirectoryBasedEnvVars() throws Exception {
+    Path dir =
+        Files.createTempDirectory(
+            FileSystems.getDefault().getPath(System.getenv("TEST_TMPDIR")), null);
+
+    Map<String, String> envvars =
+        Runfiles.create(
+                ImmutableMap.of(
+                    "RUNFILES_MANIFEST_FILE",
+                    "ignored when RUNFILES_MANIFEST_ONLY is not set to 1",
+                    "RUNFILES_DIR",
+                    dir.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());
+  }
 }