Skip to content

Conversation

ekohl
Copy link
Contributor

@ekohl ekohl commented Jun 15, 2022

On Ruby 3.1 passing the arguments as positional arguments raises warnings:

warning: Passing safe_level with the 2nd argument of ERB.new is deprecated. Do not use it, and specify other arguments as keyword arguments.
warning: Passing trim_mode with the 3rd argument of ERB.new is deprecated. Use keyword argument like ERB.new(str, trim_mode: ...) instead.

However, on Ruby < 2.7 passing as keyword arguments is not allowed. This is why a conditional with RUBY_VERSION is used.

I must admit I have not tested my changes yet and am relying on CI to tell me now.

@ekohl ekohl requested a review from a team as a code owner June 15, 2022 09:55
@ekohl ekohl requested a review from a team June 15, 2022 09:55
@puppetlabs-jenkins
Copy link
Collaborator

Can one of the admins verify this patch?

@ekohl ekohl force-pushed the PUP-11552-erb-ruby-3.1 branch from 13dc484 to fd44792 Compare June 15, 2022 09:56
Copy link
Contributor Author

@ekohl ekohl left a comment

Choose a reason for hiding this comment

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

I was unsure about a convention for ' vs " in the codebase. It looks like it's often " but not always.

@ekohl ekohl force-pushed the PUP-11552-erb-ruby-3.1 branch 3 times, most recently from 55d11e0 to c4ce7ef Compare June 21, 2022 10:43
@ekohl
Copy link
Contributor Author

ekohl commented Jun 21, 2022

This is now green. It avoids warnings on Ruby 3.1, which in itself isn't in CI, but since Ruby 2.5 - 3.0 is tested all branches should be covered.

@joshcooper
Copy link
Contributor

Thanks @ekohl!

My only concern is whether we're calling RUBY_VERSION >= "2.7" in a file that doesn't have the frozen string literal pragma and creating lots of temporary objects? One thought is to define a create_erb method in Puppet::Util

if RUBY_VERSION >= "2.7"
  def create_erb(content)
    ERB.new(content, trim_mode: '-')
  end
else
  def self.create_erb(content)
    ERB.new(content, 0, "-")
  end
end
module_function :create_erb

And then just call that method, passing in the ERB content:

Puppet::Util.create_erb(Puppet::FileSystem.read(@file, :encoding => 'utf-8'))

@ekohl
Copy link
Contributor Author

ekohl commented Jul 27, 2022

Oh, that's more elegant. Until now I've often modified scripts rather than long running daemons. I also discovered the trim_mode keyword was introduced in Ruby 2.6, not 2.7. I'll also modify that.

@ekohl ekohl force-pushed the PUP-11552-erb-ruby-3.1 branch 4 times, most recently from 7021b5f to cac7347 Compare July 27, 2022 18:16
Ruby 2.6 introduced trim_mode as a keyword argument and deprecated the
use of safe_level as well as calling it as positional arguments. On Ruby
3.1 calling it as a non-keyword argument raises a deprecation warning.

    warning: Passing safe_level with the 2nd argument of ERB.new is deprecated. Do not use it, and specify other arguments as keyword arguments.
    warning: Passing trim_mode with the 3rd argument of ERB.new is deprecated. Use keyword argument like ERB.new(str, trim_mode: ...) instead.
@ekohl ekohl force-pushed the PUP-11552-erb-ruby-3.1 branch from cac7347 to 9938cd8 Compare July 27, 2022 18:21
@ekohl
Copy link
Contributor Author

ekohl commented Jul 27, 2022

And it's green now.

@ekohl
Copy link
Contributor Author

ekohl commented Aug 3, 2022

@joshcooper mind taking another look?

@joshcooper
Copy link
Contributor

Thanks for your contribution @ekohl! I verified this fixes all of the ERB keyword test failures on Ruby 3.1. There are a few unrelated failures that should be resolved once we start on this epic https://tickets.puppetlabs.com/browse/PUP-11544

@joshcooper joshcooper merged commit ed44890 into puppetlabs:main Aug 9, 2022
@ekohl ekohl deleted the PUP-11552-erb-ruby-3.1 branch August 9, 2022 08:56
@ekohl
Copy link
Contributor Author

ekohl commented Aug 9, 2022

There are a few unrelated failures

Which ones? I run Puppet on Ruby 3.1 and haven't seen them. On the Facter side I opened puppetlabs/facter#2496 but didn't see any on the Puppet side.

@joshcooper
Copy link
Contributor

There are a few unrelated failures

Ah sorry I mean there are still some failures when running puppet rspec tests, which should get fixed in https://tickets.puppetlabs.com/browse/PUP-11406

@ekohl
Copy link
Contributor Author

ekohl commented Aug 10, 2022

Ah, sounds like errors in the test suite which you don't notice at runtime. Still, looking forward to official Ruby 3.1 support.

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

Successfully merging this pull request may close these issues.

4 participants