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

Pass more information when comparing platforms #3817

Merged
merged 3 commits into from Aug 6, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 5 additions & 1 deletion bundler/lib/bundler/index.rb
Expand Up @@ -195,7 +195,11 @@ def search_by_dependency(dependency, base = nil)
if base # allow all platforms when searching from a lockfile
dependency.matches_spec?(spec)
else
dependency.matches_spec?(spec) && Gem::Platform.match(spec.platform)
if Gem::Platform.respond_to? :match_spec?
dependency.matches_spec?(spec) && Gem::Platform.match_spec?(spec)
else
dependency.matches_spec?(spec) && Gem::Platform.match(spec.platform)
end
end
end

Expand Down
2 changes: 1 addition & 1 deletion lib/rubygems/available_set.rb
Expand Up @@ -73,7 +73,7 @@ def all_specs
end

def match_platform!
@set.reject! {|t| !Gem::Platform.match(t.spec.platform) }
@set.reject! {|t| !Gem::Platform.match_spec?(t.spec) }
@sorted = nil
self
end
Expand Down
2 changes: 1 addition & 1 deletion lib/rubygems/dependency.rb
Expand Up @@ -281,7 +281,7 @@ def matching_specs(platform_only = false)

if platform_only
matches.reject! do |spec|
spec.nil? || !Gem::Platform.match(spec.platform)
spec.nil? || !Gem::Platform.match_spec?(spec)
end
end

Expand Down
2 changes: 1 addition & 1 deletion lib/rubygems/name_tuple.rb
Expand Up @@ -59,7 +59,7 @@ def full_name
# Indicate if this NameTuple matches the current platform.

def match_platform?
Gem::Platform.match @platform
Gem::Platform.match_gem? @platform, @name
end

##
Expand Down
30 changes: 23 additions & 7 deletions lib/rubygems/platform.rb
Expand Up @@ -9,11 +9,7 @@
class Gem::Platform
@local = nil

attr_accessor :cpu

attr_accessor :os

attr_accessor :version
attr_accessor :cpu, :os, :version

def self.local
arch = RbConfig::CONFIG['arch']
Expand All @@ -22,18 +18,38 @@ def self.local
end

def self.match(platform)
Gem.platforms.any? do |local_platform|
match_platforms?(platform, Gem.platforms)
end

class << self
extend Gem::Deprecate
rubygems_deprecate :match, "Gem::Platform.match_spec?"
Copy link
Contributor Author

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?

Copy link
Member

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. :)

Copy link
Contributor Author

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.

Copy link
Contributor Author

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.

end

def self.match_platforms?(platform, platforms)
platforms.any? do |local_platform|
platform.nil? or
local_platform == platform or
(local_platform != Gem::Platform::RUBY and local_platform =~ platform)
end
end
private_class_method :match_platforms?

def self.match_spec?(spec)
match_gem?(spec.platform, spec.name)
end

def self.match_gem?(platform, gem_name)
# Note: this method might be redefined by Ruby implementations to
# customize behavior per RUBY_ENGINE, gem_name or other criteria.
match_platforms?(platform, Gem.platforms)
end

def self.installable?(spec)
if spec.respond_to? :installable_platform?
spec.installable_platform?
else
match spec.platform
match_spec? spec
end
end

Expand Down
6 changes: 3 additions & 3 deletions lib/rubygems/request_set/gem_dependency_api.rb
Expand Up @@ -379,7 +379,7 @@ def gem(name, *requirements)
Gem::Requirement.create requirements
end

return unless gem_platforms options
return unless gem_platforms name, options

groups = gem_group name, options

Expand Down Expand Up @@ -532,7 +532,7 @@ def gem_source(name, options) # :nodoc:
# Handles the platforms: option from +options+. Returns true if the
# platform matches the current platform.

def gem_platforms(options) # :nodoc:
def gem_platforms(name, options) # :nodoc:
platform_names = Array(options.delete :platform)
platform_names.concat Array(options.delete :platforms)
platform_names.concat @current_platforms if @current_platforms
Expand All @@ -543,7 +543,7 @@ def gem_platforms(options) # :nodoc:
raise ArgumentError, "unknown platform #{platform_name.inspect}" unless
platform = PLATFORM_MAP[platform_name]

next false unless Gem::Platform.match platform
next false unless Gem::Platform.match_gem? platform, name

if engines = ENGINE_MAP[platform_name]
next false unless engines.include? Gem.ruby_engine
Expand Down
2 changes: 1 addition & 1 deletion lib/rubygems/resolver/api_specification.rb
Expand Up @@ -42,7 +42,7 @@ def fetch_development_dependencies # :nodoc:
end

def installable_platform? # :nodoc:
Gem::Platform.match @platform
Gem::Platform.match_gem? @platform, @name
end

def pretty_print(q) # :nodoc:
Expand Down
2 changes: 1 addition & 1 deletion lib/rubygems/resolver/specification.rb
Expand Up @@ -104,7 +104,7 @@ def download(options)
# Returns true if this specification is installable on this platform.

