From 94a6c7bc798e8c77b4eac11ca2bbab66ebe76d03 Mon Sep 17 00:00:00 2001 From: David Dollar Date: Tue, 10 Mar 2009 10:44:24 -0400 Subject: [PATCH 1/2] Fixes a lot of brokenness in the gems rake tasks. * Fixes the chicken/egg problem present in the current gem system when gems are defined in the config that are not yet installed. * Makes the gem system understand development vs. runtime dependencies, removing the need to have hoe as a dependency of your production app. * Makes the gem 'unpacking' system a lot less fragile. * Significantly cleans up the code associated with the gem system, making it much less opaque. --- railties/lib/initializer.rb | 4 +- railties/lib/rails/gem_dependency.rb | 214 +++++++++++--------- railties/lib/tasks/gems.rake | 78 +++----- railties/test/gem_dependency_test.rb | 17 +- .../vendor/gems/dummy-gem-g-1.0.0/.specification | 2 +- 5 files changed, 163 insertions(+), 152 deletions(-) diff --git a/railties/lib/initializer.rb b/railties/lib/initializer.rb index edea4e5..a04405a 100644 --- a/railties/lib/initializer.rb +++ b/railties/lib/initializer.rb @@ -301,7 +301,9 @@ module Rails end def load_gems - @configuration.gems.each { |gem| gem.load } + unless $gems_build_rake_task + @configuration.gems.each { |gem| gem.load } + end end def check_gem_dependencies diff --git a/railties/lib/rails/gem_dependency.rb b/railties/lib/rails/gem_dependency.rb index 2dd6590..3d81b14 100644 --- a/railties/lib/rails/gem_dependency.rb +++ b/railties/lib/rails/gem_dependency.rb @@ -7,8 +7,8 @@ module Gem end module Rails - class GemDependency - attr_accessor :lib, :source + class GemDependency < Gem::Dependency + attr_accessor :lib, :source, :dep def self.unpacked_path @unpacked_path ||= File.join(RAILS_ROOT, 'vendor', 'gems') @@ -29,18 +29,6 @@ module Rails end end - def framework_gem? - @@framework_gems.has_key?(name) - end - - def vendor_rails? - Gem.loaded_specs.has_key?(name) && Gem.loaded_specs[name].loaded_from.empty? - end - - def vendor_gem? - Gem.loaded_specs.has_key?(name) && Gem.loaded_specs[name].loaded_from.include?(self.class.unpacked_path) - end - def initialize(name, options = {}) require 'rubygems' unless Object.const_defined?(:Gem) @@ -52,10 +40,11 @@ module Rails req = Gem::Requirement.default end - @dep = Gem::Dependency.new(name, req) @lib = options[:lib] @source = options[:source] @loaded = @frozen = @load_paths_added = false + + super(name, req) end def add_load_paths @@ -65,52 +54,74 @@ module Rails @load_paths_added = @loaded = @frozen = true return end - gem @dep + gem self @spec = Gem.loaded_specs[name] @frozen = @spec.loaded_from.include?(self.class.unpacked_path) if @spec @load_paths_added = true rescue Gem::LoadError end - def dependencies(options = {}) - return [] if framework_gem? || specification.nil? - - all_dependencies = specification.dependencies.map do |dependency| + def dependencies + return [] if framework_gem? + return [] unless installed? + specification.dependencies.reject do |dependency| + dependency.type == :development + end.map do |dependency| GemDependency.new(dependency.name, :requirement => dependency.version_requirements) end + end - all_dependencies += all_dependencies.map { |d| d.dependencies(options) }.flatten if options[:flatten] - all_dependencies.uniq + def specification + # code repeated from Gem.activate. Find a matching spec, or the currently loaded version. + # error out if loaded version and requested version are incompatible. + @spec ||= begin + matches = Gem.source_index.search(self) + matches << @@framework_gems[name] if framework_gem? + if Gem.loaded_specs[name] then + # This gem is already loaded. If the currently loaded gem is not in the + # list of candidate gems, then we have a version conflict. + existing_spec = Gem.loaded_specs[name] + unless matches.any? { |spec| spec.version == existing_spec.version } then + raise Gem::Exception, + "can't activate #{@dep}, already activated #{existing_spec.full_name}" + end + # we're stuck with it, so change to match + version_requirements = Gem::Requirement.create("=#{existing_spec.version}") + existing_spec + else + # new load + matches.last + end + end end - def gem_dir(base_directory) - File.join(base_directory, specification.full_name) + def requirement + r = version_requirements + (r == Gem::Requirement.default) ? nil : r end - def spec_filename(base_directory) - File.join(gem_dir(base_directory), '.specification') + def built? + # TODO: If Rubygems ever gives us a way to detect this, we should use it + false end - def load - return if @loaded || @load_paths_added == false - require(@lib || name) unless @lib == false - @loaded = true - rescue LoadError - puts $!.to_s - $!.backtrace.each { |b| puts b } + def framework_gem? + @@framework_gems.has_key?(name) end - def name - @dep.name.to_s + def frozen? + @frozen ||= vendor_rails? || vendor_gem? end - def requirement - r = @dep.version_requirements - (r == Gem::Requirement.default) ? nil : r + def installed? + Gem.loaded_specs.keys.include?(name) end - def frozen? - @frozen ||= vendor_rails? || vendor_gem? + def load_paths_added? + # always try to add load paths - even if a gem is loaded, it may not + # be a compatible version (ie random_gem 0.4 is loaded and a later spec + # needs >= 0.5 - gem 'random_gem' will catch this and error out) + @load_paths_added end def loaded? @@ -136,48 +147,49 @@ module Rails end end - def load_paths_added? - # always try to add load paths - even if a gem is loaded, it may not - # be a compatible version (ie random_gem 0.4 is loaded and a later spec - # needs >= 0.5 - gem 'random_gem' will catch this and error out) - @load_paths_added + def vendor_rails? + Gem.loaded_specs.has_key?(name) && Gem.loaded_specs[name].loaded_from.empty? end - def install - cmd = "#{gem_command} #{install_command.join(' ')}" - puts cmd - puts %x(#{cmd}) + def vendor_gem? + specification && File.exists?(unpacked_gem_directory) end - def unpack_to(directory) - return if specification.nil? || File.directory?(gem_dir(directory)) || framework_gem? - - FileUtils.mkdir_p directory - Dir.chdir directory do - Gem::GemRunner.new.run(unpack_command) + def build + require 'rails/gem_builder' + unless built? + return unless File.exists?(unpacked_specification_filename) + spec = YAML::load_file(unpacked_specification_filename) + Rails::GemBuilder.new(spec, unpacked_gem_directory).build_extensions + puts "Built gem: '#{unpacked_gem_directory}'" end - - # Gem.activate changes the spec - get the original - real_spec = Gem::Specification.load(specification.loaded_from) - write_spec(directory, real_spec) - + dependencies.each { |dep| dep.build } end - def write_spec(directory, spec) - # copy the gem's specification into GEMDIR/.specification so that - # we can access information about the gem on deployment systems - # without having the gem installed - File.open(spec_filename(directory), 'w') do |file| - file.puts spec.to_yaml + def install + unless installed? + cmd = "#{gem_command} #{install_command.join(' ')}" + puts cmd + puts %x(#{cmd}) end end - def refresh_spec(directory) + def load + return if @loaded || @load_paths_added == false + require(@lib || name) unless @lib == false + @loaded = true + rescue LoadError + puts $!.to_s + $!.backtrace.each { |b| puts b } + end + + def refresh + Rails::VendorGemSourceIndex.silence_spec_warnings = true real_gems = Gem.source_index.installed_source_index exact_dep = Gem::Dependency.new(name, "= #{specification.version}") matches = real_gems.search(exact_dep) installed_spec = matches.first - if File.exist?(File.dirname(spec_filename(directory))) + if frozen? if installed_spec # we have a real copy # get a fresh spec - matches should only have one element @@ -185,11 +197,11 @@ module Rails # spec is the same as the copy from real_gems - Gem.activate changes # some of the fields real_spec = Gem::Specification.load(matches.first.loaded_from) - write_spec(directory, real_spec) + write_specification(real_spec) puts "Reloaded specification for #{name} from installed gems." else # the gem isn't installed locally - write out our current specs - write_spec(directory, specification) + write_specification(specification) puts "Gem #{name} not loaded locally - writing out current spec." end else @@ -201,40 +213,35 @@ module Rails end end - def ==(other) - self.name == other.name && self.requirement == other.requirement + def unpack(options={}) + unless frozen? || framework_gem? + FileUtils.mkdir_p unpack_base + Dir.chdir unpack_base do + Gem::GemRunner.new.run(unpack_command) + end + # Gem.activate changes the spec - get the original + real_spec = Gem::Specification.load(specification.loaded_from) + write_specification(real_spec) + end + dependencies.each { |dep| dep.unpack } if options[:recursive] end - alias_method :"eql?", :"==" - def hash - @dep.hash + def write_specification(spec) + # copy the gem's specification into GEMDIR/.specification so that + # we can access information about the gem on deployment systems + # without having the gem installed + File.open(unpacked_specification_filename, 'w') do |file| + file.puts spec.to_yaml + end end - def specification - # code repeated from Gem.activate. Find a matching spec, or the currently loaded version. - # error out if loaded version and requested version are incompatible. - @spec ||= begin - matches = Gem.source_index.search(@dep) - matches << @@framework_gems[name] if framework_gem? - if Gem.loaded_specs[name] then - # This gem is already loaded. If the currently loaded gem is not in the - # list of candidate gems, then we have a version conflict. - existing_spec = Gem.loaded_specs[name] - unless matches.any? { |spec| spec.version == existing_spec.version } then - raise Gem::Exception, - "can't activate #{@dep}, already activated #{existing_spec.full_name}" - end - # we're stuck with it, so change to match - @dep.version_requirements = Gem::Requirement.create("=#{existing_spec.version}") - existing_spec - else - # new load - matches.last - end - end + def ==(other) + self.name == other.name && self.requirement == other.requirement end + alias_method :"eql?", :"==" private + def gem_command RUBY_PLATFORM =~ /win32/ ? 'gem.bat' : 'gem' end @@ -251,5 +258,18 @@ module Rails cmd << "--version" << "= "+specification.version.to_s if requirement cmd end + + def unpack_base + Rails::GemDependency.unpacked_path + end + + def unpacked_gem_directory + File.join(unpack_base, specification.full_name) + end + + def unpacked_specification_filename + File.join(unpacked_gem_directory, '.specification') + end + end end diff --git a/railties/lib/tasks/gems.rake b/railties/lib/tasks/gems.rake index 0932ba7..ed07bf2 100644 --- a/railties/lib/tasks/gems.rake +++ b/railties/lib/tasks/gems.rake @@ -9,71 +9,57 @@ task :gems => 'gems:base' do puts "R = Framework (loaded before rails starts)" end -def print_gem_status(gem, indent=1) - code = gem.loaded? ? (gem.frozen? ? (gem.framework_gem? ? "R" : "F") : "I") : " " - puts " "*(indent-1)+" - [#{code}] #{gem.name} #{gem.requirement.to_s}" - gem.dependencies.each { |g| print_gem_status(g, indent+1)} if gem.loaded? -end - namespace :gems do task :base do $gems_rake_task = true + require 'rubygems' + require 'rubygems/gem_runner' Rake::Task[:environment].invoke end desc "Build any native extensions for unpacked gems" task :build do - $gems_rake_task = true - require 'rails/gem_builder' - Dir[File.join(Rails::GemDependency.unpacked_path, '*')].each do |gem_dir| - spec_file = File.join(gem_dir, '.specification') - next unless File.exists?(spec_file) - specification = YAML::load_file(spec_file) - next unless ENV['GEM'].blank? || ENV['GEM'] == specification.name - Rails::GemBuilder.new(specification, gem_dir).build_extensions - puts "Built gem: '#{gem_dir}'" - end + $gems_build_rake_task = true + Rake::Task['gems:unpack'].invoke + current_gems.each &:build end - desc "Installs all required gems for this application." + desc "Installs all required gems." task :install => :base do - require 'rubygems' - require 'rubygems/gem_runner' - Rails.configuration.gems.each { |gem| gem.install unless gem.loaded? } + current_gems.each &:install end - desc "Unpacks the specified gem into vendor/gems." - task :unpack => :base do - require 'rubygems' - require 'rubygems/gem_runner' - Rails.configuration.gems.each do |gem| - next unless ENV['GEM'].blank? || ENV['GEM'] == gem.name - gem.unpack_to(Rails::GemDependency.unpacked_path) - end + desc "Unpacks all required gems into vendor/gems." + task :unpack => :install do + current_gems.each &:unpack end namespace :unpack do - desc "Unpacks the specified gems and its dependencies into vendor/gems" - task :dependencies => :unpack do - require 'rubygems' - require 'rubygems/gem_runner' - Rails.configuration.gems.each do |gem| - next unless ENV['GEM'].blank? || ENV['GEM'] == gem.name - gem.dependencies(:flatten => true).each do |dependency| - dependency.unpack_to(Rails::GemDependency.unpacked_path) - end - end + desc "Unpacks all required gems and their dependencies into vendor/gems." + task :dependencies => :install do + current_gems.each { |gem| gem.unpack(:recursive => true) } end end desc "Regenerate gem specifications in correct format." task :refresh_specs => :base do - require 'rubygems' - require 'rubygems/gem_runner' - Rails::VendorGemSourceIndex.silence_spec_warnings = true - Rails.configuration.gems.each do |gem| - next unless gem.frozen? && (ENV['GEM'].blank? || ENV['GEM'] == gem.name) - gem.refresh_spec(Rails::GemDependency.unpacked_path) if gem.loaded? - end + current_gems.each &:refresh + end +end + +def current_gems + gems = Rails.configuration.gems + gems = gems.select { |gem| gem.name == ENV['GEM'] } unless ENV['GEM'].blank? + gems +end + +def print_gem_status(gem, indent=1) + code = case + when gem.framework_gem? then 'R' + when gem.frozen? then 'F' + when gem.installed? then 'I' + else ' ' end -end \ No newline at end of file + puts " "*(indent-1)+" - [#{code}] #{gem.name} #{gem.requirement.to_s}" + gem.dependencies.each { |g| print_gem_status(g, indent+1) } +end diff --git a/railties/test/gem_dependency_test.rb b/railties/test/gem_dependency_test.rb index 8b761c4..189ad02 100644 --- a/railties/test/gem_dependency_test.rb +++ b/railties/test/gem_dependency_test.rb @@ -46,31 +46,34 @@ class GemDependencyTest < Test::Unit::TestCase end def test_gem_adds_load_paths - @gem.expects(:gem).with(Gem::Dependency.new(@gem.name, nil)) + @gem.expects(:gem).with(@gem) @gem.add_load_paths end def test_gem_with_version_adds_load_paths - @gem_with_version.expects(:gem).with(Gem::Dependency.new(@gem_with_version.name, @gem_with_version.requirement.to_s)) + @gem_with_version.expects(:gem).with(@gem_with_version) @gem_with_version.add_load_paths + assert @gem_with_version.load_paths_added? end def test_gem_loading - @gem.expects(:gem).with(Gem::Dependency.new(@gem.name, nil)) + @gem.expects(:gem).with(@gem) @gem.expects(:require).with(@gem.name) @gem.add_load_paths @gem.load + assert @gem.loaded? end def test_gem_with_lib_loading - @gem_with_lib.expects(:gem).with(Gem::Dependency.new(@gem_with_lib.name, nil)) + @gem_with_lib.expects(:gem).with(@gem_with_lib) @gem_with_lib.expects(:require).with(@gem_with_lib.lib) @gem_with_lib.add_load_paths @gem_with_lib.load + assert @gem_with_lib.loaded? end def test_gem_without_lib_loading - @gem_without_load.expects(:gem).with(Gem::Dependency.new(@gem_without_load.name, nil)) + @gem_without_load.expects(:gem).with(@gem_without_load) @gem_without_load.expects(:require).with(@gem_without_load.lib).never @gem_without_load.add_load_paths @gem_without_load.load @@ -132,8 +135,8 @@ class GemDependencyTest < Test::Unit::TestCase dummy_gem = Rails::GemDependency.new "dummy-gem-g" dummy_gem.add_load_paths dummy_gem.load - assert dummy_gem.loaded? - assert_equal 2, dummy_gem.dependencies(:flatten => true).size + assert_equal 1, dummy_gem.dependencies.size + assert_equal 1, dummy_gem.dependencies.first.dependencies.size assert_nothing_raised do dummy_gem.dependencies.each do |g| g.dependencies diff --git a/railties/test/vendor/gems/dummy-gem-g-1.0.0/.specification b/railties/test/vendor/gems/dummy-gem-g-1.0.0/.specification index 5483048..27e2991 100644 --- a/railties/test/vendor/gems/dummy-gem-g-1.0.0/.specification +++ b/railties/test/vendor/gems/dummy-gem-g-1.0.0/.specification @@ -9,7 +9,7 @@ date: 2008-10-03 00:00:00 -04:00 dependencies: - !ruby/object:Gem::Dependency name: dummy-gem-f - type: :development + type: :runtime version_requirement: version_requirements: !ruby/object:Gem::Requirement requirements: -- 1.6.1.2 From ac1513cd1a1b7341ff2e90651ce5ceeadc5e707e Mon Sep 17 00:00:00 2001 From: David Dollar Date: Sat, 14 Mar 2009 11:43:51 -0400 Subject: [PATCH 2/2] Additional tests for the gem subsystem * test_gem_ignores_development_dependencies * test_gem_guards_against_duplicate_unpacks * test_gem_does_not_unpack_framework_gems --- railties/test/gem_dependency_test.rb | 20 ++++++++ .../vendor/gems/dummy-gem-h-1.0.0/.specification | 29 ++++++++++++ .../gems/dummy-gem-h-1.0.0/lib/dummy-gem-h.rb | 1 + .../vendor/gems/dummy-gem-i-1.0.0/.specification | 49 ++++++++++++++++++++ .../gems/dummy-gem-i-1.0.0/lib/dummy-gem-i.rb | 1 + 5 files changed, 100 insertions(+), 0 deletions(-) create mode 100644 railties/test/vendor/gems/dummy-gem-h-1.0.0/.specification create mode 100644 railties/test/vendor/gems/dummy-gem-h-1.0.0/lib/dummy-gem-h.rb create mode 100644 railties/test/vendor/gems/dummy-gem-i-1.0.0/.specification create mode 100644 railties/test/vendor/gems/dummy-gem-i-1.0.0/lib/dummy-gem-i.rb diff --git a/railties/test/gem_dependency_test.rb b/railties/test/gem_dependency_test.rb index 189ad02..a44b6a8 100644 --- a/railties/test/gem_dependency_test.rb +++ b/railties/test/gem_dependency_test.rb @@ -144,4 +144,24 @@ class GemDependencyTest < Test::Unit::TestCase end end + def test_gem_ignores_development_dependencies + dummy_gem = Rails::GemDependency.new "dummy-gem-i" + dummy_gem.add_load_paths + dummy_gem.load + assert_equal 1, dummy_gem.dependencies.size + end + + def test_gem_guards_against_duplicate_unpacks + dummy_gem = Rails::GemDependency.new "dummy-gem-a" + dummy_gem.expects(:frozen?).returns(true) + dummy_gem.expects(:unpack_base).never + dummy_gem.unpack + end + + def test_gem_does_not_unpack_framework_gems + dummy_gem = Rails::GemDependency.new "dummy-gem-a" + dummy_gem.expect(:framework_gem?).returns(true) + dummy_gem.expects(:unpack_base).never + dummy_gem.unpack + end end diff --git a/railties/test/vendor/gems/dummy-gem-h-1.0.0/.specification b/railties/test/vendor/gems/dummy-gem-h-1.0.0/.specification new file mode 100644 index 0000000..b3f7930 --- /dev/null +++ b/railties/test/vendor/gems/dummy-gem-h-1.0.0/.specification @@ -0,0 +1,29 @@ +--- !ruby/object:Gem::Specification +name: dummy-gem-h +version: !ruby/object:Gem::Version + version: 1.3.0 +platform: ruby +authors: +- "Nobody" +date: 2008-10-03 00:00:00 -04:00 +dependencies: +files: +- lib +- lib/dummy-gem-h.rb +require_paths: +- lib +required_ruby_version: !ruby/object:Gem::Requirement + requirements: + - - ">=" + - !ruby/object:Gem::Version + version: "0" + version: +required_rubygems_version: !ruby/object:Gem::Requirement + requirements: + - - ">=" + - !ruby/object:Gem::Version + version: "0" + version: +requirements: [] +specification_version: 2 +summary: Dummy Gem H diff --git a/railties/test/vendor/gems/dummy-gem-h-1.0.0/lib/dummy-gem-h.rb b/railties/test/vendor/gems/dummy-gem-h-1.0.0/lib/dummy-gem-h.rb new file mode 100644 index 0000000..0f91234 --- /dev/null +++ b/railties/test/vendor/gems/dummy-gem-h-1.0.0/lib/dummy-gem-h.rb @@ -0,0 +1 @@ +DUMMY_GEM_H_VERSION="1.0.0" diff --git a/railties/test/vendor/gems/dummy-gem-i-1.0.0/.specification b/railties/test/vendor/gems/dummy-gem-i-1.0.0/.specification new file mode 100644 index 0000000..f8af41d --- /dev/null +++ b/railties/test/vendor/gems/dummy-gem-i-1.0.0/.specification @@ -0,0 +1,49 @@ +--- !ruby/object:Gem::Specification +name: dummy-gem-i +version: !ruby/object:Gem::Version + version: 1.3.0 +platform: ruby +authors: +- "Nobody" +date: 2008-10-03 00:00:00 -04:00 +dependencies: +- !ruby/object:Gem::Dependency + name: dummy-gem-f + type: :runtime + version_requirement: + version_requirements: !ruby/object:Gem::Requirement + requirements: + - - ">=" + - !ruby/object:Gem::Version + version: 1.0.0 + version: +- !ruby/object:Gem::Dependency + name: dummy-gem-h + type: :development + version_requirement: + version_requirements: !ruby/object:Gem::Requirement + requirements: + - - ">=" + - !ruby/object:Gem::Version + version: 1.0.0 + version: +files: +- lib +- lib/dummy-gem-i.rb +require_paths: +- lib +required_ruby_version: !ruby/object:Gem::Requirement + requirements: + - - ">=" + - !ruby/object:Gem::Version + version: "0" + version: +required_rubygems_version: !ruby/object:Gem::Requirement + requirements: + - - ">=" + - !ruby/object:Gem::Version + version: "0" + version: +requirements: [] +specification_version: 2 +summary: Dummy Gem I diff --git a/railties/test/vendor/gems/dummy-gem-i-1.0.0/lib/dummy-gem-i.rb b/railties/test/vendor/gems/dummy-gem-i-1.0.0/lib/dummy-gem-i.rb new file mode 100644 index 0000000..2f9a376 --- /dev/null +++ b/railties/test/vendor/gems/dummy-gem-i-1.0.0/lib/dummy-gem-i.rb @@ -0,0 +1 @@ +DUMMY_GEM_I_VERSION="1.0.0" -- 1.6.1.2