Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

incompatible_disallow_legacy_javainfo: Remove legacy (deprecated) JavaInfo constructors #5821

Closed
c-parsons opened this issue Aug 8, 2018 · 28 comments
Assignees
Labels
incompatible-change Incompatible/breaking change P2 We'll consider working on this in future. (Assignee optional) team-Rules-Java Issues for Java rules

Comments

@c-parsons
Copy link
Contributor

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

@ittaiz
Copy link
Member

ittaiz commented Aug 13, 2018 via email

@c-parsons
Copy link
Contributor Author

Any context on the blockers here? (Is there another github issue?)

@laurentlb laurentlb added P2 We'll consider working on this in future. (Assignee optional) team-Starlark incompatible-change Incompatible/breaking change team-Rules-Java Issues for Java rules and removed category: extensibility > skylark labels Nov 21, 2018
@laurentlb
Copy link
Contributor

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)

bazel-io pushed a commit that referenced this issue Jan 7, 2019
Progress on #5821

RELNOTES: There is a new flag available `--experimental_java_common_create_provider_enabled_packages` that acts as a whitelist for usages of `java_common.create_provider`. The constructor will be deprecated in Bazel 0.23.
PiperOrigin-RevId: 228164706
@laurentlb
Copy link
Contributor

laurentlb commented Jan 7, 2019

@iirina Can you help rules_scala and rules_kotlin migrate?

@iirina
Copy link
Contributor

iirina commented Jan 8, 2019

@laurentlb yes. @ittaiz What can I do to help rules_scala?

@brown
Copy link
Contributor

brown commented Jan 25, 2019

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.

@ittaiz
Copy link
Member

ittaiz commented Jan 26, 2019 via email

@brown
Copy link
Contributor

brown commented Jan 27, 2019

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.

@brown
Copy link
Contributor

brown commented Jan 28, 2019 via email

@ittaiz
Copy link
Member

ittaiz commented Jan 28, 2019 via email

@brown
Copy link
Contributor

brown commented Mar 15, 2019

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

@iirina
Copy link
Contributor

iirina commented Mar 18, 2019

@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.

@iirina
Copy link
Contributor

iirina commented Mar 18, 2019

You can control what ends-up on the runtime classpath using JavaInfo(neverlink = ...). I'm not sure why you need to control the compilation classpath as well.

@brown
Copy link
Contributor

brown commented Mar 21, 2019

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.

@brown
Copy link
Contributor

brown commented Mar 21, 2019

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.

@dkelmer
Copy link
Contributor

dkelmer commented Apr 3, 2019

This wasn't flipped in time for 0.25. Changing label to 0.26.

@lfpino lfpino removed their assignment May 9, 2019
@iirina
Copy link
Contributor

iirina commented May 21, 2019

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.

@iirina
Copy link
Contributor

iirina commented May 21, 2019

@ittaiz What's the status on the scala rules? Is there anything I can help with to migrate to the JavaInfo constructor?

@brown
Copy link
Contributor

brown commented May 29, 2019

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.

@dslomov
Copy link
Contributor

dslomov commented Jun 7, 2019

What is the migration status for this issue?

@hlopko
Copy link
Member

hlopko commented Jun 7, 2019

@dslomov am I just 5 minutes late on every issue? :)

@c-parsons
Copy link
Contributor Author

@iirina @ittaiz do you have any context here? I'm unsure of what remains to be done for rules scala

@hlopko hlopko changed the title Remove legacy (deprecated) JavaInfo constructors incompatible_disallow_legacy_javainfo: Remove legacy (deprecated) JavaInfo constructors Jun 13, 2019
@hlopko
Copy link
Member

hlopko commented Jun 18, 2019

Friendly ping @ittaiz :)

@iirina
Copy link
Contributor

iirina commented Aug 5, 2019

@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)

@dslomov
Copy link
Contributor

dslomov commented Aug 5, 2019

Is there migration tooling available/possible? How bad is the breakage?

@iirina
Copy link
Contributor

iirina commented Aug 6, 2019

@dslomov No, the migration is different from use case to use case.

I migrated the internal projects and also rules_scala in bazelbuild/rules_scala#781. There is an open PR for rules_kotlin in bazelbuild/rules_kotlin#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.

@brown
Copy link
Contributor

brown commented Aug 14, 2019 via email

@iirina
Copy link
Contributor

iirina commented Aug 21, 2019

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

bazel-io pushed a commit that referenced this issue Aug 21, 2019
as part of removing the code for --incompatible_disallow_legacy_javainfo.

See #5821.

RELNOTES: Deprecated Java-Starlark API java_common.create_provider is removed. JavaInfo() legacy args (actions, sources, source_jars, use_ijar, java_toolchain, host_javabase) are removed.
PiperOrigin-RevId: 264559900
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
incompatible-change Incompatible/breaking change P2 We'll consider working on this in future. (Assignee optional) team-Rules-Java Issues for Java rules
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants