Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add multiplex sandboxing support for Java actions #21091

Closed
DavidANeil opened this issue Jan 26, 2024 · 15 comments
Closed

Add multiplex sandboxing support for Java actions #21091

DavidANeil opened this issue Jan 26, 2024 · 15 comments
Assignees
Labels
P2 We'll consider working on this in future. (Assignee optional) team-Configurability platforms, toolchains, cquery, select(), config transitions type: bug

Comments

@DavidANeil
Copy link
Contributor

DavidANeil commented Jan 26, 2024

Description of the bug:

ERROR: /home/davidneil/.cache/bazel/_bazel_davidneil/d03a34112016843f05fb897e1bc2a558/external/rules_jvm_external~5.2/private/tools/java/com/github/bazelbuild/rules_jvm_external/jar/BUILD:3:12: Building external/rules_jvm_external~5.2/private/tools/java/com/github/bazelbuild/rules_jvm_external/jar/AddJarManifestEntry.jar (1 source file) [for tool] failed: (Exit 1): java failed: error executing Javac command (from target @@rules_jvm_external~5.2//private/tools/java/com/github/bazelbuild/rules_jvm_external/jar:AddJarManifestEntry) external/rules_java~7.3.2~toolchains~remotejdk21_linux/bin/java '--add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED' '--add-exports=jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED' ... (remaining 19 arguments skipped)
at com.google.devtools.build.buildjar.SimpleJavaLibraryBuilder.compileJavaLibrary(SimpleJavaLibraryBuilder.java:110)
        at com.google.devtools.build.buildjar.SimpleJavaLibraryBuilder.run(SimpleJavaLibraryBuilder.java:118)
        at com.google.devtools.build.buildjar.BazelJavaBuilder.build(BazelJavaBuilder.java:105)
        at com.google.devtools.build.buildjar.BazelJavaBuilder.parseAndBuild(BazelJavaBuilder.java:85)
        at com.google.devtools.build.lib.worker.WorkRequestHandler$WorkRequestHandlerBuilder.lambda$new$0(WorkRequestHandler.java:287)
        at com.google.devtools.build.lib.worker.WorkRequestHandler$WorkRequestCallback.apply(WorkRequestHandler.java:252)
        at com.google.devtools.build.lib.worker.WorkRequestHandler.respondToRequest(WorkRequestHandler.java:480)
        at com.google.devtools.build.lib.worker.WorkRequestHandler.lambda$startResponseThread$1(WorkRequestHandler.java:433)
        at java.base/java.lang.Thread.run(Thread.java:1583)
Caused by: java.nio.file.NoSuchFileException: bazel-out/cfg/bin/external/rules_jvm_external~5.2/private/tools/java/com/github/bazelbuild/rules_jvm_external/zip/libzip-hjar.jdeps
        at java.base/sun.nio.fs.UnixException.translateToIOException(UnixException.java:92)
        at java.base/sun.nio.fs.UnixException.rethrowAsIOException(UnixException.java:106)
        at java.base/sun.nio.fs.UnixException.rethrowAsIOException(UnixException.java:111)
        at java.base/sun.nio.fs.UnixFileSystemProvider.newByteChannel(UnixFileSystemProvider.java:261)
        at java.base/java.nio.file.Files.newByteChannel(Files.java:379)
        at java.base/java.nio.file.Files.newByteChannel(Files.java:431)
        at java.base/java.nio.file.spi.FileSystemProvider.newInputStream(FileSystemProvider.java:420)
        at java.base/java.nio.file.Files.newInputStream(Files.java:159)
        at com.google.devtools.build.buildjar.javac.plugins.dependency.DependencyModule.collectDependenciesFromArtifact(DependencyModule.java:277)
        ... 11 more
ERROR: /home/davidneil/.cache/bazel/_bazel_davidneil/d03a34112016843f05fb897e1bc2a558/external/protobuf~21.7/java/core/BUILD.bazel:146:13: Building external/protobuf~21.7/java/core/liblite_runtime_only.jar (91 source files) failed: (Exit 1): java failed: error executing Javac command (from target @@protobuf~21.7//java/core:lite_runtime_only) external/rules_java~7.3.2~toolchains~remotejdk21_linux/bin/java '--add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED' '--add-exports=jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED' ... (remaining 19 arguments skipped)
java.io.IOException: Cannot clean 'bazel-out/cfg/bin/external/protobuf~21.7/java/core/_javac/lite_runtime_only/liblite_runtime_only_classes'
        at com.google.devtools.build.buildjar.SimpleJavaLibraryBuilder.cleanupDirectory(SimpleJavaLibraryBuilder.java:80)
        at com.google.devtools.build.buildjar.SimpleJavaLibraryBuilder.prepareSourceCompilation(SimpleJavaLibraryBuilder.java:63)
        at com.google.devtools.build.buildjar.SimpleJavaLibraryBuilder.compileJavaLibrary(SimpleJavaLibraryBuilder.java:106)
        at com.google.devtools.build.buildjar.SimpleJavaLibraryBuilder.run(SimpleJavaLibraryBuilder.java:118)
        at com.google.devtools.build.buildjar.BazelJavaBuilder.build(BazelJavaBuilder.java:105)
        at com.google.devtools.build.buildjar.BazelJavaBuilder.parseAndBuild(BazelJavaBuilder.java:85)
        at com.google.devtools.build.lib.worker.WorkRequestHandler$WorkRequestHandlerBuilder.lambda$new$0(WorkRequestHandler.java:287)
        at com.google.devtools.build.lib.worker.WorkRequestHandler$WorkRequestCallback.apply(WorkRequestHandler.java:252)
        at com.google.devtools.build.lib.worker.WorkRequestHandler.respondToRequest(WorkRequestHandler.java:480)
        at com.google.devtools.build.lib.worker.WorkRequestHandler.lambda$startResponseThread$1(WorkRequestHandler.java:433)
        at java.base/java.lang.Thread.run(Thread.java:1583)
Caused by: java.nio.file.FileSystemException: bazel-out/cfg/bin/external/protobuf~21.7/java/core/_javac/lite_runtime_only/liblite_runtime_only_classes: failed to delete one or more files; see suppressed exceptions for details
        at com.google.common.io.MoreFiles.throwDeleteFailed(MoreFiles.java:803)
        at com.google.common.io.MoreFiles.deleteRecursively(MoreFiles.java:554)
        at com.google.devtools.build.buildjar.SimpleJavaLibraryBuilder.cleanupDirectory(SimpleJavaLibraryBuilder.java:78)
        ... 10 more
        Suppressed: java.nio.file.NoSuchFileException: liblite_runtime_only_classes
                at java.base/sun.nio.fs.UnixException.translateToIOException(UnixException.java:92)
                at java.base/sun.nio.fs.UnixException.rethrowAsIOException(UnixException.java:106)
                at java.base/sun.nio.fs.UnixException.rethrowAsIOException(UnixException.java:111)
                at java.base/sun.nio.fs.UnixSecureDirectoryStream.implDelete(UnixSecureDirectoryStream.java:210)
                at java.base/sun.nio.fs.UnixSecureDirectoryStream.deleteDirectory(UnixSecureDirectoryStream.java:224)
                at java.base/sun.nio.fs.UnixSecureDirectoryStream.deleteDirectory(UnixSecureDirectoryStream.java:42)
                at com.google.common.io.MoreFiles.deleteRecursivelySecure(MoreFiles.java:630)
                at com.google.common.io.MoreFiles.deleteRecursively(MoreFiles.java:531)
                ... 11 more
java.io.IOException: error reading deps artifact: bazel-out/cfg/bin/external/rules_jvm_external~5.2/private/tools/java/com/github/bazelbuild/rules_jvm_external/zip/libzip-hjar.jdeps
        at com.google.devtools.build.buildjar.javac.plugins.dependency.DependencyModule.collectDependenciesFromArtifact(DependencyModule.java:291)
        at com.google.devtools.build.buildjar.javac.plugins.dependency.DependencyModule.computeStrictClasspath(DependencyModule.java:257)
        at com.google.devtools.build.buildjar.ReducedClasspathJavaLibraryBuilder.compileSources(ReducedClasspathJavaLibraryBuilder.java:52)

Which category does this issue belong to?

Java Rules

What's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

I can reproduce this in the bazel repository with

bazel build --experimental_output_paths=strip //src:bazel
$ java --version
openjdk 11.0.22 2024-01-16 LTS
OpenJDK Runtime Environment Corretto-11.0.22.7.1 (build 11.0.22+7-LTS)
OpenJDK 64-Bit Server VM Corretto-11.0.22.7.1 (build 11.0.22+7-LTS, mixed mode)

What is the output of bazel info release?

release 7.0.0

@fmeum
Copy link
Collaborator

fmeum commented Jan 26, 2024

bazel build --experimental_output_paths=strip //src:bazel passes for me with #19872. Could you give that a try?

@fmeum fmeum added team-Configurability platforms, toolchains, cquery, select(), config transitions P2 We'll consider working on this in future. (Assignee optional) and removed team-Rules-Java Issues for Java rules untriaged labels Jan 26, 2024
@fmeum fmeum self-assigned this Jan 26, 2024
@DavidANeil
Copy link
Contributor Author

That did not work for me.
I also tried rebasing it on master, and it failed with the same error as originally.

Not sure if this is important, but the first time it fails, it doesn't print an error message, only these:

ERROR: /home/davidneil/github/bazel/src/main/java/com/google/devtools/build/lib/analysis/BUILD:572:13: output 'src/main/java/com/google/devtools/build/lib/analysis/libbuild_info.jar' was not created
ERROR: /home/davidneil/github/bazel/src/main/java/com/google/devtools/build/lib/analysis/BUILD:572:13: output 'src/main/java/com/google/devtools/build/lib/analysis/libbuild_info.jdeps' was not created
ERROR: /home/davidneil/github/bazel/src/main/java/com/google/devtools/build/lib/analysis/BUILD:572:13: output 'src/main/java/com/google/devtools/build/lib/analysis/libbuild_info-native-header.jar' was not created
ERROR: /home/davidneil/github/bazel/src/main/java/com/google/devtools/build/lib/analysis/BUILD:572:13: output 'src/main/java/com/google/devtools/build/lib/analysis/libbuild_info.jar_manifest_proto' was not created
ERROR: /home/davidneil/github/bazel/src/main/java/com/google/devtools/build/lib/analysis/BUILD:572:13: Building src/main/java/com/google/devtools/build/lib/analysis/libbuild_info.jar (1 source file) failed: not all outputs were created or valid

And then on a re-run it prints a message like the one in the original post.

@fmeum
Copy link
Collaborator

fmeum commented Jan 27, 2024

@DavidANeil Sorry, I think my instructions may not have been clear: Did you build Bazel from that PR and then used that bazel to run the command? That's what I meant, just checking out the PR and running the build with the latest Bazel release wouldn't work.

@DavidANeil
Copy link
Contributor Author

Here are the commands I ran:

git checkout 0c97e298603f51f02affc6fe0e5931c9fef5fab3
bazel build //src:bazel
bazel-bin/src/bazel --nohome_rc build --experimental_output_paths=strip //src:bazel

git pull --rebase origin master
bazel build //src:bazel
bazel-bin/src/bazel --nohome_rc build --experimental_output_paths=strip //src:bazel

bazel-io pushed a commit to bazel-io/bazel that referenced this issue Feb 6, 2024
…ization

`JavaHeaderCompileAction` can apply an optimization where it compiles the source files only against direct dependencies, making use of the fact that Turbine includes sufficient information about transitive dependencies into the direct header jars in a special directory.

In order to ensure that downstream consumers of header compilation action can use its `.jdeps` file regardless of which of these actions actually uses path mapping, all such paths to transitive jars have to be unmapped.

With this commit, actions can declare additional artifacts whose paths they want to apply path mapping to. By always passing all transitive jars into the path mapper, even when only the direct jars are inputs to the action, the transitive header jar paths can be unmapped and stripped path collisions between them correctly result in a noop `PathMapper` being used for the current action.

Fixes bazelbuild#21091

Closes bazelbuild#19872.

PiperOrigin-RevId: 604772306
Change-Id: I21d25785b2e909aace8554251f41b1012185138a
@fmeum
Copy link
Collaborator

fmeum commented Feb 6, 2024

Reopening, I accidentally left the issue reference in the PR description even after you confirmed the error. Something seems to have regressed on master, I will investigate further.

@fmeum fmeum reopened this Feb 6, 2024
@fmeum
Copy link
Collaborator

fmeum commented Feb 6, 2024

@DavidANeil Nvm, I forgot to mention that path mapping doesn't support the unsandboxed multiplex worker mode used by Java actions (see https://youtu.be/Et1rjb7ixUU?t=1300). Could you try again with --worker_sandboxing --noexperimental_worker_multiplex?

@DavidANeil
Copy link
Contributor Author

git checkout 0c97e298603f51f02affc6fe0e5931c9fef5fab3
bazel build //src:bazel
bazel-bin/src/bazel --nohome_rc build --experimental_output_paths=strip --worker_sandboxing --noexperimental_worker_multiplex //src:bazel

Seems to build correctly.

We use multiplexing for our Scala compiles, so I can't just disable that, would it make sense to re-title this issue, or open a new one, or is there an existing issue for tracking making the two features compatible?

@fmeum
Copy link
Collaborator

fmeum commented Feb 7, 2024

What I know is #19719 (comment). Feel free to update the title.

@hvadehra Has the internal situation regarding sandboxed multiplexed Javac workers changed? Would you accept a PR along the lines of https://bazel-review.googlesource.com/c/bazel/+/179090 that adds support, possibly optional and/or for Bazel only?

@hvadehra
Copy link
Member

hvadehra commented Feb 8, 2024

@wilwell I think you took this over from Lars?

github-merge-queue bot pushed a commit that referenced this issue Feb 8, 2024
…th optimization (#21227)

`JavaHeaderCompileAction` can apply an optimization where it compiles
the source files only against direct dependencies, making use of the
fact that Turbine includes sufficient information about transitive
dependencies into the direct header jars in a special directory.

In order to ensure that downstream consumers of header compilation
action can use its `.jdeps` file regardless of which of these actions
actually uses path mapping, all such paths to transitive jars have to be
unmapped.

With this commit, actions can declare additional artifacts whose paths
they want to apply path mapping to. By always passing all transitive
jars into the path mapper, even when only the direct jars are inputs to
the action, the transitive header jar paths can be unmapped and stripped
path collisions between them correctly result in a noop `PathMapper`
being used for the current action.

Fixes #21091

Closes #19872.

Commit
fd196bf

PiperOrigin-RevId: 604772306
Change-Id: I21d25785b2e909aace8554251f41b1012185138a

Co-authored-by: Fabian Meumertzheim <fabian@meumertzhe.im>
@wilwell
Copy link

wilwell commented Feb 13, 2024

Hi @fmeum!

Thanks for mentioning of this CL. Right now Lars is not working with Javac workers, so probably I am the POC for this CL.

We have observed issues internally with enabling of Javac sandboxed multiplex workers. The largest issue was connected with Java plugins. In general we are not ready to push this change. Even pushing only in Bazel means that we need to support this feature, but it's not well tested and we are not sure about its behaviour.

To end on a good note I want to say, that we are going to spend time for enabling Javac sandboxed multiplex workers this year, because we see large benefits from using multiplex workers, but probably it's gonna happen in second half of the year.

@fmeum
Copy link
Collaborator

fmeum commented Feb 13, 2024

@wilwell Thanks for the update! Glad to hear that this is at least on the roadmap.

In the meantime, what do you think of merging just the JavaBuilder and worker part of the CL but not the changes to the Java toolchain setup that set the execution requirement? This way the feature wouldn't be enabled by default in Bazel, but interested users could already try it out via --modify_execution_info and start fixing their plugins.

@cushon
Copy link
Contributor

cushon commented Feb 14, 2024

If it would be helpful to merge just the JavaBuilder parts of that, I can help with the reviews for that if you want to pick it up.

Is it basically just the OptionsParser changes to handle making all of the paths relative to the sandbox directory? I wonder if we could do more simply in JavaLibraryBuildRequest, maybe around here:

private static ImmutableList<Path> asPaths(Collection<String> paths) {
return paths.stream().map(Paths::get).collect(toImmutableList());
}
@Nullable
private static Path asPath(@Nullable String path) {
return path != null ? Paths.get(path) : null;
}

@iancha1992
Copy link
Member

iancha1992 commented Feb 22, 2024

@fmeum I believe this is fixed now in #21401? Should I close this issue?

@fmeum fmeum closed this as completed Feb 22, 2024
@fmeum fmeum reopened this Feb 22, 2024
@fmeum
Copy link
Collaborator

fmeum commented Feb 22, 2024

@iancha1992 This is actually a separate issue. Could you change the title to Add multiplex sandboxing support for Java actions?

@DavidANeil DavidANeil changed the title --experimental_output_paths=strip fails for some Java actions --experimental_output_paths=strip fails for multiplex sandboxed Java actions Feb 22, 2024
@iancha1992 iancha1992 changed the title --experimental_output_paths=strip fails for multiplex sandboxed Java actions Add multiplex sandboxing support for Java actions Feb 23, 2024
copybara-service bot pushed a commit that referenced this issue Mar 6, 2024
This does not include adding support to Java actions yet.

Work towards #21091

Closes #21370.

PiperOrigin-RevId: 613231195
Change-Id: I87e76d2cebed3075a1eedfd8445b910ebd7604b9
fmeum added a commit to fmeum/bazel that referenced this issue Mar 6, 2024
This does not include adding support to Java actions yet.

Work towards bazelbuild#21091

Closes bazelbuild#21370.

PiperOrigin-RevId: 613231195
Change-Id: I87e76d2cebed3075a1eedfd8445b910ebd7604b9
github-merge-queue bot pushed a commit that referenced this issue Mar 6, 2024
This does not include adding support to Java actions yet.

Work towards #21091

Closes #21370.

PiperOrigin-RevId: 613231195
Change-Id: I87e76d2cebed3075a1eedfd8445b910ebd7604b9

Closes #21518
fmeum added a commit to fmeum/bazel that referenced this issue Mar 27, 2024
Javac actions gained support for multiplex sandboxing with
rules_java 7.5.0, which makes it possible to test path mapping in this
mode.

Fixes bazelbuild#21091
bazel-io pushed a commit to bazel-io/bazel that referenced this issue Jul 29, 2024
Javac actions gained support for multiplex sandboxing with rules_java 7.5.0, which makes it possible to test path mapping in this mode.

Fixes bazelbuild#21091

Closes bazelbuild#21837.

PiperOrigin-RevId: 657111790
Change-Id: Ib08f2c0bc48d210cce013e328da0e871f73278aa
github-merge-queue bot pushed a commit that referenced this issue Jul 29, 2024
Javac actions gained support for multiplex sandboxing with rules_java
7.5.0, which makes it possible to test path mapping in this mode.

Fixes #21091

Closes #21837.

PiperOrigin-RevId: 657111790
Change-Id: Ib08f2c0bc48d210cce013e328da0e871f73278aa

Commit
8e79fe0

Co-authored-by: Fabian Meumertzheim <fabian@meumertzhe.im>
@iancha1992
Copy link
Member

A fix for this issue has been included in Bazel 7.3.0 RC1. Please test out the release candidate and report any issues as soon as possible.
If you're using Bazelisk, you can point to the latest RC by setting USE_BAZEL_VERSION=7.3.0rc1. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2 We'll consider working on this in future. (Assignee optional) team-Configurability platforms, toolchains, cquery, select(), config transitions type: bug
Projects
None yet
8 participants