No kotlin-reflect.jar on classpath for annotation processors that need it

[pettermahlen]

When trying to run moshi-kotlin-codegen through Bazel, I get the following error:

Caused by: kotlin.jvm.KotlinReflectionNotSupportedError: Kotlin reflection implementation is not found at runtime. Make sure you have kotlin-reflect.jar in the classpath
    at kotlin.jvm.internal.ClassReference.error(ClassReference.kt:79)
    at kotlin.jvm.internal.ClassReference.getQualifiedName(ClassReference.kt:15)
    at com.squareup.kotlinpoet.ClassNames.get(ClassName.kt:49)
    at com.squareup.moshi.kotlinpoet.classinspector.elements.ElementsClassInspector.<clinit>(ElementsClassInspector.kt:493)
    at com.squareup.moshi.kotlin.codegen.JsonClassCodegenProcessor.process(JsonClassCodegenProcessor.kt:99)
    at org.jetbrains.kotlin.kapt3.base.incremental.IncrementalProcessor.process(incrementalProcessors.kt)
    at org.jetbrains.kotlin.kapt3.base.ProcessorWrapper.process(annotationProcessing.kt:147)
    at jdk.compiler/com.sun.tools.javac.processing.JavacProcessingEnvironment.callProcessor(JavacProcessingEnvironment.java:980)
    ... 48 more

moshi-kotlin-codegen depends on kotlinpoet which depends on kotlin-reflect.

For a small example project manifesting the problem, see https://github.com/pettermahlen/codegen-repro.

It seems like a bug to me, but it's of course possible that I'm doing something wrong - and I'm of course also not sure that the bug is in rules_kotlin.

[pettermahlen]

I found out that I could get some more troubleshooting info, and also that I should probably exclude the old stdlib and reflect versions that were pulled in by kotlinpoet. That led to still getting the same exception, with the following info in the params file:

--classpath
bazel-out/darwin-fastbuild/bin/external/maven/v1/https/repo1.maven.org/maven2/com/squareup/moshi/moshi/1.9.2/stamped_moshi-1.9.2.jar
external/com_github_jetbrains_kotlin/lib/annotations-13.0.jar
external/com_github_jetbrains_kotlin/lib/kotlin-stdlib.jar
external/com_github_jetbrains_kotlin/lib/kotlin-stdlib-jdk7.jar
external/com_github_jetbrains_kotlin/lib/kotlin-stdlib-jdk8.jar
bazel-out/darwin-fastbuild/bin/external/maven/v1/https/repo1.maven.org/maven2/com/squareup/okio/okio/1.16.0/stamped_okio-1.16.0.jar
bazel-out/host/bin/external/maven/v1/https/repo1.maven.org/maven2/com/squareup/moshi/moshi-kotlin-codegen/1.9.2/stamped_moshi-kotlin-codegen-1.9.2.jar
bazel-out/host/bin/external/maven/v1/https/repo1.maven.org/maven2/com/squareup/kotlinpoet/1.4.4/stamped_kotlinpoet-1.4.4.jar
bazel-out/host/bin/external/maven/v1/https/repo1.maven.org/maven2/org/jetbrains/kotlin/kotlin-stdlib-jdk7/1.3.50/stamped_kotlin-stdlib-jdk7-1.3.50.jar
bazel-out/host/bin/external/maven/v1/https/repo1.maven.org/maven2/com/squareup/okio/okio/1.16.0/stamped_okio-1.16.0.jar
bazel-out/host/bin/external/maven/v1/https/repo1.maven.org/maven2/com/squareup/moshi/moshi/1.9.2/stamped_moshi-1.9.2.jar
external/com_github_jetbrains_kotlin/lib/kotlin-reflect.jar
--sources
java/com/pettermahlen/Test.kt
--processors
com.squareup.moshi.kotlin.codegen.JsonClassCodegenProcessor
--processorpath
bazel-out/host/bin/external/maven/v1/https/repo1.maven.org/maven2/com/squareup/moshi/moshi-kotlin-codegen/1.9.2/stamped_moshi-kotlin-codegen-1.9.2.jar
bazel-out/host/bin/external/maven/v1/https/repo1.maven.org/maven2/com/squareup/kotlinpoet/1.4.4/stamped_kotlinpoet-1.4.4.jar
bazel-out/host/bin/external/maven/v1/https/repo1.maven.org/maven2/org/jetbrains/kotlin/kotlin-stdlib-jdk7/1.3.50/stamped_kotlin-stdlib-jdk7-1.3.50.jar
bazel-out/host/bin/external/maven/v1/https/repo1.maven.org/maven2/com/squareup/okio/okio/1.16.0/stamped_okio-1.16.0.jar
bazel-out/host/bin/external/maven/v1/https/repo1.maven.org/maven2/com/squareup/moshi/moshi/1.9.2/stamped_moshi-1.9.2.jar
external/com_github_jetbrains_kotlin/lib/kotlin-reflect.jar

