Generate Dep files about rules_kotlin HOT 14 OPEN

The approach for generating deps files using the jdk jdeps tool won't work for inline functions and type aliases (afaik). Using the jdeps tool is not cheap and the diagnostics are poor.

I've implemented a kotlin compiler plugin to generate the jdeps file. The kotlin compiler infrastructure doesn't have analysis hooks for Java files though. The options are:

1. Get some help from Jetbrains to get java analysis going.
2. Use JavaBuilder_deploy.jar to compile the java and merge the 2 jdeps files together.
3. Use turbine. This is a small dependency but it seems be for a different purpose -- compiling ABI's ? Turbine also has a source-jar in output.jar out interface. The flexibility of javac is needed specifically to:
   • supply directories of classes on the classpath. Currently only jars are supported (stub classes and classes directory needs to be put on the claspath).
   • compiling to a directory rather than a jar, packaging the jar and stamping it should be left to the kotlin builder.

Notes on classpaths:

• The kotlin compiler and it's plugin is currently in it's own classloader, this is a child of the system classloader.
• JavaBuilder_deploy.jar contains unshaded versions of protobuf, protobuf_util so depending on how it is used it could replace these 2 deps. It also contains guava, jacoco and asm all of these could be used if it is placed on the boot classpath.

What I have tried.

• Using Turbine to compile the java -- did not verify the bytecode.
• Attempt to port the SJD plugin to Kotlin and drive it via JavacTool jsr-199 api -- SJD uses internal apis -- e.g.,Symbol, Flags and currently getting Kotlin to compile this with the bootstrap macros is not feasible. Kotlins jdk9 support needs work (no --add-exports), and enabling javac for java anlysis in jdk9 leads to bugs in general.

from rules_kotlin.

@cushon could do with your thoughts on this.

from rules_kotlin.

cc @kevin1e100

Have you considered using the bytecode based strict deps enforcement / .jdeps generation Bazel uses for aar_import, etc.?

https://github.com/bazelbuild/bazel/blob/42389d9468a954f3793a19f8e026b022b39aefca/src/java_tools/import_deps_checker/java/com/google/devtools/build/importdeps/Main.java#L45

from rules_kotlin.

I'm a bit unclear what's desired here: strict_deps enforcement or .jdeps output, or both? As an aside note that computing .jdeps for .kt and .java files from two separately computed .jdeps files likely involves filtering out the pseudo-dependency between the the two sets of files.

The importdeps tool mentioned in the previous comment is handy in that it can generate .jdeps files (and enforce strict_deps if desired) for any given .jar file(s), meaning it can be run over kotlinc's and javac's output (or just one of the two). Note that its notion of strictness is "stricter" than JavaBuilder's, in that it requires direct dependencies on all types (outside the Java bootclasspath) mentioned in the given Jar, but it's likely easy to hook up with the Kotlin rules as they are and and can compute .jdeps files regardless.

Looks like the Kotlin worker currently compiles .java files by invoking javac's main directly, IIUC. By using JavaBuilder instead it seems you'd indeed get strict_deps and .jdeps for .java files as desired. Doing that seems desirable anyway so the Kotlin rules compile .java files more like Bazel's Java rules. An alternative may be to use Bazel's built-in Java support by compiling .java files in a separate action using java_common.compile (which will run in its own worker); that should also give you strict_deps as well as a .jdeps file for the given .java files.

from rules_kotlin.

I'm a bit unclear what's desired here: strict_deps enforcement or .jdeps output, or both?

Both. The kotlin compiler plugin I wrote can do both in a manner similar to the JavaBuilder.

I can't see a way of driving the kotlin frontend to do a Java analysis -- if it could analyze and compile the this would be ideal. I created an issue for it.

In general getting kotlinc todo javac compilation has bugs. Especially with the embedded jdk9. So it might be quite a while till we can rely on this

two separately computed .jdeps files likely involves filtering out the pseudo-dependency between the the two sets of files

Kotlin won't need a filtering step as it compiles with the java source files.

If the directory containing the class files compiled by Kotlin is on the classpath rather than a jar then a filtering step might not be needed. For example this is how I invoke javac at the moment. When this is run ${d.classes} contains the class files compiled by kotlinc.

The importdeps tool mentioned in the previous comment is handy in that it can generate .jdeps files (and enforce strict_deps if desired) for any given .jar file(s), meaning it can be run over kotlinc's and javac's output (or just one of the two).

The importdeps tool looks interesting, it's a similar approach to what I am currently doing. But, it wouldn't work for Kotlin bytecode unless the metadata annotations are processed, typealias and inline function references cannot be discerned from the bytecode. Inline classes are also in the pipeline now.

It could still be leveraged somehow.

Looks like the Kotlin worker currently compiles .java files by invoking javac's main directly, IIUC. By using JavaBuilder instead it seems you'd indeed get strict_deps and .jdeps for .java files as desired. Doing that seems desirable anyway so the Kotlin rules compile .java files more like Bazel's Java rules.

I am favouring this approach. I think I can use com.google.devtools.build.java.bazel.BazelJavaCompiler to compile the java. In a fashion similar to how am using javac at the moment. Psuedo dep filtering shouldn't be required.

This would mean putting JavaBuilder_deploy.jar on the kotlin builders classpath. With this approach the kotlin builder would only depend on the kotlin compiler repository directly. It also lets me use the worker and deps jar directly.

