Add --incompatible_disallow_legacy_javainfo flag.

When set to true, old-style JavaInfo provider construction methods become an error. RELNOTES[INC]: Add --incompatible_disallow_legacy_javainfo flag. PiperOrigin-RevId: 195104452
agoulti · May 2, 2018 · e374970 · e374970
1 parent 94b8702
commit e374970
Show file tree

Hide file tree

Showing 9 changed files with 174 additions and 3 deletions.
diff --git a/site/docs/skylark/backward-compatibility.md b/site/docs/skylark/backward-compatibility.md
@@ -44,6 +44,7 @@ guarded behind flags in the current release:
 *   [Disable objc provider resources](#disable-objc-provider-resources)
 *   [Remove native git repository](#remove-native-git-repository)
 *   [Remove native http archive](#remove-native-http-archive)
+*   [New-style JavaInfo constructor](#new-style-java_info)
 
 
 ### Dictionary concatenation
@@ -265,5 +266,75 @@ should be used instead.
 *   Flag: `--incompatible_remove_native_http_archive`
 *   Default: `false`
 
+### New-style JavaInfo constructor
+
+When set, `java_common.create_provider` and certain arguments to `JavaInfo` are deprecated. The
+deprecated arguments are: `actions`, `sources`, `source_jars`, `use_ijar`, `java_toolchain`,
+and `host_javabase`.
+
+Example migration from `create_provider`:
+
+```python
+# Before
+provider = java_common.create_provider(
+    ctx.actions,
+    compile_time_jars = [output_jar],
+    use_ijar = True,
+    java_toolchain = ctx.attr._java_toolchain,
+    transitive_compile_time_jars = transitive_compile_time,
+    transitive_runtime_jars = transitive_runtime_jars,
+)
+
+# After
+compile_jar = java_common.run_ijar(
+    ctx.actions,
+    jar = output_jar,
+    target_label = ctx.label,
+    java_toolchain = ctx.attr._java_toolchain,
+)
+provider = JavaInfo(
+    output_jar = output_jar,
+    compile_jar = compile_jar,
+    deps = deps,
+    runtime_deps = runtime_deps,
+)
+```
+
+Example migration from deprecated `JavaInfo` arguments:
+
+```python
+# Before
+provider = JavaInfo(
+  output_jar = my_jar,
+  use_ijar = True,
+  sources = my_sources,
+  deps = my_compile_deps,
+  runtime_deps = my_runtime_deps,
+  actions = ctx.actions,
+  java_toolchain = my_java_toolchain,
+  host_javabase = my_host_javabase,
+)
+
+# After
+my_ijar = java_common.run_ijar(
+  ctx.actions,
+  jar = my_jar,
+  target_label = ctx.label,
+  java_toolchain, my_java_toolchain,
+)
+my_source_jar = java_common.pack_sources(
+  ctx.actions,
+  sources = my_sources,
+  java_toolchain = my_java_toolchain,
+  host_javabase = my_host_javabase,
+)
+provider = JavaInfo(
+  output_jar = my_jar,
+  compile_jar = my_ijar,
+  source_jar = my_source_jar,
+  deps = my_compile_deps,
+  runtime_deps = my_runtime_deps,
+)
+```
 
 <!-- Add new options here -->
diff --git a/src/main/java/com/google/devtools/build/lib/packages/SkylarkSemanticsCodec.java b/src/main/java/com/google/devtools/build/lib/packages/SkylarkSemanticsCodec.java
@@ -50,6 +50,7 @@ public void serialize(
     codedOut.writeBoolNoTag(semantics.incompatibleDisableObjcProviderResources());
     codedOut.writeBoolNoTag(semantics.incompatibleDisallowDictPlus());
     codedOut.writeBoolNoTag(semantics.incompatibleDisallowFileType());
+    codedOut.writeBoolNoTag(semantics.incompatibleDisallowLegacyJavaInfo());
     codedOut.writeBoolNoTag(semantics.incompatibleDisallowOldStyleArgsAdd());
     codedOut.writeBoolNoTag(semantics.incompatibleDisallowSlashOperator());
     codedOut.writeBoolNoTag(semantics.incompatibleNewActionsApi());
@@ -73,6 +74,7 @@ public SkylarkSemantics deserialize(DeserializationContext context, CodedInputSt
     builder.incompatibleDisableObjcProviderResources(codedIn.readBool());
     builder.incompatibleDisallowDictPlus(codedIn.readBool());
     builder.incompatibleDisallowFileType(codedIn.readBool());
+    builder.incompatibleDisallowLegacyJavaInfo(codedIn.readBool());
     builder.incompatibleDisallowOldStyleArgsAdd(codedIn.readBool());
     builder.incompatibleDisallowSlashOperator(codedIn.readBool());
     builder.incompatibleNewActionsApi(codedIn.readBool());

diff --git a/src/main/java/com/google/devtools/build/lib/packages/SkylarkSemanticsOptions.java b/src/main/java/com/google/devtools/build/lib/packages/SkylarkSemanticsOptions.java
@@ -157,6 +157,18 @@ public class SkylarkSemanticsOptions extends OptionsBase implements Serializable
   )
   public boolean incompatibleDisallowFileType;
 
+  @Option(
+      name = "incompatible_disallow_legacy_javainfo",
+      defaultValue = "false",
+      documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
+      effectTags = {OptionEffectTag.UNKNOWN},
+      metadataTags = {
+        OptionMetadataTag.INCOMPATIBLE_CHANGE,
+        OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES
+      },
+      help = "If set to true, old-style JavaInfo provider construction is disallowed.")
+  public boolean incompatibleDisallowLegacyJavaInfo;
+
   @Option(
     name = "incompatible_disallow_slash_operator",
     defaultValue = "false",
@@ -280,6 +292,7 @@ public SkylarkSemantics toSkylarkSemantics() {
         .incompatibleDisableObjcProviderResources(incompatibleDisableObjcProviderResources)
         .incompatibleDisallowDictPlus(incompatibleDisallowDictPlus)
         .incompatibleDisallowFileType(incompatibleDisallowFileType)
+        .incompatibleDisallowLegacyJavaInfo(incompatibleDisallowLegacyJavaInfo)
         .incompatibleDisallowOldStyleArgsAdd(incompatibleDisallowOldStyleArgsAdd)
         .incompatibleDisallowSlashOperator(incompatibleDisallowSlashOperator)
         .incompatibleNewActionsApi(incompatibleNewActionsApi)

diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaInfo.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaInfo.java
@@ -326,6 +326,14 @@ protected JavaInfo createInstanceFromSkylark(Object[] args, Environment env, Loc
                   || javaToolchain != null
                   || hostJavabase != null;
           if (hasLegacyArg) {
+            if (env.getSemantics().incompatibleDisallowLegacyJavaInfo()) {
+              throw new EvalException(
+                  loc,
+                  "Cannot use deprecated argument when "
+                      + "--incompatible_disallow_legacy_javainfo is set. "
+                      + "Deprecated arguments are 'actions', 'sources', 'source_jars', "
+                      + "'use_ijar', 'java_toolchain', 'host_javabase'.");
+            }
             boolean hasNewArg = compileJar != null || sourceJar != null;
             if (hasNewArg) {
               throw new EvalException(

diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaSkylarkCommon.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaSkylarkCommon.java
@@ -31,6 +31,7 @@
 import com.google.devtools.build.lib.skylarkinterface.ParamType;
 import com.google.devtools.build.lib.skylarkinterface.SkylarkCallable;
 import com.google.devtools.build.lib.skylarkinterface.SkylarkModule;
+import com.google.devtools.build.lib.syntax.Environment;
 import com.google.devtools.build.lib.syntax.EvalException;
 import com.google.devtools.build.lib.syntax.Runtime;
 import com.google.devtools.build.lib.syntax.SkylarkList;
@@ -154,7 +155,8 @@ public class JavaSkylarkCommon {
                 "A list or set of output source jars that contain the uncompiled source files "
                     + "including the source files generated by annotation processors if the case.")
       },
-      useLocation = true)
+      useLocation = true,
+      useEnvironment = true)
   public JavaInfo create(
       @Nullable Object actionsUnchecked,
       Object compileTimeJars,
@@ -164,9 +166,16 @@ public JavaInfo create(
       Object transitiveCompileTimeJars,
       Object transitiveRuntimeJars,
       Object sourceJars,
-      Location location)
+      Location location,
+      Environment environment)
       throws EvalException {
-
+    if (environment.getSemantics().incompatibleDisallowLegacyJavaInfo()) {
+      throw new EvalException(
+          location,
+          "create_provider is deprecated and cannot be used when "
+              + "--incompatible_disallow_legacy_javainfo is set. "
+              + "Please migrate to the JavaInfo constructor.");
+    }
     return JavaInfoBuildHelper.getInstance()
         .create(
             actionsUnchecked,

diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSemantics.java b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSemantics.java
@@ -53,6 +53,8 @@ public abstract class SkylarkSemantics {
 
   public abstract boolean incompatibleDisallowFileType();
 
+  public abstract boolean incompatibleDisallowLegacyJavaInfo();
+
   public abstract boolean incompatibleDisallowOldStyleArgsAdd();
 
   public abstract boolean incompatibleDisallowSlashOperator();
@@ -91,6 +93,7 @@ public static Builder builderWithDefaults() {
           .incompatibleDisableObjcProviderResources(false)
           .incompatibleDisallowDictPlus(false)
           .incompatibleDisallowFileType(false)
+          .incompatibleDisallowLegacyJavaInfo(false)
           .incompatibleDisallowOldStyleArgsAdd(false)
           .incompatibleDisallowSlashOperator(false)
           .incompatibleNewActionsApi(false)
@@ -120,6 +123,8 @@ public abstract static class Builder {
 
     public abstract Builder incompatibleDisallowFileType(boolean value);
 
+    public abstract Builder incompatibleDisallowLegacyJavaInfo(boolean value);
+
     public abstract Builder incompatibleDisallowOldStyleArgsAdd(boolean value);
 
     public abstract Builder incompatibleDisallowSlashOperator(boolean value);

diff --git a/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java b/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java
@@ -126,6 +126,7 @@ private static SkylarkSemanticsOptions buildRandomOptions(Random rand) throws Ex
         "--incompatible_disable_objc_provider_resources=" + rand.nextBoolean(),
         "--incompatible_disallow_dict_plus=" + rand.nextBoolean(),
         "--incompatible_disallow_filetype=" + rand.nextBoolean(),
+        "--incompatible_disallow_legacy_javainfo=" + rand.nextBoolean(),
         "--incompatible_disallow_old_style_args_add=" + rand.nextBoolean(),
         "--incompatible_disallow_slash_operator=" + rand.nextBoolean(),
         "--incompatible_new_actions_api=" + rand.nextBoolean(),
@@ -150,6 +151,7 @@ private static SkylarkSemantics buildRandomSemantics(Random rand) {
         .incompatibleDisableObjcProviderResources(rand.nextBoolean())
         .incompatibleDisallowDictPlus(rand.nextBoolean())
         .incompatibleDisallowFileType(rand.nextBoolean())
+        .incompatibleDisallowLegacyJavaInfo(rand.nextBoolean())
         .incompatibleDisallowOldStyleArgsAdd(rand.nextBoolean())
         .incompatibleDisallowSlashOperator(rand.nextBoolean())
         .incompatibleNewActionsApi(rand.nextBoolean())

diff --git a/src/test/java/com/google/devtools/build/lib/rules/java/JavaInfoSkylarkApiTest.java b/src/test/java/com/google/devtools/build/lib/rules/java/JavaInfoSkylarkApiTest.java
@@ -24,6 +24,7 @@
 import com.google.devtools.build.lib.packages.SkylarkProvider.SkylarkKey;
 import com.google.devtools.build.lib.rules.java.JavaRuleOutputJarsProvider.OutputJar;
 import com.google.devtools.build.lib.testutil.TestConstants;
+import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.Parameterized;
@@ -47,6 +48,13 @@ public JavaInfoSkylarkApiTest(boolean legacyJavaInfoConstructor) {
     this.legacyJavaInfoConstructor = legacyJavaInfoConstructor;
   }
 
+  @Before
+  public void setIncompatibleFlag() throws Exception {
+    if (legacyJavaInfoConstructor) {
+      setSkylarkSemanticsOptions("--noincompatible_disallow_legacy_javainfo");
+    }
+  }
+
   @Test
   public void buildHelperCreateJavaInfoWithOutputJarOnly() throws Exception {
     ruleBuilder().build();
@@ -677,6 +685,32 @@ public void testMixMatchNewAndLegacyArgsIsError() throws Exception {
         "my_rule(name = 'my_skylark_rule')");
   }
 
+  @Test
+  public void testIncompatibleDisallowLegacyJavaInfo() throws Exception {
+    setSkylarkSemanticsOptions("--incompatible_disallow_legacy_javainfo");
+    ImmutableList.Builder<String> lines = ImmutableList.builder();
+    lines.add(
+        "result = provider()",
+        "def _impl(ctx):",
+        "  output_jar = ctx.actions.declare_file('output_jar')",
+        "  source_jar = ctx.actions.declare_file('source_jar')",
+        "  javaInfo = JavaInfo(",
+        "    output_jar = output_jar,",
+        "    source_jars = [source_jar],", // No longer allowed
+        "  )",
+        "  return [result(property = javaInfo)]",
+        "my_rule = rule(",
+        "  implementation = _impl,",
+        ")");
+    scratch.file("foo/extension.bzl", lines.build().toArray(new String[] {}));
+    checkError(
+        "foo",
+        "my_skylark_rule",
+        "Cannot use deprecated argument when --incompatible_disallow_legacy_javainfo is set. ",
+        "load(':extension.bzl', 'my_rule')",
+        "my_rule(name = 'my_skylark_rule')");
+  }
+
   private RuleBuilder ruleBuilder() {
     return new RuleBuilder();
   }

diff --git a/src/test/java/com/google/devtools/build/lib/rules/java/JavaSkylarkApiTest.java b/src/test/java/com/google/devtools/build/lib/rules/java/JavaSkylarkApiTest.java
@@ -1642,6 +1642,33 @@ public void javaToolchainFlag_set() throws Exception {
     assertThat(javaToolchainLabel.toString()).isEqualTo("//java/com/google/test:toolchain");
   }
 
+  @Test
+  public void testIncompatibleDisallowLegacyJavaInfo() throws Exception {
+    setSkylarkSemanticsOptions("--incompatible_disallow_legacy_javainfo");
+    scratch.file(
+        "java/test/custom_rule.bzl",
+        "def _impl(ctx):",
+        "  jar = ctx.actions.declare_file('jar')",
+        "  java_common.create_provider(",
+        "      compile_time_jars = [jar],",
+        "      transitive_compile_time_jars = [jar],",
+        "      runtime_jars = [jar],",
+        "      use_ijar = False,",
+        "  )",
+        "java_custom_library = rule(",
+        "  implementation = _impl,",
+        ")");
+    checkError(
+        "java/test",
+        "custom",
+        "create_provider is deprecated and cannot be used when "
+            + "--incompatible_disallow_legacy_javainfo is set. ",
+        "load(':custom_rule.bzl', 'java_custom_library')",
+        "java_custom_library(",
+        "  name = 'custom',",
+        ")");
+  }
+
   private static boolean javaCompilationArgsHaveTheSameParent(
       JavaCompilationArgsProvider args, JavaCompilationArgsProvider otherArgs) {
     if (!nestedSetsOfArtifactHaveTheSameParent(