That looks a lot like the kotlin-reflect.jar file should be present. This is from adding the following to the maven_install command:

    excluded_artifacts = [
       "org.jetbrains.kotlin:kotlin-reflect",
       "org.jetbrains.kotlin:kotlin-stdlib",
    ],

And the following to the deps section of the moshi_kotlin_codegen/BUILD.bazel file:

      "@com_github_jetbrains_kotlin//:kotlin-reflect",

I will be pushing this to the repro repo since I think it's a better config, even if it's still breaking in the same way.

[pettermahlen]

I've add a pom.xml to the repro repo, and Maven is perfectly capable of compiling the code and generating the expected classes. Maven provides a lot of output about how it configures kapt3 when run with mvn -X compile, but I've not been able to figure out what the difference is. I suppose there could be a difference in how arguments are passed to kotlinc, (just going on the comment in KaptCompilerPluginArgsEncoder.PluginArgs about the parameter being undocumented), but I'm not sure how to try to figure anything out about that.

If someone has a pointer as to where I might look for the issue, that would be much welcomed, I'm rabbitholing myself too deeply now, I think, so I need to drop this and do something else.

[pettermahlen]

Even though this doesn't feel like my yak to shave, I seem to be unable to keep reaching for the razor. Having looked through the commit log of the ClassReference file in kotlin-stdlib, I noticed that the method that kotlinpoet tries to call has been changed from throwing an error to actually working, even without reflection. Upgrading the WORKSPACE file to use kotlinc 1.3.72 makes this code compile.

A horrible workaround that seems to indicate that the stdlib that is used at compile time isn't the one passed in to the annotation processor on the classpath, and which seems to indicate that passing in the kotlin-reflect jar on the classpath may also be ineffective. Note that Maven doesn't exhibit the same behaviour, it works whether I set kotlin.version to 1.3.50, 1.3.60 or 1.3.72. Also, Maven doesn't produce the message warning: some JAR files in the classpath have the Kotlin Runtime library bundled into them. This may cause difficult to debug problems if there's a different version of the Kotlin Runtime library in the classpath. Consider removing these libraries from the classpath. So there's clearly some difference in how the kotlin-maven-plugin works and the Kotlin toolchain in these rules. Haven't figured out how that's different either, and maybe I'll just go with the workaround and finally drop this - probably, a similar problem will show up when someone tries to use some other reflective method, though, so it might be worthwhile trying to figure out a real solution.

[restingbull]

Which release are you running on?

[restingbull]

Ah, nevermind. I see in the repro.

[restingbull]

Oh. I see the issue -- the stdlib comes before reflect on the classpath, meaning that the stdlib ClassReference is found before reflect. Ew.

[pettermahlen]

I've done some (not super comprehensive) research that leads me to think that the issue isn't the classpath ordering, but more something like versioning, or perhaps something that gets confused by ijars or something along those lines. For other build systems, ordering doesn't seem relevant, but having different versions of stdlib and reflect on the classpath seems to break in a similar way.

[pettermahlen]

I think I have to walk back my previous comment about how it worked if I just changed the compiler version. For me, right now, the repro project is failing with KotlinReflectionNotSupportedError. In my previous experiments, I was running against a local version of rules_kotlin, where I had modified the KotlinToolchain.kotlinStandardLibraries list to include kotlin-reflect.jar.

In another, internal repo (where the original problem manifested itself), I am seeing the following kotlin-stdlib entries in the jar-0.params file:

external/com_github_jetbrains_kotlin/lib/kotlin-stdlib.jar
external/org_jetbrains_kotlin__kotlin_stdlib__1_3_72/file/kotlin-stdlib-1.3.72.jar

They point to two different files, of different sizes, on the file system. Both with a MANIFEST.mf file claiming that they are version 1.3.72-release-468, but with a difference in the Implementation-Version value:

kotlin-stdlib.jar:

