nidi3 / graphviz-java

Use graphviz with pure java
Apache License 2.0
935 stars 106 forks source link

Discuss: Kotlin common port #169

Open mikesamuel opened 4 years ago

mikesamuel commented 4 years ago

I really like your Graph building API and was wondering whether you'd like contributions aimed at making large chunks of the API available as Kotlin common modules.

[Kotlin Multplatform Programming] Working on all platforms is an explicit goal for Kotlin, but we see it as a premise to a much more important goal: sharing code between platforms.

each project contains two source sets, commonMain and commonTest, where one can place all the code that should be shared between all of the target platforms


I ask because I'm using Kotlin-multiplatform for a compiler that

It'd be nice if shared parts of the compiler to be able to describe themselves as DOT graphs for easy debugging, ideally via a nice builder API like you've developed, but I only need to generate an actual image in Java code since the IDEs I'm targeting are Java based.


If I were to fork the project and do the below, would you be interested in possibly re-integrating that at some future point?

  1. Convert the Java files into Kotlin using the Java ⇒ Kotlin converter.
  2. Make it build as a Kotlin multiplatform project. This probably requires switching from Maven to Gradle :(
  3. Use expect to move the parts that depend on JVM-only facilities like java.io and ProcessBuilder out of the main codebase.
  4. Introduce actual stubs that raise UnsupportedOperation for other backends including JS, Linux native, and maybe android.
  5. Add JUnit tests written as .java files, not .kt, to ensure Java API stability.

(I notice you provide a Kotlin DSL so apologies if I'm over-explaining the language.)

nidi3 commented 4 years ago

Hi there, sorry for the late answer. If I understand correctly, you propose to convert the "pure" part of the library (API and the generation of a dot file, the attribute and model, maybe parse packages) into a kotlin multiplatform module? The rest of the project (i.e. calling dot to produce an image) could stay as it is now and use the JVM impl of the new multiplatform module.

I think this is interesting and I love kotlin, so I would be glad to integrating a fork into the main project if it works seamlessly.

Probably, it wouldn't even need expect clauses because the pure packages should not have any IO or other platform dependent code.

mikesamuel commented 4 years ago

Probably, it wouldn't even need expect clauses because the pure packages should not have any IO or other platform dependent code.

Yeah. I could put the non-pure packages under src/jvmMain/ and skip expect/actual that way.

I would be glad to integrating a fork into the main project if it works seamlessly.

I've worked with Kotlin enough that I'm pretty confident I can get an accurate port, but there's non-purely-code issues that might have seams.


Do you guarantee bytecode compatibility?

For example, just porting unit tests is not going to catch problems when a Java class has a method with an internal JVM signature like

parse(java/io/Reader)

If the Kotlin version produced a class with

parse(java/io/Readable)

it would not present a problem for any Java code compiled with a new version, but would cause a bytecode verifier error for classes compiled against an old version but loaded alongside a new version.

I can try to avoid such differences, but without building new test tooling have low confidence that I would catch them all.


How tolerant are you of seams outside the code, e.g. in the build system?

I'm less confident of my ability to do it within maven. Getting Kotlin multiplatform working without the gradle plugin is going to be tough.

Also, Kotlin has public, private, internal. The last does not really map to Java's package private. If you rely heavily on package-private to keep separate things separate, then I'd probably have to put together a multi-platform & multi-project build to recreate those divisions.


Maybe the best way to proceed would be for me to migrate a small set of packages that have no dependencies on the rest of the code so you can see what it affects.

I'm not super familiar with the layout of the code, but

$ find graphviz-java/src/main/java/guru/nidi/graphviz/{model,attribute,parse} -name \*.java \
    | xargs egrep -n 'import *(static *)?guru' \
    | egrep -v '[.](attribute|model|parse)'
graphviz-java/src/main/java/guru/nidi/graphviz/model/package-info.java:19:import guru.nidi.graphviz.NonnullApi;
graphviz-java/src/main/java/guru/nidi/graphviz/attribute/package-info.java:19:import guru.nidi.graphviz.NonnullApi;
graphviz-java/src/main/java/guru/nidi/graphviz/parse/package-info.java:19:import guru.nidi.graphviz.NonnullApi;

suggests that (...attribute, ...attribute.validate, ...model, ...parse) might be a good place to start.

The validators and model have a few external dependencies on loggesr, which I could actual/expect out:

https://github.com/mikesamuel/graphviz-java/blob/c664cabd0f68d2d16d9d5776b68f4d11f24dd0ee/graphviz-java/src/main/java/guru/nidi/graphviz/attribute/validate/AttributeValidator.java#L102-L105

https://github.com/mikesamuel/graphviz-java/blob/c664cabd0f68d2d16d9d5776b68f4d11f24dd0ee/graphviz-java/src/main/java/guru/nidi/graphviz/attribute/validate/ValidatorMessage.java#L36

SVGElementFinder seems to use java.io and javax.xml as implementation details. The JS backend can use XMLDocument when on the browser with CSS selectors instead of XPath, but I might only be able to provide a best-effort port of this initially.

https://github.com/mikesamuel/graphviz-java/blob/c664cabd0f68d2d16d9d5776b68f4d11f24dd0ee/graphviz-java/src/main/java/guru/nidi/graphviz/model/SvgElementFinder.java#L34-L58

Lexer and Parser use java.io classes in public APIs but not in ways that should cause problems.

https://github.com/mikesamuel/graphviz-java/blob/c664cabd0f68d2d16d9d5776b68f4d11f24dd0ee/graphviz-java/src/main/java/guru/nidi/graphviz/parse/Lexer.java#L39-L43

nidi3 commented 4 years ago

Thanks for the thorough analysis, that makes me positive regarding the quality of your PR ;)

Do you guarantee bytecode compatibility?

No.

How tolerant are you of seams outside the code, e.g. in the build system?

I'm aware that for multiplatform projects, gradle is a must. As long as you can setup it (I can't), I'm fine. A multi project setup would also be ok and maybe easier to start with. As for package-private visibility, I heavily rely on it. Visibility inside the project itself is not so important, but I definitely don't want to see java users artifacts they don't see now.

Maybe the best way to proceed would be for me to migrate a small set of packages that have no dependencies on the rest of the code so you can see what it affects.

I agree. attribute, model and parse have no dependencies on any of the other packages. SVGElementFinder is currently only provided as a helper and is not used inside this project, so I could even live without a working non-JVM impl for it.