Upgrade to Asciidoctor 1.5.6 and Asciidoctor-Diagram 1.5.5

[robertpanzer]

This PR updates Asciidoctor core to 1.5.6 and Asciidoctor-Diagram to 1.5.5.
(Asciidoctor 1.5.6 will not run with any other version Asciidoctor-Diagram – see asciidoctor/asciidoctor-diagram#154 and asciidoctor/asciidoctor-diagram#159)

To fix asciidoctor/asciidoctor-diagram#159 the TCCL is changed while rendering the document to the current JRubyClassLoader. This is necessary because asciidoctor-diagram requires access to other jars in that gem via the TCCL.
This might impact extensions though, that require the TCCL to be set to sth else, possibly extensions that run in a JavaEE environment. I can't say anything about how OSGi class loading would be impacted due to my lack of knowledge.

What is missing though is the update to the latest epub3 converter, the first test failed and I'll look into it.

[robertpanzer]

Seems like simply upgrading asciidoctor-epub3 failed due to the minimum required version 0.3.6 of thread-safe, and AsciidoctorJ was still on 0.3.5.
Fixed this problem by bumping thread-safe in AsciidoctorJ also to 0.3.6.

[mojavelinux]

Asciidoctor 1.5.6 will not run with any other version Asciidoctor-Diagram

We may want to base AsciidoctorJ on Asciidoctor core 1.5.6.1, which will be backwards compatible. I'll push the release out by 07/19.

To fix asciidoctor/asciidoctor-diagram#159 the TCCL is changed while rendering the document to the current JRubyClassLoader.

Is this going to cause a problem in OSGi?

[mojavelinux]

Oh, I see you commented on that. Perhaps we can get some input from @lefou.

[robertpanzer]

We may want to base AsciidoctorJ on Asciidoctor core 1.5.6.1, which will be backwards compatible.
Sounds good!

But we would still deliver it with asciidoctor-diagram 1.5.5, right?

Another solution for the class loading issue would be to change asciidoctor-diagram to load the implementations via the TCCL and the class loader that loaded the CommandServer: https://github.com/pepijnve/asciidoctor-diagram-java/blob/master/server/src/main/java/org/asciidoctor/diagram/CommandServer.java#L42
I.e. change it like so:

        ServiceLoader<DiagramGenerator> ownGeneratorLoader = ServiceLoader.load(DiagramGenerator.class, getClass().getClassLoader());
        for (DiagramGenerator generator : ownGeneratorLoader) {
            generatorMap.put(generator.getName(), generator);
        }
        ServiceLoader<DiagramGenerator> generatorLoader = ServiceLoader.load(DiagramGenerator.class);
        for (DiagramGenerator generator : generatorLoader) {
            generatorMap.put(generator.getName(), generator);
        }

[mojavelinux]

But we would still deliver it with asciidoctor-diagram 1.5.5, right?

That's up to you. You would now have a choice. But I'm leaning towards yes.

[mojavelinux]

Another solution for the class loading issue would be to change asciidoctor-diagram to load the implementations via the TCCL and the class loader that loaded the CommandServer

This feels like a good compromise to me (but I'm definitely not the right one to make the call).

[robertpanzer]

Created pepijnve/asciidoctor-diagram-java#1
If we could get a release for that we could avoid modifying the TCCL.

[robertpanzer]

But we would still deliver it with asciidoctor-diagram 1.5.5, right?

That's up to you. You would now have a choice. But I'm leaning towards yes.

Thinking about it a bit more, I am actually inclined to not include asciidoctor-diagram 1.5.5.
Changing the classloader might have other implications that I cannot oversee at the moment.
I searched too often for class loading problems with libraries that worked with the TCCL, for example commons-logging, I am not really looking forward to go down that path again or sending others into that.

But I am still getting the "frozen" errors. Should these have been resolved?

asciidoctor-diagram: ERROR: Failed to generate image: can't modify frozen #<Class:#<Asciidoctor::Diagram::PlantUmlBlockProcessor:0x222726ed>>

Another question: Do we want to get the extension deregistration into the AsciidoctorJ 1.5.6 release?

[mojavelinux]

But I am still getting the "frozen" errors. Should these have been resolved?

Nope. That was an actual bug in Asciidoctor Diagram (which it was getting a pass on due to a bug in core). We'd either have to upgrade Asciidoctor Diagram to 1.5.5 or persuade Pepijn to release a 1.5.4.1 version that fixes this one issue.

Another question: Do we want to get the extension deregistration into the AsciidoctorJ 1.5.6 release?

This depends on which version of Asciidoctor @lefou is using. If it's the 1.5 series, then absolutely. I've asked in the upstream issue.

[robertpanzer]

I would say now that we should get the unregistration into master. Now the PR is for master anyway.

I am still unsure though about the situation with asciidoctor-diagram.
If I think about the way AsciidoctorJ and JRuby have to be deployed as a module in JBossAS/Wildfly I would say that changing the TCCL while rendering would make any calls to EE module impossible.
(But that's just wild guessing now, I didn't test it)

[mojavelinux]

@pepijnve Would you be willing to release 1.5.4.1 that fixes the freeze issue? If so, I can send the necessary PR starting at the 1.5.4 tag.

We definitely want to get to 1.5.5, but more testing is needed and we don't want that to be rushed by the AsciidoctorJ 1.5.6 release.

[pepijnve]

@mojavelinux sure. I don't see any reason not to if that helps you out with the asciidoctorj release.

[mojavelinux]

Thanks @pepijnve! I really appreciate it. I'll follow-up shortly.

Btw, I want to be clear that the frozen bug was my fault. You couldn't have known that was a bug in core when you made Asciidoctor Diagram. We just want to move forward so we don't have to roll back the fix for the bug in core.

[pepijnve]

@mojavelinux I cherry-picked your fix from master onto a new 1.5.4.1. I can release that whenever you need it. Is there anything else that you need to fix for that release?

[mojavelinux]

Thanks @pepijnve! I think as long as the tests in AsciidoctorJ Diagram pass, we're good to go. @robertpanzer do you need me to test it or you can you configure it to test against this branch?

[robertpanzer]

I'll give it a go over the weekend and report my results here.

[robertpanzer]

@pepijnve & @mojavelinux : Just tested that it should work fine.
I applied the patch manually and the distribution tests passed.
So I would be very happy to get an asciidoctor-diagram 1.5.4.1 :)

[robertpanzer]

Integrated the unregistration of extension from #564 into this branch (squashed it first)

[pepijnve]

diagram 1.5.4.1 has been built and published

[robertpanzer]

Thanks, @pepijnve !

@mojavelinux : I updated the PR to use asciidoctor-diagram 1.5.4.1 and added a few comments about the ExtensionGroup API to the README.
If you are fine with it I would merge this and then do a release of asciidoctorj, asciidoctorj-diagram and asciidoctorj-epub3.

[mojavelinux]

Good work guys! I vote to proceed with the release. 🎉

[mojavelinux]

Please test and use Asciidoctor PDF 1.5.0.alpha.16. (I likely won't make a new Asciidoctor EPUB3 release in time, so don't worry about that component).

[robertpanzer]

Downgraded jruby-complete for the dist to 9.1.8.0.
Otherwise it will fail on Windows.

If nothing's speaking against it and nothing else comes in between I would do the release tonight.