Skip to content

Fixes for Ruby 4.0 compatibility - #311

Merged
nmburgan merged 1 commit into
OpenVoxProject:mainfrom
silug:ruby-4
Apr 7, 2026
Merged

Fixes for Ruby 4.0 compatibility#311
nmburgan merged 1 commit into
OpenVoxProject:mainfrom
silug:ruby-4

Conversation

@silug

@silug silug commented Feb 4, 2026

Copy link
Copy Markdown
Contributor

No description provided.

@silug
silug marked this pull request as draft February 4, 2026 19:32

it "allows stdout and stderr to share a file" do
command = "ruby -e '(1..10).each {|i| (i%2==0) ? $stdout.puts(i) : $stderr.puts(i)}'"
command = "ruby -W0 -e '(1..10).each {|i| (i%2==0) ? $stdout.puts(i) : $stderr.puts(i)}'"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

-W0 makes it less verbose? Does it hide some warnings now?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I do not understand why this is needed, but without the -W0 you get the following failure:

Failures:

  1) Puppet::Util::Execution#execute allows stdout and stderr to share a file
     Failure/Error: expect(Puppet::Util::Execution.execute(command, :combine => true).split).to match_array([*'1'..'10'])
     
       expected collection contained:  ["1", "10", "2", "3", "4", "5", "6", "7", "8", "9"]
       actual collection contained:    ["/usr/local/lib/ruby/4.0.0/rubygems/platform.rb:279:", "/usr/local/lib/ruby/4.0.0/rubygems/platform...., "warning:", "warning:", "warning:", "was", "was", "was", "was", "was", "was", "was", "was", "was"]
       the extra elements were:        ["/usr/local/lib/ruby/4.0.0/rubygems/platform.rb:279:", "/usr/local/lib/ruby/4.0.0/rubygems/platform...., "warning:", "warning:", "warning:", "was", "was", "was", "was", "was", "was", "was", "was", "was"]
     # ./spec/integration/util/execution_spec.rb:57:in 'block (3 levels) in <top (required)>'
     # ./spec/spec_helper.rb:182:in 'block (2 levels) in <top (required)>'
     # ./vendor/ruby/4.0.0/gems/webmock-3.26.1/lib/webmock/rspec.rb:39:in 'block (2 levels) in <top (required)>'

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It does hide warnings. Default is -W2 and -W0 is silent mode. So something is generating warnings and the integration test is catching those.

I wouldn't be surprised if a user would see those same warnings at runtime.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think this is just a workaround in the test though. I'm assuming there is an environment variable being passed from rspec or something.

I will note that this PR will not eliminate all warnings under Ruby 4.0. It will only get us to the point that tests pass. There will need to be more cleanup.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm never a fan of the big PRs that combine many fixes because reviewing is a pain. I'd appreciate it if the PR was split up. At least into logical commits that can be reviewed but ideally into multiple focused PRs that have a well defined scope.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The warnings are coming from the ruby that is used to create some data on stdout/stderr. I ran the command in a pry in that block, and here are the warnings:

/workspaces/openvox/vendor/ruby/4.0.0/gems/bundler-2.6.9/lib/bundler/rubygems_ext.rb:64: warning: already initialized constant Gem::Platform::JAVA
/usr/local/lib/ruby/4.0.0/rubygems/platform.rb:279: warning: previous definition of JAVA was here
/workspaces/openvox/vendor/ruby/4.0.0/gems/bundler-2.6.9/lib/bundler/rubygems_ext.rb:65: warning: already initialized constant Gem::Platform::MSWIN
/usr/local/lib/ruby/4.0.0/rubygems/platform.rb:280: warning: previous definition of MSWIN was here
/workspaces/openvox/vendor/ruby/4.0.0/gems/bundler-2.6.9/lib/bundler/rubygems_ext.rb:66: warning: already initialized constant Gem::Platform::MSWIN64
/usr/local/lib/ruby/4.0.0/rubygems/platform.rb:281: warning: previous definition of MSWIN64 was here
/workspaces/openvox/vendor/ruby/4.0.0/gems/bundler-2.6.9/lib/bundler/rubygems_ext.rb:67: warning: already initialized constant Gem::Platform::MINGW
/usr/local/lib/ruby/4.0.0/rubygems/platform.rb:282: warning: previous definition of MINGW was here
/workspaces/openvox/vendor/ruby/4.0.0/gems/bundler-2.6.9/lib/bundler/rubygems_ext.rb:68: warning: already initialized constant Gem::Platform::X64_MINGW
/usr/local/lib/ruby/4.0.0/rubygems/platform.rb:284: warning: previous definition of X64_MINGW was here
/workspaces/openvox/vendor/ruby/4.0.0/gems/bundler-2.6.9/lib/bundler/rubygems_ext.rb:70: warning: already initialized constant Gem::Platform::UNIVERSAL_MINGW
/usr/local/lib/ruby/4.0.0/rubygems/platform.rb:285: warning: previous definition of UNIVERSAL_MINGW was here
/workspaces/openvox/vendor/ruby/4.0.0/gems/bundler-2.6.9/lib/bundler/rubygems_ext.rb:71: warning: already initialized constant Gem::Platform::WINDOWS
/usr/local/lib/ruby/4.0.0/rubygems/platform.rb:286: warning: previous definition of WINDOWS was here
/workspaces/openvox/vendor/ruby/4.0.0/gems/bundler-2.6.9/lib/bundler/rubygems_ext.rb:72: warning: already initialized constant Gem::Platform::X64_LINUX
/usr/local/lib/ruby/4.0.0/rubygems/platform.rb:287: warning: previous definition of X64_LINUX was here
/workspaces/openvox/vendor/ruby/4.0.0/gems/bundler-2.6.9/lib/bundler/rubygems_ext.rb:73: warning: already initialized constant Gem::Platform::X64_LINUX_MUSL
/usr/local/lib/ruby/4.0.0/rubygems/platform.rb:288: warning: previous definition of X64_LINUX_MUSL was here

Note that these warnings are coming from ruby internally, not any of our code. I can also make the test run successfully by replacing the command = line with this:

command = "bash -c 'for ((n=1; n<=10; n++)); do if ((n % 2 == 0)); then echo $n; else echo $n >&2; fi; done'"

Personally, I think quieting the warnings, which are absolutely just noise in this case, is a perfectly reasonable approach.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I can also trigger the warnings in my dev container completely outside of rspec by setting RUBYOPT to the same value it has under rspec:

$ RUBYOPT="-r/workspaces/openvox/vendor/ruby/4.0.0/gems/bundler-2.6.9/lib/bundler/setup" ruby -e '(1..10).each {|i| (i%2==0) ? $stdout.puts(i) : $stderr.puts(i)}'
/workspaces/openvox/vendor/ruby/4.0.0/gems/bundler-2.6.9/lib/bundler/rubygems_ext.rb:64: warning: already initialized constant Gem::Platform::JAVA
/usr/local/lib/ruby/4.0.0/rubygems/platform.rb:279: warning: previous definition of JAVA was here
/workspaces/openvox/vendor/ruby/4.0.0/gems/bundler-2.6.9/lib/bundler/rubygems_ext.rb:65: warning: already initialized constant Gem::Platform::MSWIN
/usr/local/lib/ruby/4.0.0/rubygems/platform.rb:280: warning: previous definition of MSWIN was here
/workspaces/openvox/vendor/ruby/4.0.0/gems/bundler-2.6.9/lib/bundler/rubygems_ext.rb:66: warning: already initialized constant Gem::Platform::MSWIN64
/usr/local/lib/ruby/4.0.0/rubygems/platform.rb:281: warning: previous definition of MSWIN64 was here
/workspaces/openvox/vendor/ruby/4.0.0/gems/bundler-2.6.9/lib/bundler/rubygems_ext.rb:67: warning: already initialized constant Gem::Platform::MINGW
/usr/local/lib/ruby/4.0.0/rubygems/platform.rb:282: warning: previous definition of MINGW was here
/workspaces/openvox/vendor/ruby/4.0.0/gems/bundler-2.6.9/lib/bundler/rubygems_ext.rb:68: warning: already initialized constant Gem::Platform::X64_MINGW
/usr/local/lib/ruby/4.0.0/rubygems/platform.rb:284: warning: previous definition of X64_MINGW was here
/workspaces/openvox/vendor/ruby/4.0.0/gems/bundler-2.6.9/lib/bundler/rubygems_ext.rb:70: warning: already initialized constant Gem::Platform::UNIVERSAL_MINGW
/usr/local/lib/ruby/4.0.0/rubygems/platform.rb:285: warning: previous definition of UNIVERSAL_MINGW was here
/workspaces/openvox/vendor/ruby/4.0.0/gems/bundler-2.6.9/lib/bundler/rubygems_ext.rb:71: warning: already initialized constant Gem::Platform::WINDOWS
/usr/local/lib/ruby/4.0.0/rubygems/platform.rb:286: warning: previous definition of WINDOWS was here
/workspaces/openvox/vendor/ruby/4.0.0/gems/bundler-2.6.9/lib/bundler/rubygems_ext.rb:72: warning: already initialized constant Gem::Platform::X64_LINUX
/usr/local/lib/ruby/4.0.0/rubygems/platform.rb:287: warning: previous definition of X64_LINUX was here
/workspaces/openvox/vendor/ruby/4.0.0/gems/bundler-2.6.9/lib/bundler/rubygems_ext.rb:73: warning: already initialized constant Gem::Platform::X64_LINUX_MUSL
/usr/local/lib/ruby/4.0.0/rubygems/platform.rb:288: warning: previous definition of X64_LINUX_MUSL was here
1
2
3
4
5
6
7
8
9
10

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yeah, I suppose squelching the warnings is fine here. Expected production use is for puppet to be running directly under /opt/puppetlabs/puppet/bin/ruby and not bundle exec, so the constant re-definition that the warnings are carping about shouldn't show up.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@bastelfreak do you agree? Can I resolve this conversation?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Leaving my agree here, since it's a test file and not production code and Ruby likes to be noisy these days. We might be able to be more targeted, but I don't think it's worth spending time on for now.

@silug
silug marked this pull request as ready for review February 10, 2026 20:16
@bastelfreak
bastelfreak requested a review from ekohl February 10, 2026 20:16
# We are intentionally using 'pipe' with open to launch a process
open("| #{command_str} 2>&1") do |pipe| # rubocop:disable Security/Open
# Use IO.popen instead of open for Ruby 4 compatibility
IO.popen("#{command_str} 2>&1", "r") do |pipe|

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is the much smaller version of #295.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I missed #295 when I started working on this.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@ekohl do you prefer this solution or yours? I build packages with ruby 4 / openssl 3.5 and #311: https://artifacts.voxpupuli.org/openvox-agent/8.25.0.9.g086d1fd67/

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If this works, I'm OK with it. Perhaps mine is a good OpenVox 9 item because it could change the API for providers. Security wise it's IMHO better and if a major is planned anyway ...

Comment thread rakelib/parallel.rake Outdated
Comment thread openvox.gemspec
spec.add_runtime_dependency('concurrent-ruby', '~> 1.0')
spec.add_runtime_dependency('deep_merge', '~> 1.0')
spec.add_runtime_dependency('fast_gettext', '>= 2.1', '< 5')
spec.add_runtime_dependency('fiddle', '~> 1.1')

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks like this is only used in the Solaris build. https://stdgems.org/fiddle/ says that it wasn't bundled since Ruby 3.2 so is anyone using OpenVox 8 on Solaris?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It was getting used indirectly on Windows. Tests fail without that.

@silug
silug force-pushed the ruby-4 branch 2 times, most recently from f591739 to 2b19c1f Compare February 19, 2026 19:10
Comment thread openvox.gemspec
spec.add_runtime_dependency('racc', '~> 1.5')
spec.add_runtime_dependency('scanf', '~> 1.0')
spec.add_runtime_dependency('semantic_puppet', '~> 1.0')
spec.add_runtime_dependency('win32ole', '>= 1.8', '< 2.0') if Gem.win_platform?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

* Explicitly require fiddle gem
* Explicitly require win32ole gem
* `parallel.rake` fixes

Co-authored-by: Ewoud Kohl van Wijngaarden <ewoud@kohlvanwijngaarden.nl>
Signed-off-by: Steven Pritchard <steven.pritchard@gmail.com>
@nmburgan
nmburgan merged commit bad82dc into OpenVoxProject:main Apr 7, 2026
15 checks passed
@silug
silug deleted the ruby-4 branch April 7, 2026 21:16
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.

5 participants