From ef247efab4526547e12161b1d74cb624c0e0bd71 Mon Sep 17 00:00:00 2001 From: Laszlo Csomor Date: Mon, 16 Apr 2018 03:29:37 -0700 Subject: [PATCH] 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 --- src/test/py/bazel/runfiles_test.py | 47 +------- .../testdata/runfiles_test/foo/BUILD.mock | 2 + .../bazel/testdata/runfiles_test/foo/Foo.java | 55 +++++++++- .../build/runfiles/DirectoryBased.java | 22 +++- .../build/runfiles/DirectoryBasedTest.java | 17 ++- .../build/runfiles/ManifestBased.java | 29 ++++- .../devtools/build/runfiles/Runfiles.java | 19 ++++ .../devtools/build/runfiles/RunfilesTest.java | 103 ++++++++++++++++-- 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 21ed5fd855bf36..fb2b0e180f88be 100644 --- a/src/test/py/bazel/runfiles_test.py +++ b/src/test/py/bazel/runfiles_test.py @@ -31,50 +31,12 @@ def testAttemptToBuildRunfilesOnWindows(self): 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 @@ def _AssertPythonRunfilesLibraryInBazelToolsRepo(self, family, lang_name): 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 04fc23ebace7e3..48d3108f677b47 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 0e9f5cfcd0b1aa..44a01847ce12c2 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 4376810a4ce4a0..0215a7c26bfe34 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 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; + } } 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 b15f12a58da9c9..abd90731799ddd 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 @@ 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 void testRlocation() throws Exception { // 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 void testCtorArgumentValidation() throws Exception { // 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 7c47b60aaa7a5f..30b3bd24961071 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.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 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 @@ private static Map loadRunfiles(String path) throws IOException 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 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; + } } 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 bfe571c8eccca8..c34298550004ca 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 @@ * Runfiles runfiles = Runfiles.create(); * File p = new File(runfiles.rlocation("io_bazel/src/bazel")); * + * + *

If you want to start subprocesses that also need runfiles, you need to set the right + * environment variables for them: + * + * 

+ *   String path = r.rlocation("path/to/binary");
+ *   ProcessBuilder pb = new ProcessBuilder(path);
+ *   pb.environment().putAll(r.getEnvVars());
+ *   ...
+ *   Process p = pb.start();
+ *
*/ public abstract class Runfiles { @@ -103,6 +114,14 @@ public final String rlocation(String path) { return rlocationChecked(path); } + /** + * Returns environment variables for subprocesses. + * + *

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(); + /** Returns true if the platform supports runfiles only via manifests. */ private static boolean isManifestOnly(Map 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 0d1904aff0e382..af9e8d6ced7c3e 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.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 @@ private void assertRlocationArg(Runfiles runfiles, String path, @Nullable String @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 void testCreatesManifestBasedRunfiles() throws Exception { @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 void testFailsToCreateManifestBasedBecauseManifestDoesNotExist() throws E 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 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 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()); + } }