From 5e2899daf9f6816ee3811a48d09526e75c85ed0e Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Thu, 31 Aug 2023 08:34:43 +0200 Subject: [PATCH 1/2] fix(native_image): clean up construction of inputs `depset` structures should be kept as flat as possible. C++ toolchain files are also already `depset`s and thus shouldn't be passed into `tools`. Signed-off-by: Sam Gammon --- internal/native_image/rules.bzl | 35 ++++++++++++++------------------- 1 file changed, 15 insertions(+), 20 deletions(-) diff --git a/internal/native_image/rules.bzl b/internal/native_image/rules.bzl index d9dc74cd..c667cf12 100644 --- a/internal/native_image/rules.bzl +++ b/internal/native_image/rules.bzl @@ -103,7 +103,9 @@ def _graal_binary_implementation(ctx): dep[JavaInfo].transitive_runtime_jars for dep in ctx.attr.deps ]) - all_deps = depset([], transitive = [classpath_depset]) + + direct_inputs = [] + transitive_inputs = [classpath_depset] if graal_attr == None: # resolve via toolchains @@ -117,16 +119,14 @@ def _graal_binary_implementation(ctx): ] + extra_tool_deps) graal = graal_inputs.to_list()[0] - all_deps = depset(transitive = [ - classpath_depset, - gvm_toolchain.gvm_files.files, - ]) + transitive_inputs.append(gvm_toolchain.gvm_files[DefaultInfo].files) else: # otherwise, use the legacy code path graal_inputs, _, _ = ctx.resolve_command(tools = [ graal_attr, ]) graal = graal_inputs[0] + direct_inputs.append(graal_inputs) if graal_attr == None: # still failed to resolve: cannot resolve via either toolchains or attributes. @@ -158,6 +158,7 @@ def _graal_binary_implementation(ctx): feature_configuration = feature_configuration, action_name = CPP_LINK_DYNAMIC_LIBRARY_ACTION_NAME, ) + transitive_inputs.append(cc_toolchain.all_files) env = {} path_set = {} @@ -251,38 +252,32 @@ def _graal_binary_implementation(ctx): if ctx.attr.reflection_configuration != None: args.add(ctx.file.reflection_configuration, format = "-H:ReflectionConfigurationFiles=%s") - all_deps = depset([ctx.file.reflection_configuration], transitive = [all_deps]) + direct_inputs.append(ctx.file.reflection_configuration) if ctx.attr.include_resources != None: args.add(ctx.attr.include_resources, format = "-H:IncludeResources=%s") if ctx.attr.jni_configuration != None: args.add(ctx.file.jni_configuration, format = "-H:JNIConfigurationFiles=%s") - all_deps = depset([ctx.file.jni_configuration], transitive = [all_deps]) + direct_inputs.append(ctx.file.jni_configuration) args.add("-H:+JNI") + inputs = depset(direct_inputs, transitive = transitive_inputs) run_params = { "outputs": [binary], "arguments": [args], "executable": graal, + "inputs": inputs, "mnemonic": "NativeImage", "env": env, } - # fix: on bazel4, the `tools` parameter is expected to be a `File or FilesToRunProvider`, whereas - # on later versions it is expected to be a `depset`. there is also no `toolchain` parameter at all. if not ctx.attr._legacy_rule: - run_params.update(**{ - "inputs": all_deps, - "use_default_shell_env": ctx.attr.enable_default_shell_env, - "progress_message": "Compiling native image %{label}", - "toolchain": Label(_GVM_TOOLCHAIN_TYPE), - "tools": [ - ctx.attr._cc_toolchain.files, - ], - }) - else: - run_params["inputs"] = classpath_depset + run_params.update( + use_default_shell_env = ctx.attr.enable_default_shell_env, + progress_message = "Compiling native image %{label}", + toolchain = Label(_GVM_TOOLCHAIN_TYPE), + ) ctx.actions.run( **run_params From 07d3eedba5d67d1c93c61eee3cdb0f7b7ad19ea7 Mon Sep 17 00:00:00 2001 From: Sam Gammon Date: Thu, 31 Aug 2023 18:10:12 -0700 Subject: [PATCH 2/2] fix: modern rule check - fix: explicitly check against modern rule status before resolving toolchain, unconditionally take it if running modern - fix: direct deps for `native_image_tool` attr - fix: preserve legacy rule behavior applies on top of sgammon/rules_graalvm#70 Signed-off-by: Sam Gammon --- internal/native_image/rules.bzl | 28 ++++++++++++++++++++-------- 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/internal/native_image/rules.bzl b/internal/native_image/rules.bzl index c667cf12..03fa5277 100644 --- a/internal/native_image/rules.bzl +++ b/internal/native_image/rules.bzl @@ -107,28 +107,39 @@ def _graal_binary_implementation(ctx): direct_inputs = [] transitive_inputs = [classpath_depset] - if graal_attr == None: + # if we aren't handed a specific `native-image` tool binary, or we are running + # under the modern `native_image` rule, then attempt to resolve via toolchains... + if graal_attr == None and not (ctx.attr._legacy_rule): # resolve via toolchains info = ctx.toolchains[_GVM_TOOLCHAIN_TYPE].graalvm - graal_attr = info.native_image_bin + + # but fall back to explicitly-provided tool, which should override, with the + # remainder of the resolved toolchain present + resolved_graal = graal_attr or info.native_image_bin gvm_toolchain = info extra_tool_deps.append(info.gvm_files) graal_inputs, _ = ctx.resolve_tools(tools = [ - graal_attr, + resolved_graal, ] + extra_tool_deps) graal = graal_inputs.to_list()[0] + + # if we're using an explicit tool, add it to the direct inputs + if graal_attr: + direct_inputs.append(graal_inputs) + + # add toolchain files to transitive inputs transitive_inputs.append(gvm_toolchain.gvm_files[DefaultInfo].files) - else: - # otherwise, use the legacy code path + elif graal_attr != None: + # otherwise, use the legacy code path. the `graal` value is used in the run + # `executable` so it isn't listed in deps for earlier Bazel versions. graal_inputs, _, _ = ctx.resolve_command(tools = [ graal_attr, ]) graal = graal_inputs[0] - direct_inputs.append(graal_inputs) - - if graal_attr == None: + direct_inputs.append(graal) + else: # still failed to resolve: cannot resolve via either toolchains or attributes. fail(""" No `native-image` tool found. Please either define a `native_image_tool` in your target, @@ -263,6 +274,7 @@ def _graal_binary_implementation(ctx): args.add("-H:+JNI") inputs = depset(direct_inputs, transitive = transitive_inputs) + run_params = { "outputs": [binary], "arguments": [args],