def installable_platform?
Gem::Platform.match spec.platform
Gem::Platform.match_spec? spec
end

def local? # :nodoc:
Expand Down
2 changes: 1 addition & 1 deletion lib/rubygems/spec_fetcher.rb
Expand Up @@ -98,7 +98,7 @@ def search_for_dependency(dependency, matching_platform=true)

found[source] = specs.select do |tup|
if dependency.match?(tup)
if matching_platform and !Gem::Platform.match(tup.platform)
if matching_platform and !Gem::Platform.match_gem?(tup.platform, tup.name)
pm = (
rejected_specs[dependency] ||= \
Gem::PlatformMismatch.new(tup.name, tup.version))
Expand Down
4 changes: 2 additions & 2 deletions lib/rubygems/specification.rb
Expand Up @@ -804,7 +804,7 @@ def self.stubs
stubs = stubs.uniq {|stub| stub.full_name }

_resort!(stubs)
@@stubs_by_name = stubs.select {|s| Gem::Platform.match s.platform }.group_by(&:name)
@@stubs_by_name = stubs.select {|s| Gem::Platform.match_spec? s }.group_by(&:name)
stubs
end
end
Expand All @@ -831,7 +831,7 @@ def self.stubs_for(name)
@@stubs_by_name[name]
else
pattern = "#{name}-*.gemspec"
stubs = installed_stubs(dirs, pattern).select {|s| Gem::Platform.match s.platform } + default_stubs(pattern)
stubs = installed_stubs(dirs, pattern).select {|s| Gem::Platform.match_spec? s } + default_stubs(pattern)
stubs = stubs.uniq {|stub| stub.full_name }.group_by(&:name)
stubs.each_value {|v| _resort!(v) }

Expand Down
67 changes: 63 additions & 4 deletions test/rubygems/test_gem_platform.rb
Expand Up @@ -11,10 +11,69 @@ def test_self_local
end

def test_self_match
assert Gem::Platform.match(nil), 'nil == ruby'
assert Gem::Platform.match(Gem::Platform.local), 'exact match'
assert Gem::Platform.match(Gem::Platform.local.to_s), '=~ match'
assert Gem::Platform.match(Gem::Platform::RUBY), 'ruby'
Gem::Deprecate.skip_during do
assert Gem::Platform.match(nil), 'nil == ruby'
assert Gem::Platform.match(Gem::Platform.local), 'exact match'
assert Gem::Platform.match(Gem::Platform.local.to_s), '=~ match'
assert Gem::Platform.match(Gem::Platform::RUBY), 'ruby'
end
end

def test_self_match_gem?
assert Gem::Platform.match_gem?(nil, 'json'), 'nil == ruby'
assert Gem::Platform.match_gem?(Gem::Platform.local, 'json'), 'exact match'
assert Gem::Platform.match_gem?(Gem::Platform.local.to_s, 'json'), '=~ match'
assert Gem::Platform.match_gem?(Gem::Platform::RUBY, 'json'), 'ruby'
end

def test_self_match_spec?
make_spec = -> platform do
util_spec 'mygem-for-platform-match_spec', '1' do |s|
s.platform = platform
end
end

assert Gem::Platform.match_spec?(make_spec.call(nil)), 'nil == ruby'
assert Gem::Platform.match_spec?(make_spec.call(Gem::Platform.local)), 'exact match'
assert Gem::Platform.match_spec?(make_spec.call(Gem::Platform.local.to_s)), '=~ match'
assert Gem::Platform.match_spec?(make_spec.call(Gem::Platform::RUBY)), 'ruby'
end

def test_self_match_spec_with_match_gem_override
make_spec = -> name, platform do
util_spec name, '1' do |s|
s.platform = platform
end
end

class << Gem::Platform
alias_method :original_match_gem?, :match_gem?
def match_gem?(platform, gem_name)
# e.g., sassc and libv8 are such gems, their native extensions do not use the Ruby C API
if gem_name == 'gem-with-ruby-impl-independent-precompiled-ext'
match_platforms?(platform, [Gem::Platform::RUBY, Gem::Platform.local])
else
match_platforms?(platform, Gem.platforms)
end
end
end

platforms = Gem.platforms
Gem.platforms = [Gem::Platform::RUBY]
begin
assert_equal true, Gem::Platform.match_spec?(make_spec.call('mygem', Gem::Platform::RUBY))
assert_equal false, Gem::Platform.match_spec?(make_spec.call('mygem', Gem::Platform.local))

name = 'gem-with-ruby-impl-independent-precompiled-ext'
assert_equal true, Gem::Platform.match_spec?(make_spec.call(name, Gem::Platform.local))
ensure
Gem.platforms = platforms
class << Gem::Platform
remove_method :match_gem?
alias_method :match_gem?, :original_match_gem? # rubocop:disable Lint/DuplicateMethods
remove_method :original_match_gem?
end
end
end

def test_self_new
Expand Down