Repository: bazelbuild/bazel
Issue: incompatible_disallow_legacy_javainfo: Remove legacy (deprecated) JavaInfo constructors
Labels: P2
team-Rules-Java
incompatible-change

This is a tracking issue for offering a migration solution for
`--incompatible_disallow_legacy_javainfo`




Issue comments:
@iirina,
have we removed the blockers from this?
We still haven't been able to move rules_scala to the new one AFAIR and
unfortunately due to the strict-deps false negative we're also stuck with a
rather old rules_scala version

On Thu, Aug 9, 2018 at 1:32 AM c-parsons <notifications@github.com> wrote:

> This is a tracking issue for offering a migration solution for
> --incompatible_disallow_legacy_javainfo
>
> —
> You are receiving this because you are subscribed to this thread.
> Reply to this email directly, view it on GitHub
> <https://github.com/bazelbuild/bazel/issues/5821>, or mute the thread
> <https://github.com/notifications/unsubscribe-auth/ABUIF8onFHTeQvFHV5_D19wEkgVhoAEpks5uO2bmgaJpZM4V0xLv>
> .
>

Any context on the blockers here? (Is there another github issue?)
I got this error message:
> create_provider is deprecated and cannot be used when --incompatible_disallow_legacy_javainfo is set. Please migrate to the JavaInfo constructor.