@kevin1e100 WDYT ?

An alternative may be to use Bazel's built-in Java support by compiling .java files in a separate action using java_common.compile (which will run in its own worker); that should also give you strict_deps as well as a .jdeps file for the given .java files.

I think in practise this will might be too complex I think. We'd have to generate a temporary jar for the kotlin classes filter this as a pseudo dep somehow, go back into the kotlin builder to merge the jars and jdeps files etc.

from rules_kotlin.

I’ve figured out how to so the analysis in the kotlin compiler. Will open a PR soon, it will be a big one.

from rules_kotlin.

The Kotlin compiler plugin as I have implemented it will produce a diagnostic for import path and everywhere a symbol is used. This is probably too verbose.

The java jdeps plugin doesn't produce report entries on import paths. I haven't tried an unused import path yet -- or investigated multiple usage of a symbol -- I wonder if it would produces an entry on unused import paths.

What would be the most desireable heuristic ?

1. One diagnostic per file.
2. Target symbol once per file, prefering a non import path entry.
3. One diagnostic per target dependency.
4. Ignore import paths entirely.

I can't see anything wrong with 4 -- unused dep entries would just end up not treated as strict dep violations.

@cushon wdyt ?

from rules_kotlin.

I'm not sure what "import path" means here?

The Java SJD implementation requires direct deps for imports, even if they're unused.

I think the Java implementation will report multiple diagnostics for repeated uses of transitive deps, but it deduplicates the suggested fixes, and as long as there's a suggested fix I think people typically don't look at the other diagnostics very carefully.

from rules_kotlin.

Aah sorry I meant import statements, not import list.

Thanks for the feedback i'll have a look at a deduplication strategy.

It might be better left for a another PR.

from rules_kotlin.

@hsyed did you end up making any progress on this?

from rules_kotlin.

I think we can rework the Kotlin rules so that:-

java_info_from_javac = java_common.compile(...)
java_info_from_kotlinc = doKotlinCompilation(...) # Include @hsyed Kotlin compiler plugin

final_java_info = java_common.merge(java_info_from_javac, java_info_from_kotlinc)

Then the final JavaInfo will contain two sets of output jars, compile jars, jdeps (rather than try and merge the individual jars + jdeps themselves)

Does this sound reasonable?

We really want this so we can support a reduced classpath mode similar to JavaBuilder. Incremental builds with Kotlin are really painful otherwise.

One issue blocking this is that there seems no way to parse jdeps inside of a Starlark rule. Since JavaInfo contains JavaInfo.outputs.jdeps and provides JavaInfo.transitive_compile_time_jars would it be reasonable to have JavaInfo also support something like JavaInfo.reduced_transitive_compile_time_jars? @cushon - we chatted about this on Bazel Discuss... the nice thing about this is it would allow Kotlin and Java to use the same logic to compute the reduced classapth.

One other, unrelated thing is that java_info.compile() will run any annotation processors found in deps which is redundant because for Kotlin, annotation processing happens through KAPT. Would you be supportive of adding another named and defaulted parameter to `java_common.compile(skip_annotation_processing = False) so that we can avoid the additional annotation processing from Kotlin rules?

from rules_kotlin.

I've been away a long time. It seems a lifetime. I had some personal issues and just dropped off the face of the planet. received your email a while back @jongerrish sorry for not replying.

I'm trying to see if there I interest in Bazel in my new role. If there is I might be able to contribute again.

I made a lot of progress on the Kotlin compiler but it was 2 years ago and it was deep inside the guts of the compiler. The internals of the compiler have probably evolved since then. I can dig up the code but best bet is to talk to JB and ask for support on this from them.

from rules_kotlin.

Hey @hsyed I'm planning to write a kotlin compiler plugin to produce the kotlin side of the jdeps. If you have any code that I could resurrect and get a head start with that would be cool :-)

from rules_kotlin.

Update: The Kotlin rules now use java_common.compile() to build Java source.

We produce these outputs:

1. compile jar - merging the hjar produced from the Java Builder with the output of the Kotlin ABI jar compiler plugin
2. output jar - by merging the output jars from full Java + Kotlin compilation.
3. Jdeps - by calling Java Jdeps tool, parsing the merged output jar and converting it to Bazel's deps.proto format. We're still throwing away the jdeps produced by the Java Builder right now.
4. source jar - calling java_common.pack_sources()

It appears that its an error to return two providers of the same type and JavaInfo doesn't seem to have APIs to support multiple triples of (ijar, class_jar, jdeps) so we'd need to continue merging all three outputs from Java + Kotlin Builders.

If this is true then I think the work required would be

1. Kotlin Jdeps compiler plugin (To take into account Kotlin metadata as @hsyed mentioned previously)
2. Jdeps merging action to merge jdeps from Java + Kotlin builders.

Does this sound reasonable?

Would there be any interest in adding some jdeps support to the Java Skylark APIs? In particular I'm thinking

1. FileApi java_common.merge_jdeps(Sequence) # Merge sequence of Jdeps inputs to a single Jdeps
2. Sequence java_common.getReducedClassPath(Sequence) # Our internal hacks show big wins from a reduced classpath but looking to build something more robust/correct that we can upstream.

Otherwise we can add this to the Kotlin Builder.

Thoughts @kevin1e100 @cushon - any insight you can give on what you're doing with the internal KT rules?

from rules_kotlin.