Use @argument files in the Java binary wrapper script #6354

cushon · 2018-10-10T22:49:46Z

The Java binary wrapper script works around command line length limits by creating a jar file with a Class-Path manifest entry containing the long classpath (related: #6289).

JDK 9 adds support for flag files using the @argument-file syntax, see "java Command-Line Argument Files" in https://docs.oracle.com/javase/9/tools/java.htm.

We should use that instead of the manifest jar hack when targeting JDK 9.

We could back-fill support for that syntax for older --javabase versions by adding logic to the wrapper script.

The text was updated successfully, but these errors were encountered:

petroseskinder · 2018-10-15T22:26:56Z

@cushon will this change resolve the long classpath issue documented in #6289?
Additionally, is there anything we can do in the ensuing time period to stop the bleeding?

We have ran unused_deps and have removed all obviously unnecessary dependencies.

cushon · 2018-10-15T23:18:46Z

(previous bug about @argfiles: #1392)

cushon · 2018-10-15T23:19:27Z

@petroseskinder I filed this as a reminder to look at @argfiles if/when it's worth adding features that only work for JDK > 9. I think we need a different short-term fix for #6289.

petroseskinder · 2018-10-17T14:58:54Z

I see. Thank you. Does the suggestion by @brown suffice for the short term (pre-JDK >9) fix?

cushon · 2018-10-17T18:08:33Z

I actually didn't realize there was a patch there because github mangled the formatting :/
I left a comment: #6289 (comment)

laszlocsomor · 2019-01-07T08:25:16Z

#6471 was pushed on 2018-10-29, and IIUC it fixed #6289. According to GitHub this patch is included in Bazel 0.20.0.

@petroseskinder : did this Bazel version "stop the bleeding" with older JDKs?
@cushon : with #6289 fixed, what's the motivation to resolve this bug, is it just to avoid writing a classpath jar if we can get away with writing an argument file?

cushon · 2019-01-07T18:25:26Z

what's the motivation to resolve this bug

I don't see any reason to prefer the current approach over @argument files, aside from supporting legacy JDK versions? @argument files are a standard solution to the same problem that probably performs better and won't need maintenance in the future.

ittaiz · 2022-07-13T14:23:13Z

Should this be fixed? Like @cushon I have a hunch (no data unfortunately) that using an argument file will perform better than a classpath jar

cushon · 2022-07-13T15:50:46Z

I think the main challenge is that we can only rely on this feature for JDK 9 and newer.

There was some related discussion prompted by https://github.com/bazelbuild/bazel/pull/15487/files

One option is to add some information to java_runtime that tells Bazel if it can rely on argument files, e.g.

java_runtime(..., supports_params_files = True)
java_runtime(..., feature_version = 9)
java_runtime(..., features = ["params_file"], ...)

ittaiz · 2022-07-14T11:02:21Z

I see. Thanks.
For better and for worse rules_scala has its own fork of java_stub_template so it can do this for itself.

ittaiz · 2022-07-14T12:14:33Z

@cushon isn't the version already accessible on the toolchain now? I forgot already what it contains but the Java version sounds like something that should be there and then in the template you "just" check if it's bigger than 8

ittaiz · 2022-07-14T12:16:04Z

One last thing (and really sorry for spamming) but from a few runs this is shaving for us 1s-1.5s internally and we have thousands of not more of test targets with large class paths (partially because we have fine grain build targets)

cushon · 2022-07-14T16:41:58Z

I forgot already what it contains but the Java version sounds like something that should be there

There isn't anything in java_runtime that reliably declares what version the toolchain is.

from a few runs this is shaving for us 1s-1.5s internally and we have thousands of not more of test targets with large class paths (partially because we have fine grain build targets)

Nice! This is definitely something I'd like to see happen, we just need a way to do it conditionally for newer JDKs, or wait for JDK 8 support to be dropped.

And I think for Google we're not seeing this be a bottleneck because we're not relying on the Class-Path jar logic, our JDK has a higher command line length limit.

ittaiz · 2022-07-15T06:01:06Z

Thanks!
We'll roll out the change today or on Sunday probably but this is a divergence I obviously would like to remove.
Btw, the measurement is on my M1. We still have many people on intel chips and my colleagues tell me that there this preparation takes ~4s on many targets.
Really interested in the full rollout 😁🤞🏽

ittaiz · 2022-11-10T07:19:22Z

@cushon do you know if there are any plans on dropping JDK 8 support?

1. As suggested in bazelbuild#6354 (comment), add a JDK version attribute to the java_runtime rule. This will allow changing behavior based on the Java runtime's version. 2. As an application of the above, pass -Djava.security.manager=allow to Java tests on JDK 17+. This makes the Bazel Java test runner to work on JDK 19. Fixes bazelbuild#14502. 3. To make the above generally useful, the remote_java_repository and local_java_repository must set the new version attribute in the java_runtime rules that they create. This means the old static jdk.BUILD file no longer suffices. Move the contents of jdk.BUILD into a Starlark file, so the version can be interpolated in by JDK repository rules and macros. (This isn't the nicest, but local_java_repository is not a repository rule, so the template cannot be in a non-Starlark file.)

1. As suggested in #6354 (comment), add a JDK version attribute to the java_runtime rule. This will allow changing behavior based on the Java runtime's version. 2. As an application of the above, pass -Djava.security.manager=allow to Java tests on JDK 17+. This makes the Bazel Java test runner to work on JDK 19. Fixes #14502. 3. To make the above generally useful, the remote_java_repository and local_java_repository must set the new version attribute in the java_runtime rules that they create. This means the old static jdk.BUILD file no longer suffices. Move the contents of jdk.BUILD into a Starlark file, so the version can be interpolated in by JDK repository rules and macros. (This isn't the nicest, but local_java_repository is not a repository rule, so the template cannot be in a non-Starlark file.) Closes #17775. PiperOrigin-RevId: 518860040 Change-Id: I8223b6407dd09528a4e5a6bf12354e5fc68278c6

1. As suggested in bazelbuild#6354 (comment), add a JDK version attribute to the java_runtime rule. This will allow changing behavior based on the Java runtime's version. 2. As an application of the above, pass -Djava.security.manager=allow to Java tests on JDK 17+. This makes the Bazel Java test runner to work on JDK 19. Fixes bazelbuild#14502. 3. To make the above generally useful, the remote_java_repository and local_java_repository must set the new version attribute in the java_runtime rules that they create. This means the old static jdk.BUILD file no longer suffices. Move the contents of jdk.BUILD into a Starlark file, so the version can be interpolated in by JDK repository rules and macros. (This isn't the nicest, but local_java_repository is not a repository rule, so the template cannot be in a non-Starlark file.) Closes bazelbuild#17775. PiperOrigin-RevId: 518860040 Change-Id: I8223b6407dd09528a4e5a6bf12354e5fc68278c6

1. As suggested in #6354 (comment), add a JDK version attribute to the java_runtime rule. This will allow changing behavior based on the Java runtime's version. 2. As an application of the above, pass -Djava.security.manager=allow to Java tests on JDK 17+. This makes the Bazel Java test runner to work on JDK 19. Fixes #14502. 3. To make the above generally useful, the remote_java_repository and local_java_repository must set the new version attribute in the java_runtime rules that they create. This means the old static jdk.BUILD file no longer suffices. Move the contents of jdk.BUILD into a Starlark file, so the version can be interpolated in by JDK repository rules and macros. (This isn't the nicest, but local_java_repository is not a repository rule, so the template cannot be in a non-Starlark file.) Closes #17775. PiperOrigin-RevId: 518860040 Change-Id: I8223b6407dd09528a4e5a6bf12354e5fc68278c6 (cherry picked from commit 7556e11)

@argument

As suggested in Use @argument files in the Java binary wrapper script #6354 (comment), add a JDK version attribute to the java_runtime rule. This will allow changing behavior based on the Java runtime's version. As an application of the above, pass -Djava.security.manager=allow to Java tests on JDK 17+. This makes the Bazel Java test runner to work on JDK 19. Fixes Bazel@HEAD on JDK 17: GoogleTestSecurityManager WARNING: System::setSecurityManager will be removed in a future release #14502. To make the above generally useful, the remote_java_repository and local_java_repository must set the new version attribute in the java_runtime rules that they create. This means the old static jdk.BUILD file no longer suffices. Move the contents of jdk.BUILD into a Starlark file, so the version can be interpolated in by JDK repository rules and macros. (This isn't the nicest, but local_java_repository is not a repository rule, so the template cannot be in a non-Starlark file.) Closes #17775. PiperOrigin-RevId: 518860040 Change-Id: I8223b6407dd09528a4e5a6bf12354e5fc68278c6

1. As suggested in #6354 (comment), add a JDK version attribute to the java_runtime rule. This will allow changing behavior based on the Java runtime's version. 2. As an application of the above, pass -Djava.security.manager=allow to Java tests on JDK 17+. This makes the Bazel Java test runner to work on JDK 19. Fixes #14502. 3. To make the above generally useful, the remote_java_repository and local_java_repository must set the new version attribute in the java_runtime rules that they create. This means the old static jdk.BUILD file no longer suffices. Move the contents of jdk.BUILD into a Starlark file, so the version can be interpolated in by JDK repository rules and macros. (This isn't the nicest, but local_java_repository is not a repository rule, so the template cannot be in a non-Starlark file.) Closes #17775. PiperOrigin-RevId: 518860040 Change-Id: I8223b6407dd09528a4e5a6bf12354e5fc68278c6 (cherry picked from commit 7556e11)

* Add version to JavaRuntimeInfo. 1. As suggested in #6354 (comment), add a JDK version attribute to the java_runtime rule. This will allow changing behavior based on the Java runtime's version. 2. As an application of the above, pass -Djava.security.manager=allow to Java tests on JDK 17+. This makes the Bazel Java test runner to work on JDK 19. Fixes #14502. 3. To make the above generally useful, the remote_java_repository and local_java_repository must set the new version attribute in the java_runtime rules that they create. This means the old static jdk.BUILD file no longer suffices. Move the contents of jdk.BUILD into a Starlark file, so the version can be interpolated in by JDK repository rules and macros. (This isn't the nicest, but local_java_repository is not a repository rule, so the template cannot be in a non-Starlark file.) Closes #17775. PiperOrigin-RevId: 518860040 Change-Id: I8223b6407dd09528a4e5a6bf12354e5fc68278c6 (cherry picked from commit 7556e11) * Use string.replace() instead of string.format() to avoid issues with select{} in the template string * Remove test and fix formatting * Fix starlark_repository_test Change the grep pattern to exactly match what the test is looking for. The old pattern would match BUILD file content from the jdk_build_file.bzl template --------- Co-authored-by: Benjamin Peterson <[email protected]>

1. As suggested in bazelbuild#6354 (comment), add a JDK version attribute to the java_runtime rule. This will allow changing behavior based on the Java runtime's version. 2. As an application of the above, pass -Djava.security.manager=allow to Java tests on JDK 17+. This makes the Bazel Java test runner to work on JDK 19. Fixes bazelbuild#14502. 3. To make the above generally useful, the remote_java_repository and local_java_repository must set the new version attribute in the java_runtime rules that they create. This means the old static jdk.BUILD file no longer suffices. Move the contents of jdk.BUILD into a Starlark file, so the version can be interpolated in by JDK repository rules and macros. (This isn't the nicest, but local_java_repository is not a repository rule, so the template cannot be in a non-Starlark file.) Closes bazelbuild#17775. PiperOrigin-RevId: 518860040 Change-Id: I8223b6407dd09528a4e5a6bf12354e5fc68278c6

github-actions · 2024-01-19T01:29:36Z

Thank you for contributing to the Bazel repository! This issue has been marked as stale since it has not had any activity in the last 1+ years. It will be closed in the next 90 days unless any other activity occurs. If you think this issue is still relevant and should stay open, please post any comment here and the issue will no longer be marked as stale.

cushon · 2024-01-19T01:33:31Z

If you think this issue is still relevant and should stay open, please post any comment here and the issue will no longer be marked as stale.

Please keep this one open for now

cushon added the team-Rules-Java Issues for Java rules label Oct 10, 2018

cushon mentioned this issue Oct 10, 2018

Support @argument files in the Java binary wrapper script #6355

Closed

irengrig added type: feature request untriaged labels Oct 11, 2018

cushon mentioned this issue Oct 15, 2018

Java binary with long classpath fails on read-only file system #6289

Closed

cushon mentioned this issue Dec 18, 2018

Tests which use java stub template with large classpaths fails because of local_jdk implicit dependency #6955

Closed

lberki added P3 We're not considering working on this, but happy to review a PR. (No assignee) P2 We'll consider working on this in future. (Assignee optional) and removed untriaged P3 We're not considering working on this, but happy to review a PR. (No assignee) labels Mar 14, 2019

comius added P3 We're not considering working on this, but happy to review a PR. (No assignee) and removed P2 We'll consider working on this in future. (Assignee optional) labels Nov 21, 2020

cushon mentioned this issue May 21, 2021

Classpath JAR is incorrect when running java_binary remotely on Windows #13484

Closed

cushon mentioned this issue Apr 22, 2022

Default to a UTF-8 locale in Java stub template #15159

Closed

ittaiz mentioned this issue Jul 14, 2022

java_stub_template to use argument file instead of classpath jar bazelbuild/rules_scala#1410

Merged

benjaminp mentioned this issue Mar 14, 2023

Add version to JavaRuntimeInfo. #17775

Closed

ShreeM01 mentioned this issue Mar 24, 2023

[6.2.0]Add version to JavaRuntimeInfo. #17881

Closed

ShreeM01 mentioned this issue Mar 28, 2023

[6.2.0]Add version to JavaRuntimeInfo. #17898

Closed

hvadehra mentioned this issue Mar 29, 2023

[6.2.0] Add version to JavaRuntimeInfo #17913

Merged

cushon mentioned this issue Jul 27, 2023

java_stub_template.txt leaves orphan Java process when terminated #19036

Open

cushon mentioned this issue Sep 12, 2023

Long runtime classpaths slow down unit test execution #19480

Closed

github-actions bot added the stale Issues or PRs that are stale (no activity for 30 days) label Jan 19, 2024

meteorcloudy added not stale Issues or PRs that are inactive but not considered stale and removed stale Issues or PRs that are stale (no activity for 30 days) labels Jan 19, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use @argument files in the Java binary wrapper script #6354

Use @argument files in the Java binary wrapper script #6354

cushon commented Oct 10, 2018 •

edited by lberki

Loading

petroseskinder commented Oct 15, 2018 •

edited

Loading

cushon commented Oct 15, 2018

cushon commented Oct 15, 2018

petroseskinder commented Oct 17, 2018

cushon commented Oct 17, 2018

laszlocsomor commented Jan 7, 2019 •

edited

Loading

cushon commented Jan 7, 2019 •

edited

Loading

ittaiz commented Jul 13, 2022

cushon commented Jul 13, 2022

ittaiz commented Jul 14, 2022

ittaiz commented Jul 14, 2022

ittaiz commented Jul 14, 2022

cushon commented Jul 14, 2022

ittaiz commented Jul 15, 2022

ittaiz commented Nov 10, 2022

github-actions bot commented Jan 19, 2024

cushon commented Jan 19, 2024

Use @argument files in the Java binary wrapper script #6354

Use @argument files in the Java binary wrapper script #6354

Comments

cushon commented Oct 10, 2018 • edited by lberki Loading

petroseskinder commented Oct 15, 2018 • edited Loading

cushon commented Oct 15, 2018

cushon commented Oct 15, 2018

petroseskinder commented Oct 17, 2018

cushon commented Oct 17, 2018

laszlocsomor commented Jan 7, 2019 • edited Loading

cushon commented Jan 7, 2019 • edited Loading

ittaiz commented Jul 13, 2022

cushon commented Jul 13, 2022

ittaiz commented Jul 14, 2022

ittaiz commented Jul 14, 2022

ittaiz commented Jul 14, 2022

cushon commented Jul 14, 2022

ittaiz commented Jul 15, 2022

ittaiz commented Nov 10, 2022

github-actions bot commented Jan 19, 2024

cushon commented Jan 19, 2024

cushon commented Oct 10, 2018 •

edited by lberki

Loading

petroseskinder commented Oct 15, 2018 •

edited

Loading

laszlocsomor commented Jan 7, 2019 •

edited

Loading

cushon commented Jan 7, 2019 •

edited

Loading