Skip to content

Commit

Permalink
Make native JavaOutput a StructApi to allow both native and Starl…
Browse files Browse the repository at this point in the history
…ark instances to be added to a `depset`

`depset` type-checking relies on `Starlark.type()` which finds the lowest ancestor class with a `@StarlarkBuiltins` annotation. For Starlark providers this is `StructApi`, so we have to make it the same for our native class.

This is needed so we can de-duplicate `JavaOutput` instances efficiently in Starlark (otherwise we run into #17170).

Also updates `JavaOutput` translation in `JavaPluginInfo` that was missed in an earlier change.

PiperOrigin-RevId: 549520899
Change-Id: I7811c57bbbce41ab92b50938889d4e2c1b8cf34b
  • Loading branch information
hvadehra authored and copybara-github committed Jul 20, 2023
1 parent e81cdb9 commit cda08cc
Show file tree
Hide file tree
Showing 3 changed files with 69 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,8 @@ public JavaPluginInfo wrap(Info value) throws RuleErrorException {
try {
StructImpl info = (StructImpl) value;
return new AutoValue_JavaPluginInfo(
Sequence.cast(info.getValue("java_outputs"), JavaOutput.class, "java_outputs")
.getImmutableList(),
JavaOutput.wrapSequence(
Sequence.cast(info.getValue("java_outputs"), Object.class, "java_outputs")),
JavaPluginData.wrap(info.getValue("plugins")),
JavaPluginData.wrap(info.getValue("api_generating_plugins")));
} catch (EvalException e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,20 +14,16 @@

package com.google.devtools.build.lib.starlarkbuildapi.java;

import com.google.devtools.build.docgen.annot.DocCategory;
import com.google.devtools.build.lib.starlarkbuildapi.FileApi;
import com.google.devtools.build.lib.starlarkbuildapi.core.StructApi;
import javax.annotation.Nullable;
import net.starlark.java.annot.StarlarkBuiltin;
import net.starlark.java.annot.StarlarkMethod;
import net.starlark.java.eval.EvalException;
import net.starlark.java.eval.Starlark;
import net.starlark.java.eval.StarlarkSemantics;
import net.starlark.java.eval.StarlarkValue;

/** A tuple of a java classes jar and its associated source and interface archives. */
@StarlarkBuiltin(
name = "java_output",
category = DocCategory.BUILTIN,
doc = "The outputs of Java compilation.")
public interface JavaOutputApi<FileT extends FileApi> extends StarlarkValue {
public interface JavaOutputApi<FileT extends FileApi> extends StructApi {

@StarlarkMethod(name = "class_jar", doc = "A classes jar file.", structField = true)
FileT getClassJar();
Expand Down Expand Up @@ -121,4 +117,14 @@ public interface JavaOutputApi<FileT extends FileApi> extends StarlarkValue {
structField = true)
@Nullable
Object getSrcJarsStarlark(StarlarkSemantics semantics);

@Override
default String toProto() throws EvalException {
throw Starlark.errorf("unsupported method");
}

@Override
default String toJson() throws EvalException {
throw Starlark.errorf("unsupported method");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
import com.google.errorprone.annotations.CanIgnoreReturnValue;
import java.io.IOException;
import java.util.Map;
import net.starlark.java.eval.Starlark;
import net.starlark.java.eval.StarlarkList;
import org.junit.Test;
import org.junit.runner.RunWith;
Expand Down Expand Up @@ -872,6 +873,25 @@ public void buildHelperCreateJavaInfoWithModuleFlags() throws Exception {
.containsExactly("--add-opens=java.base/java.lang=ALL-UNNAMED");
}

@Test
public void starlarkJavaOutputsCanBeAddedToJavaPluginInfo() throws Exception {
Artifact classJar = createArtifact("foo.jar");
StarlarkInfo starlarkJavaOutput =
makeStruct(ImmutableMap.of("source_jars", Starlark.NONE, "class_jar", classJar));
StarlarkInfo starlarkPluginInfo =
makeStruct(
ImmutableMap.of(
"java_outputs", StarlarkList.immutableOf(starlarkJavaOutput),
"plugins", JavaPluginData.empty(),
"api_generating_plugins", JavaPluginData.empty()));

JavaPluginInfo pluginInfo = JavaPluginInfo.PROVIDER.wrap(starlarkPluginInfo);

assertThat(pluginInfo).isNotNull();
assertThat(pluginInfo.getJavaOutputs()).hasSize(1);
assertThat(pluginInfo.getJavaOutputs().get(0).getClassJar()).isEqualTo(classJar);
}

@Test
public void javaOutputSourceJarsReturnsListWithIncompatibleFlagDisabled() throws Exception {
setBuildLanguageOptions("--noincompatible_depset_for_java_output_source_jars");
Expand Down Expand Up @@ -932,6 +952,39 @@ public void javaOutputSourceJarsReturnsDepsetWithIncompatibleFlagEnabled() throw
assertThat(info.getValue("source_jars")).isInstanceOf(Depset.class);
}

@Test
public void nativeAndStarlarkJavaOutputsCanBeAddedToADepset() throws Exception {
scratch.file(
"foo/extension.bzl",
"def _impl(ctx):",
" f = ctx.actions.declare_file(ctx.label.name + '.jar')",
" ctx.actions.write(f, '')",
" return [JavaInfo(output_jar=f, compile_jar=None)]",
"",
"my_rule = rule(implementation = _impl)");
scratch.file(
"foo/BUILD",
//
"load(':extension.bzl', 'my_rule')",
"my_rule(name = 'my_starlark_rule')");
JavaOutput nativeOutput =
JavaOutput.builder().setClassJar(createArtifact("native.jar")).build();
StarlarkList<?> starlarkOutputs =
((StarlarkInfo)
getConfiguredTarget("//foo:my_starlark_rule").get(JavaInfo.PROVIDER.getKey()))
.getValue("java_outputs", StarlarkList.class);

Depset depset =
Depset.fromDirectAndTransitive(
Order.STABLE_ORDER,
/* direct= */ ImmutableList.builder().add(nativeOutput).addAll(starlarkOutputs).build(),
/* transitive= */ ImmutableList.of(),
/* strict= */ true);

assertThat(depset).isNotNull();
assertThat(depset.toList()).hasSize(2);
}

@Test
public void translateStarlarkJavaInfo_minimal() throws Exception {
ImmutableMap<String, Object> fields = getBuilderWithMandataryFields().buildOrThrow();
Expand Down

0 comments on commit cda08cc

Please sign in to comment.