Manifest-Version: 1.0
Implementation-Title: kotlin-stdlib
Kotlin-Runtime-Component: Main
Kotlin-Version: 1.3
Implementation-Version: 1.3.72-release-468
Implementation-Vendor: JetBrains

kotlin-stdlib-1.3.72.jar:

Manifest-Version: 1.0
Implementation-Title: kotlin-stdlib
Kotlin-Runtime-Component: Main
Kotlin-Version: 1.3
Implementation-Version: 1.3.72-release-468 (1.3.72)
Implementation-Vendor: JetBrains

I wonder if the problem could maybe be the different Implementation-Version? I get the feeling that the kotlin-stdlib.jar file gets included on the classpath through the kotlinStandardLibraries list, whereas the other one comes through the dependencies. If Kotlin tries to find a kotlin-reflect.jar whose Implementation-Version exactly matches the kotlin-stdlib.jar, then that could be the explanation.

Really grasping at straws here, and I'd very much appreciate some it if someone who is familiar with the design of rules_kotlin and its compiler invocation could help guide my search a bit. Where do these jars come from and why are they included in the classpath the way they are?

[pettermahlen]

One more tidbit here that indicates that classpath order (stdlib before or after reflect) shouldn't matter for whether reflection should work:

    static {
        ReflectionFactory impl;
        try {
            Class<?> implClass = Class.forName("kotlin.reflect.jvm.internal.ReflectionFactoryImpl");
            impl = (ReflectionFactory) implClass.newInstance();
        }
        catch (ClassCastException e) { impl = null; }
        catch (ClassNotFoundException e) { impl = null; }
        catch (InstantiationException e) { impl = null; }
        catch (IllegalAccessException e) { impl = null; }

        factory = impl != null ? impl : new ReflectionFactory();
    }

This is from kotlin.jvm.internal.Reflection in kotlin-stdlib, and the first ReflectionFactoryImpl class is defined in kotlin-reflect. So if the Reflection class is loaded at a point in time when the kotlin.reflect.jvm.internal.ReflectionFactoryImpl class can be instantiated, then reflection should work.

[pettermahlen]

@restingbull it looks like you made the latest changes to KotlinToolchain.kt - I get the feeling this problem might be similar to the comment you left about 'plugins must be preloaded', but I'm not sure. I feel like the Reflection class might get instantiated at a point in time when kotlin-reflect isn't on the classpath. Could that be a valid theory?

[pettermahlen]

Even more data collection, trying to figure out which conditions make it possible to build and which don't:

