Add kotlin-reflect on the builder classpath

[pettermahlen]

The issue in #342 boils down to how Java classloaders and reflection in Kotlin work. Classloaders form a tree, where a classloader will first ask its parent for any requested class, and only if the parent doesn't provide it, it will try itself. Kotlin reflection works by the kotlin-stdlib Reflection class attempting to load a ReflectionFactory implementation defined in kotlin-reflect, and using a default one if that fails. The current setup in rules_kotlin, where the builder loads kotlin-stdlib into the System classloader, and kotlin-reflect may be added as a dependency of an annotation processor or compiler plugin, leads to the following sequence of events:

1. Some code in an annotation processor tries to do reflection.
2. The annotation processor classloader asks its parent for the Reflection class.
3. Parents are asked all the way down to the boot loader (which doesn't have it).
4. Requests bounce back up to the System classloader, which has the class.
5. The Reflection class initialises itself, and tries to load kotlin.reflect.jvm.internal.ReflectionFactoryImpl - but that is only available in a child classloader (that of the annotation processor), so it fails.
6. Reflection is not fully supported in the annotation processor.

Kotlin reflection support is distributed separately for size reasons, and that shouldn't be an issue in this context, so supporting it in the builder is safe.

This change is a workaround, however, in the sense that the builder chooses the same kotlin-stdlib version as is selected for the compiler, not the one that is selected in dependencies. So users may be confused when they, for instance, ask to use compiler version 1.3.72 and kotlin-stdlib+reflect version 1.3.50, and in fact the application gets built with stdlib+reflect version 1.3.72. (I won't swear that that's what actually happens, but it's what it looks like to me). It would be better to separate the class loaders so that the system classloader stays kotlin-free and builder gets loaded into a separate branch from the compiler and annotation processor, meaning that annotation processor classpaths and compiler classpaths actually work. That change is quite large, though, and this change is at least an improvement over the current state.

[pettermahlen]

It looks like the build issues are caused by something unrelated, I see that other PRs are failing in the same way.

[mauriciogg]

@cgruber @restingbull We are running into this issue internally. Are there any concerns with merging this PR.

[restingbull]

@cgruber @restingbull We are running into this issue internally. Are there any concerns with merging this PR.

Yes -- the classpath for the builder shouldn't be part of the execution.

Are you running into the issue using plugins, or something else?

[mauriciogg]

@cgruber @restingbull We are running into this issue internally. Are there any concerns with merging this PR.

Yes -- the classpath for the builder shouldn't be part of the execution.

Are you running into the issue using plugins, or something else?

Yeah, this is only for plugins, since the plugin is executed but has no access to its transitive deps.. Alternatively we could add the dependencies of the plugin to the classpath at compile time.

[restingbull]

@cgruber @restingbull We are running into this issue internally. Are there any concerns with merging this PR.

Yes -- the classpath for the builder shouldn't be part of the execution. Are you running into the issue using plugins, or something else?

Yeah, this is only for plugins, since the plugin is executed but has no access to its transitive deps.. Alternatively we could add the dependencies of the plugin to the classpath at compile time.

Alternatively we could add the dependencies of the plugin to the classpath at compile time. This is the ideal solution if it works -- the plugin should have what it needs to execute.

[pettermahlen]

The problem, as I understand it, is that the standard library and the reflection library need to be found in the same classloader. And that seems pretty hard to achieve. I recall when I was looking into this a couple of years back that I checked out how Maven does that - their solution was based on some library for managing class loaders, so it could be something that is brought in here as well. But it didn't look easy to me.

Isn't this - the potential/desired difference between the JVM/kotlin/whatever version that is used by the Bazel process, and the JVM/kotlin/whatever version that should be used to compile classes a problem that is more general than just for rules_kotlin? It seems to me that the same kind of problem should potentially happen to any Java compilation. If so, perhaps the best place to solve it would be deeper in Bazel - I don't really know.