Skip to content

Commit

Permalink
Bump minimum Ruby version required to 2.7
Browse files Browse the repository at this point in the history
  • Loading branch information
@mogest
mogest committed Jun 16, 2024
1 parent d6d0169 commit f98879d
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 18 deletions.
19 changes: 2 additions & 17 deletions .github/workflows/test.yml
View file
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
name: test
on: [push, pull_request]
jobs:
rake_new:
test:
runs-on: ubuntu-latest
strategy:
fail-fast: false
matrix:
ruby: [2.6, 2.7, '3.0', 3.1, 3.2, 3.3]
ruby: [2.7, '3.0', 3.1, 3.2, 3.3]
steps:
- uses: actions/checkout@v4
- name: Set up Ruby
Expand All @@ -18,18 +18,3 @@ jobs:
run: bundle exec rake
env:
RUBYOPT: "--enable-frozen-string-literal"
rake_old:
runs-on: ubuntu-latest
strategy:
fail-fast: false
matrix:
ruby: [2.5]
steps:
- uses: actions/checkout@v4
- name: Set up Ruby
uses: ruby/setup-ruby@v1
with:
bundler-cache: true
ruby-version: ${{ matrix.ruby }}
- name: Run tests
run: bundle exec rake
2 changes: 1 addition & 1 deletion prawn-svg.gemspec
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ Gem::Specification.new do |gem|
gem.require_paths = ['lib']
gem.metadata = { "rubygems_mfa_required" => "true" }

gem.required_ruby_version = '>= 2.5.0'
gem.required_ruby_version = '>= 2.7.0'

This comment has been minimized.

Sign in to view

Copy link
@mojavelinux

mojavelinux Jun 22, 2024

Contributor

FYI, this breaks JRuby 9.2 and 9.3, which only claim Ruby 2.5 and 2.6 compliance, respectively. Thus, the only option is JRuby 9.4, which not all users have switched to.

This comment has been minimized.

Sign in to view

Copy link
@mogest

mogest Jun 23, 2024

Author Owner

Good to know, I don't follow JRuby. Is this a problem for the asciidoctor-pdf userbase? I'm guessing the vast majority will be on MRI?

This comment has been minimized.

Sign in to view

Copy link
@mogest

mogest Jun 23, 2024

Author Owner

BTW prawn is min ruby version 2.7 as well—I typically keep aligned with prawn's version and wanted to use the newish case in syntax for the parser I wrote for this release.

This comment has been minimized.

Sign in to view

Copy link
@mojavelinux

mojavelinux Jun 23, 2024 •
edited
Loading

Contributor

Yes, it's a huge problem for our user base. I'd say about half of all Asciidoctor PDF users are using it from Java, and thus through JRuby. And JRuby 9.2 and 9.3 are still very much in production use.

BTW prawn is min ruby version 2.7 as well—I typically keep aligned with prawn's version and wanted to use the newish case in syntax for the parser I wrote for this release.

I get that, and using Ruby 2.7 features isn't necessarily an issue. But JRuby only self reports a fully compliant Ruby version, and thus is usually quite behind the latest version of Ruby. The best way I have found to deal with this is to not set the minimum version of Ruby in the gemspec, but rather just document it in the README and CI workflow. That way, it is still possible to install the gem even when the Ruby runtime is not self reporting a sufficient version number.

This comment has been minimized.

Sign in to view

Copy link
@mojavelinux

mojavelinux Jun 23, 2024

Contributor

It's also very challenging for me when a change is made like this in a minor version. Typically these kinds of things are done in a major version that isn't just geared towards fixing bugs or adding minor enhancements, as is the case here. So it's not entirely clear why this would happen between 0.34.0 and 0.35.0.

This comment has been minimized.

Sign in to view

Copy link
@mogest

mogest Jun 23, 2024

Author Owner

Hmm, I'm confused. Does JRuby 9.2/9.3 support case in? If you have anything for me to read on the topic I'd be happy to.

Given prawn has a hard 2.7 dependency, and prawn-svg has a hard prawn dependency, I'm not sure about the benefit of removing it from the gemspec. It's still going to result in a hard 2.7 dependency, isn't it?

0.x releases are unstable, according to semver point 4. I treat 0.x releases as potentially breaking, and 0.x.y releases as bugfixes. I'm not sure prawn-svg will ever be at a version 1 due to its lack of support for all SVG 1.1, but I also don't want to cause undue issue for anyone downstream. So keen to hear your thoughts.

This comment has been minimized.

Sign in to view

Copy link
@mojavelinux

mojavelinux Jun 23, 2024

Contributor

Does JRuby 9.2/9.3 support case in?

In this particular situation, no they do not support that syntax feature. It's typical for JRuby to support a subset of features for a Ruby language version as it catches up, but this is not one of those features. What that means is that Asciidoctor PDF will have to pin to 0.34.0 for the foreseeable future and forgo improvements. In the future, I'd request that you at least support one JRuby release back (in this case 9.3) to give users ample time to upgrade.

Given prawn has a hard 2.7 dependency, and prawn-svg has a hard prawn dependency, I'm not sure about the benefit of removing it from the gemspec.

Personally, I find the practice to be hostile. You cannot account for all the situations under which a gem is packaged and this just makes it harder downstream (such as Linux packaging). I prefer for the user to decide based on the information given and the environment in which they are using it. But that's my personal stance and yours may differ.

I treat 0.x releases as potentially breaking, and 0.x.y releases as bugfixes.

I used to follow this approach of using 0.* releases, but I've since stopped because I found that it frustrated users. I think the first version of any software should be 1.0.0 and to go up from there. Then you can make use of the whole range that semver offers. I encourage you to give up the emotional attachment to 1.0.0 and rather just see it as just another version of something. 0.* is just playing games with versioning.

This comment has been minimized.

Sign in to view

Copy link
@mojavelinux

mojavelinux Jun 23, 2024

Contributor

Asciidoctor PDF will have to pin to 0.34.0 for the foreseeable future and forgo improvements.

The foreseeable future just means that I now have to do an assessment to determine whether enough of the AsciidoctorJ user base has transitioned to JRuby 9.4 to warrant enforcing its usage (at least in the current point releases of Asciidoctor PDF).

This comment has been minimized.

Sign in to view

Copy link
@mogest

mogest Jun 23, 2024

Author Owner

I prefer for the user to decide based on the information given and the environment in which they are using it.

I recognise it's not a black-and-white situation, but in this case I've got 2.7-requiring code and I'd rather the user was told they can't install it because their Ruby version isn't new enough rather than install it and it randomly breaks with a cryptic error message while parsing SVG. My experience leads me to believe that most people don't read documentation unless they are forced to.

0.* is just playing games with versioning.

I just don't agree with this sentiment. I don't think I'm playing games. 0.x is an industry standard. However I don't have any emotional attachment to it in the slightest, if there's a pragmatic reason to go 1.x I will. Practically I'm not sure what the difference between pinning ~> 0.34.0 and ~> 0.34 is though.

In any case, a way forward.

If prawn was still on Ruby 2.5, I would have not used case in and lived with the considerably messier code. My mistake was thinking that because prawn was on 2.7, downstream would be fine with 2.7 as well. I see in the asciidoctor-pdf gemspec that you've pinned prawn ~> 2.4.0 which is why you haven't run into this problem yet because the prawn 2.4 versions are Ruby 2.5.

prawn-svg releases are pretty sparse these days and for future development I'm only planning on fixing bugs as they are reported. I guess either you stick with prawn 2.4.x & prawn-svg 0.34.x for a while, and I can backport any new fixes onto a 0.34 branch, or I/someone rewrites the parser code and we reduce the min ruby version to 2.5 again. I am not at all excited about the prospect of rewriting that code, but I also want to serve the community.

The fact that this parser bug (using apostrophes inside a CSS url() function call) came up only now when prawn-svg has existed for 14 years probably indicates it's not overly common and is not going to make a huge difference to the community. But being closer to the users you will know better than I will.

This comment has been minimized.

Sign in to view

Copy link
@mojavelinux

mojavelinux Jun 23, 2024

Contributor

My mistake was thinking that because prawn was on 2.7

That was only a very recent release after an extremely long hiatus. The new release is already incompatible in other ways and we simply can't upgrade to it yet without more investigation.

