Skip to content

Allow construction of a full jdk repo from a build target #17

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions src/main/starlark/builtins_bzl/common/java/java_runtime.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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

Choose a reason for hiding this comment

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

medium

Consider adding a comment explaining why ctx.executable.java is now used instead of ctx.file.java. This will help future maintainers understand the change and its implications.

# Use ctx.executable.java to allow for a full jdk repo from a build target
    java = ctx.executable.java

if java:
if paths.is_absolute(java_home):
fail("'java_home' with an absolute path requires 'java' to be empty.")
Expand Down Expand Up @@ -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 = """
Expand Down
91 changes: 89 additions & 2 deletions src/test/shell/integration/bazel_java_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Comment on lines +339 to +340

Choose a reason for hiding this comment

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

medium

The log expectations have been updated to reflect the new dummy executables. It would be helpful to add a comment explaining why foo and bar are no longer expected and why dummy_java and dummy_javac are now expected. This will help understand the test logic and the changes introduced by the pull request.

Suggested change
expect_log "dummy_java"
expect_log "dummy_javac"
# Test the genrule that specifically depends on :bar_runtime.
# foo and bar are no longer expected because we are now using dummy_java and dummy_javac
expect_log "dummy_java"
expect_log "dummy_javac"

expect_not_log "embedded_jdk"
expect_not_log "remotejdk_"
expect_not_log "remotejdk11_"
Expand All @@ -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."
Loading