search

puppetlabs / puppet-strings

The next generation Puppet documentation extraction and presentation tool.
http://puppetlabs.github.io/puppet-strings/
Apache License 2.0
90 stars 84 forks source link

Minimum Ruby version is incompatible with current code #301

Closed danielparks closed 2 years ago

danielparks commented 2 years ago

Describe the Bug

puppet-strings.gemspec reports that the minimum version of Ruby supported is 2.3.0, but there is unsupported code (match?) since at least 08936f963e6e0b98c41d66be166b8635f513d18c.

There are also a bunch of unsupported dependency problems, but that can be solved with the brute force addition of a lot of version constraints.

Switching to ruby 2.6.0p0 makes the code work fine without problems.

Expected Behavior

You should be able to install dependencies and run unit tests with the minimum ruby version.

Steps to Reproduce

Steps to reproduce the behavior:

  1. rbenv install 2.3.0 && rbenv local 2.3.0
  2. gem install bundler:1.13.7 (later versions may work)
  3. bundle install --path .bundle/gems (this will fail)
  4. Add a bunch of version constraints to Gemfile.
  5. bundle install --path .bundle/gems
  6. bundle exec rake spec
. . .
      RuntimeError:
        Processing (stdin):13 with . . . => undefined method `match?' for /ruby4x/:Regexp
. . .

Environment

danielparks commented 2 years ago

I updated this to better reflect reality. I’m not sure what the implications of updating the minimum version would be.

I can prep a PR to fix all the errors with Ruby 2.3.0, but it involves specifying a bunch of additional dependencies with version information. It will eventually break as dependencies stop supporting 2.3.0, which will require additional constraints to be added, but that’s unavoidable with any Ruby version (perhaps unless Gemfile.lock gets added, though I don’t think that’s supported by bundler with Ruby 2.3.0).

danielparks commented 2 years ago

To give you an idea of the number of dependencies that need to be modified: https://github.com/puppetlabs/puppet-strings/compare/main...danielparks:puppet-strings:fix_ruby_2.3.0

I only made very minimal effort to specify them in the correct section; it’s not ready to be a PR.

chelnak commented 2 years ago

@danielparks I think it's reasonable to bump the minimum version. I'll discuss internally too just incase anyone can see an issue with it.

danielparks commented 2 years ago

Cool! That seems a lot cleaner to me, but I since Puppet comes packaged with Ruby there might be implications I haven’t thought of.

chelnak commented 2 years ago

Yeah agree.

I just had a quick chat with @david22swan about this. It looks like 2.5 is the current minimum version so it would be nice to align here.

I can test this out shortly.

chelnak commented 2 years ago

Tested with ruby 2.5.7p206 (2019-10-01 revision 67816) [x86_64-darwin20] and Bundler version 2.3.6.

This appears to work so think this might be a nice compromise given that it brings it inline with the rest of the ecosystem.

danielparks commented 2 years ago

Does that mean that the minimum Puppet version should be bumped to 7?

danielparks commented 2 years ago

PR #313. Hopefully we weren’t both working on this; feel free to delete, etc.

chelnak commented 2 years ago

It does slightly overlap. I am currently battling with some rubocop/spec issues that have occurred.

I will merge your changes and integrate in to my branch. Thanks!

With regards to the puppet version. I think it's OK to leave at 6 for now.

chelnak commented 2 years ago

Your changes are now in my branch

danielparks commented 2 years ago

Hurrah!