This comment has been minimized.

Sign in to view

Copy link
@mojavelinux

mojavelinux Jun 23, 2024

Contributor

The fact that this parser bug (using apostrophes inside a CSS url() function call) came up only now when prawn-svg has existed for 14 years probably indicates it's not overly common and is not going to make a huge difference to the community.

I agree, it's not a huge issue. One of the reasons stuff like this comes up is because of diagram generators. The number of them is increasing rapidly and the ways in which they make SVGs varies widely. So you get these kinds of errors now that more SVG generators are entering the space. That's been my observation.

This comment has been minimized.

Sign in to view

Copy link
@mojavelinux

mojavelinux Jun 23, 2024 •
edited
Loading

Contributor

As for the versioning, we can agree to disagree. It's your library and your choice. I just don't like 0.* versions anymore because they are very difficult to track and, to me, just don't mean much. It just comes across as a reluctance to really ever commit to the code that was published.

This comment has been minimized.

Sign in to view

Copy link
@mogest

mogest Jun 23, 2024

Author Owner

OK. Well, let's say pin to 0.34.x for now, I'll maintain a 0.34 branch with any fixes, and either

  1. JRuby users will upgrade and you can go back to the head prawn & prawn-svg releases, or
  2. the new SVG generators quoting their URLs grows to the point where it's a persistent issue and I'll rewrite the code.

Does that sound reasonable?

This comment has been minimized.

Sign in to view

Copy link
@mojavelinux

mojavelinux Jun 23, 2024

Contributor

Yep, that's reasonable.

I've started the assessment to see if we can bump the minimum version of JRuby to 9.4, at least in the main branch (likely). I can't do it for 2.3.x since that would be considered a breaking change, so 2.3.x will need to stay pinned to 0.34.x.

This comment has been minimized.

Sign in to view

Copy link
@uidzip

uidzip via email Jun 23, 2024

This comment has been minimized.

Sign in to view

Copy link
@mogest

mogest Jun 23, 2024

Author Owner

Thanks for the context @uidzip, that's sad to hear. Lends weight to the rewrite option.

gem.add_runtime_dependency 'css_parser', '~> 1.6'
gem.add_runtime_dependency 'matrix', '~> 0.4.2'
Expand Down

1 comment on commit f98879d

@janwesterkamp
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, first let me say I am very happy, that there is such a (even little complex) tool chain available to create and maintain high quality documentation - many thanks!

I am a fan of the Docs-as-Code approach and semver, but as I am not a Ruby programmer (most of the time a Java one instead) I am one of these JRuby users Dan mentioned.
I am with my recent projects on JRuby 9.4, but in some others migrating it to this version(s) it's more difficult as this i.e. needs additional support from the working group of these projects to make changes. With Java and Maven this JRuby migration could be managed transitively, but when users had overwritten the version number manually and did not maintained it, this update will result in a breaking change because of JRuby (which should not happen in a Minor Release update in general by the way, except 0.x versions).

So personally I could live with the resulting requirement of JRuby 9.4, but many others (not following this thread) may be surpiesed.

As my intention was to get this bug/feature fixed in the root cause finally, as I was falling about it the 2nd time in two years (and did not remembered that any more first) and not preferring adding a documentation warning with a description of the PNG-workaround instead.
May be the root cause is also that strange formatting in the diagram generator, but getting it fixed in bpmn-js (JavaScript) and the corresponding tooling seems a tough challenge too (and there might be a valid reason to do it in such a way), so I am very happy to have prawn-svg being a tolerant reader here now.

Both way would work for me, having a new (Major?) release of asciidoctor(j)-pdf with prawn-svg 0.35.0 and JRuby 9.4+ dependencies in the long run and a 0.34.x Patch Release with a backport of the fix, when this can be included in a Patch Release of asciidoctor(j)-pdf to get it fixed in short run. However, the last one would make it possible to get rid much sooner of the technical debt with the current workaround to use PNG rendered diagrams instead of SVG with higher quality result and lower footprint.
Especially when it takes time to fix all the issues resulting from higher version dependencies, this would be very appreciated.
Thanks, Jan

Please sign in to comment.