Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Binary incompatible changes in 1.6 #697

Closed
wilkinsona opened this issue Oct 17, 2018 · 14 comments
Closed

Binary incompatible changes in 1.6 #697

wilkinsona opened this issue Oct 17, 2018 · 14 comments

Comments

@wilkinsona
Copy link

wilkinsona commented Oct 17, 2018

This change that's in 1.6 means that it isn't binary compatible with 1.5. When an extension that was compiled against 1.5 is run with 1.6, a failure like this will occur:

Exception in thread "main" java.lang.NoSuchMethodError: org.asciidoctor.extension.JavaExtensionRegistry.docinfoProcessor(Lorg/asciidoctor/extension/DocinfoProcessor;)V
        at io.spring.asciidoctor.SpringAsciidoctorExtensionRegistry.register(SpringAsciidoctorExtensionRegistry.java:31)
        at org.asciidoctor.extension.internal.ExtensionRegistryExecutor.registerAllExtensions(ExtensionRegistryExecutor.java:21)
        at org.asciidoctor.internal.JRubyAsciidoctor.registerExtensions(JRubyAsciidoctor.java:118)
        at org.asciidoctor.internal.JRubyAsciidoctor.processRegistrations(JRubyAsciidoctor.java:107)
        at org.asciidoctor.internal.JRubyAsciidoctor.create(JRubyAsciidoctor.java:87)
        at org.asciidoctor.internal.JRubyAsciidoctor.create(JRubyAsciidoctor.java:83)
        at org.asciidoctor.Asciidoctor$Factory.create(Asciidoctor.java:823)
        at org.asciidoctor.gradle.remote.AsciidoctorJavaExec.getAsciidoctorInstance(AsciidoctorJavaExec.groovy:80)
        at org.asciidoctor.gradle.remote.AsciidoctorJavaExec.run(AsciidoctorJavaExec.groovy:42)
        at org.asciidoctor.gradle.remote.AsciidoctorJavaExec.main(AsciidoctorJavaExec.groovy:128)

Can you please consider reverting this change? While it's possible to work around the breaking change by using reflection, that will require changes in every extension in the ecosystem. In my opinion, that cost outweighs the benefit of being able to chain methods when working with JavaExtensionRegistry.

@robertpanzer
Copy link
Member

1.6 isn't only binary incompatible, but also the API changed.
In particular if you use extensions, these will have to be updated for 1.6.

I understand that it looks frustrating to fail at runtime for such a reason, but 1.6 really will be a major update with more breaking changes.

@wilkinsona
Copy link
Author

Thanks for the info. If it's going to contain more breaking changes, could you please consider calling it 2.0? When it's only a minor version change, I think many people expect that it will be compatible and that existing extensions will just work. It would help to set expectations if it was a 2.0 release where people are more likely to anticipate breaking changes.

@mojavelinux
Copy link
Member

mojavelinux commented Oct 17, 2018

We've been clear from the beginning that 1.6 is not going to be binary compatible with 1.5. The whole intention of 1.6 is to redesign the model so that it's more robust, complete, and logical.

We've also been clear that anyone writing extensions for AsciidoctorJ should be using 1.6, not 1.5. This wasn't just because we like the 1.6 code better. It's because the extension and document model in 1.5 was substantially flawed (which we learned from experience) and the only way to fix it was to redesign it.

Please understand that this break was the only way forward. There were a lot of lessons learned about how to map a Java API to a Ruby API, and we know more now than we did then. Some of it was wrong and it needed to be corrected.

@mojavelinux
Copy link
Member

mojavelinux commented Oct 17, 2018

We can certainly consider naming it 2.0 (but it will delay the release)

Having said that, we've also been clear that Asciidoctor does not yet follow semantic versioning. That's the primary focus of the Asciidoctor 2.0 release. Once we get to 2.0.0, we will follow semantic versioning strictly. Again, another mistake we made in Asciidoctor's history that we have to rectify.

@wilkinsona
Copy link
Author

We've also been clear that anyone writing extensions for AsciidoctorJ should be using 1.6, not 1.5.

1.6 is still alpha isn't it? Many people, myself included, won't want to use an alpha with the implied instability that brings. I also wouldn't want to release and try to support an extension with alpha dependencies.

While the intention may be that extension authors use 1.6 rather than 1.5, I think the current version numbers are encouraging the opposite behaviour.

@mojavelinux
Copy link
Member

@wilkinsona Your reasoning for calling it 2.0 regardless is sound even if we aren't in a semantic versioning model right now.

@wilkinsona
Copy link
Author

Just to be clear, I am no objecting to the breaking changes themselves. Far from it, as I know they're a big step forward. My concern is solely around the current versioning, the expectations it sets and the ways in which it will influence the decisions that people make. I think there's room for improvement that will help to guide people towards the new and improved code without placing too much burden on extension authors and without splitting the community into those stuck on 1.5 and those able to use 1.6.

@mojavelinux
Copy link
Member

mojavelinux commented Oct 17, 2018

I'll be the first to admit we have a serious versioning problem right now. We've been trying to get out of it for years, but I've made mistakes that prevented it from being rectified. So we're trying to rectify it now.

1.6 is still alpha isn't it?

It's even hard for me to describe what that alpha label actually means. It was meant to be temporary, but temporary had a way of becoming rather long term.

The alpha label was there so people didn't assume that it was providing Asciidoctor 1.6 (which doesn't yet exist...and will now never exist). The code is more stable than 1.5 by orders of magnitude. So it's not really alpha software at all.

...but the API has continued to change, so in that regard the alpha label has allowed the API to continue changing. That's why recently I proposed we drop the alpha label and just release 1.6.0.

@mojavelinux
Copy link
Member

If I could go back, we would have released 1.6.0 a long time ago. The fact that we didn't is my fault.

@mojavelinux
Copy link
Member

mojavelinux commented Oct 17, 2018

Suffice to say, reverting one change isn't going to make 1.6 binary compatible with 1.5. Maybe for a particular use case, but there would be a dozen more behind it. Trying to make it binary compatible would mean rolling back 2 years of changes.

The switch from 1.5 to 1.6 (or 2.0) is going to be a major one. Perhaps that is reason enough to say that we can't release this code as 1.6 (semantic versioning or not), but rather as 2.0.

@robertpanzer
Copy link
Member

I can definitely proceed with a release of 1.6.0 soon.
There's a few issues to sort out still, but nothing big right now.

The reasoning for the versioning in the past was to align the version of AsciidoctorJ with the version of Asciidoctor, so that users know what is inside the package.

@mojavelinux
Copy link
Member

I can definitely proceed with a release of 1.6.0 soon.

This probably warrants a dedicated issue to decide how to get the 1.6 branch on a path to a release.

@mojavelinux
Copy link
Member

When I wrote above "we've been clear", what I meant to say was "we've tried to be clear", because clearly we weren't clear enough ;)

We're going to give this some thought and come up with plan of action for how to proceed. Stay tuned.

@robertpanzer
Copy link
Member

Closing this issue for now.
We'll try to follow semantic versioning now as best as we can from 2.0.0 onward.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants