Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CI: Use RUBYOPT: "--enable-frozen-string-literal" to find any incompatibilities #1354

Closed
wants to merge 7 commits into from

Conversation

olleolleolle
Copy link
Contributor

@olleolleolle olleolleolle commented Aug 11, 2021

This PR makes a try-out with the "frozen string literals" setting.

If it works, then great, if not, we learned something. Update: We learned something.

Now, the only failures that are left come from the multipart-post gem, and its next release will fix those, too.

This PR fixes the parts in octokit.rb which used strings in incompatible ways.


Changes

  • build_error_message could use a mutable string 4dcea1c, 9f9c7a0
  • CI: use an updated version (still 2.x) of RubyGems for Ruby 2.5

Learnings

If it works, then great, if not, we learned something.
@olleolleolle
Copy link
Contributor Author

olleolleolle commented Aug 11, 2021

I learned 📖 that one file in RubyGems is modifying a string in-place: with Ruby 2.5, this became a problem.

https://github.com/rubygems/rubygems/blob/master/lib/rubygems/bundler_version_finder.rb#L87

Output from test:

/opt/hostedtoolcache/Ruby/2.5.9/x64/lib/ruby/2.5.0/rubygems/bundler_version_finder.rb:89:in `untaint': can't modify frozen String (FrozenError)
	from /opt/hostedtoolcache/Ruby/2.5.9/x64/lib/ruby/2.5.0/rubygems/bundler_version_finder.rb:89:in `lockfile_contents'
	from /opt/hostedtoolcache/Ruby/2.5.9/x64/lib/ruby/2.5.0/rubygems/bundler_version_finder.rb:65:in `lockfile_version'
	from /opt/hostedtoolcache/Ruby/2.5.9/x64/lib/ruby/2.5.0/rubygems/bundler_version_finder.rb:19:in `bundler_version_with_reason'
	from /opt/hostedtoolcache/Ruby/2.5.9/x64/lib/ruby/2.5.0/rubygems/bundler_version_finder.rb:4:in `bundler_version'
	from /opt/hostedtoolcache/Ruby/2.5.9/x64/lib/ruby/2.5.0/rubygems/bundler_version_finder.rb:42:in `filter!'
	from /opt/hostedtoolcache/Ruby/2.5.9/x64/lib/ruby/2.5.0/rubygems/dependency.rb:283:in `matching_specs'
	from /opt/hostedtoolcache/Ruby/2.5.9/x64/lib/ruby/2.5.0/rubygems.rb:273:in `find_spec_for_exe'
	from /opt/hostedtoolcache/Ruby/2.5.9/x64/lib/ruby/2.5.0/rubygems.rb:303:in `activate_bin_path'
	from /opt/hostedtoolcache/Ruby/2.5.9/x64/bin/bundle:23:in `<main>'

Update: this has been fixed in later releases of RubyGems.

In order to support --enable-frozen-literal, this change explicitly uses a mutable string - the unary plus does that.
@olleolleolle olleolleolle changed the title CI: Use RUBYOPT: "--enable-frozen-string-literal" CI: Use RUBYOPT: "--enable-frozen-string-literal" to find any incompatibilities Aug 11, 2021
olleolleolle and others added 3 commits August 11, 2021 10:25
For diagnostic reasons, we may need this if reporting an issue to RubyGems.

I was not able to locate the information anywhere near here, so I output an extra bit of information.
@timrogers
Copy link
Contributor

Sadly, it looks like the fix on multipart-post still hasn't been released into a version, so this is stuck until then. I'll try creating an issue over there.

@timrogers
Copy link
Contributor

Changed merged in #1426.

@timrogers timrogers closed this Jun 6, 2022
@olleolleolle olleolleolle deleted the patch-2 branch June 6, 2022 15:00
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.

3 participants