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

Allow private invocation of ruby2_keywords #1714

Merged
merged 1 commit into from
Oct 8, 2021
Merged

Conversation

marcotc
Copy link
Member

@marcotc marcotc commented Oct 7, 2021

Fixes #1712

#ruby2_keywords can be sometimes private: https://github.com/ruby/ruby2_keywords/blob/92ad9c5c3fff591b8383ada8b93c3da1279d24ad/lib/ruby2_keywords.rb#L1-L12

We already accounted for this by passing true to respond_to?(:ruby2_keywords, true), but we didn't invoke it with a private-safe way in case it was actually private. This PR makes such change.

@marcotc marcotc added the bug Involves a bug label Oct 7, 2021
@marcotc marcotc requested a review from ivoanjo October 7, 2021 21:42
@marcotc marcotc self-assigned this Oct 7, 2021
@marcotc marcotc requested a review from a team October 7, 2021 21:42
@gedimin45
Copy link

Thanks for the amazingly swift fix @marcotc! Looking forward to the release so I can put this into production 🙂

Copy link
Member

@ivoanjo ivoanjo left a comment

Choose a reason for hiding this comment

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

Thanks @marcotc for the lightning fast fix.

I think it's definitely worth merging and releasing, but I don't quite understand why this happened in #1712. The backtrace mentions Ruby 2.4.0, which definitely did not have ruby2_keywords support. I thought perhaps that the ruby2_keywords compatibility gem was in use, but I tried every version from 0.0.0 to the latest released 0.0.5 and this method was still not private for procs there.

So I'm left wondering where that ruby2_keywords monkey patch was coming from.

Second observation: cthread.rb has been really annoying to get right. I plan to replace it with a better approach and completely delete it, and it cannot arrive soon enough.

@marcotc marcotc merged commit 9427cdc into master Oct 8, 2021
@marcotc marcotc deleted the private-ruby2_keywords branch October 8, 2021 21:40
@github-actions github-actions bot added this to the 0.54.0 milestone Oct 8, 2021
@marcotc
Copy link
Member Author

marcotc commented Oct 8, 2021

this method was still not private for procs there.

@ivoanjo I was thinking the same thing, but I feel like a program can "mess up" the global Proc class by mixing-in the private Module#ruby2_keywords somewhere 🤷. Given we have empirical evidence, I'm incline to believe it :)

@ivoanjo
Copy link
Member

ivoanjo commented Oct 11, 2021

@ivoanjo I was thinking the same thing, but I feel like a program can "mess up" the global Proc class by mixing-in the private Module#ruby2_keywords somewhere 🤷. Given we have empirical evidence, I'm incline to believe it :)

Oh yeah! That's exactly what I meant to convey as well -- I'm curious what's up in this case, but since a) we have evidence this can happen + b) the fix is trivial and does not impact customers or our work at all + c) hopefully this whole thing will be removed soonish, I think it's 100% a win to merge this in anyway :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Involves a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Passenger compilation fails with "private method `ruby2_keywords' called"
3 participants