Attempt to fix missing class error by not using tree artifacts for bootclasspath (#340) A few times a year across many thousands of builds we encounter a rare error about the `DumpPlatformClassPath` class being missing. Our Bazel setup uses dynamic execution, builds without the bytes, remote execution + remote caching, and path mapping. The error we encounter is as follows: ``` Error: Could not find or load main class DumpPlatformClassPath Caused by: java.lang.ClassNotFoundException: DumpPlatformClassPath ``` I'm guessing that this is happening due to some kind of Bazel bug that happens with our Bazel setup and tree artifacts, i.e., declare_directory. Best I can tell this is happening because the `DumpPlatformClassPath.class` file is somehow not materializing correctly. I'm not 100% confident about that, but it's my leading hypothesis at this point in time. This commit changes the actions in `bootclasspath.bzl` to not rely on tree artifacts. Instead, they rely JDK 11+'s ability to launch single-file programs (introduced in JEP 330). This avoids the `javac` action and `declare_directory` previously required to compile `DumpPlatformClassPath`. Problem is this makes rules_java not compatible with JDK's older than 11. I'm very open to alternative solutions to this, but I haven't yet come up with a robust, cross platform solution that avoids tree artifacts while also maintaining compatibility with JDKs older than 11. I wanted to open this PR to get some discussion going. Closes #340 COPYBARA_INTEGRATE_REVIEW=https://github.com/bazelbuild/rules_java/pull/340 from lucidsoftware:bootclasspath-avoid-tree-artifact 34809e22c0022d4bb72088c0b1cfc864ea0cfb31 PiperOrigin-RevId: 880851895 Change-Id: I04c7142981a22b9f4995f25e2789ac6aa2aaf097
diff --git a/test/check_remote_java_tools_configs.sh b/test/check_remote_java_tools_configs.sh index 7e69dc0..957aa78 100755 --- a/test/check_remote_java_tools_configs.sh +++ b/test/check_remote_java_tools_configs.sh
@@ -22,7 +22,11 @@ TMP_FILE=$(mktemp -q /tmp/remotejavatools.XXXXXX) echo "fetching $name from $url to ${TMP_FILE}" curl --silent -o ${TMP_FILE} -L "$url" - actual_hash=`sha256sum ${TMP_FILE} | cut -d' ' -f1` + if command -v sha256sum &> /dev/null; then + actual_hash=`sha256sum ${TMP_FILE} | cut -d' ' -f1` + else + actual_hash=`shasum -a 256 ${TMP_FILE} | cut -d' ' -f1` + fi if [ "${hash}" != "${actual_hash}" ]; then echo "ERROR: wrong hash for ${name}! wanted: ${hash}, got: ${actual_hash}" exit 1
diff --git a/test/toolchains/bootclasspath_tests.bzl b/test/toolchains/bootclasspath_tests.bzl index e7ad1ca..15ccd4f 100644 --- a/test/toolchains/bootclasspath_tests.bzl +++ b/test/toolchains/bootclasspath_tests.bzl
@@ -68,6 +68,53 @@ system_path = target[java_common.BootClassPathInfo]._system_path env.expect.that_str(system_path).contains("local_jdk") +def _test_jdk8_uses_tree_artifact(name): + analysis_test( + name = name, + impl = _test_jdk8_uses_tree_artifact_impl, + target = Label("//toolchains:platformclasspath"), + config_settings = { + "//command_line_option:tool_java_runtime_version": "remotejdk_8", + }, + ) + +def _test_jdk8_uses_tree_artifact_impl(env, target): + env.expect.that_target(target).action_named( + "JavaToolchainCompileClasses", + ).argv().contains_at_least([ + "-d", + "{bindir}/{package}/{name}_classes", + "toolchains/DumpPlatformClassPath.java", + ]).in_order() + env.expect.that_target(target).action_named( + "JavaToolchainCompileBootClasspath", + ).argv().contains_at_least([ + "--add-exports=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED", + "-cp", + "{bindir}/{package}/{name}_classes", + "DumpPlatformClassPath", + "{bindir}/{package}/{name}_unstripped.jar", + ]).in_order() + +def _test_jdk11_uses_source_launcher(name): + analysis_test( + name = name, + impl = _test_jdk11_uses_source_launcher_impl, + target = Label("//toolchains:platformclasspath"), + config_settings = { + "//command_line_option:tool_java_runtime_version": "remotejdk_11", + }, + ) + +def _test_jdk11_uses_source_launcher_impl(env, target): + env.expect.that_target(target).action_named( + "JavaToolchainCompileBootClasspath", + ).argv().contains_at_least([ + "--add-exports=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED", + "toolchains/DumpPlatformClassPath.java", + "{bindir}/{package}/{name}_unstripped.jar", + ]).in_order() + def bootclasspath_tests(name): test_suite( name = name, @@ -76,5 +123,7 @@ _test_incompatible_language_version_bootclasspath_disabled, _test_incompatible_language_version_bootclasspath_enabled_versioned, _test_incompatible_language_version_bootclasspath_enabled_unversioned, + _test_jdk8_uses_tree_artifact, + _test_jdk11_uses_source_launcher, ], )
diff --git a/toolchains/bootclasspath.bzl b/toolchains/bootclasspath.bzl index 93da5de..256c032 100644 --- a/toolchains/bootclasspath.bzl +++ b/toolchains/bootclasspath.bzl
@@ -129,29 +129,35 @@ exec_javabase = ctx.attr.java_runtime_alias[java_common.JavaRuntimeInfo] env = ctx.attr._utf8_environment[Utf8EnvironmentInfo].environment - class_dir = ctx.actions.declare_directory("%s_classes" % ctx.label.name) + # If possible, use JDK 11+'s ability to run a single Java file to avoid a + # separate action to compile DumpPlatformClassPath. + use_source_launcher = exec_javabase.version >= 11 - args = ctx.actions.args() - args.add("-source") - args.add("8") - args.add("-target") - args.add("8") - args.add("-Xlint:-options") - args.add("-J-XX:-UsePerfData") - args.add("-d") - args.add_all([class_dir], expand_directories = False) - args.add(ctx.file.src) + class_dir = None + if not use_source_launcher: + class_dir = ctx.actions.declare_directory("%s_classes" % ctx.label.name) - ctx.actions.run( - executable = "%s/bin/javac" % exec_javabase.java_home, - mnemonic = "JavaToolchainCompileClasses", - inputs = [ctx.file.src] + ctx.files.java_runtime_alias, - outputs = [class_dir], - arguments = [args], - env = env, - execution_requirements = _SUPPORTS_PATH_MAPPING, - use_default_shell_env = True, - ) + args = ctx.actions.args() + args.add("-source") + args.add("8") + args.add("-target") + args.add("8") + args.add("-Xlint:-options") + args.add("-J-XX:-UsePerfData") + args.add("-d") + args.add_all([class_dir], expand_directories = False) + args.add(ctx.file.src) + + ctx.actions.run( + executable = "%s/bin/javac" % exec_javabase.java_home, + mnemonic = "JavaToolchainCompileClasses", + inputs = [ctx.file.src] + ctx.files.java_runtime_alias, + outputs = [class_dir], + arguments = [args], + env = env, + execution_requirements = _SUPPORTS_PATH_MAPPING, + use_default_shell_env = True, + ) unstripped_bootclasspath = ctx.actions.declare_file("%s_unstripped.jar" % ctx.label.name) @@ -161,8 +167,13 @@ args.add("--add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED") args.add("--add-exports=jdk.compiler/com.sun.tools.javac.platform=ALL-UNNAMED") args.add("--add-exports=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED") - args.add_all("-cp", [class_dir], expand_directories = False) - args.add("DumpPlatformClassPath") + + if use_source_launcher: + args.add(ctx.file.src) + else: + args.add_all("-cp", [class_dir], expand_directories = False) + args.add("DumpPlatformClassPath") + args.add(unstripped_bootclasspath) if ctx.attr.language_version_bootstrap_runtime: @@ -211,7 +222,8 @@ if len(system) != len(system_files): system = None - inputs = depset([class_dir] + ctx.files.java_runtime_alias, transitive = [any_javabase.files]) + classpath_input = ctx.file.src if use_source_launcher else class_dir + inputs = depset([classpath_input] + ctx.files.java_runtime_alias, transitive = [any_javabase.files]) ctx.actions.run( executable = str(exec_javabase.java_executable_exec_path), mnemonic = "JavaToolchainCompileBootClasspath",