Documentation is not great (https://docs.bazel.build/versions/master/skylark/lib/java_common.html#create_provider doesn't even say it's deprecated) and it's not clear how to migrate the existing code.

(cc @lberki)
@iirina Can you help `rules_scala` and `rules_kotlin` migrate?
@laurentlb yes. @ittaiz What can I do to help `rules_scala`?
I use the deprecated create_provider API as follows:

```
new_providers.append(
    java_common.create_provider(
        use_ijar = False,
        compile_time_jars = provider.compile_jars,
        runtime_jars = provider.runtime_output_jars,
        transitive_compile_time_jars = compile_time_jars,
        transitive_runtime_jars = runtime_jars,
        ),
    )
```

in order to remove specific jars from the compile and runtime classpath.  I do not believe I can implement the same functionality with the JavaInfo constructor.  If it's possible, please tell me how so that I can port my code before the deprecated create_provider is removed from Bazel.  Thanks.

Can you say a bit more why you need to remove specific jars? Maybe it will
help think what you can do
On Fri, 25 Jan 2019 at 21:14 Robert Brown <notifications@github.com> wrote:

> I use the deprecated create_provider API as follows:
>
> new_providers.append(
> java_common.create_provider(
> use_ijar = False,
> compile_time_jars = provider.compile_jars,
> runtime_jars = provider.runtime_output_jars,
> transitive_compile_time_jars = compile_time_jars,
> transitive_runtime_jars = runtime_jars,
> ),
> )
>
> in order to remove specific jars from the compile and runtime classpath. I
> do not believe I can implement the same functionality with the JavaInfo
> constructor. If it's possible, please tell me how so that I can port my
> code before the deprecated create_provider is removed from Bazel. Thanks.
>
> —
> You are receiving this because you were mentioned.
>
>
> Reply to this email directly, view it on GitHub
> <https://github.com/bazelbuild/bazel/issues/5821#issuecomment-457690483>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/ABUIFwJRigGu2m6aO1-PQEevHBWv0nNzks5vG1eRgaJpZM4V0xLv>
> .
>

Java developers expect to be able to override the library dependencies of the libraries their code depends upon.  For instance, suppose my code depends on two libraries, lib1 and lib2.  The lib1 target has a dependency on guava22 and the lib2 target depends on guava25.  I want to use both in my application.

My Starlark code implements a rule called java_exclude_library that removes jars from the compilation and run-time Java classpaths.  You use it like this:
```
java_exclude_library(
    name = "lib1_without_guava",
    excludes = [
        "//third_party/com.google.guava/guava22",
    ],
    deps = [
        ":lib1",
    ],
)
```
to create lib1_without_guava from java_library lib1.  The lib1_without_guava target is identical to lib1, except that the jar for guava22 does not appear on its compilation and run-time classpath.  My application depends on targets lib1_without_guava and lib2.  All the code in my application ends up using guava25 at run time.

I added a comment to the issue explaining why I need a JavaInfo constructor
that allows control over classpath jars.

On Sat, Jan 26, 2019 at 3:59 PM Ittai Zeidman <notifications@github.com>
wrote:

> Can you say a bit more why you need to remove specific jars? Maybe it will
> help think what you can do
> On Fri, 25 Jan 2019 at 21:14 Robert Brown <notifications@github.com>
> wrote:
>
> > I use the deprecated create_provider API as follows:
> >
> > new_providers.append(
> > java_common.create_provider(
> > use_ijar = False,
> > compile_time_jars = provider.compile_jars,
> > runtime_jars = provider.runtime_output_jars,
> > transitive_compile_time_jars = compile_time_jars,
> > transitive_runtime_jars = runtime_jars,
> > ),
> > )
> >
> > in order to remove specific jars from the compile and runtime classpath.
> I
> > do not believe I can implement the same functionality with the JavaInfo
> > constructor. If it's possible, please tell me how so that I can port my
> > code before the deprecated create_provider is removed from Bazel. Thanks.
> >
> > —
> > You are receiving this because you were mentioned.
> >
> >
> > Reply to this email directly, view it on GitHub
> > <https://github.com/bazelbuild/bazel/issues/5821#issuecomment-457690483
> >,
> > or mute the thread
> > <
> https://github.com/notifications/unsubscribe-auth/ABUIFwJRigGu2m6aO1-PQEevHBWv0nNzks5vG1eRgaJpZM4V0xLv
> >
> > .
> >
>
> —
> You are receiving this because you commented.
> Reply to this email directly, view it on GitHub
> <https://github.com/bazelbuild/bazel/issues/5821#issuecomment-457865518>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/AABqsUwdYr11YsbJKpNQSbwh24N7HKIaks5vHMGrgaJpZM4V0xLv>
> .
>

👍🏽
I think maybe the discussion on the specific (and interesting) use case
should continue on a separate issue but I’ll leave that decision to @iirina
and @lberki
On Mon, 28 Jan 2019 at 18:16 Robert Brown <notifications@github.com> wrote:

> I added a comment to the issue explaining why I need a JavaInfo constructor
> that allows control over classpath jars.
>
> On Sat, Jan 26, 2019 at 3:59 PM Ittai Zeidman <notifications@github.com>
> wrote:
>
> > Can you say a bit more why you need to remove specific jars? Maybe it
> will
> > help think what you can do
> > On Fri, 25 Jan 2019 at 21:14 Robert Brown <notifications@github.com>
> > wrote:
> >
> > > I use the deprecated create_provider API as follows:
> > >
> > > new_providers.append(
> > > java_common.create_provider(
> > > use_ijar = False,
> > > compile_time_jars = provider.compile_jars,
> > > runtime_jars = provider.runtime_output_jars,
> > > transitive_compile_time_jars = compile_time_jars,
> > > transitive_runtime_jars = runtime_jars,
> > > ),
> > > )
> > >
> > > in order to remove specific jars from the compile and runtime
> classpath.
> > I
> > > do not believe I can implement the same functionality with the JavaInfo
> > > constructor. If it's possible, please tell me how so that I can port my
> > > code before the deprecated create_provider is removed from Bazel.
> Thanks.
> > >
> > > —
> > > You are receiving this because you were mentioned.
> > >
> > >
> > > Reply to this email directly, view it on GitHub
> > > <
> https://github.com/bazelbuild/bazel/issues/5821#issuecomment-457690483
> > >,
> > > or mute the thread
> > > <
> >
> https://github.com/notifications/unsubscribe-auth/ABUIFwJRigGu2m6aO1-PQEevHBWv0nNzks5vG1eRgaJpZM4V0xLv
> > >
> > > .
> > >
> >
> > —
> > You are receiving this because you commented.
> > Reply to this email directly, view it on GitHub
> > <https://github.com/bazelbuild/bazel/issues/5821#issuecomment-457865518
> >,
> > or mute the thread
> > <
> https://github.com/notifications/unsubscribe-auth/AABqsUwdYr11YsbJKpNQSbwh24N7HKIaks5vHMGrgaJpZM4V0xLv
> >
> > .
> >
>
> —
> You are receiving this because you were mentioned.
>
>
> Reply to this email directly, view it on GitHub
> <https://github.com/bazelbuild/bazel/issues/5821#issuecomment-458195838>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/ABUIF0vDWtHoEvQKhu3NUp5ya3uuHjgjks5vHyJggaJpZM4V0xLv>
> .
>

Here is a link to the ts_java_exclude_library Starlark rule that uses java_common.create_provider.
I do not believe this rule can be implemented once the create_provider API is removed.

https://github.com/twosigma/bazel-rules/blob/master/ts_java_exclude.bzl

@brown Thanks for letting me know. Can you explain briefly what the rule is trying to accomplish, especially the method `_create_exclude_providers`? I'm trying to understand your use case and come up with a solution.


You can control what ends-up on the runtime classpath using [JavaInfo(neverlink = ...)](https://docs.bazel.build/versions/master/skylark/lib/JavaInfo.html#JavaInfo). I'm not sure why you need to control the compilation classpath as well.
I don't think the neverlink parameter to JavaInfo helps.

The goal is to create a Java library from other Java libraries, but with some transitive dependencies removed from the run-time class path.

Let say one group develops Java library1, which has many transitive dependencies including a java_import of Guava version 19. 

A second group develops Java library2, which has many transitive dependencies including a java_import of Guava version 20.

I want to create a Java binary that depends on both library1 and library2, but if my Java code depends on both libraries, then both versions of Guava will be on my application's class path, which can cause problems.

I can use the ts_java_exclude_library rule to make a new library that combines both library1 and library2, but excludes Guava 19:

    ts_java_exclude_library(
        name = "libraries_without_guava19",
        excludes = [ "//ext/public/google/guava/19/0:jars" ],
        deps = [
            ":library1",
            ":library2",
        ],
    )

Above, the excluded target is the java_import of Guava 19 jar files that library1 transitively depends on.

My Java binary depends on libraries_without_guava19.  It includes library1 and its dependencies, which have been compiled with Guava 19.  It also includes library2 and its dependencies, which have been compiled with Guava 20.  Both library1 and library2 use Guava 20 when my binary runs, because Guava 19 has been excluded from the class path.

The _create_exclude_providers function iterates through a list of Java Providers.  For each Provider it creates a new one that's identical to the original, except when specific jar files occur in the original's run-time or compile-time class paths.  In that case the new Provider lacks the jars in its class paths.

The code may not need to remove jars from the compile-time class path.

This wasn't flipped in time for 0.25. Changing label to 0.26. 
> The goal is to create a Java library from other Java libraries, but with some transitive dependencies removed from the run-time class path.

This is what `neverlink` is supposed to do, and you can use it on any kind of java rule targets. Assuming the guava targets defined via java_import(name = 'guava19',...) and java_import(name = 'guava20',...), you can set  `neverlink = true` on guava19 and it won't end up on the runtime classpath. Can you think of disadvantages of always setting a java_import/java_library as neverlink in your project? I'm more inclined towards a solution with higher abstraction rather than cherry picking classpath jars.
@ittaiz What's the status on the scala rules? Is there anything I can help with to migrate to the JavaInfo constructor?
I too would be happier with a API that does not involve directly modifying Java class paths, but I currently don't have any good ideas for implementing an exclude feature without the ability to manipulate class path lists.

Two Sigma has a large monorepo containing thousands of external Java libraries, most of which have been imported from Maven.  The Maven dependencies between packages are programmatically converted into dependencies between Bazel java_import libraries.  With these dependencies a Two Sigma library can depend on an external Java package and all of that package's run time needs appear on an application's Java class path.  If some external libraries have neverlink set to True, then libraries and/or binaries that depend on them will have to use runtime_deps to add back required run time dependencies.  Managing runtime_deps lists with hundreds of entries would be a nightmare.

Let's say I took your suggestion and set neverlink to True for the java_import of library guava19.  That could be used to remove guava19 from my application's or library's Java class path.  But what about all the other binaries and libraries in my monorepo that prefer to use guava19?  I have to track down those Bazel targets and add guava19 to the runtime_deps of binaries and/or libraries.

The deprecated JavaInfo constructor is the only Bazel API that allows a rule to remove a dependency from the Java class path once it has been added by a transitive dependency.  In the Two Sigma monorepo there are hundreds of instances where a jar is excluded from a Java class path.  Google doesn't encounter this problem primarily because of its one version policy for external library dependencies.  In a company with a monorepo containing several versions of many external libraries, the ability to depend on an internal library but selectively exclude some of its transitive external dependencies is valuable.

What is the migration status for this issue?
@dslomov am I just 5 minutes late on every issue? :)
@iirina @ittaiz do you have any context here? I'm unsure of what remains to be done for rules scala
Friendly ping @ittaiz :)
@brown The flag needs to be flipped in Bazel 1.0. I put together some snippets to help you migrate your use case and still be able to exclude some jars. Please let me know how I can help further.


