-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Pass more information when comparing platforms #3817
Conversation
51e7307
to
8ea2b52
Compare
class << self | ||
|
||
extend Gem::Deprecate | ||
rubygems_deprecate :match, "Gem::Platform.match_spec?" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This might be annoying when using Bundler 1, and as long as there is the RubyGems Bundler version switcher (when removed, then Bundler 2+ would always be used and so just having a recent Bundler would avoid the issue).
Any idea how to "soft deprecate" this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO if you are on a RubyGems new enough to get this deprecation it's ok to tell you to upgrade to Bundler 2? But open to hearing from people that would be a problem for. :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One case I can imagine is using e.g. Ruby 2.8/3.0 or update RubyGems (which would include this code),
and then bundle install
on a project with Gemfile.lock
BUNDLED WITH 1.x.y
.
Then Bundler 1 would be used (due to the version switcher) and the warning would be shown.
I think a good way to fix that is to remove the version switcher, so Bundler 2 would be used in that case.
BTW, since it seems Bundler 1 seems no longer maintained (last release of Bundler 1 in 2018), it seems important to use Bundler 2 for all cases if installed, even if the Gemfile.lock still has BUNDLED WITH 1.x.y
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I filed #3879 about this.
8ea2b52
to
bad8323
Compare
@deivid-rodriguez Could you review this? Do you think we could merge it? |
* Passing the gem specification enables more flexibility when checking whether a gem is compatible with the various aspects of the "current platform" including OS, architecture, RUBY_ENGINE, etc. * Relates to rubygems#2945 * Based on oracle/truffleruby@ae83667
* So it also works with older versions of RubyGems.
bad8323
to
b9b0758
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIRC @deivid-rodriguez is out this week taking a break, and this looks reasonable to me. Let's merge this once it's green, and we can get additional feedback or make additional changes if needed after he gets back.
Thanks for merging! |
Pass more information when comparing platforms (cherry picked from commit 8977719)
Pass more information when comparing platforms (cherry picked from commit 8977719)
Pass more information when comparing platforms (cherry picked from commit 8977719)
Although this argument isn't used in the CRuby implementation, other implementations (specifically TruffleRuby) reserve the right to re-implement this method with special cases for specific gems. More context at rubygems/rubygems#3817
Although this argument isn't used in the CRuby implementation, other implementations (specifically TruffleRuby) reserve the right to re-implement this method with special cases for specific gems. More context at rubygems/rubygems#3817
when checking whether a gem is compatible
with the various aspects of the "current platform"
including OS, architecture, RUBY_ENGINE, etc.
What was the end-user or developer problem that led to this PR?
As mentioned in #2945 and in other issues (notably about including the libc in the "platform"), the current Gem::Platform captures too little and is sometimes not enough to check if a precompiled extension is actually compatible with the local machine installing the gem.
One example is the
sassc
andlibv8
gems both have extensions which do not use the Ruby C API and are therefore independent of the Ruby implementation. So such precompiled "extensions" for such gems can be reused on TruffleRuby, JRuby, etc, while typical precompiled C extensions cannot due to having a different ABI.Also consider that while
sassc
can be compiled from source, it's "just" rather slow.But for
libv8
it's both hard to build (needs dependencies typically not installed by Rubyists) and it takes >15 minutes to compile (oracle/truffleruby#1827 (comment)).What is your fix for the problem, implemented in this PR?
This PR makes a step forwards by giving more information and therefore flexibility when checking whether a Gem's platform is compatible with the local machine.
Tasks:
I will abide by the code of conduct.