From c43ace2a9644c262be41c0d25c867161d7cfd903 Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Thu, 7 Mar 2024 00:28:19 +0100 Subject: [PATCH] [7.1.0] Add multiplex sandboxing support to JavaBuilder (#21598) This does not include adding support to Java actions yet. Work towards #21091 Closes #21370. PiperOrigin-RevId: 613231195 Change-Id: I87e76d2cebed3075a1eedfd8445b910ebd7604b9 Closes https://github.com/bazelbuild/bazel/issues/21518 --- .../build/buildjar/BazelJavaBuilder.java | 19 ++++++--- .../buildjar/JavaLibraryBuildRequest.java | 41 ++++++++++++------- .../lib/rules/java/JavaCompilationHelper.java | 3 ++ .../lib/rules/java/JavaToolchainProvider.java | 5 +++ .../lib/rules/java/JavaToolchainRule.java | 4 ++ .../worker/JsonWorkerMessageProcessor.java | 10 +++++ .../common/java/java_toolchain.bzl | 3 ++ src/test/shell/bazel/bazel_java_test.sh | 32 +++++++++++++++ 8 files changed, 96 insertions(+), 21 deletions(-) diff --git a/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/BazelJavaBuilder.java b/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/BazelJavaBuilder.java index c8449a0073174f..62e01a5e829c68 100644 --- a/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/BazelJavaBuilder.java +++ b/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/BazelJavaBuilder.java @@ -31,6 +31,7 @@ import java.io.PrintWriter; import java.io.Writer; import java.nio.charset.Charset; +import java.nio.file.Path; import java.time.Duration; import java.util.Arrays; import java.util.List; @@ -46,7 +47,12 @@ public static void main(String[] args) { if (args.length == 1 && args[0].equals("--persistent_worker")) { WorkRequestHandler workerHandler = new WorkRequestHandlerBuilder( - builder::parseAndBuild, + new WorkRequestHandler.WorkRequestCallback( + (workRequest, printWriter) -> + builder.parseAndBuild( + workRequest.getArgumentsList(), + Path.of(workRequest.getSandboxDir()), + printWriter)), System.err, new ProtoWorkerMessageProcessor(System.in, System.out)) .setCpuUsageBeforeGc(Duration.ofSeconds(10)) @@ -66,7 +72,7 @@ public static void main(String[] args) { new PrintWriter(new OutputStreamWriter(System.err, Charset.defaultCharset())); int returnCode; try { - returnCode = builder.parseAndBuild(Arrays.asList(args), pw); + returnCode = builder.parseAndBuild(Arrays.asList(args), Path.of(""), pw); } finally { pw.flush(); } @@ -74,9 +80,9 @@ public static void main(String[] args) { } } - public int parseAndBuild(List args, PrintWriter pw) { + public int parseAndBuild(List args, Path workDir, PrintWriter pw) { try { - JavaLibraryBuildRequest build = parse(args); + JavaLibraryBuildRequest build = parse(args, workDir); try (SimpleJavaLibraryBuilder builder = build.getDependencyModule().reduceClasspath() ? new ReducedClasspathJavaLibraryBuilder() @@ -122,12 +128,13 @@ protected int build( * @throws InvalidCommandLineException on any command line error */ @VisibleForTesting - public JavaLibraryBuildRequest parse(List args) + public JavaLibraryBuildRequest parse(List args, Path workDir) throws IOException, InvalidCommandLineException { OptionsParser optionsParser = new OptionsParser(args, JavacOptions.createWithWarningsAsErrorsDefault(ImmutableList.of())); ImmutableList plugins = ImmutableList.of(new ErrorPronePlugin(BazelScannerSuppliers.bazelChecks())); - return new JavaLibraryBuildRequest(optionsParser, plugins, new DependencyModule.Builder()); + return new JavaLibraryBuildRequest( + optionsParser, plugins, new DependencyModule.Builder(), workDir); } } diff --git a/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/JavaLibraryBuildRequest.java b/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/JavaLibraryBuildRequest.java index c7e0f273b7592d..46b6c393a810c6 100644 --- a/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/JavaLibraryBuildRequest.java +++ b/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/JavaLibraryBuildRequest.java @@ -34,7 +34,6 @@ import com.google.protobuf.ByteString; import java.io.IOException; import java.nio.file.Path; -import java.nio.file.Paths; import java.util.ArrayList; import java.util.Collection; import java.util.List; @@ -44,7 +43,10 @@ /** All the information needed to perform a single Java library build operation. */ public final class JavaLibraryBuildRequest { - private ImmutableList javacOpts; + private final ImmutableList javacOpts; + + /** The directory relative to which all other paths should be resolved. */ + private final Path workDir; /** Where to store source files generated by annotation processors. */ private final Path sourceGenDir; @@ -104,7 +106,7 @@ public final class JavaLibraryBuildRequest { public JavaLibraryBuildRequest( OptionsParser optionsParser, List extraPlugins) throws InvalidCommandLineException, IOException { - this(optionsParser, extraPlugins, new DependencyModule.Builder()); + this(optionsParser, extraPlugins, new DependencyModule.Builder(), Path.of("")); } /** @@ -114,14 +116,16 @@ public JavaLibraryBuildRequest( * @param optionsParser the parsed command line args. * @param extraPlugins extraneous plugins to use in addition to the strict dependency module. * @param depsBuilder a preconstructed dependency module builder. + * @param workDir the directory relative to which all other paths should be resolved. * @throws InvalidCommandLineException on any command line error */ public JavaLibraryBuildRequest( OptionsParser optionsParser, List extraPlugins, - DependencyModule.Builder depsBuilder) + DependencyModule.Builder depsBuilder, + Path workDir) throws InvalidCommandLineException, IOException { - this(optionsParser, extraPlugins, depsBuilder, ImmutableMap.of(), OptionalInt.empty()); + this(optionsParser, extraPlugins, depsBuilder, ImmutableMap.of(), workDir, OptionalInt.empty()); } /** @@ -139,19 +143,22 @@ public JavaLibraryBuildRequest( List extraPlugins, DependencyModule.Builder depsBuilder, ImmutableMap inputsAndDigest, + Path workDir, OptionalInt requestId) throws InvalidCommandLineException, IOException { + // Keep this first as it is used by asPath. + this.workDir = workDir; depsBuilder.setDirectJars( - optionsParser.directJars().stream().map(Paths::get).collect(toImmutableSet())); + optionsParser.directJars().stream().map(this::asPath).collect(toImmutableSet())); if (optionsParser.getStrictJavaDeps() != null) { depsBuilder.setStrictJavaDeps(optionsParser.getStrictJavaDeps()); } if (optionsParser.getOutputDepsProtoFile() != null) { - depsBuilder.setOutputDepsProtoFile(Paths.get(optionsParser.getOutputDepsProtoFile())); + depsBuilder.setOutputDepsProtoFile(asPath(optionsParser.getOutputDepsProtoFile())); } depsBuilder.addDepsArtifacts(asPaths(optionsParser.getDepsArtifacts())); depsBuilder.setPlatformJars( - optionsParser.getBootClassPath().stream().map(Paths::get).collect(toImmutableSet())); + optionsParser.getBootClassPath().stream().map(this::asPath).collect(toImmutableSet())); if (optionsParser.reduceClasspathMode() != OptionsParser.ReduceClasspathMode.NONE) { depsBuilder.setReduceClasspath(); } @@ -165,7 +172,7 @@ public JavaLibraryBuildRequest( AnnotationProcessingModule.Builder processingBuilder = AnnotationProcessingModule.builder(); processingBuilder.setSourceGenDir(sourceGenDir); if (optionsParser.getManifestProtoPath() != null) { - processingBuilder.setManifestProtoPath(Paths.get(optionsParser.getManifestProtoPath())); + processingBuilder.setManifestProtoPath(asPath(optionsParser.getManifestProtoPath())); } this.processingModule = processingBuilder.build(); @@ -217,25 +224,29 @@ public JavaLibraryBuildRequest( */ // TODO(b/169793789): kill this with fire if javahotswap starts using jars instead of classes @VisibleForTesting - static Path deriveDirectory(String label, String outputJar, String suffix) throws IOException { + static Path deriveDirectory(String label, String outputJar, String suffix, Path workDir) { checkArgument(label != null, "--target_label is required"); checkArgument(outputJar != null, "--output is required"); checkArgument( label.contains(":"), "--target_label must be a canonical label (containing a `:`)"); - Path path = Paths.get(outputJar); + Path path = workDir.resolve(outputJar); String name = MoreFiles.getNameWithoutExtension(path); String base = label.substring(label.lastIndexOf(':') + 1); return path.resolveSibling("_javac").resolve(base).resolve(name + suffix); } - private static ImmutableList asPaths(Collection paths) { - return paths.stream().map(Paths::get).collect(toImmutableList()); + private Path deriveDirectory(String label, String outputJar, String suffix) { + return deriveDirectory(label, outputJar, suffix, workDir); + } + + private ImmutableList asPaths(Collection paths) { + return paths.stream().map(this::asPath).collect(toImmutableList()); } @Nullable - private static Path asPath(@Nullable String path) { - return path != null ? Paths.get(path) : null; + private Path asPath(@Nullable String path) { + return path != null ? workDir.resolve(path) : null; } public ImmutableList getJavacOpts() { diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationHelper.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationHelper.java index 005d4783876f24..e50e5cd1b8fe07 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationHelper.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationHelper.java @@ -429,6 +429,9 @@ private ImmutableMap getExecutionInfo() throws RuleErrorExceptio if (javaToolchain.getJavacSupportsWorkerCancellation()) { modifiableExecutionInfo.put(ExecutionRequirements.SUPPORTS_WORKER_CANCELLATION, "1"); } + if (javaToolchain.getJavacSupportsWorkerMultiplexSandboxing()) { + modifiableExecutionInfo.put(ExecutionRequirements.SUPPORTS_MULTIPLEX_SANDBOXING, "1"); + } ImmutableMap.Builder executionInfo = ImmutableMap.builder(); executionInfo.putAll( getConfiguration() diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaToolchainProvider.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaToolchainProvider.java index 355d8220a48791..6fb9290da487c3 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaToolchainProvider.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaToolchainProvider.java @@ -316,6 +316,11 @@ public boolean getJavacSupportsWorkerCancellation() throws RuleErrorException { return getUnderlyingValue("_javac_supports_worker_cancellation", Boolean.class); } + /** Returns whether JavaBuilders supports running multiplex persistent workers in sandbox mode */ + public boolean getJavacSupportsWorkerMultiplexSandboxing() throws RuleErrorException { + return getUnderlyingValue("_javac_supports_worker_multiplex_sandboxing", Boolean.class); + } + /** Returns the global {@code java_package_configuration} data. */ public ImmutableList packageConfiguration() throws RuleErrorException { diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaToolchainRule.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaToolchainRule.java index 80bb2261b518ec..ef383fd49e2570 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaToolchainRule.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaToolchainRule.java @@ -130,6 +130,10 @@ The Java target version (e.g., '6' or '7'). It specifies for which Java runtime True if JavaBuilder supports cancellation of persistent workers, false if it doesn't. */ .add(attr("javac_supports_worker_cancellation", BOOLEAN).value(true)) + /* + True if JavaBuilder supports multiplex sandboxing, false if it doesn't. + */ + .add(attr("javac_supports_worker_multiplex_sandboxing", BOOLEAN).value(false)) /* Labels of tools available for label-expansion in jvm_opts. */ diff --git a/src/main/java/com/google/devtools/build/lib/worker/JsonWorkerMessageProcessor.java b/src/main/java/com/google/devtools/build/lib/worker/JsonWorkerMessageProcessor.java index d1e840adfab0f6..0d1ef8b968168f 100644 --- a/src/main/java/com/google/devtools/build/lib/worker/JsonWorkerMessageProcessor.java +++ b/src/main/java/com/google/devtools/build/lib/worker/JsonWorkerMessageProcessor.java @@ -105,6 +105,7 @@ public WorkRequest readWorkRequest() throws IOException { List inputs = null; Integer requestId = null; Integer verbosity = null; + String sandboxDir = null; try { reader.beginObject(); while (reader.hasNext()) { @@ -134,6 +135,12 @@ public WorkRequest readWorkRequest() throws IOException { } verbosity = reader.nextInt(); break; + case "sandboxDir": + if (sandboxDir != null) { + throw new IOException("Work response cannot have more than one sandboxDir"); + } + sandboxDir = reader.nextString(); + break; default: // As per https://bazel.build/docs/creating-workers#work-responses, // unknown fields are ignored. @@ -159,6 +166,9 @@ public WorkRequest readWorkRequest() throws IOException { if (verbosity != null) { requestBuilder.setVerbosity(verbosity); } + if (sandboxDir != null) { + requestBuilder.setSandboxDir(sandboxDir); + } return requestBuilder.build(); } diff --git a/src/main/starlark/builtins_bzl/common/java/java_toolchain.bzl b/src/main/starlark/builtins_bzl/common/java/java_toolchain.bzl index 7b274b62c42f3b..9a96df3695c2e0 100644 --- a/src/main/starlark/builtins_bzl/common/java/java_toolchain.bzl +++ b/src/main/starlark/builtins_bzl/common/java/java_toolchain.bzl @@ -62,6 +62,7 @@ JavaToolchainInfo, _new_javatoolchaininfo = provider( "_javac_supports_workers": _PRIVATE_API_DOC_STRING, "_javac_supports_multiplex_workers": _PRIVATE_API_DOC_STRING, "_javac_supports_worker_cancellation": _PRIVATE_API_DOC_STRING, + "_javac_supports_worker_multiplex_sandboxing": _PRIVATE_API_DOC_STRING, "_jspecify_info": _PRIVATE_API_DOC_STRING, "_local_java_optimization_config": _PRIVATE_API_DOC_STRING, "_one_version_tool": _PRIVATE_API_DOC_STRING, @@ -116,6 +117,7 @@ def _java_toolchain_impl(ctx): _javac_supports_workers = ctx.attr.javac_supports_workers, _javac_supports_multiplex_workers = ctx.attr.javac_supports_multiplex_workers, _javac_supports_worker_cancellation = ctx.attr.javac_supports_worker_cancellation, + _javac_supports_worker_multiplex_sandboxing = ctx.attr.javac_supports_worker_multiplex_sandboxing, _jspecify_info = _get_jspecify_info(ctx), _local_java_optimization_config = ctx.files._local_java_optimization_configuration, _one_version_tool = ctx.attr.oneversion.files_to_run if ctx.attr.oneversion else None, @@ -242,6 +244,7 @@ _java_toolchain = rule( "javac_supports_workers": attr.bool(default = True), "javac_supports_multiplex_workers": attr.bool(default = True), "javac_supports_worker_cancellation": attr.bool(default = True), + "javac_supports_worker_multiplex_sandboxing": attr.bool(default = False), "javacopts": attr.string_list(default = []), "jspecify_implicit_deps": attr.label(cfg = "exec", allow_single_file = True, executable = True), "jspecify_javacopts": attr.string_list(), diff --git a/src/test/shell/bazel/bazel_java_test.sh b/src/test/shell/bazel/bazel_java_test.sh index 83cd617fbc1b07..92db25e7c9fa02 100755 --- a/src/test/shell/bazel/bazel_java_test.sh +++ b/src/test/shell/bazel/bazel_java_test.sh @@ -1949,4 +1949,36 @@ EOF bazel build //pkg:a >& $TEST_log || fail "build failed" } +function test_sandboxed_multiplexing() { + if [[ "${JAVA_TOOLS_ZIP}" == released ]]; then + # TODO: Enable test after the next java_tools release. + return 0 + fi + + mkdir -p pkg + cat << 'EOF' > pkg/BUILD +load("@bazel_tools//tools/jdk:default_java_toolchain.bzl", "default_java_toolchain") +default_java_toolchain( + name = "java_toolchain", + source_version = "17", + target_version = "17", + javac_supports_worker_multiplex_sandboxing = True, +) +java_library(name = "a", srcs = ["A.java"], deps = [":b"]) +java_library(name = "b", srcs = ["B.java"]) +EOF + cat << 'EOF' > pkg/A.java +public class A extends B {} +EOF + cat << 'EOF' > pkg/B.java +public class B {} +EOF + + bazel build //pkg:a \ + --experimental_worker_multiplex_sandboxing \ + --java_language_version=17 \ + --extra_toolchains=//pkg:java_toolchain_definition \ + >& $TEST_log || fail "build failed" +} + run_suite "Java integration tests"