```
# after computing runtime_jars and compile_time_jars

transitive_compile_providers = [JavaInfo(
    output_jar = compile_time_jar,
    compile_jar = compile_time_jar,
    neverlink = True,
)
for compile_time_jar in compile_time_jars]

transitive_runtime_providers = [JavaInfo(
    output_jar = runtime_jar,
    compile_jar = runtime_jar,
)
for runtime_jar in runtime_jars]

compile_providers = [JavaInfo(
    output_jar = compile_jar,
    compile_jar = compile_jar,
    neverlink = True,
)
for compile_jar in provider.compile_jars]

runtime_providers = [JavaInfo(
    output_jar = runtime_jar,
    compile_jar = runtime_jar,
    deps = transitive_runtime_providers,
)
for runtime_jar in provider.runtime_output_jars]

new_providers.append(java_common.merge(compile_providers + runtime_providers)
```
Is there migration tooling available/possible? How bad is the breakage?
@dslomov No, the migration is different from use case to use case. 

I migrated the internal projects and also `rules_scala` in https://github.com/bazelbuild/rules_scala/pull/781. There is an open PR for `rules_kotlin` in https://github.com/bazelbuild/rules_kotlin/pull/197, and one internal open change for migrating intellij. 
Other than that, `rules_jvm_external` have to upgrade their `rules_scala` and `rules_kotlin` versions (cc @jin). No other breakages on our CI, but I don't know if projects that are not on BazelCI have migrated.
Thanks very much for the snippet of code.  It seems to work well.
I will patch the library rule.  Thanks again!


