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 --experimental_java_classpath=bazel_no_fallback option #24876

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

rsalvador
Copy link
Contributor

This change introduces a new option, --experimental_java_classpath=bazel_no_fallback, which operates similarly to --experimental_java_classpath=bazel but does not fall back to the full transitive classpath. Instead, the build fails if javac cannot compile with the reduced classpath.

Fixes #24875

@rsalvador rsalvador requested review from lberki and a team as code owners January 9, 2025 09:29
@github-actions github-actions bot added team-Rules-Java Issues for Java rules awaiting-review PR is awaiting review from an assigned reviewer labels Jan 9, 2025
@rsalvador
Copy link
Contributor Author

test_build_hello_world_reduced_classpath_no_fallback in bazel_java_test.sh fails with:

java.lang.IllegalArgumentException: No enum constant com.google.devtools.build.buildjar.OptionsParser.ReduceClasspathMode.BAZEL_REDUCED_NO_FALLBACK
	at java.base/java.lang.Enum.valueOf(Enum.java:293)
	at com.google.devtools.build.buildjar.OptionsParser$ReduceClasspathMode.valueOf(OptionsParser.java:55)

does this mean that the change would need to be back-compatible with versions of java_tools that don't include the src/java_tools changes in this PR?.

@meisterT meisterT requested a review from hvadehra January 9, 2025 12:40
Copy link
Contributor

@cushon cushon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left some notes in the bug

classpathLine.add("--reduce_classpath_mode", fallback ? "BAZEL_FALLBACK" : "BAZEL_REDUCED");
classpathLine.add("--reduce_classpath_mode",
fallback ? "BAZEL_FALLBACK"
: (classpathMode == JavaClasspathMode.BAZEL ? "BAZEL_REDUCED" : "BAZEL_REDUCED_NO_FALLBACK"));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of adding the new --reduce_classpath_mode=BAZEL_REDUCED_NO_FALLBACK mode, did you consider having Bazel invoke JavaBuilder with a reduced classpath and pass --reduce_classpath_mode=NONE, and then handle not doing fallback on the Bazel side? I think that would provide equivalent behaviour without JavaBuilder changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thx, that's much better. Pushed the change.

There is still 1 test failure, but seems unrelated to the change:

test_size_less_than_385MB FAILED: terminated because this command returned a non-zero status:
/private/var/tmp/_bazel_buildkite/1198ee8566c303641c6d358da6949c16/sandbox/darwin-sandbox/4799/execroot/_main/bazel-out/darwin_arm64-fastbuild/bin/src/test/shell/integration/minimal_jdk_test.runfiles/_main/src/test/shell/integration/minimal_jdk_test:50: in call to test_size_less_than_385MB

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting-review PR is awaiting review from an assigned reviewer team-Rules-Java Issues for Java rules
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot fix or prevent javac fallbacks when using --experimental_java_classpath=bazel
2 participants