asciidoctor / asciidoctorj

:coffee: Java bindings for Asciidoctor. Asciidoctor on the JVM!
http://asciidoctor.org
Apache License 2.0
625 stars 173 forks source link

release 3.0.0-alpha.1 is gravely incompatible with earlier releases #1220

Closed verhas closed 1 year ago

verhas commented 1 year ago

Preprocessors are required to implement the org.asciidoctor.extension.Preprocessor abstract class. From version 2.5.8 version to 3.0.0-alpha-1 the method signature of

public abstract void process(Document document, PreprocessorReader reader);

changed to

public abstract Reader process(Document document, PreprocessorReader reader);

A preprocessor cannot implement both incompatible methods. When I develop the Jamal preprocessor Asciidoctor integration, I either go with the 2.5.8 version and support all those implementations that use that version (e.g., the IntelliJ Asciidoctor plugin as of today, 2023-06-16). My software starts not working when the plugin upgrade uses the new version. Or I opt for the new, unusable version until the new plugin version comes out.

However, when the new version is out, I will have users who did NOT upgrade to the newest version and users who did.

Or I invest a great amount of technical development and create a code that makes serious class loading tricks, decides on the fly during run-time which version is needed, and registers the one which fits. It is doable but hacky, adding an extra reflective call overhead for each render.

How can you fix this? Can you revert this release and have a version that keeps compatibility with the API interface?

Also, it would be nice to see how you decide on such a compatibility-breaking change before it gets into release and makes extension developers' life a nightmare.

verhas commented 1 year ago

https://github.com/verhas/jamal/blob/master/jamal-asciidoc258/README.md

describes the workaround

abelsromero commented 1 year ago

Thanks for the feedback! But I must say that this is a major version and it has been part of its goals to finally implement some improvements that break backward compatibility. As with any piece of software, that's bound to happen at some point. We are aware the changes won't be "nice" and that's why an alpha release to allow users to start testing was released. We hope we can catch bugs and improve upon the work done, but we cannot maintain interfaces that limited the functionality. Branch v2.5.x will still be supported for some time, so you don't need to upgrade as soon as possible. I can't comment on the other modules like IntelliJ plugin. If you integrate with that too, you could try to reach the maintainer about a possible calendar, https://asciidoctor.zulipchat.com would be the best place. The truth is we don't have any closed date for the final release, but we could coordinate with him and others if it helps the community.

If it's within your grasp I can suggest using a branching strategy to start working on v3.0.0. We are doing it for the maven-plugin. That helps keep things separated, at the cost of some duplication of course.

PS: we have prepared special "v25x to v30" documentation to facilitate the migration https://github.com/asciidoctor/asciidoctorj/tree/main/docs/modules/guides/pages.

verhas commented 1 year ago

Thanks for coming back to me that fast. I appreciate.

I understand that significant versions break backward compatibility. I was reading the old comments why this method was void in the first place, and I can tell, IMHO, it is absolutely reasonable to change the API this direction. The direction is okay, the way, however, could be better paved.

I suggest creating the new SPI in a way that leaves room for implementing a preprocessor supporting both versions. I would like to create a preprocessor that can work with the 2.X.X and the 3.X.X versions simultaneously, if possible.

You could, for example, rename the method for the 3.X.X versions and deprecate or even delete the old one. The implementation can implement both methods.

Your approach makes it challenging to create an implementation supporting both versions. It is not impossible because Java is highly versatile in this sense. However, I had to create a new module in my project with the dependencies on the 2.5.10 version and implement a code, which determines which AsciidoctorJ version is calling it.

It is a technical bravado, I can write an article about it, and I enjoyed the two hours of writing and debugging this hack. Now it is ready, so Jamal supports 2.X.X and 3.X.X versions simultaneously as of its next release 2.3.0.

However, making your fellow developers' life easy is in your best interest. Preprocessor developers want to focus on the functionality of their software and not on compatibility upgrade changes. I know; I am one. Creating different releases and different branches is possible, but it has its cost. It is not your cost, but still something that we can save.

verhas commented 1 year ago

BTW: the document https://github.com/asciidoctor/asciidoctorj/blob/main/docs/modules/guides/pages/api-migration-guide-v25x-to-v30.adoc is not there. I mean, it is there, listing other documents, which all return 404.

abelsromero commented 1 year ago

First, these are MY comments, I am one of the maintainers and I think I can speak for the whole, but the final decision is open to discussion with the rest. In that regard, can you please update your doc to refer to AsciidoctorJ developers? You are entitled to your opinions, but please don't extend them to others, Asciidoctor refers to the Ruby core implementation maintained by other members of the community.


I am glad you could find a solution. I must say I don't consider the complexity of an argument in this kind of conversation. It is a decision for the integrator to decide what versions to support and how to do it. That should not impose restrictions on the consumed libraries. Believe it or not, maintaining multiple products (repos) with multiple release versions is part of my day-to-day job (between 5 and 3 release branches for each repo). Even more in asciidoctor-maven-plugin we maintain also several branches based on AsciidoctorJ, maven-site v4-milestones, and so far we are lucky we don't need them for Maven v4-milestones :crossed_fingers: Believe me when I say I understand your pain, but precisely I clearly see the need for libraries to be ale to evolve independently.

But you brought up some fair points:

mojavelinux commented 1 year ago

As an observer, I'd like to point out that I find this comment in the referenced doc to be extremely unfriendly and uncalled for:

This module is a patch to the terrible decision of the Asciidoctor developers to change the SPI in an incompatible way.

First, you're referring to an alpha. Everyone understands that an alpha is a proposal. So to say that a decision was made is completely inaccurate. A decision has been proposed at best. And you seem to have written that accusation before you even brought the issue to the attention of the project and provided ample time for a response. It's entirely possible that the consequences of such a proposal weren't even understood at the time.

In short, be nice. And I don't find that comment to be nice. We're all trying to create great software and we don't get there by shaming others. That's not how open source works when it works.

verhas commented 1 year ago

Dear Abel,

Believe it or not, ... is part of my day-to-day job

That is exactly the point. In my everyday job, I program Java 8, maintain different versions, and administer many things. It is possible, but it is effort; it is cost. I was just hoping that when we are not in a corporate world we can be more flexible.

Dear Dan,

you are right, and thank you for the comment. I was a bit pissed when I wrote that sentence. I'm sorry, I fixed it.

Everyone understands that an alpha is a proposal.

I suspected that, but I did not find the document in the project about the release strategy. I did not look for it too long, that is true. Alphas are usually not final releases, but APIs are usually fixed when it is not a SNAPSHOT anymore. You have the right to make incompatible changes in major releases. I am on the sideline so whatever I say is a suggestion from the user side of the API.

We're all trying to create great software

and you do.

verhas commented 1 year ago

BTW: the version number is outdated in the README.adoc Jamal is precisely to solve issues like that.

If you say you would consider it, I would be happy to create a pull request renaming README.adoc to README.adoc.jam with the proper macros that fetch the latest version during the build (or when you edit with the plugin we just discussed) and adding Jamal to the build (well, I am not experienced with Gradle, but I will give it a try).

mojavelinux commented 1 year ago

Thanks @verhas. I appreciate your candid reply.

mojavelinux commented 1 year ago

Jamal is precisely to solve issues like that.

Cool! Sounds like it could be a good fit.

robertpanzer commented 1 year ago

Started a thread on Zulip to discuss the path forward. https://asciidoctor.zulipchat.com/#narrow/stream/279656-dev-.F0.9F.9B.A0.EF.B8.8F/topic/Breaking.20changes.20in.20AsciidoctorJ.203.2E0.2E0/near/369414330