pubref / rules_kotlin

Bazel rules for Kotlin
Other
159 stars 20 forks source link

_deploy.jar does not include transitive kotlin classes via kotlin classes #52

Open trevorsummerssmith opened 6 years ago

trevorsummerssmith commented 6 years ago

Issue: When using the kotlin_binary rule to create an uberjar, one expects that direct and transitive dependencies are included. It appears that direct kotlin classes are included, but transitive kotlin classes that are transitive via a kotlin class are not. However, kotlin transitive dependencies that are transitive via a java class are included.

Disclaimer: I am new to both bazel and kotlin so likely this is user error.

Reproduce: Using a modification of rules_kotlin examples see https://github.com/trevorsummerssmith/rules_kotlin/commit/2b9cb1bc106645a56d827e1034dfbebf8cd8c7c2

We have a new Foo kotlin class. Rules (kotlin class) is modified to depend upon Foo. Main should now transitively depend upon foo.

bazel build examples/helloworld:main_kt_deploy.jar
# we expect the following to run but it does not
> java -jar bazel-bin/examples/helloworld/main_kt_deploy.jar
Exception in thread "main" java.lang.NoClassDefFoundError: examples/helloworld/Foo
    at examples.helloworld.KotlinBinaryRule.<init>(rules.kt:14)
    at examples.helloworld.MainKt.main(main.kt:11)
Caused by: java.lang.ClassNotFoundException: examples.helloworld.Foo
    at java.base/jdk.internal.loader.BuiltinClassLoader.loadClass(BuiltinClassLoader.java:582)
    at java.base/jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass(ClassLoaders.java:185)
    at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:496)
    ... 2 more

# Do a local modification to make Milk (a java target) depend upon Foo
# and now it works!
> java -jar bazel-bin/examples/helloworld/main_kt_deploy.jar
Foo(name=foo)
I am Kotlin! ......
... But what is soy milk?
What if soy milk is just regular milk introducing itself in Spanish?

I am happy to do some more digging here if someone can point me in the right direction.

Hypothesis (this could be totally wrong): I saw #47 and locally changed my rules.bzl to use transitive_compile_time_jars instead of full_compile_jars and then built the examples with --nouse_ijars, thinking that this would give the full transitive dependencies. However I saw no change in behavior.

trevorsummerssmith commented 6 years ago

@pcj and @hsyed I am curious if either of you have thoughts on this that could point me in a direction of a fix, or tell me I am doing something wrong? Thanks!

hsyed commented 6 years ago

@trevorsummerssmith The documented rules are currently implemented as macros. kotlin_binary, kotlin_library and kotlin_test all forward to the real rule kotlin_compile. I found this to be problematic dependency wise and I'm rewriting the rules now so that they are all first class self contained rules like like the native java rules -- modelling this work on the Scala rules.

I would recommend using the kotlin_library rule to setup your Kotlin compilations and then to build binaries and tests using java_binary and java_test. From the kotlin_library rule use the _kt output -- this strategy lead to behaviour that works predictably for me.

pcj should be able to help you debug your issue.

trevorsummerssmith commented 6 years ago

@hsyed thank you for your reply! I tried what I understand your suggestion to be: use the java_binary rule instead of kotlin_binary on the example I gave above. I pushed the code here - https://github.com/trevorsummerssmith/rules_kotlin/commit/1cf13531e38db64162800bfabb6c883a7883fdb4

Unfortunately that does not work for me either:

> bazel build --nouse_ijars examples/helloworld:main_kt_java_deploy.jar
# ... 
> java -jar bazel-bin/examples/helloworld/main_kt_java_deploy.jar
Exception in thread "main" java.lang.NoClassDefFoundError: examples/helloworld/KotlinBinaryRule
        at examples.helloworld.MainKt.main(main.kt:11)
Caused by: java.lang.ClassNotFoundException: examples.helloworld.KotlinBinaryRule
        at java.base/jdk.internal.loader.BuiltinClassLoader.loadClass(BuiltinClassLoader.java:582)
        at java.base/jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass(ClassLoaders.java:185)
        at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:496)
        ... 1 more

We don't get the KotlinBinaryRule class because it is a transitive dependency via our kotlin library rule now. This does show another variant of this bug that I did not previously test.

I did more reading on the macro vs rule distinction and what you say makes sense to my rudimentary understanding of bazel. Is this something you are actively working on? / Do you need any help?

trevorsummerssmith commented 6 years ago

@hsyed Happy new year! I am curious if you are currently working on the port of macros -> rules? Thank you.

hsyed commented 6 years ago

@trevorsummerssmith yes I have completely forked the rules alongside porting the new bazel intelij plugin extension I have been working on (these haven't been merged in yet). The fork is available here. It's a barebones rework and the documentation is minimal. We have enabled it in our mono repo and it work well enough.

Happy to get feedback !

trevorsummerssmith commented 6 years ago

@hsyed ok good to know. I will take a look at what you have going on this week.