• Works:

  • local rules_kotlin on current master (https://github.com/bazelbuild/rules_kotlin/commit/d897c946d37f0851295c6c5c588a7140722a5a25), compiler version 1.3.72
  • local rules_kotlin at (https://github.com/bazelbuild/rules_kotlin/commit/53cd4b28108694ec11bd64ea3ad5a52ccd363963), compiler version 1.3.72
  • local rules_kotlin on legacy-1.4.0-rc3, compiler version 1.3.72

• KotlinReflectionNotSupportedError:

  • local rules_kotlin on current master (https://github.com/bazelbuild/rules_kotlin/commit/d897c946d37f0851295c6c5c588a7140722a5a25), default compiler version
  • local rules_kotlin at (https://github.com/bazelbuild/rules_kotlin/commit/53cd4b28108694ec11bd64ea3ad5a52ccd363963), default compiler version
  • local rules_kotlin on legacy-1.4.0-rc3, default compiler
  • released version of rules_kotlin with default compiler
  • released version of rules_kotlin with compiler 1.3.72

IOW, it looks like the combination of using 1.3.72 (where kotlin-stdlib supports looking up the qualified class name) as the compiler and building locally is necessary for the workaround to work. As an experiment, I tried to add

load("@io_bazel_rules_kotlin//kotlin:dependencies.bzl", "kt_download_local_dev_dependencies")
kt_download_local_dev_dependencies()

when building with the released version of rules_kotlin, but that didn't change anything.

Whether or not the workaround can be applied is secondary, I guess - there is still the root problem of supporting Kotlin reflection in annotation processors (of which the workaround only allows a very limited form). But at least this investigation helped me figure out why the workaround suddenly stopped working.

I'm not sure what other conclusions to draw from it though. It looks like building rules_kotlin locally using compiler 1.3.72 means that the follow-up compilation of the 'repro' uses a different classpath (because kotlin-stdlib.jar 1.3.72 gets used instead of the one that's specified in the project) than if rules_kotlin is downloaded. That seems like non-hermetic builds to me? If so, that's another issue, I suppose.

[pettermahlen]

I think I have a theory about what's going on that seems reasonable now.

rules_kotlin gets loaded into the System classloader in the Bazel CLI process. The rules_kotlin classpath doesn't include kotlin-reflect.jar. For some reason that I don't yet understand, the classpath is different when building rules_kotlin locally than when using the downloaded rules, so you get different versions of kotlin-stdlib, exhibiting different behaviours. Something in rules_kotlin triggers the load of the Reflection class into the System classloader and it gets initialised without access to ReflectionFactoryImpl.

              System Classloader  <--------------  rules_kotlin
                /                                              
               /                                               
              /                                                
             /                                                 
       Kotlinc Classloader     

If correct, there might be two possible solutions: either add kotlin-reflect.jar to the classpath of rules_kotlin, so that reflection is always available, or not loading rules_kotlin into the System classloader:

                   System Classloader    <------------ Kotlin banned, no stdlib                                       
                      /          \                                                          
                     /            \                                                         
                    /              \                                                        
                   /                \                                                       
                  /                  \                                                      
        Kotlinc Classloader          rules_kotlin Classloader   <-------------- rules_kotlin loaded here

I feel like the latter is safer, if the explanation is correct. It would separate the kotlinc invocation more cleanly from the rules_kotlin invocation. If anyone is listening to this, it would be great to get some feedback on whether you think this theory is likely or not.

[pettermahlen]

If the theory is correct and the second option is best, then it looks like changes might need to be made to KotlinBuilderMain.java to launch the builder in a separate classloader (providing access to the necessary kotlin libraries), and to bootstrap.bzl to remove kotlin libraries from the system classpath - I'm not sure how to read bootstrap.bzl, so not quite sure where things should be changed, or if that's the right place to look.

EDIT: having looked through the code a bit more, it's clear that the above suggestion isn't enough. Basically, everything up until the kotlinc invocation would have to be written in Java, I suppose. Or Kotlin could be shaded in the builder... Which would seem weird. I also wonder what the impact of a persistent worker is in a setup like this, if I understand that concept right? If one builds some Bazel subproject that loads stdlib which initialises the Reflection class without kotlin-reflect, I assume that that might be left over to the next invocation of Bazel? If so, that would be a problem for every class that makes decisions at classloading time and be problematic for hermetic builds?

[pettermahlen]

FWIW, adding

diff --git a/src/main/kotlin/io/bazel/kotlin/builder/BUILD b/src/main/kotlin/io/bazel/kotlin/builder/BUILD
index b4bd2d0..9db72a9 100644
--- a/src/main/kotlin/io/bazel/kotlin/builder/BUILD
+++ b/src/main/kotlin/io/bazel/kotlin/builder/BUILD
@@ -30,6 +30,7 @@ java_library(
         "//src/main/protobuf:worker_protocol_java_proto",
         "//third_party:dagger",
         "@com_github_jetbrains_kotlin//:annotations",
+        "@com_github_jetbrains_kotlin//:kotlin-reflect",
         "@com_github_jetbrains_kotlin//:kotlin-stdlib",
         "@kotlin_rules_maven//:javax_annotation_javax_annotation_api",
         "@kotlin_rules_maven//:javax_inject_javax_inject",

to rules_kotlin legacy-1.4.0-rc3 means that 'local build with default compiler' works, but that there are warnings about "some JAR files on the classpath" having the Kotlin runtime in them:

warning: some JAR files in the classpath have the Kotlin Runtime library bundled into them. This may cause difficult to debug problems if there's a different version of the Kotlin Runtime library in the classpath. Consider removing these libraries from the classpath
bazel-out/host/bin/external/maven/v1/https/repo1.maven.org/maven2/org/jetbrains/kotlin/kotlin-reflect/1.3.50/stamped_kotlin-reflect-1.3.50.jar: warning: library has Kotlin runtime bundled into it
bazel-out/host/bin/external/maven/v1/https/repo1.maven.org/maven2/org/jetbrains/kotlin/kotlin-stdlib/1.3.50/stamped_kotlin-stdlib-1.3.50.jar: warning: library has Kotlin runtime bundled into it

I think this is verification that the problem is that the Kotlin libraries loaded by the builder affect the output of the compiler. It seems like it would be more correct to let the project being built specify this, so you could use kotlinc 1.3.72 with 1.3.50 of stdlib and reflect, for instance.