On Mon, Aug 5, 2019 at 10:40 AM Irina Iancu <notifications@github.com>
wrote:

> @brown <https://github.com/brown> The flag needs to be flipped in Bazel
> 1.0. I put together some snippets to help you migrate your use case and
> still be able to exclude some jars. Please let me know how I can help
> further.
>
> # after computing runtime_jars and compile_time_jars
>
> transitive_compile_providers = [JavaInfo(
>     output_jar = compile_time_jar,
>     compile_jar = compile_time_jar,
>     neverlink = True,
> )
> for compile_time_jar in compile_time_jars]
>
> transitive_runtime_providers = [JavaInfo(
>     output_jar = runtime_jar,
>     compile_jar = runtime_jar,
> )
> for runtime_jar in runtime_jars]
>
> compile_providers = [JavaInfo(
>     output_jar = compile_jar,
>     compile_jar = compile_jar,
>     neverlink = True,
> )
> for compile_jar in provider.compile_jars]
>
> runtime_providers = [JavaInfo(
>     output_jar = runtime_jar,
>     compile_jar = runtime_jar,
>     deps = transitive_runtime_providers,
> )
> for runtime_jar in provider.runtime_output_jars]
>
> new_providers.append(java_common.merge(compile_providers + runtime_providers)
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/bazelbuild/bazel/issues/5821?email_source=notifications&email_token=AAAGVMO5U7QV5EVP3IR3V5TQDA3VJA5CNFSM4FOTCLX2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD3SA3BY#issuecomment-518262151>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/AAAGVMOUKW3FOVV4ZLVKETTQDA3VJANCNFSM4FOTCLXQ>
> .
>

@brown That's great to hear, thanks for trying it out!

Pull request review comments:

Patch that resolved the issue:
diff --git a/src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java b/src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java
index 8f8fe4dcb2b852..a7c881250024c4 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java
@@ -361,7 +361,7 @@ public class StarlarkSemanticsOptions extends OptionsBase implements Serializabl
 
   @Option(
       name = "incompatible_disallow_legacy_javainfo",
-      defaultValue = "false",
+      defaultValue = "true",
       documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS,
       effectTags = {OptionEffectTag.BUILD_FILE_SEMANTICS},
       metadataTags = {
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java b/src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java
index bf55bdf3087e98..cbd85b8acc1c13 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java
@@ -269,7 +269,7 @@ public static Builder builderWithDefaults() {
           .incompatibleDisallowDictPlus(true)
           .incompatibleDisallowEmptyGlob(false)
           .incompatibleDisallowLegacyJavaProvider(false)
-          .incompatibleDisallowLegacyJavaInfo(false)
+          .incompatibleDisallowLegacyJavaInfo(true)
           .incompatibleDisallowOldStyleArgsAdd(true)
           .incompatibleDisallowRuleExecutionPlatformConstraintsAllowed(true)
           .incompatibleDisallowStructProviderSyntax(false)


Test patch:
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
index acf7976ee2d227..7ac2224737ff97 100644
--- 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
@@ -693,32 +693,6 @@ public void buildHelperCreateJavaInfoWithJdeps_javaRuleOutputJarsProvider() thro
     assertThat(ruleOutputs.getJdeps().prettyPrint()).isEqualTo("foo/my_jdeps.pb");
   }
 
-  @Test
-  public void testMixMatchNewAndLegacyArgsIsError() throws Exception {
-    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_jar = source_jar,",
-        "    source_jars = [source_jar],",
-        "  )",
-        "  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 arguments at the same time",
-        "load(':extension.bzl', 'my_rule')",
-        "my_rule(name = 'my_skylark_rule')");
-  }
-
   @Test
   public void testIncompatibleDisallowLegacyJavaInfo() throws Exception {
     setSkylarkSemanticsOptions("--incompatible_disallow_legacy_javainfo");
@@ -740,7 +714,7 @@ public void testIncompatibleDisallowLegacyJavaInfo() throws Exception {
     checkError(
         "foo",
         "my_skylark_rule",
-        "Cannot use deprecated argument when --incompatible_disallow_legacy_javainfo is set. ",
+        "Cannot use deprecated argument when --incompatible_disallow_legacy_javainfo is set",
         "load(':extension.bzl', 'my_rule')",
         "my_rule(name = 'my_skylark_rule')");
   }
diff --git a/src/test/shell/bazel/bazel_java_test.sh b/src/test/shell/bazel/bazel_java_test.sh
index e42556c7a92166..d2af85c4f71959 100755
--- a/src/test/shell/bazel/bazel_java_test.sh
+++ b/src/test/shell/bazel/bazel_java_test.sh
@@ -1424,7 +1424,7 @@ def _impl(ctx):
     host_javabase = ctx.attr._host_javabase[java_common.JavaRuntimeInfo],
   )
 
-  imported_provider = JavaInfo(output_jar = imported_jar, use_ijar=False);
+  imported_provider = JavaInfo(output_jar = imported_jar, compile_jar = imported_jar);
 
   final_provider = java_common.merge([compilation_provider, imported_provider])
 
@@ -1453,81 +1453,6 @@ EOF
   expect_log "<generated file java/com/google/sandwich/libb.jar>"
 }
 
-
-
-function test_java_info_constructor_with_ijar_unset_actions() {
-  mkdir -p java/com/google/foo
-  touch java/com/google/foo/{BUILD,my_rule.bzl}
-  cat > java/com/google/foo/BUILD << EOF
-load(":my_rule.bzl", "my_rule")
-my_rule(
-  name = 'my_skylark_rule',
-  output_jar = 'my_skylark_rule_lib.jar',
-  source_jars = ['my_skylark_rule_src.jar']
- )
-EOF
-
-  cat > java/com/google/foo/my_rule.bzl << EOF
-result = provider()
-def _impl(ctx):
-  javaInfo = JavaInfo(
-    output_jar = ctx.file.output_jar,
-    source_jars = ctx.files.source_jars,
-    use_ijar = True,
-    java_toolchain = ctx.attr._java_toolchain[java_common.JavaToolchainInfo]
-  )
-  return [result(property = javaInfo)]
-
-my_rule = rule(
-  implementation = _impl,
-  attrs = {
-    'output_jar' : attr.label(allow_single_file=True),
-    'source_jars' : attr.label_list(allow_files=['.jar']),
-    "_java_toolchain": attr.label(default = Label("@bazel_tools//tools/jdk:remote_toolchain"))
-  }
-)
-EOF
-
-  bazel build java/com/google/foo:my_skylark_rule >& "$TEST_log" && fail "Unexpected success"
-  expect_log "The value of use_ijar is True. Make sure the ctx.actions argument is valid."
-}
-
-function test_java_info_constructor_with_ijar_unset_java_toolchain() {
-  mkdir -p java/com/google/foo
-  touch java/com/google/foo/{BUILD,my_rule.bzl}
-  cat > java/com/google/foo/BUILD << EOF
-load(":my_rule.bzl", "my_rule")
-my_rule(
-  name = 'my_skylark_rule',
-  output_jar = 'my_skylark_rule_lib.jar',
-  source_jars = ['my_skylark_rule_src.jar']
- )
-EOF
-
-  cat > java/com/google/foo/my_rule.bzl << EOF
-result = provider()
-def _impl(ctx):
-  javaInfo = JavaInfo(
-    output_jar = ctx.file.output_jar,
-    source_jars = ctx.files.source_jars,
-    use_ijar = True,
-    actions = ctx.actions
-  )
-  return [result(property = javaInfo)]
-
-my_rule = rule(
-  implementation = _impl,
-  attrs = {
-    'output_jar' : attr.label(allow_single_file=True),
-    'source_jars' : attr.label_list(allow_files=['.jar'])
-  }
-)
-EOF
-
-  bazel build java/com/google/foo:my_skylark_rule >& "$TEST_log" && fail "Unexpected success"
-  expect_log "The value of use_ijar is True. Make sure the java_toolchain argument is valid."
-}
-
 function test_java_info_constructor_e2e() {
   mkdir -p java/com/google/foo
   touch java/com/google/foo/{BUILD,my_rule.bzl}