Skip to content
Browse files

Introduce Gem::AvailableSet

AvailableSet abstracts tracking and picking which gems to install from
which sources.

Previously, the [[spec, source], ...] array had an implicit order that
was required in order for RubyGems to function properly. This is now
properly documented and maintained by AvailableSet.
  • Loading branch information...
1 parent a099e44 commit 432fc4818bbf14dfa5e49bbc1950eaa6d3fde133 @evanphx evanphx committed Oct 9, 2012
View
95 lib/rubygems/available_set.rb
@@ -0,0 +1,95 @@
+module Gem
+ class AvailableSet
+ Tuple = Struct.new(:spec, :source)
+
+ def initialize
+ @set = []
+ @sorted = nil
+ end
+
+ attr_reader :set
+
+ def add(spec, source)
+ @set << Tuple.new(spec, source)
+ @sorted = nil
+ self
+ end
+
+ def <<(o)
+ case o
+ when AvailableSet
+ s = o.set
+ when Array
+ s = o.map do |sp,so|
+ if !sp.kind_of?(Specification) or !so.kind_of?(Source)
+ raise TypeError, "Array must be in [[spec, source], ...] form"
+ end
+
+ Tuple.new(sp,so)
+ end
+ else
+ raise TypeError, "Must be an AvailableSet"
+ end
+
+ @set += s
+ @sorted = nil
+
+ self
+ end
+
+ def empty?
+ @set.empty?
+ end
+
+ def all_specs
+ @set.map { |t| t.spec }
+ end
+
+ def match_platform!
+ @set.reject! { |t| !Gem::Platform.match(t.spec.platform) }
+ @sorted = nil
+ self
+ end
+
+ def sorted
+ @sorted ||= @set.sort do |a,b|
+ i = b.spec <=> a.spec
+ i != 0 ? i : (a.source <=> b.source)
+ end
+ end
+
+ def size
+ @set.size
+ end
+
+ def source_for(spec)
+ f = @set.find { |t| t.spec == spec }
+ f.source
+ end
+
+ def pick_best!
+ return self if empty?
+
+ @set = [sorted.first]
+ @sorted = nil
+ self
+ end
+
+ def remove_installed!(dep)
+ @set.reject! do |t|
+ # already locally installed
+ Gem::Specification.any? do |installed_spec|
+ dep.name == installed_spec.name and
+ dep.requirement.satisfied_by? installed_spec.version
+ end
+ end
+
+ @sorted = nil
+ self
+ end
+
+ def inject_into_list(dep_list)
+ @set.each { |t| dep_list.add t.spec }
+ end
+ end
+end
View
53 lib/rubygems/dependency_installer.rb
@@ -6,6 +6,7 @@
require 'rubygems/user_interaction'
require 'rubygems/source_local'
require 'rubygems/source_specific_file'
+require 'rubygems/available_set'
##
# Installs a gem along with all its dependencies from local and remote gems.
@@ -120,14 +121,14 @@ def consider_remote?
# local gems preferred over remote gems.
def find_gems_with_sources(dep)
- gems_and_sources = []
+ set = Gem::AvailableSet.new
if consider_local?
sl = Gem::Source::Local.new
if spec = sl.find_gem(dep.name)
if dep.matches_spec? spec
- gems_and_sources << [spec, sl]
+ set.add spec, sl
end
end
end
@@ -142,7 +143,7 @@ def find_gems_with_sources(dep)
@errors = errors
end
- gems_and_sources.push(*found)
+ set << found
rescue Gem::RemoteFetcher::FetchError => e
# FIX if there is a problem talking to the network, we either need to always tell
@@ -156,17 +157,15 @@ def find_gems_with_sources(dep)
end
end
- gems_and_sources.sort_by do |gem, source|
- [gem, source] # local gems win
- end
+ set
end
##
# Gathers all dependencies necessary for the installation from local and
# remote sources unless the ignore_dependencies was given.
def gather_dependencies
- specs = @specs_and_sources.map { |spec,_| spec }
+ specs = @available.all_specs
# these gems were listed by the user, always install them
keep_names = specs.map { |spec| spec.full_name }
@@ -232,21 +231,14 @@ def add_found_dependencies to_do, dependency_list
results = find_gems_with_sources(dep)
- results.reject! do |dep_spec,|
- to_do.push dep_spec
-
- # already locally installed
- Gem::Specification.any? do |installed_spec|
- dep.name == installed_spec.name and
- dep.requirement.satisfied_by? installed_spec.version
- end
+ results.sorted.each do |t|
+ to_do.push t.spec
end
- results.each do |dep_spec, source_uri|
- @specs_and_sources << [dep_spec, source_uri]
+ results.remove_installed! dep
- dependency_list.add dep_spec
- end
+ @available << results
+ results.inject_into_list dependency_list
end
end
@@ -262,39 +254,37 @@ def find_spec_by_name_and_version(gem_name,
version = Gem::Requirement.default,
prerelease = false)
- spec_and_source = nil
+ set = Gem::AvailableSet.new
if consider_local?
if File.exists? gem_name
src = Gem::Source::SpecificFile.new(gem_name)
- spec_and_source = [src.spec, src]
+ set.add src.spec, src
else
local = Gem::Source::Local.new
if s = local.find_gem(gem_name, version)
- spec_and_source = [s, local]
+ set.add s, local
end
end
end
- unless spec_and_source then
+ if set.empty?
dep = Gem::Dependency.new gem_name, version
# HACK Dependency objects should be immutable
dep.prerelease = true if prerelease
- spec_and_sources = find_gems_with_sources(dep)
- spec_and_source = spec_and_sources.find { |spec, source|
- Gem::Platform.match spec.platform
- }
+ set = find_gems_with_sources(dep)
+ set.match_platform!
end
- if spec_and_source.nil? then
+ if set.empty?
raise Gem::GemNotFoundException.new(
"Could not find a valid gem '#{gem_name}' (#{version}) locally or in a repository",
gem_name, version, @errors)
end
- @specs_and_sources = [spec_and_source]
+ @available = set
end
##
@@ -317,7 +307,7 @@ def install dep_or_name, version = Gem::Requirement.default
else
dep = dep_or_name.dup
dep.prerelease = @prerelease
- @specs_and_sources = [find_gems_with_sources(dep).first]
+ @available = find_gems_with_sources(dep).pick_best!
end
@installed_gems = []
@@ -335,7 +325,8 @@ def install dep_or_name, version = Gem::Requirement.default
# TODO: make this sorta_verbose so other users can benefit from it
say "Installing gem #{spec.full_name}" if Gem.configuration.really_verbose
- _, source = @specs_and_sources.assoc spec
+ source = @available.source_for spec
+
begin
# REFACTOR make the fetcher to use configurable
local_gem_path = source.download spec, @cache_dir
View
106 test/rubygems/test_gem_available_set.rb
@@ -0,0 +1,106 @@
+require 'rubygems/test_case'
+require 'rubygems/available_set'
+require 'rubygems/security'
+
+class TestGemAvailableSet < Gem::TestCase
+ def setup
+ super
+
+ @source = Gem::Source.new(@gem_repo)
+ end
+
+ def test_add_and_empty
+ a1, _ = util_gem 'a', '1'
+
+ set = Gem::AvailableSet.new
+ assert set.empty?
+
+ set.add a1, @source
+
+ refute set.empty?
+
+ assert_equal [a1], set.all_specs
+ end
+
+ def test_match_platform
+ a1, _ = util_gem 'a', '1' do |g|
+ g.platform = "something-weird-yep"
+ end
+
+ a1c, _ = util_gem 'a', '2' do |g|
+ g.platform = Gem::Platform.local
+ end
+
+ a2, _ = util_gem 'a', '2'
+
+ set = Gem::AvailableSet.new
+ set.add a1, @source
+ set.add a1c, @source
+ set.add a2, @source
+
+ set.match_platform!
+
+ assert_equal [a1c, a2], set.all_specs
+ end
+
+ def test_best
+ a1, _ = util_gem 'a', '1'
+ a2, _ = util_gem 'a', '2'
+
+ set = Gem::AvailableSet.new
+ set.add a1, @source
+ set.add a2, @source
+
+ set.pick_best!
+
+ assert_equal [a2], set.all_specs
+ end
+
+ def test_remove_installed_bang
+ a1, _ = util_gem 'a', '1'
+
+ a1.activate
+
+ set = Gem::AvailableSet.new
+ set.add a1, @source
+
+ dep = Gem::Dependency.new "a", ">= 0"
+
+ set.remove_installed! dep
+
+ assert set.empty?
+ end
+
+ def test_sorted_normal_versions
+ a1, _ = util_gem 'a', '1'
+ a2, _ = util_gem 'a', '2'
+
+ set = Gem::AvailableSet.new
+ set.add a1, @source
+ set.add a2, @source
+
+ g = set.sorted
+
+ assert_equal a2, g[0].spec
+ assert_equal a1, g[1].spec
+ end
+
+ def test_sorted_respect_pre
+ a1a, _ = util_gem 'a', '1.a'
+ a1, _ = util_gem 'a', '1'
+ a2a, _ = util_gem 'a', '2.a'
+ a2, _ = util_gem 'a', '2'
+ a3a, _ = util_gem 'a', '3.a'
+
+ set = Gem::AvailableSet.new
+ set.add a1, @source
+ set.add a1a, @source
+ set.add a3a, @source
+ set.add a2a, @source
+ set.add a2, @source
+
+ g = set.sorted.map { |t| t.spec }
+
+ assert_equal [a3a, a2, a2a, a1, a1a], g
+ end
+end
View
50 test/rubygems/test_gem_commands_install_command.rb
@@ -276,7 +276,53 @@ def test_execute_conflicting_install_options
assert_equal expected, @ui.error
end
- def test_execute_prerelease
+ def test_execute_prerelease_skipped_when_no_flag_set
+ util_setup_fake_fetcher :prerelease
+ util_clear_gems
+ util_setup_spec_fetcher @a1, @a2_pre
+
+ @fetcher.data["#{@gem_repo}gems/#{@a1.file_name}"] =
+ read_binary(@a1.cache_file)
+ @fetcher.data["#{@gem_repo}gems/#{@a2_pre.file_name}"] =
+ read_binary(@a2_pre.cache_file)
+
+ @cmd.options[:prerelease] = false
+ @cmd.options[:args] = [@a2_pre.name]
+
+ use_ui @ui do
+ e = assert_raises Gem::SystemExitException do
+ @cmd.execute
+ end
+ assert_equal 0, e.exit_code, @ui.error
+ end
+
+ assert_equal %w[a-1], @cmd.installed_specs.map { |spec| spec.full_name }
+ end
+
+ def test_execute_prerelease_wins_over_previous_ver
+ util_setup_fake_fetcher :prerelease
+ util_clear_gems
+ util_setup_spec_fetcher @a1, @a2_pre
+
+ @fetcher.data["#{@gem_repo}gems/#{@a1.file_name}"] =
+ read_binary(@a1.cache_file)
+ @fetcher.data["#{@gem_repo}gems/#{@a2_pre.file_name}"] =
+ read_binary(@a2_pre.cache_file)
+
+ @cmd.options[:prerelease] = true
+ @cmd.options[:args] = [@a2_pre.name]
+
+ use_ui @ui do
+ e = assert_raises Gem::SystemExitException do
+ @cmd.execute
+ end
+ assert_equal 0, e.exit_code, @ui.error
+ end
+
+ assert_equal %w[a-2.a], @cmd.installed_specs.map { |spec| spec.full_name }
+ end
+
+ def test_execute_prerelease_skipped_when_non_pre_available
util_setup_fake_fetcher :prerelease
util_clear_gems
util_setup_spec_fetcher @a2, @a2_pre
@@ -296,7 +342,7 @@ def test_execute_prerelease
assert_equal 0, e.exit_code, @ui.error
end
- assert_equal %w[a-2.a], @cmd.installed_specs.map { |spec| spec.full_name }
+ assert_equal %w[a-2], @cmd.installed_specs.map { |spec| spec.full_name }
end
def test_execute_rdoc
View
118 test/rubygems/test_gem_dependency_installer.rb
@@ -53,17 +53,57 @@ def test_install
def test_install_prerelease
util_setup_gems
- a1_pre_data = File.read(@a1_pre_gem)
+ p1a, gem = util_gem 'a', '10.a'
- @fetcher.data['http://gems.example.com/gems/a-1.a.gem'] = a1_pre_data
- inst = nil
+ util_setup_spec_fetcher(p1a, @a1, @a1_pre)
+ util_clear_gems
+
+ p1a_data = File.read(gem)
+
+ @fetcher.data['http://gems.example.com/gems/a-10.a.gem'] = p1a_data
dep = Gem::Dependency.new "a"
inst = Gem::DependencyInstaller.new :prerelease => true
inst.install dep
- assert_equal %w[a-1.a], Gem::Specification.map(&:full_name)
- assert_equal [@a1_pre], inst.installed_gems
+ assert_equal %w[a-10.a], Gem::Specification.map(&:full_name)
+ assert_equal [p1a], inst.installed_gems
+ end
+
+ def test_install_when_only_prerelease
+ p1a, gem = util_gem 'p', '1.a'
+
+ util_setup_spec_fetcher(p1a)
+ util_clear_gems
+
+ p1a_data = File.read(gem)
+
+ @fetcher.data['http://gems.example.com/gems/p-1.a.gem'] = p1a_data
+
+ dep = Gem::Dependency.new "p"
+ inst = Gem::DependencyInstaller.new
+ inst.install dep
+
+ assert_equal %w[], Gem::Specification.map(&:full_name)
+ assert_equal [], inst.installed_gems
+ end
+
+ def test_install_prerelease_skipped_when_normal_ver
+ util_setup_gems
+
+ util_setup_spec_fetcher(@a1, @a1_pre)
+ util_clear_gems
+
+ p1a_data = File.read(@a1_gem)
+
+ @fetcher.data['http://gems.example.com/gems/a-1.gem'] = p1a_data
+
+ dep = Gem::Dependency.new "a"
+ inst = Gem::DependencyInstaller.new :prerelease => true
+ inst.install dep
+
+ assert_equal %w[a-1], Gem::Specification.map(&:full_name)
+ assert_equal [@a1], inst.installed_gems
end
def test_install_all_dependencies
@@ -171,6 +211,44 @@ def test_install_dependencies_satisfied
assert_equal %w[b-1], inst.installed_gems.map { |s| s.full_name }
end
+ # This asserts that if a gem's dependency is satisfied by an
+ # already installed gem, RubyGems doesn't installed a newer
+ # version
+ def test_install_doesnt_upgrade_installed_depedencies
+ util_setup_gems
+
+ a2, a2_gem = util_gem 'a', '2'
+ a3, a3_gem = util_gem 'a', '3'
+
+ util_setup_spec_fetcher @a1, a3, @b1
+
+ FileUtils.rm_rf File.join(@gemhome, 'gems')
+
+ Gem::Specification.reset
+
+ FileUtils.mv @a1_gem, @tempdir
+ FileUtils.mv a2_gem, @tempdir # not in index
+ FileUtils.mv @b1_gem, @tempdir
+ FileUtils.mv a3_gem, @tempdir
+
+ inst = nil
+
+ Dir.chdir @tempdir do
+ inst = Gem::DependencyInstaller.new
+ inst.install 'a', Gem::Requirement.create("= 2")
+ end
+
+ FileUtils.rm File.join(@tempdir, a2.file_name)
+
+ Dir.chdir @tempdir do
+ inst = Gem::DependencyInstaller.new
+ inst.install 'b'
+ end
+
+ assert_equal %w[a-2 b-1], Gem::Specification.map(&:full_name)
+ assert_equal %w[b-1], inst.installed_gems.map { |s| s.full_name }
+ end
+
def test_install_dependency
util_setup_gems
@@ -690,8 +768,14 @@ def test_find_gems_gems_with_sources
Gem::Specification.reset
- assert_equal [[@b1, Gem::Source.new(@gem_repo)]],
- inst.find_gems_with_sources(dep)
+ set = inst.find_gems_with_sources(dep)
+
+ assert_kind_of Gem::AvailableSet, set
+
+ s = set.set.first
+
+ assert_equal @b1, s.spec
+ assert_equal Gem::Source.new(@gem_repo), s.source
end
def test_find_gems_with_sources_local
@@ -700,21 +784,23 @@ def test_find_gems_with_sources_local
FileUtils.mv @a1_gem, @tempdir
inst = Gem::DependencyInstaller.new
dep = Gem::Dependency.new 'a', '>= 0'
- gems = nil
+ set = nil
Dir.chdir @tempdir do
- gems = inst.find_gems_with_sources dep
+ set = inst.find_gems_with_sources dep
end
+ gems = set.sorted
+
assert_equal 2, gems.length
local = gems.first
- assert_equal 'a-1', local.first.full_name, 'local spec'
+ assert_equal 'a-1', local.spec.full_name, 'local spec'
assert_equal File.join(@tempdir, @a1.file_name),
- local.last.download(local.first), 'local path'
+ local.source.download(local.spec), 'local path'
remote = gems.last
- assert_equal 'a-1', remote.first.full_name, 'remote spec'
- assert_equal Gem::Source.new(@gem_repo), remote.last, 'remote path'
+ assert_equal 'a-1', remote.spec.full_name, 'remote spec'
+ assert_equal Gem::Source.new(@gem_repo), remote.source, 'remote path'
end
@@ -726,15 +812,15 @@ def test_find_gems_with_sources_prerelease
dependency = Gem::Dependency.new('a', Gem::Requirement.default)
releases =
- installer.find_gems_with_sources(dependency).map { |gems, *| gems }
+ installer.find_gems_with_sources(dependency).all_specs
assert releases.any? { |s| s.name == 'a' and s.version.to_s == '1' }
refute releases.any? { |s| s.name == 'a' and s.version.to_s == '1.a' }
dependency.prerelease = true
prereleases =
- installer.find_gems_with_sources(dependency).map { |gems, *| gems }
+ installer.find_gems_with_sources(dependency).all_specs
assert_equal [@a1_pre, @a1], prereleases
end
@@ -748,7 +834,7 @@ def test_find_gems_with_sources_with_bad_source
out = installer.find_gems_with_sources(dep)
- assert_equal [], out
+ assert out.empty?
assert_kind_of Gem::SourceFetchProblem, installer.errors.first
end
View
9 test/rubygems/test_gem_source_local.rb
@@ -71,4 +71,13 @@ def test_download
assert_equal File.expand_path(@a.file_name), path
end
+
+ def test_compare
+ uri = URI.parse "http://gems.example/foo"
+ s = Gem::Source.new uri
+
+ assert_equal -1, (@sl <=> s)
+ assert_equal 1, (s <=> @sl)
+ assert_equal 0, (@sl <=> @sl)
+ end
end

0 comments on commit 432fc48

Please sign in to comment.
Something went wrong with that request. Please try again.