From 90e2ea6ed27d15cf4e691e9aa0b4b90e0dfa511f Mon Sep 17 00:00:00 2001 From: JonPez2 Date: Fri, 28 Jun 2024 09:00:03 +0100 Subject: [PATCH 1/2] Don't restrict the 'java' attr to a single file. This means that developers can create a java_runtime from a build target by constructing an appropriately laid-out jdk in a custom rule, and then point to that rule in the java attr. --- .../builtins_bzl/common/java/java_runtime.bzl | 3 +- src/test/shell/integration/bazel_java_test.sh | 91 ++++++++++++++++++- 2 files changed, 90 insertions(+), 4 deletions(-) diff --git a/src/main/starlark/builtins_bzl/common/java/java_runtime.bzl b/src/main/starlark/builtins_bzl/common/java/java_runtime.bzl index 7b9e509a8558db..78c548ec81874b 100644 --- a/src/main/starlark/builtins_bzl/common/java/java_runtime.bzl +++ b/src/main/starlark/builtins_bzl/common/java/java_runtime.bzl @@ -100,7 +100,7 @@ def _java_runtime_rule_impl(ctx): java_binary_exec_path = paths.get_relative(java_home, _get_bin_java(ctx)) java_binary_runfiles_path = _get_runfiles_java_executable(ctx, java_home, ctx.label) - java = ctx.file.java + java = ctx.executable.java.path if java: if paths.is_absolute(java_home): fail("'java_home' with an absolute path requires 'java' to be empty.") @@ -204,7 +204,6 @@ The libraries that are statically linked with the launcher for hermetic deployme """, ), "java": attr.label( - allow_single_file = True, executable = True, cfg = "target", doc = """ diff --git a/src/test/shell/integration/bazel_java_test.sh b/src/test/shell/integration/bazel_java_test.sh index c72e2b58fa8e4f..f8d59accdd0eff 100755 --- a/src/test/shell/integration/bazel_java_test.sh +++ b/src/test/shell/integration/bazel_java_test.sh @@ -336,8 +336,8 @@ EOF # Test the genrule that specifically depends on :bar_runtime. bazel cquery --max_config_changes_to_show=0 --implicit_deps 'deps(//:with_java)' >& $TEST_log - expect_not_log "foo" - expect_log "bar" + expect_log "dummy_java" + expect_log "dummy_javac" expect_not_log "embedded_jdk" expect_not_log "remotejdk_" expect_not_log "remotejdk11_" @@ -353,4 +353,91 @@ EOF expect_not_log "remotejdk11_" } + +function test_fully_generated_jdk() { + cat << EOF >> WORKSPACE +load("@bazel_tools//tools/jdk:local_java_repository.bzl", "local_java_repository") +local_java_repository( + name = "foo_javabase", + java_home = "$PWD/foo", + version = "11", +) +EOF + + mkdir -p foo/bin bar/bin + + cat << EOF > defs.bzl + +def _make_jdk_impl(ctx): + dummy_java = ctx.actions.declare_file(ctx.attr.name + "_dummy_java") + ctx.actions.write(dummy_java, "dummy_java", is_executable = True) + dummy_javac = ctx.actions.declare_file(ctx.attr.name + "_dummy_javac") + ctx.actions.write(dummy_javac, "dummy_javac", is_executable = True) + + return [DefaultInfo( + executable = dummy_java, + files = depset([dummy_java, dummy_javac]) + )] + +make_jdk = rule(implementation = _make_jdk_impl, executable = True) + +EOF + + cat << EOF > BUILD +load(":defs.bzl", "make_jdk") + +make_jdk(name = "generated_jdk") + +java_runtime( + name = "bar_runtime", + visibility = ["//visibility:public"], + executable = "generated_jdk", +) + +genrule( + name = "without_java", + srcs = ["in"], + outs = ["out_without"], + cmd = "cat \$(SRCS) > \$(OUTS)", +) + +genrule( + name = "with_java", + srcs = ["in"], + outs = ["out_with"], + cmd = "echo \$(JAVA) > \$(OUTS)", + toolchains = [":bar_runtime"], +) +EOF + + # Use --max_config_changes_to_show=0, as changed option names may otherwise + # erroneously match the expected regexes. + + # Test the genrule with no java dependencies. + bazel cquery --max_config_changes_to_show=0 --implicit_deps 'deps(//:without_java)' >& $TEST_log + expect_not_log "dummy_java" + expect_not_log "dummy_javac" + expect_not_log "embedded_jdk" + expect_not_log "remotejdk_" + expect_not_log "remotejdk11_" + + # Test the genrule that specifically depends on :bar_runtime. + bazel cquery --max_config_changes_to_show=0 --implicit_deps 'deps(//:with_java)' >& $TEST_log + expect_log "dummy_java" + expect_log "dummy_javac" + expect_not_log "embedded_jdk" + expect_not_log "remotejdk_" + expect_not_log "remotejdk11_" + + # Setting the javabase should not change the use of :bar_runtime from the + # roolchains attribute. + bazel cquery --max_config_changes_to_show=0 --implicit_deps 'deps(//:with_java)' \ + --tool_java_runtime_version=foo_javabase >& $TEST_log + expect_log "dummy_java" + expect_log "dummy_javac" + expect_not_log "embedded_jdk" + expect_not_log "remotejdk_" + expect_not_log "remotejdk11_" +} + run_suite "Tests of specifying custom server_javabase/host_javabase and javabase." From c5f372015d98b82b1c8399083d6103f77f3651b2 Mon Sep 17 00:00:00 2001 From: JonPez2 Date: Fri, 28 Jun 2024 09:09:19 +0100 Subject: [PATCH 2/2] Bugfix --- src/main/starlark/builtins_bzl/common/java/java_runtime.bzl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/starlark/builtins_bzl/common/java/java_runtime.bzl b/src/main/starlark/builtins_bzl/common/java/java_runtime.bzl index 78c548ec81874b..eeabf1b6c64e36 100644 --- a/src/main/starlark/builtins_bzl/common/java/java_runtime.bzl +++ b/src/main/starlark/builtins_bzl/common/java/java_runtime.bzl @@ -100,7 +100,7 @@ def _java_runtime_rule_impl(ctx): java_binary_exec_path = paths.get_relative(java_home, _get_bin_java(ctx)) java_binary_runfiles_path = _get_runfiles_java_executable(ctx, java_home, ctx.label) - java = ctx.executable.java.path + java = ctx.executable.java if java: if paths.is_absolute(java_home): fail("'java_home' with an absolute path requires 'java' to be empty.")