diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCommon.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCommon.java index 652594eb46d0f1..12a215539f008a 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCommon.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCommon.java @@ -830,7 +830,7 @@ public RuleContext getRuleContext() { private JavaCompilationInfoProvider createCompilationInfoProvider() throws RuleErrorException { return new JavaCompilationInfoProvider.Builder() - .setJavacOpts(javacOpts) + .setJavacOpts(JavaHelper.detokenizeJavaOptions(javacOpts)) .setBootClasspath(getBootClasspath()) .setCompilationClasspath(getCompileTimeClasspath()) .setRuntimeClasspath(getRuntimeClasspath()) diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationInfoProvider.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationInfoProvider.java index 272c288bbf25f7..559b0f8be5c3c1 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationInfoProvider.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationInfoProvider.java @@ -83,8 +83,7 @@ public static JavaCompilationInfoProvider fromStarlarkCompilationInfo(Object val Builder builder = new Builder() .setJavacOpts( - Sequence.cast(info.getValue("javac_options"), String.class, "javac_options") - .getImmutableList()) + Depset.cast(info.getValue("javac_options"), String.class, "javac_options")) .setBootClasspath( NestedSetBuilder.wrap( Order.NAIVE_LINK_ORDER, @@ -112,13 +111,13 @@ public boolean isImmutable() { /** Builder for {@link JavaCompilationInfoProvider}. */ public static class Builder { - private ImmutableList javacOpts = ImmutableList.of(); + private NestedSet javacOpts = NestedSetBuilder.emptySet(Order.NAIVE_LINK_ORDER); private NestedSet runtimeClasspath; private NestedSet compilationClasspath; private NestedSet bootClasspath = NestedSetBuilder.emptySet(Order.STABLE_ORDER); @CanIgnoreReturnValue - public Builder setJavacOpts(@Nonnull ImmutableList javacOpts) { + public Builder setJavacOpts(@Nonnull NestedSet javacOpts) { this.javacOpts = javacOpts; return this; } @@ -143,15 +142,21 @@ public Builder setBootClasspath(NestedSet bootClasspath) { public JavaCompilationInfoProvider build() throws RuleErrorException { return new AutoValue_JavaCompilationInfoProvider( - JavaCompilationHelper.internJavacOpts(javacOpts), - runtimeClasspath, - compilationClasspath, - bootClasspath); + javacOpts, runtimeClasspath, compilationClasspath, bootClasspath); } } + public abstract NestedSet getJavacOpts(); + @Override - public abstract ImmutableList getJavacOpts(); + public Depset getJavacOptsStarlark() { + return Depset.of(String.class, getJavacOpts()); + } + + @Override + public ImmutableList getJavacOptsList() { + return JavaCompilationHelper.internJavacOpts(JavaHelper.tokenizeJavaOptions(getJavacOpts())); + } @Nullable public abstract NestedSet runtimeClasspath(); @@ -203,7 +208,7 @@ public final boolean equals(Object obj) { return false; } JavaCompilationInfoProvider other = (JavaCompilationInfoProvider) obj; - return Objects.equals(getJavacOpts(), other.getJavacOpts()) + return getJavacOpts().shallowEquals(other.getJavacOpts()) && Objects.equals(getRuntimeClasspath(), other.getRuntimeClasspath()) && Objects.equals(getCompilationClasspath(), other.getCompilationClasspath()) && bootClasspath().shallowEquals(other.bootClasspath()); diff --git a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/java/JavaCompilationInfoProviderApi.java b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/java/JavaCompilationInfoProviderApi.java index 3147b5b5e51e10..47f510cc9fcd52 100644 --- a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/java/JavaCompilationInfoProviderApi.java +++ b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/java/JavaCompilationInfoProviderApi.java @@ -29,8 +29,13 @@ doc = "Provides access to compilation information for Java rules.") public interface JavaCompilationInfoProviderApi extends StarlarkValue { - @StarlarkMethod(name = "javac_options", structField = true, doc = "Options to java compiler.") - ImmutableList getJavacOpts(); + @StarlarkMethod( + name = "javac_options", + structField = true, + doc = + "A depset of options to java compiler. To get the exact list of options passed to javac" + + " in the correct order, use the tokenize_javacopts utility in rules_java") + Depset getJavacOptsStarlark(); @StarlarkMethod( name = "javac_options_list", @@ -39,9 +44,7 @@ public interface JavaCompilationInfoProviderApi extends S "A list of options to java compiler. This exists temporarily for migration purposes. " + "javac_options will return a depset in the future, and this method will be dropped " + "once all usages have been updated to handle depsets.") - default ImmutableList getJavacOptsList() { - return getJavacOpts(); - } + ImmutableList getJavacOptsList(); @StarlarkMethod( name = "runtime_classpath", diff --git a/src/main/starlark/builtins_bzl/common/java/android_lint.bzl b/src/main/starlark/builtins_bzl/common/java/android_lint.bzl index f6e61e050a12ef..4a9f43fbeb7a41 100644 --- a/src/main/starlark/builtins_bzl/common/java/android_lint.bzl +++ b/src/main/starlark/builtins_bzl/common/java/android_lint.bzl @@ -14,8 +14,12 @@ """Creates the android lint action for java rules""" +load(":common/java/java_helper.bzl", "helper") load(":common/java/java_semantics.bzl", "semantics") +def _tokenize_opts(opts_depset): + return helper.tokenize_javacopts(ctx = None, opts = opts_depset) + def _android_lint_action(ctx, source_files, source_jars, compilation_info): """ Creates an action that runs Android lint against Java source files. @@ -97,8 +101,9 @@ def _android_lint_action(ctx, source_files, source_jars, compilation_info): args.add("--target_label", ctx.label) javac_opts = compilation_info.javac_options - if (javac_opts): - args.add_all("--javacopts", javac_opts) + if javac_opts: + # wrap in a list so that map_each passes the depset to _tokenize_opts + args.add_all("--javacopts", [javac_opts], map_each = _tokenize_opts) args.add("--") args.add("--lintopts") diff --git a/src/main/starlark/builtins_bzl/common/java/compile_action.bzl b/src/main/starlark/builtins_bzl/common/java/compile_action.bzl index 267c81ac5dbc70..eb1982e98de28f 100644 --- a/src/main/starlark/builtins_bzl/common/java/compile_action.bzl +++ b/src/main/starlark/builtins_bzl/common/java/compile_action.bzl @@ -162,7 +162,7 @@ def compile_action( runfiles = [output_class_jar] if source_files or source_jars or resources else [], # TODO(ilist): collect compile_jars from JavaInfo in deps & exports compilation_classpath = java_info.compilation_info.compilation_classpath, - javac_options = java_info.compilation_info.javac_options_list, + javac_options = java_info.compilation_info.javac_options, plugins = _collect_plugins(deps, plugins), ) 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 8862b394b43008..a1dc5236a4ac06 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 @@ -282,7 +282,7 @@ def compile( direct_runtime_jars = [] compilation_info = struct( - javac_options = all_javac_opts_list, + javac_options = all_javac_opts, javac_options_list = all_javac_opts_list, # needs to be flattened because the public API is a list boot_classpath = (bootclasspath.bootclasspath if bootclasspath else java_toolchain.bootclasspath).to_list(), diff --git a/src/main/starlark/builtins_bzl/common/java/java_info.bzl b/src/main/starlark/builtins_bzl/common/java/java_info.bzl index f53000b3106de3..fd0554699cc0d9 100644 --- a/src/main/starlark/builtins_bzl/common/java/java_info.bzl +++ b/src/main/starlark/builtins_bzl/common/java/java_info.bzl @@ -82,7 +82,9 @@ JavaCompilationInfo = provider( doc = "Compilation information in Java rules, for perusal of aspects and tools.", fields = { "boot_classpath": "Boot classpath for this Java target.", - "javac_options": "Options to the java compiler.", + "javac_options": """Depset of options to the java compiler. To get the + exact list of options passed to javac in the correct order, use the + tokenize_javacopts utility in rules_java""", "javac_options_list": """A list of options to java compiler. This exists temporarily for migration purposes. javac_options will return a depset in the future, and this method will be dropped once all usages have @@ -475,7 +477,7 @@ def java_info_for_compilation( if compilation_info: result.update( compilation_info = JavaCompilationInfo( - javac_options = _java_common_internal.intern_javac_opts(compilation_info.javac_options), + javac_options = compilation_info.javac_options, javac_options_list = _java_common_internal.intern_javac_opts(compilation_info.javac_options_list), boot_classpath = compilation_info.boot_classpath, compilation_classpath = compilation_info.compilation_classpath, diff --git a/src/test/java/com/google/devtools/build/lib/rules/java/JavaInfoStarlarkApiTest.java b/src/test/java/com/google/devtools/build/lib/rules/java/JavaInfoStarlarkApiTest.java index 9b0a5accd0a20f..5d6606632969b0 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/java/JavaInfoStarlarkApiTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/java/JavaInfoStarlarkApiTest.java @@ -1079,7 +1079,10 @@ public void translateStarlarkJavaInfo_compilationInfo() throws Exception { "compilation_info", makeStruct( ImmutableMap.of( - "javac_options", StarlarkList.immutableOf("opt1", "opt2"), + "javac_options", + Depset.of( + String.class, + NestedSetBuilder.create(Order.NAIVE_LINK_ORDER, "opt1", "opt2")), "boot_classpath", StarlarkList.immutableOf(createArtifact("cp.jar"))))) .buildOrThrow(); StarlarkInfo starlarkInfo = makeStruct(fields); @@ -1088,7 +1091,7 @@ public void translateStarlarkJavaInfo_compilationInfo() throws Exception { assertThat(javaInfo).isNotNull(); assertThat(javaInfo.getCompilationInfoProvider()).isNotNull(); - assertThat(javaInfo.getCompilationInfoProvider().getJavacOpts()) + assertThat(javaInfo.getCompilationInfoProvider().getJavacOptsList()) .containsExactly("opt1", "opt2"); assertThat(javaInfo.getCompilationInfoProvider().getBootClasspathList()).hasSize(1); assertThat(prettyArtifactNames(javaInfo.getCompilationInfoProvider().getBootClasspathList())) @@ -1107,13 +1110,16 @@ public void translatedStarlarkCompilationInfoEqualsNativeInstance() throws Excep ImmutableMap.of( "compilation_classpath", Depset.of(Artifact.class, compilationClasspath), "runtime_classpath", Depset.of(Artifact.class, runtimeClasspath), - "javac_options", StarlarkList.immutableOf("opt1", "opt2"), + "javac_options", + Depset.of( + String.class, + NestedSetBuilder.create(Order.NAIVE_LINK_ORDER, "opt1", "opt2")), "boot_classpath", StarlarkList.immutableOf(bootClasspathArtifact))); JavaCompilationInfoProvider nativeCompilationInfo = new JavaCompilationInfoProvider.Builder() .setCompilationClasspath(compilationClasspath) .setRuntimeClasspath(runtimeClasspath) - .setJavacOpts(ImmutableList.of("opt1", "opt2")) + .setJavacOpts(NestedSetBuilder.create(Order.NAIVE_LINK_ORDER, "opt1", "opt2")) .setBootClasspath( NestedSetBuilder.create(Order.NAIVE_LINK_ORDER, bootClasspathArtifact)) .build(); 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 a69c7060462a6f..31edd567aea1d0 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 @@ -681,7 +681,7 @@ public void testJavaCommonCompileCompilationInfo() throws Exception { assertThat(prettyArtifactNames(compilationInfo.getRuntimeClasspath().toList(Artifact.class))) .containsExactly("java/test/libdep.jar", "java/test/libcustom.jar"); - assertThat(compilationInfo.getJavacOpts()).contains("-XDone"); + assertThat(compilationInfo.getJavacOptsList()).contains("-XDone"); } @Test @@ -2188,7 +2188,7 @@ public void javaInfoGetCompilationInfoProvider() throws Exception { prettyArtifactNames( javaCompilationInfoProvider.getRuntimeClasspath().getSet(Artifact.class))) .containsExactly("foo/libmy_java_lib_a.jar"); - assertThat(javaCompilationInfoProvider.getJavacOpts()).contains("opt1"); + assertThat(javaCompilationInfoProvider.getJavacOpts().toList()).contains("opt1"); assertThat(javaCompilationInfoProvider.getJavacOptsList()).contains("opt1"); }