Skip to content

Commit

Permalink
Delete private API default_javac_opts
Browse files Browse the repository at this point in the history
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
  • Loading branch information
hvadehra authored and copybara-github committed Nov 3, 2023
1 parent 304e5e9 commit c7d8d1e
Show file tree
Hide file tree
Showing 7 changed files with 16 additions and 45 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -264,18 +264,6 @@ public ImmutableList<String> getCompatibleJavacOptionsAsList(String key)
return JavaHelper.tokenizeJavaOptions(getCompatibleJavacOptions(key));
}

/** Returns the list of default options for the java compiler. */
public NestedSet<String> getJavacOptions(RuleContext ruleContext) throws RuleErrorException {
NestedSetBuilder<String> 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<String> javacOptions() throws RuleErrorException {
return getUnderlyingNestedSet("_javacopts", String.class);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
8 changes: 3 additions & 5 deletions src/main/starlark/builtins_bzl/common/java/java_common.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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):
Expand All @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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")

Expand Down Expand Up @@ -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(
Expand Down
7 changes: 5 additions & 2 deletions src/main/starlark/builtins_bzl/common/java/java_toolchain.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit c7d8d1e

Please sign in to comment.