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

Only call Object#taint when the taint method exists #1419

Merged
merged 1 commit into from
Jan 3, 2022
Merged

Only call Object#taint when the taint method exists #1419

merged 1 commit into from
Jan 3, 2022

Conversation

jcouball
Copy link
Contributor

@jcouball jcouball commented Jan 1, 2022

Description

Running YARD with Ruby 3.2 (current Ruby HEAD) results in the error:

$ rake yard
NoMethodError: undefined method `taint' for "# frozen_string_literal: true\n...":String

            module_eval(File.read(setup_file).taint, setup_file, 1)
                                             ^^^^^^

/home/runner/work/ruby-git/ruby-git/vendor/bundle/ruby/3.2.0/gems/yard-0.9.27/lib/yard/templates/template.rb:179:in `load_setup_rb'

This is because the taint mechanism, which was deprecated in Ruby 2.7 is to be removed from Ruby 3.2. In Ruby 2.7, the taint checking functionality was removed and Object#taint, Object#trust, Object#untaint, Object#untrust and related methods have no-op implementations. This means all objects by default are deemed untainted.

In Ruby 3.2 (the current HEAD), Object#taint, Object#trust, Object#untaint, Object#untrust are completely removed.

See the article Ruby 2.7 removes taint checking mechanism for a summary of the timeline for removal of the taint mechanism and links to Ruby core team discussions.

Caveats

I ran tests locally after making these changes in Ruby 2.4.x so the CI matrix build will need to be completed to make sure that these changes do not impact tests on all supported versions of Ruby.

Completed Tasks

  • I have read the Contributing Guide.
  • The pull request is complete (implemented / written).
  • Git commits have been cleaned up (squash WIP / revert commits).
  • I ran bundle exec rake locally (if code is attached to PR).

Copy link
Owner

@lsegal lsegal left a comment

Choose a reason for hiding this comment

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

We would still want to support taint for Ruby versions that support it.

Signed-off-by: James Couball <jcouball@yahoo.com>
@jcouball
Copy link
Contributor Author

jcouball commented Jan 3, 2022

@lsegal I changed the implementation to call taint when the receiving object respond_to?(:taint). Can you take another look?

@jcouball jcouball changed the title Remove calls to deprecated Object#taint Only call Object#taint when the taint method exists Jan 3, 2022
Copy link
Owner

@lsegal lsegal left a comment

Choose a reason for hiding this comment

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

Great stuff! Thanks so much for this PR!

@lsegal lsegal merged commit 7f399d3 into lsegal:main Jan 3, 2022
kyrylo added a commit to airbrake/airbrake-ruby that referenced this pull request Jan 10, 2022
Fails with ```load_setup_rb': undefined method `taint'``

Fixed in YARD but not released yet:
lsegal/yard#1419
@ShockwaveNN
Copy link

@lsegal Any chance you will release this change soon?

Really want to test my CI with ruby-head, but this fix is required (

@jcouball
Copy link
Contributor Author

@ShockwaveNN here is what I did as a temporary workaround until a new YARD version is published: ruby-git/ruby-git#573

@ShockwaveNN
Copy link

@jcouball Thanks, not a big fan of installing something from master )

I'm interesting if there is some specific problem why owner cannot release (something like v0.9.28) with this fix

@jcouball
Copy link
Contributor Author

@ShockwaveNN 100% agree, but I was tired of failing builds.

While I understand that YARD is only a dev dependency and Ruby 3.2 isn't released, fingers crossed that @lsegal will make a release soon with this fix.

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