Skip to content

Commit

Permalink
Pass ct.sym to direct version of Turbine
Browse files Browse the repository at this point in the history
This ensures that Turbine has access to `ct.sym` and thus supports the `--release` flag even if it is compiled into a Graal native image, which doesn't have access to a `JAVA_HOME`.

This requires exposing the `ct.sym` file on a new `lib_ct_sym` attribute of `java_runtime` and passing it into Turbine via the `turbine.ctSymPath` system property introduced in v0.3.1.

Along the way this commit fixes the setup code for testing unreleased versions of the remote Java and coverage tools, which silently broke with the flip of `--enable_bzlmod`.

Work towards bazelbuild/stardoc#195

Closes #20294.

PiperOrigin-RevId: 585866870
Change-Id: I416b787e324bd3a01e223edbda4f9ba137c21241
  • Loading branch information
fmeum authored and copybara-github committed Nov 28, 2023
1 parent b2cca31 commit 4a29f08
Show file tree
Hide file tree
Showing 15 changed files with 272 additions and 201 deletions.
2 changes: 1 addition & 1 deletion MODULE.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ maven.install(
"com.google.http-client:google-http-client-gson:1.42.0",
"com.google.http-client:google-http-client:1.42.0",
"com.google.j2objc:j2objc-annotations:1.3",
"com.google.turbine:turbine:0.3.0",
"com.google.turbine:turbine:0.3.1",
"com.ryanharter.auto.value:auto-value-gson-extension:1.3.1",
"com.ryanharter.auto.value:auto-value-gson-runtime:1.3.1",
"com.ryanharter.auto.value:auto-value-gson-factory:1.3.1",
Expand Down
36 changes: 18 additions & 18 deletions MODULE.bazel.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 4 additions & 4 deletions maven_install.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"__AUTOGENERATED_FILE_DO_NOT_MODIFY_THIS_FILE_MANUALLY": "THERE_IS_NO_DATA_ONLY_ZUUL",
"__INPUT_ARTIFACTS_HASH": -922870577,
"__RESOLVED_ARTIFACTS_HASH": 1291237234,
"__INPUT_ARTIFACTS_HASH": 143984014,
"__RESOLVED_ARTIFACTS_HASH": -866086153,
"conflict_resolution": {
"com.google.code.gson:gson:2.8.9": "com.google.code.gson:gson:2.9.0",
"com.google.errorprone:error_prone_annotations:2.3.2": "com.google.errorprone:error_prone_annotations:2.23.0",
Expand Down Expand Up @@ -278,9 +278,9 @@
},
"com.google.turbine:turbine": {
"shasums": {
"jar": "3fee92239ec300ef4142e2e32ac81af3123cce9cdc552ca78429086ed8c5a445"
"jar": "c0778917005294a59c805a60a5ace2ec847dcece41ced70680f675d822fc03a7"
},
"version": "0.3.0"
"version": "0.3.1"
},
"com.ryanharter.auto.value:auto-value-gson-extension": {
"shasums": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,16 @@ public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment env)
Files in the runtime needed for hermetic deployments.
<!-- #END_BLAZE_RULE.ATTRIBUTE --> */
.add(attr("hermetic_srcs", LABEL_LIST).allowedFileTypes(FileTypeSet.ANY_FILE))
/* <!-- #BLAZE_RULE(java_runtime).ATTRIBUTE(lib_ct_sym) -->
The lib/ct.sym file needed for compilation with <code>--release</code>. If not specified and
there is exactly one file in <code>srcs</code> whose path ends with
<code>/lib/ct.sym</code>, that file is used.
<!-- #END_BLAZE_RULE.ATTRIBUTE --> */
.add(
attr("lib_ct_sym", LABEL)
.singleArtifact()
.allowedFileTypes(FileTypeSet.ANY_FILE)
.exec())
/* <!-- #BLAZE_RULE(java_runtime).ATTRIBUTE(lib_modules) -->
The lib/modules file needed for hermetic deployments.
<!-- #END_BLAZE_RULE.ATTRIBUTE --> */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,8 @@ static JavaToolchainTool fromStarlark(@Nullable StructImpl struct) throws RuleEr
public abstract NestedSet<Artifact> data();

/**
* JVM flags to invoke the tool with, or empty if it is not a {@code _deploy.jar}. Location
* expansion is performed on these flags using the inputs in {@link #data}.
* JVM flags to invoke the tool with. Location expansion is performed on these flags using the
* inputs in {@link #data}.
*/
public abstract NestedSet<String> jvmOpts();

Expand Down Expand Up @@ -105,7 +105,7 @@ private CustomCommandLine extractCommandLine(JavaToolchainProvider toolchain)

Artifact executable = tool().getExecutable();
if (!executable.getExtension().equals("jar")) {
command.addExecPath(executable);
command = command.addExecPath(executable).addAll(jvmOpts());
} else {
command
.addPath(toolchain.getJavaRuntime().javaBinaryExecPathFragment())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,15 @@ public interface JavaRuntimeInfoApi extends StructApi {
structField = true)
Depset starlarkHermeticInputs();

/** The lib/ct.sym file. */
@StarlarkMethod(
name = "lib_ct_sym",
doc = "Returns the lib/ct.sym file.",
structField = true,
allowReturnNones = true)
@Nullable
FileApi libCtSym();

/** The lib/modules file. */
@StarlarkMethod(
name = "lib_modules",
Expand Down
13 changes: 13 additions & 0 deletions src/main/starlark/builtins_bzl/common/java/java_runtime.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ JavaRuntimeInfo, _new_javaruntimeinfo = provider(
This should only be used when one needs to access the JDK during the execution
of a binary or a test built by Bazel. In particular, when one needs the JDK
during an action, java_home should be used instead.""",
"lib_ct_sym": "Returns the lib/ct.sym file.",
"lib_modules": "Returns the lib/modules file.",
"version": "The Java feature version of the runtime. This is 0 if the version is unknown.",
},
Expand Down Expand Up @@ -75,6 +76,15 @@ def _get_runfiles_java_executable(java_home, label):
def _is_java_binary(path):
return path.endswith("bin/java") or path.endswith("bin/java.exe")

def _get_lib_ct_sym(srcs, explicit_lib_ct_sym):
if explicit_lib_ct_sym:
return explicit_lib_ct_sym
candidates = [src for src in srcs if src.path.endswith("/lib/ct.sym")]
if len(candidates) == 1:
return candidates[0]
else:
return None

def _java_runtime_rule_impl(ctx):
all_files = [] # [depset[File]]
all_files.append(depset(ctx.files.srcs))
Expand Down Expand Up @@ -105,6 +115,7 @@ def _java_runtime_rule_impl(ctx):
hermetic_inputs = depset(ctx.files.hermetic_srcs)
all_files.append(hermetic_inputs)

lib_ct_sym = _get_lib_ct_sym(ctx.files.srcs, ctx.file.lib_ct_sym)
lib_modules = ctx.file.lib_modules
hermetic_static_libs = [dep[CcInfo] for dep in ctx.attr.hermetic_static_libs]

Expand All @@ -128,6 +139,7 @@ def _java_runtime_rule_impl(ctx):
java_executable_runfiles_path = java_binary_runfiles_path,
java_home = java_home,
java_home_runfiles_path = java_home_runfiles_path,
lib_ct_sym = lib_ct_sym,
lib_modules = lib_modules,
version = ctx.attr.version,
)
Expand All @@ -152,6 +164,7 @@ java_runtime = rule(
"hermetic_static_libs": attr.label_list(providers = [CcInfo]),
"java": attr.label(allow_single_file = True, executable = True, cfg = "target"),
"java_home": attr.string(),
"lib_ct_sym": attr.label(allow_single_file = True),
"lib_modules": attr.label(allow_single_file = True, executable = True, cfg = "target"),
"output_licenses": attr.license() if hasattr(attr, "license") else attr.string_list(),
"srcs": attr.label_list(allow_files = True),
Expand Down
20 changes: 16 additions & 4 deletions src/main/starlark/builtins_bzl/common/java/java_toolchain.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -78,11 +78,18 @@ JavaToolchainInfo, _new_javatoolchaininfo = provider(
def _java_toolchain_impl(ctx):
javac_opts_list = _get_javac_opts(ctx)
bootclasspath_info = _get_bootclasspath_info(ctx)
java_runtime = _get_java_runtime(ctx)
if java_runtime and java_runtime.lib_ct_sym:
header_compiler_direct_data = [java_runtime.lib_ct_sym]
header_compiler_direct_jvm_opts = ["-Dturbine.ctSymPath=" + java_runtime.lib_ct_sym.path]
else:
header_compiler_direct_data = []
header_compiler_direct_jvm_opts = []
java_toolchain_info = _new_javatoolchaininfo(
bootclasspath = bootclasspath_info.bootclasspath,
ijar = ctx.attr.ijar.files_to_run if ctx.attr.ijar else None,
jacocorunner = ctx.attr.jacocorunner.files_to_run if ctx.attr.jacocorunner else None,
java_runtime = _get_java_runtime(ctx),
java_runtime = java_runtime,
jvm_opt = depset(_java_common_internal.expand_java_opts(ctx, "jvm_opts", tokenize = False, exec_paths = True)),
label = ctx.label,
proguard_allowlister = ctx.attr.proguard_allowlister.files_to_run if ctx.attr.proguard_allowlister else None,
Expand All @@ -100,7 +107,12 @@ def _java_toolchain_impl(ctx):
_gen_class = ctx.file.genclass,
_header_compiler = _get_tool_from_ctx(ctx, "header_compiler", "turbine_data", "turbine_jvm_opts"),
_header_compiler_builtin_processors = depset(ctx.attr.header_compiler_builtin_processors),
_header_compiler_direct = _get_tool_from_executable(ctx, "header_compiler_direct"),
_header_compiler_direct = _get_tool_from_executable(
ctx,
"header_compiler_direct",
data = header_compiler_direct_data,
jvm_opts = header_compiler_direct_jvm_opts,
),
_javabuilder = _get_tool_from_ctx(ctx, "javabuilder", "javabuilder_data", "javabuilder_jvm_opts"),
_javacopts = helper.detokenize_javacopts(javac_opts_list),
_javacopts_list = javac_opts_list,
Expand Down Expand Up @@ -174,14 +186,14 @@ def _get_tool_from_ctx(ctx, tool_attr, data_attr, opts_attr):
jvm_opts = depset([ctx.expand_location(opt, data) for opt in getattr(ctx.attr, opts_attr)]),
)

def _get_tool_from_executable(ctx, attr_name):
def _get_tool_from_executable(ctx, attr_name, data = [], jvm_opts = []):
dep = getattr(ctx.attr, attr_name)
if not dep:
return None
files_to_run = dep.files_to_run
if not files_to_run or not files_to_run.executable:
fail(dep.label, "does not refer to a valid executable target")
return struct(tool = files_to_run, data = depset(), jvm_opts = depset())
return struct(tool = files_to_run, data = depset(data), jvm_opts = depset(jvm_opts))

def _get_compatible_javacopts(ctx):
result = {}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3566,6 +3566,97 @@ public void hermeticStaticLibs() throws Exception {
.containsExactly("a/libStatic");
}

@Test
public void implicitLibCtSym() throws Exception {
scratch.file("a/libStatic.a");
scratch.file(
"a/BUILD",
"load(':rule.bzl', 'jrule')",
"load('"
+ TestConstants.TOOLS_REPOSITORY
+ "//tools/jdk:java_toolchain_alias.bzl', 'java_runtime_alias')",
"java_runtime(",
" name='jvm',",
" srcs=[",
" 'foo/bar/bin/java',",
" 'foo/bar/lib/ct.sym',",
" ],",
" java='foo/bar/bin/java',",
")",
"java_runtime_alias(name='alias')",
"jrule(name='r')",
"toolchain(",
" name = 'java_runtime_toolchain',",
" toolchain = ':jvm',",
" toolchain_type = '"
+ TestConstants.TOOLS_REPOSITORY
+ "//tools/jdk:runtime_toolchain_type',",
")");
scratch.file(
"a/rule.bzl",
"load('//myinfo:myinfo.bzl', 'MyInfo')",
"def _impl(ctx):",
" provider = ctx.attr._java_runtime[java_common.JavaRuntimeInfo]",
" return MyInfo(",
" lib_ct_sym = provider.lib_ct_sym,",
" )",
"jrule = rule(_impl, attrs = { '_java_runtime': attr.label(default=Label('//a:alias'))})");

useConfiguration("--extra_toolchains=//a:all");
ConfiguredTarget ct = getConfiguredTarget("//a:r");
StructImpl myInfo = getMyInfoFromTarget(ct);
@SuppressWarnings("unchecked")
Artifact libCtSym = (Artifact) myInfo.getValue("lib_ct_sym");
assertThat(libCtSym).isNotNull();
assertThat(libCtSym.getExecPathString()).isEqualTo("a/foo/bar/lib/ct.sym");
}

@Test
public void explicitLibCtSym() throws Exception {
scratch.file("a/libStatic.a");
scratch.file(
"a/BUILD",
"load(':rule.bzl', 'jrule')",
"load('"
+ TestConstants.TOOLS_REPOSITORY
+ "//tools/jdk:java_toolchain_alias.bzl', 'java_runtime_alias')",
"java_runtime(",
" name='jvm',",
" srcs=[",
" 'foo/bar/bin/java',",
" 'foo/bar/lib/ct.sym',",
" ],",
" java='foo/bar/bin/java',",
" lib_ct_sym='lib/ct.sym',",
")",
"java_runtime_alias(name='alias')",
"jrule(name='r')",
"toolchain(",
" name = 'java_runtime_toolchain',",
" toolchain = ':jvm',",
" toolchain_type = '"
+ TestConstants.TOOLS_REPOSITORY
+ "//tools/jdk:runtime_toolchain_type',",
")");
scratch.file(
"a/rule.bzl",
"load('//myinfo:myinfo.bzl', 'MyInfo')",
"def _impl(ctx):",
" provider = ctx.attr._java_runtime[java_common.JavaRuntimeInfo]",
" return MyInfo(",
" lib_ct_sym = provider.lib_ct_sym,",
" )",
"jrule = rule(_impl, attrs = { '_java_runtime': attr.label(default=Label('//a:alias'))})");

useConfiguration("--extra_toolchains=//a:all");
ConfiguredTarget ct = getConfiguredTarget("//a:r");
StructImpl myInfo = getMyInfoFromTarget(ct);
@SuppressWarnings("unchecked")
Artifact libCtSym = (Artifact) myInfo.getValue("lib_ct_sym");
assertThat(libCtSym).isNotNull();
assertThat(libCtSym.getExecPathString()).isEqualTo("a/lib/ct.sym");
}

@Test
@TestParameters({
"{module: java_config, api: use_ijars}",
Expand Down
Loading

0 comments on commit 4a29f08

Please sign in to comment.