swiftlang / swift-java

Apache License 2.0
721 stars 27 forks source link

JavaCompilerPlugin doesn't handle outputs correctly #114

Open jakepetroules opened 2 weeks ago

jakepetroules commented 2 weeks ago

The JavaCompilerPlugin has a serious flaw in that it doesn't handle outputs correctly -- it simply maps each .java file path to an equivalent .class file path, but due to a combination of use of certain language features like inner classes, and compiler implementation details which can vary across versions and implementations, there is NO guarantee that compilation of a set of Java sources will produce corresponding .class files at a 1:1 relationship.

This means the outputFiles may fail to capture class files which are generated, leading to broken products and/or broken incremental builds.

In Qbs I implemented this via a tool using compiler APIs to get the list of the .class files which will be produced: https://github.com/qbs/qbs/blob/master/share/qbs/modules/java/io/qt/qbs/tools/utils/JavaCompilerScanner.java. But this is a more involved solution and is only needed if you want to pass individual .class files to downstream build rules for further processing.

For now, can we instead produce a jar file with the set of generated classes, and list THAT as the build tool plugin command output? This simplifies the solution significantly.

ktoso commented 2 weeks ago

Yeah that's a good observation; I'm a bit worried about the JavaCompilerPlugin exploding in complexity to be honest, especially once we add dependencies to the mix.

For this specific topic, I think it might be easier if we give up on it being a "build plugin" and make it a "prebuild plugin" as these do not have to list all the exact outputs: https://github.com/swiftlang/swift-package-manager/blob/main/Documentation/Plugins.md

We would have to do some smarter caching in the prebuild plugin then though, so it's not ideal but may be easier than figuring out what javac might decide to emit (with closures etc this may also become "fun").

So maybe that's a direction to consider?

ktoso commented 2 weeks ago

The jar producing idea also isn't bad, whoever picks up the work can figure out what'll be nicer 👍