enable and fix upstream build

ahus1 commented 3 years ago

I found that the upstream build is not triggered any more due to a mismatch of the condition. This PR also tries to fix what is broken in the build.

Kind of change

[ ] Bug fix
[ ] New non-breaking feature
[ ] New breaking feature
[ ] Documentation update
[x] Build improvement

Description

What is the goal of this pull request? Enable upstream build, and as a follow-up to tackle #40

How does it achieve that? Enable upstream build and see that shell script test-asciidoctor-upstream.sh runs without errors

Are there any alternative ways to implement this? (maybe skip upstream build? don't know if that is an option)

Are there any implications of this pull request? Anything a user must know? No

Issue

(doesn't fix any issue)

Release notes

Please add a corresponding entry to the file CHANGELOG.adoc

ahus1 commented 3 years ago

@robertpanzer - I kept this PR as a draft with additional debugging enabled as I got one question:

The shell script now completes without an error, but how can I check that it is really using the correct upstream version of asciidoctor-pdf?

I see that asciidoctorPdfGemVersion is passed to Gradle as expected, but looking at the output I don't know how to figure out the right version of the Gem is used in the tests. Could you please support me here? Thanks!

robertpanzer commented 3 years ago

Thank you for looking at this! I wasn't aware that I left the upstream build in such a bad shape. There is a bug here: https://github.com/asciidoctor/asciidoctorj-pdf/blob/8d7ea8cb2b6bab2535df6124ee9a3edb282c7aa9/asciidoctorj-pdf/build.gradle#L9

This must rather look like this

  gems("rubygems:asciidoctor-pdf:$asciidoctorPdfGemVersion") {

Additionally a more recent version of AsciidoctorJ (2.x) has to be used instead of the 1.5.x which is still used as the reference.

Then the current asciidoctorj-pdf head is installed at asciidoctorj-pdf/build/preparedGems/gems/asciidoctor-pdf-2.0.0.dev

However the tests are still failing now for some reason that I don't understand yet:

org.jruby.exceptions.LoadError: (MissingSpecVersionError) Gem::MissingSpecVersionError
    at RUBY.to_specs(uri:classloader:/META-INF/jruby.home/lib/ruby/stdlib/rubygems/dependency.rb:313)
    at RUBY.activate_dependencies(uri:classloader:/META-INF/jruby.home/lib/ruby/stdlib/rubygems/specification.rb:1400)
    at org.jruby.RubyArray.each(org/jruby/RubyArray.java:1809)
    at RUBY.activate_dependencies(uri:classloader:/META-INF/jruby.home/lib/ruby/stdlib/rubygems/specification.rb:1389)
    at RUBY.activate(uri:classloader:/META-INF/jruby.home/lib/ruby/stdlib/rubygems/specification.rb:1371)
    at RUBY.try_activate(uri:classloader:/META-INF/jruby.home/lib/ruby/stdlib/rubygems.rb:217)
    at RUBY.require(uri:classloader:/META-INF/jruby.home/lib/ruby/stdlib/rubygems/core_ext/kernel_require.rb:151)
    at RUBY.<main>(<script>:1)

I also tried with a more recent version of JRuby.

robertpanzer commented 3 years ago

Ah, prawn also needs to be updated to 2.4.0, then the upstream build passes again for me. We should probably make these versions configurable too.

robertpanzer commented 3 years ago

I created a PR for this PR so that we can all have more PRs at https://github.com/ahus1/asciidoctorj-pdf/pull/1 :)

ahus1 commented 3 years ago

Thank you for your help! I checked the following box for this PR, so you should be able to push to my branch directly:

I rebased the changes and cleaned out my pending TODOs.

As the build is now green again, I wonder if it should be refactored/can be simplified:

the continuous-integration.yaml proved to be quite complex. I wonder if the upstream build should go to a separate job definition, that would eliminate the check if: matrix.os == 'ubuntu-latest' && matrix.java == '1.8', and you'd see separate jobs failing for local and for upstream builds. Suggested name: upstream.yml
I wonder if it would be sufficient for the nightly build to just run the upstream build. Is there any other reason to run also the local build nightly - is there a chance that transitive dependencies change "over night"?

Please let me know and I'll update this as part of this PR.

robertpanzer commented 3 years ago

If the build pipeline can be simplified I am all for it. i was already happy that I got it working at all, if we can enhance it, even better..

It would probably be fine if we would only run the upstream build nightly. I think there are still a few variable transitive dependencies that might change. we probably should try to fix them to get reproducible builds.

ahus1 commented 3 years ago

I committed my attempt to reduce the complexity for a for the builds, please let me know what you think.

The PR is now no longer "draft"; if everything is fine, feel free to merge.

UPDATE: with check-out action V2, "fetch-depth: 1" is now the default, therefore I shortened it.

robertpanzer commented 3 years ago

Thank you again for taking care of this! :+1:

asciidoctor / asciidoctorj-pdf