From c7d8d1e3f16ac6db37b134358b6cfdb5e3c8f6b0 Mon Sep 17 00:00:00 2001 From: Googler Date: Fri, 3 Nov 2023 05:21:51 -0700 Subject: [PATCH] Delete private API `default_javac_opts` Unfortunately, we store the options in list form on `java_toolchain` since we don't have `ctx` to tokenize them when requested. Once usages[^1] are migrated to handling depsets, this will be dropped. PiperOrigin-RevId: 579150744 Change-Id: I1b8058b102a9d65818ef10af3c5c2c1365291b29 --- .../lib/rules/java/JavaStarlarkCommon.java | 17 ----------------- .../lib/rules/java/JavaToolchainProvider.java | 12 ------------ .../starlarkbuildapi/java/JavaCommonApi.java | 8 ++++---- .../builtins_bzl/common/java/java_common.bzl | 8 +++----- .../java/java_common_internal_for_builtins.bzl | 7 +++---- .../builtins_bzl/common/java/java_toolchain.bzl | 7 +++++-- .../lib/rules/java/JavaStarlarkApiTest.java | 2 +- 7 files changed, 16 insertions(+), 45 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaStarlarkCommon.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaStarlarkCommon.java index d95e59491afed2..c1346ddc831d41 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaStarlarkCommon.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaStarlarkCommon.java @@ -57,7 +57,6 @@ import net.starlark.java.eval.Starlark; import net.starlark.java.eval.StarlarkList; import net.starlark.java.eval.StarlarkThread; -import net.starlark.java.eval.StarlarkValue; /** A module that contains Starlark utilities for Java support. */ public class JavaStarlarkCommon @@ -255,22 +254,6 @@ public void createCompilationAction( compilationHelper.createCompileAction(outputs); } - @Override - // TODO(b/78512644): migrate callers to passing explicit javacopts or using custom toolchains, and - // delete - public StarlarkValue getDefaultJavacOpts(Info javaToolchainUnchecked, boolean asDepset) - throws EvalException, RuleErrorException { - JavaToolchainProvider javaToolchain = - JavaToolchainProvider.PROVIDER.wrap(javaToolchainUnchecked); - // We don't have a rule context if the default_javac_opts.java_toolchain parameter is set - if (asDepset) { - return Depset.of(String.class, javaToolchain.getJavacOptions(/* ruleContext= */ null)); - } else { - return StarlarkList.immutableCopyOf( - javaToolchain.getJavacOptionsAsList(/* ruleContext= */ null)); - } - } - @Override public String getTargetKind(Object target, StarlarkThread thread) throws EvalException { checkPrivateAccess(thread); diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaToolchainProvider.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaToolchainProvider.java index 355d8220a48791..d12d9ba664c542 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaToolchainProvider.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaToolchainProvider.java @@ -264,18 +264,6 @@ public ImmutableList getCompatibleJavacOptionsAsList(String key) return JavaHelper.tokenizeJavaOptions(getCompatibleJavacOptions(key)); } - /** Returns the list of default options for the java compiler. */ - public NestedSet getJavacOptions(RuleContext ruleContext) throws RuleErrorException { - NestedSetBuilder result = NestedSetBuilder.naiveLinkOrder(); - result.addTransitive(javacOptions()); - if (ruleContext != null) { - // TODO(b/78512644): require ruleContext to be non-null after java_common.default_javac_opts - // is turned down - result.addTransitive(ruleContext.getFragment(JavaConfiguration.class).getDefaultJavacFlags()); - } - return result.build(); - } - private NestedSet javacOptions() throws RuleErrorException { return getUnderlyingNestedSet("_javacopts", String.class); } diff --git a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/java/JavaCommonApi.java b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/java/JavaCommonApi.java index 69e6504a367045..dde801def08762 100644 --- a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/java/JavaCommonApi.java +++ b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/java/JavaCommonApi.java @@ -606,10 +606,10 @@ void createCompilationAction( documented = false, defaultValue = "False") }) - // TODO(b/78512644): migrate callers to passing explicit javacopts or using custom toolchains, and - // delete - StarlarkValue getDefaultJavacOpts(Info javaToolchain, boolean asDepset) - throws EvalException, RuleErrorException; + default StarlarkValue getDefaultJavacOpts(Info javaToolchain, boolean asDepset) { + // method exists solely for documentation + throw new UnsupportedOperationException(); + } @StarlarkMethod( name = "JavaToolchainInfo", diff --git a/src/main/starlark/builtins_bzl/common/java/java_common.bzl b/src/main/starlark/builtins_bzl/common/java/java_common.bzl index 4620396a59b729..c8a5d8c3638686 100644 --- a/src/main/starlark/builtins_bzl/common/java/java_common.bzl +++ b/src/main/starlark/builtins_bzl/common/java/java_common.bzl @@ -164,6 +164,7 @@ def _pack_sources( mnemonic = "JavaSourceJar", ) +# TODO: b/78512644 - migrate callers to passing explicit javacopts or using custom toolchains, and delete def _default_javac_opts(java_toolchain): """Experimental! Get default javacopts from a java toolchain @@ -173,7 +174,7 @@ def _default_javac_opts(java_toolchain): Returns: ([str]) A list of javac options """ - return _java_common_internal.default_javac_opts(java_toolchain = java_toolchain) + return java_toolchain._javacopts_list # temporary for migration def _default_javac_opts_depset(java_toolchain): @@ -185,10 +186,7 @@ def _default_javac_opts_depset(java_toolchain): Returns: (depset[str]) A depset of javac options that should be tokenized before passing to javac """ - return _java_common_internal.default_javac_opts( - java_toolchain = java_toolchain, - as_depset = True, - ) + return java_toolchain._javacopts def _merge(providers): """Merges the given providers into a single JavaInfo. diff --git a/src/main/starlark/builtins_bzl/common/java/java_common_internal_for_builtins.bzl b/src/main/starlark/builtins_bzl/common/java/java_common_internal_for_builtins.bzl index 142b048a89c2f2..6115d207d23f8d 100644 --- a/src/main/starlark/builtins_bzl/common/java/java_common_internal_for_builtins.bzl +++ b/src/main/starlark/builtins_bzl/common/java/java_common_internal_for_builtins.bzl @@ -23,6 +23,7 @@ load( "merge_plugin_info_without_outputs", ) load(":common/java/java_semantics.bzl", "semantics") +load(":common/java/java_toolchain.bzl", "JavaToolchainInfo") load(":common/java/sharded_javac.bzl", "experimental_sharded_javac", "use_sharded_javac") load(":common/paths.bzl", "paths") @@ -111,15 +112,13 @@ def compile( Returns: (JavaInfo) """ + _java_common_internal.check_provider_instances([java_toolchain], "java_toolchain", JavaToolchainInfo) _java_common_internal.check_provider_instances(plugins, "plugins", JavaPluginInfo) plugin_info = merge_plugin_info_without_outputs(plugins + deps) all_javac_opts = [] # [depset[str]] - all_javac_opts.append(_java_common_internal.default_javac_opts( - java_toolchain = java_toolchain, - as_depset = True, - )) + all_javac_opts.append(java_toolchain._javacopts) all_javac_opts.append(ctx.fragments.java.default_javac_flags_depset) all_javac_opts.append(semantics.compatible_javac_options( diff --git a/src/main/starlark/builtins_bzl/common/java/java_toolchain.bzl b/src/main/starlark/builtins_bzl/common/java/java_toolchain.bzl index a9284744794af4..726bbf5b4864f8 100644 --- a/src/main/starlark/builtins_bzl/common/java/java_toolchain.bzl +++ b/src/main/starlark/builtins_bzl/common/java/java_toolchain.bzl @@ -59,6 +59,7 @@ JavaToolchainInfo, _new_javatoolchaininfo = provider( "_header_compiler_direct": _PRIVATE_API_DOC_STRING, "_javabuilder": _PRIVATE_API_DOC_STRING, "_javacopts": _PRIVATE_API_DOC_STRING, + "_javacopts_list": _PRIVATE_API_DOC_STRING, "_javac_supports_workers": _PRIVATE_API_DOC_STRING, "_javac_supports_multiplex_workers": _PRIVATE_API_DOC_STRING, "_javac_supports_worker_cancellation": _PRIVATE_API_DOC_STRING, @@ -75,6 +76,7 @@ JavaToolchainInfo, _new_javatoolchaininfo = provider( ) def _java_toolchain_impl(ctx): + javac_opts_list = _get_javac_opts(ctx) bootclasspath_info = _get_bootclasspath_info(ctx) java_toolchain_info = _new_javatoolchaininfo( bootclasspath = bootclasspath_info.bootclasspath, @@ -100,7 +102,8 @@ def _java_toolchain_impl(ctx): _header_compiler_builtin_processors = depset(ctx.attr.header_compiler_builtin_processors), _header_compiler_direct = _get_tool_from_executable(ctx, "header_compiler_direct"), _javabuilder = _get_tool_from_ctx(ctx, "javabuilder", "javabuilder_data", "javabuilder_jvm_opts"), - _javacopts = _get_javac_opts(ctx), + _javacopts = helper.detokenize_javacopts(javac_opts_list), + _javacopts_list = javac_opts_list, _javac_supports_workers = ctx.attr.javac_supports_workers, _javac_supports_multiplex_workers = ctx.attr.javac_supports_multiplex_workers, _javac_supports_worker_cancellation = ctx.attr.javac_supports_worker_cancellation, @@ -141,7 +144,7 @@ def _get_javac_opts(ctx): opts.append("-Xlint:" + ",".join(ctx.attr.xlint)) opts.extend(_java_common_internal.expand_java_opts(ctx, "misc", tokenize = True)) opts.extend(_java_common_internal.expand_java_opts(ctx, "javacopts", tokenize = True)) - return helper.detokenize_javacopts(opts) + return opts def _get_android_lint_tool(ctx): if not ctx.attr.android_lint_runner: diff --git a/src/test/java/com/google/devtools/build/lib/rules/java/JavaStarlarkApiTest.java b/src/test/java/com/google/devtools/build/lib/rules/java/JavaStarlarkApiTest.java index 81e3f85c7ff012..b8fa4a30baa675 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/java/JavaStarlarkApiTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/java/JavaStarlarkApiTest.java @@ -2959,7 +2959,7 @@ public void testConfiguredTargetToolchain() throws Exception { reporter.removeHandler(failFastHandler); getConfiguredTarget("//a:r"); - assertContainsEvent("got value of type 'ToolchainInfo', want 'JavaToolchainInfo'"); + assertContainsEvent("got element of type ToolchainInfo, want JavaToolchainInfo"); } @Test