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

Transitive dependencies missing in compilation #132

Closed
jfancher opened this issue Aug 23, 2018 · 5 comments
Closed

Transitive dependencies missing in compilation #132

jfancher opened this issue Aug 23, 2018 · 5 comments

Comments

@jfancher
Copy link

This is reported in pubref/rules_kotlin#47 with no resolution, but I don't see any discussion in the context of the new fork. It seems pretty significant, so I thought I'd raise it. Apologies if it is discussed elsewhere and I missed it.

kt_jvm_library seems to omit transitive dependencies when compiling, requiring many more things to be declared as direct deps compared to java_library.

Here are some reproducers:
https://gist.github.com/jfancher/e56cdd806c3241177267c6542943c54d
https://gist.github.com/jfancher/d9db1a94277ae898aa3c0e6e2a1e3483

In both of these, bazel build //:third fails without adding :first as a dep, due to C0 not being on the compilation classpath. In both cases the equivalent Java code and java_library declarations would be fine.

Any plans or opportunities to improve this? Is it blocked on Skylark support? bazelbuild/bazel#4549 seems related but it's not clear to me whether there's a blocker for this simple use case or just more advanced Kotlin-specific features (it seems framed as the latter).

@hsyed
Copy link
Contributor

hsyed commented Aug 23, 2018

Current state is that strict deps are emulated, proper dep analysis is required before the transitive classpath can be presented to the compiler. I am using the jdeps tool from the jdk to perform analysis and this is not suitable for Kotlin (references to inline methods, typealiases would not get registered).

The good news is the last couple of days I have been working on strict dep support. This means adapting the kotlin compiler environment and adding compiler extensions. One such extension is strict dep analysis. I am now working on the analysis code. I am not a compiler engineer so it's trial and error.

After I get kotlin to do a proper analysis it needs to be integrated with strict dep analysis performed by javac. I'll look into using the turbine compiler from bazel in place of javac if it's not too hard.

I've got wip commits that update the rules with the various strict dep modes (EMULATED , OFF , WARN, ERROR). EMULATED is what we have at the moment. The other three modes use the full transitive classpath. I might checkpoint some of this into master. When I do this the rules will pick up some tag --e.g., strict_deps_off for example and use the full classpath.

@cgruber
Copy link
Collaborator

cgruber commented Feb 27, 2019

I've just run smack into this, and it applies even to java compiled by the kotlin compiler, so it's a problem for migration. A property of JavaInfo was added for scala's strict deps analysis, which is transitive_compile_time_deps, and pivoting off that should help, as in my testing of the rxjava2 dep which requires reactive-streaming APIs (inherits from them), both jars are present in the transitive_compile_time_deps depset. (c.f. square/bazel_maven_repository#54)

cgruber added a commit to cgruber/bazel_maven_repository that referenced this issue Feb 27, 2019
…ransitive deps are present.

If B deps on A as a part of API, e.g. inheriting from a class, A must be in the compilation classpath.  This case adds a java_library example and a kt_jvm_library example of just such a case (rxjava2's Flowable and reactive-streaming's Publisher).  This succeeds in the java_library case, so it's clear bazel_maven_repository's construction of a JavaInfo is sound. It fails in kotlin (even java compiled by the kotlin rules' kotlinc invocation) because it has half-baked strict-deps support, and ends up not consuming the proper classpath. Because it fails, it's commented out with a note pointing at the issue bazelbuild/rules_kotlin#132.  This can be commented out at hte point where it's fixed.
cgruber added a commit to cgruber/bazel_maven_repository that referenced this issue Feb 27, 2019
…ransitive deps are present.

If B deps on A as a part of API, e.g. inheriting from a class, A must be in the compilation classpath.  This case adds a java_library example and a kt_jvm_library example of just such a case (rxjava2's Flowable and reactive-streaming's Publisher).  This succeeds in the java_library case, so it's clear bazel_maven_repository's construction of a JavaInfo is sound. It fails in kotlin (even java compiled by the kotlin rules' kotlinc invocation) because it has half-baked strict-deps support, and ends up not consuming the proper classpath. Because it fails, it's commented out with a note pointing at the issue bazelbuild/rules_kotlin#132.  This can be commented out at hte point where it's fixed.
@CooperBills
Copy link

Bumping up against this as well. I hit a blocker: grpc-java is structured with internal transitive dependencies that are flagged as private, so Bazel won't allow depending on them directly. I set up a small reproduction here: https://github.com/CoopeyB/bazel-kotlin-deps-bug

Looks like this specific issue was discussed over a year ago: grpc/grpc-java#4258 - @hsyed, has there been any resolution to this?

cgruber added a commit to cgruber/rules_kotlin that referenced this issue Jun 12, 2019
…path. Currently, because there's no strict deps mechanism, that means basically use the full transitive closure. This fixes (or works around) but bazelbuild#132
IljaKroonen added a commit to Selmaai/rules_kotlin that referenced this issue Jul 1, 2019
…le classpath. Currently, because there's no strict deps mechanism, that means basically use the full transitive closure. This fixes (or works around) but bazelbuild#132"

This reverts commit 2d86189.
@cgruber
Copy link
Collaborator

cgruber commented Oct 10, 2019

Fixed as of #205

@cgruber cgruber closed this as completed Oct 10, 2019
jongerrish added a commit to jongerrish/rules_kotlin that referenced this issue Apr 16, 2020
…path. Currently, because there's no strict deps mechanism, that means basically use the full transitive closure. This fixes (or works around) but bazelbuild#132
richmowd411 added a commit to richmowd411/Rules-Kotlin that referenced this issue Apr 12, 2022
…path. Currently, because there's no strict deps mechanism, that means basically use the full transitive closure. This fixes (or works around) but bazelbuild/rules_kotlin#132
@mykola-ignatiev
Copy link

It seems like it was hardcoded to be always in transitive mode.
Shouldn't it rather be similar to Java rule controlled by a flag?

https://bazel.build/reference/command-line-reference#flag--experimental_strict_java_deps

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants