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

Add --chruby option to use a Ruby managed by chruby #146

Merged
merged 2 commits into from
Dec 12, 2022

Conversation

k0kubun
Copy link
Member

@k0kubun k0kubun commented Dec 9, 2022

This is a shorthand for -e option #111, which works like:

# same as: ./run_benchmarks.rb -e "3.1.3::/opt/rubies/3.1.3/bin/ruby"
./run_benchmarks.rb --chruby 3.1.3

# same as: ./run_benchmarks.rb -e "yjit::/opt/rubies/ruby-master/bin/ruby --yjit"
./run_benchmarks.rb --chruby "yjit::ruby-master --yjit"

Similar to -e, this is the same interface as what ruby/ruby's benchmark driver has.

@k0kubun k0kubun requested a review from a team December 9, 2022 23:35
Copy link
Contributor

@maximecb maximecb left a comment

Choose a reason for hiding this comment

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

Good idea, but should ideally provide example usage in the readme

@k0kubun
Copy link
Member Author

k0kubun commented Dec 12, 2022

👍 updated README as well 9fc2cd3

@maximecb maximecb merged commit 39aab8c into Shopify:main Dec 12, 2022
@k0kubun k0kubun deleted the chruby branch December 12, 2022 18:31
version = name # allow skipping `NAME::`
end
version, *options = version.shellsplit
unless executable = ["/opt/rubies/#{version}/bin/ruby", "#{ENV["HOME"]}/.rubies/#{version}/bin/ruby"].find { |path| File.executable?(path) }
Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, this is a good fix. I had tried a similar thing with running chruby, but it's hard to get the environment right.

This won't change the gem environment, though, right? So it would only work if the Rubies are both using the same built native extensions?

Copy link
Contributor

Choose a reason for hiding this comment

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

This won't change the gem environment, though, right? So it would only work if the Rubies are both using the same built native extensions?

I wondered the same, but the script unsets GEM_HOME/GEM_PATH above so the gem homes are the default inside the Rubies prefixes (or bundle config get path if that's set).

Copy link
Contributor

@noahgibbs noahgibbs Dec 13, 2022

Choose a reason for hiding this comment

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

Ah, okay, so we're doing that anyway. Fair. Thanks!

Wait, no. Huh, okay, sounds like I need to play with this and figure out what it does. Though still, thanks! :-)

Copy link
Contributor

Choose a reason for hiding this comment

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

It's https://github.com/k0kubun/yjit-bench/blob/9fc2cd39d2ddebdb58f0ae7965530390a225cc7f/run_benchmarks.rb#L263-L267
And that's only done if it's not the first ruby in PATH, I missed that.
So then the behavior seems:

  • If it's the first Ruby in PATH use GEM_HOME/GEM_PATH as set by chruby or anything else
  • Otherwise use the default gem home inside the prefix

Copy link
Member Author

Choose a reason for hiding this comment

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

What eregon said is the intention. I've effectively been doing the same thing as --chruby with -e for a while since I merged #138, and it's been working fine.

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.

None yet

4 participants