See comment on patch below. Other than my comments, I'm fine with this.

Eric / Rich, could you weigh in?

 - Evan

On Apr 25, 2010, at 11:11 AM, Nobuyoshi Nakada wrote:

> Hi,
>=20
> At Fri, 16 Apr 2010 03:08:27 +0900,
> Evan Phoenix wrote in [ruby-core:29537]:
>> This moves the RubyGem custom require into the prelude and
>> triggers the full loading of RubyGems itself if the normal
>> require raises a LoadError. It fixes the problem because the
>> highest version gems are no longer pushed on $LOAD_PATH,
>> allowing RubyGems to activate gems and the proper
>> dependencies.
>=20
> I don't like the duplication, so changed to keep #require
> staying in custom_require.rb and introduce Gem.try_activate.
>=20
>=20
> diff --git c/common.mk i/common.mk
> index 36174c3..30b31c8 100644
> --- c/common.mk
> +++ i/common.mk
> @@ -733,7 +733,9 @@ known_errors.inc: =
$(srcdir)/template/known_errors.inc.tmpl $(srcdir)/defs/known_
> miniprelude.c: $(srcdir)/tool/compile_prelude.rb $(srcdir)/prelude.rb
> 	$(BASERUBY) -I$(srcdir) $(srcdir)/tool/compile_prelude.rb =
$(srcdir)/prelude.rb $@
>=20
> -prelude.c: $(srcdir)/tool/compile_prelude.rb $(RBCONFIG) =
$(srcdir)/lib/rubygems/defaults.rb $(PRELUDE_SCRIPTS) $(PREP)
> +prelude.c: $(srcdir)/tool/compile_prelude.rb $(RBCONFIG) \
> +	   $(srcdir)/lib/rubygems/defaults.rb =
$(srcdir)/lib/rubygems/custom_require.rb \
> +	   $(PRELUDE_SCRIPTS) $(PREP)
> 	$(COMPILE_PRELUDE) $(PRELUDE_SCRIPTS) $@
>=20
> golf_prelude.c: $(srcdir)/tool/compile_prelude.rb $(RBCONFIG) =
$(srcdir)/prelude.rb $(srcdir)/golf_prelude.rb $(PREP)
> diff --git c/gem_prelude.rb i/gem_prelude.rb
> index 71f30bd..09b8f3a 100644
> --- c/gem_prelude.rb
> +++ i/gem_prelude.rb
> @@ -1,4 +1,3 @@
> -# depends on: array.rb dir.rb env.rb file.rb hash.rb module.rb =
regexp.rb
> # vim: filetype=3Druby
>=20
> # NOTICE: Ruby is during initialization here.
> @@ -13,7 +12,8 @@ if defined?(Gem) then
>   module Kernel
>=20
>     def gem(gem_name, *version_requirements)
> -      Gem.push_gem_version_on_load_path(gem_name, =
*version_requirements)
> +      Gem::QuickLoader.load_full_rubygems_library
> +      gem gem_name, *version_requirements
>     end
>     private :gem
>   end
> @@ -133,29 +133,27 @@ if defined?(Gem) then
>     end
>=20
>     module QuickLoader
> -
> -      @loaded_full_rubygems_library =3D false
> -
>       def self.load_full_rubygems_library
> -        return if @loaded_full_rubygems_library
> +        return @loaded_full_rubygems_library if =
defined?(@loaded_full_rubygems_library)
>=20
> -        @loaded_full_rubygems_library =3D true
> +        @loaded_full_rubygems_library =3D false
>=20
>         class << Gem
>           undef_method(*Gem::GEM_PRELUDE_METHODS)
> -          undef_method :const_missing
> -          undef_method :method_missing
> -        end
> -
> -        Kernel.module_eval do
> -          undef_method :gem if method_defined? :gem
>         end
>=20
>         $".delete path_to_full_rubygems_library
>         if $".any? {|path| path.end_with?('/rubygems.rb')}
>           raise LoadError, "another rubygems is already loaded from =
#{path}"
>         end
> +
> +        verbose, debug =3D $VERBOSE, $DEBUG
> +        $VERBOSE =3D $DEBUG =3D nil
> +
>         require 'rubygems'
> +        @loaded_full_rubygems_library =3D true
> +      ensure
> +        $VERBOSE, $DEBUG =3D verbose, debug
>       end
>=20
>       def self.fake_rubygems_as_loaded
> @@ -177,95 +175,6 @@ if defined?(Gem) then
>         end
>       end
>=20
> -      GemPaths =3D {}
> -      GemVersions =3D {}
> -
> -      def push_gem_version_on_load_path(gem_name, =
*version_requirements)
> -        if version_requirements.empty?
> -          unless GemPaths.has_key?(gem_name) then
> -            raise Gem::LoadError, "Could not find RubyGem #{gem_name} =
(>=3D 0)\n"
> -          end
> -
> -          # highest version gems already active
> -          return false
> -        else
> -          if version_requirements.length > 1 then
> -            QuickLoader.load_full_rubygems_library
> -            return gem(gem_name, *version_requirements)
> -          end
> -
> -          requirement, version =3D version_requirements[0].split
> -          requirement.strip!
> -
> -          if loaded_version =3D GemVersions[gem_name] then
> -            case requirement
> -            when ">", ">=3D" then
> -              return false if
> -                (loaded_version <=3D> Gem.integers_for(version)) >=3D =
0
> -            when "~>" then
> -              required_version =3D Gem.integers_for version
> -
> -              return false if loaded_version.first =3D=3D =
required_version.first
> -            end
> -          end
> -
> -          QuickLoader.load_full_rubygems_library
> -          gem gem_name, *version_requirements
> -        end
> -      end
> -
> -      def integers_for(gem_version)
> -        numbers =3D gem_version.split(".").collect {|n| n.to_i}
> -        numbers.pop while numbers.last =3D=3D 0
> -        numbers << 0 if numbers.empty?
> -        numbers
> -      end
> -
> -      def push_all_highest_version_gems_on_load_path
> -        Gem.path.each do |path|
> -          gems_directory =3D File.join(path, "gems")
> -
> -          if File.exist?(gems_directory) then
> -            Dir.entries(gems_directory).each do |gem_directory_name|
> -              next if gem_directory_name =3D=3D "." || =
gem_directory_name =3D=3D ".."
> -
> -              next unless gem_name =3D =
gem_directory_name[/(.*)-(.*)/, 1]
> -              new_version =3D integers_for($2)
> -              current_version =3D GemVersions[gem_name]
> -
> -              if !current_version or (current_version <=3D> =
new_version) < 0 then
> -                GemVersions[gem_name] =3D new_version
> -                GemPaths[gem_name] =3D File.join(gems_directory, =
gem_directory_name)
> -              end
> -            end
> -          end
> -        end
> -
> -        require_paths =3D []
> -
> -        GemPaths.each_value do |path|
> -          if File.exist?(file =3D File.join(path, ".require_paths")) =
then
> -            paths =3D File.read(file).split.map do |require_path|
> -              File.join path, require_path
> -            end
> -
> -            require_paths.concat paths
> -          else
> -            require_paths << file if File.exist?(file =3D =
File.join(path, "bin"))
> -            require_paths << file if File.exist?(file =3D =
File.join(path, "lib"))
> -          end
> -        end
> -
> -        # "tag" the first require_path inserted into the $LOAD_PATH =
to enable
> -        # indexing correctly with rubygems proper when it inserts an =
explicitly
> -        # gem version
> -        unless require_paths.empty? then
> -          =
require_paths.first.instance_variable_set(:@gem_prelude_index, true)
> -        end
> -        # gem directories must come after -I and ENV['RUBYLIB']
> -        =
$:[$:.index{|e|e.instance_variable_defined?(:@gem_prelude_index)}||-1,0] =
=3D require_paths
> -      end
> -
>       def const_missing(constant)
>         QuickLoader.load_full_rubygems_library
>=20
> @@ -285,10 +194,15 @@ if defined?(Gem) then
>=20
>     extend QuickLoader
>=20
> +    def self.try_activate(path) # :nodoc:
> +      QuickLoader.load_full_rubygems_library
> +    end
> +
>   end

Doesn't the try_activate in prelude need to load full rubygems, then =
call try_activate again so that the proper version is called the first =
time try_activate is used?

>=20
>   begin
> -    Gem.push_all_highest_version_gems_on_load_path
> +    require 'lib/rubygems/custom_require.rb'
> +
>     Gem::QuickLoader.fake_rubygems_as_loaded
>   rescue Exception =3D> e
>     puts "Error loading gem paths on load path in gem_prelude"
> diff --git c/lib/rubygems.rb i/lib/rubygems.rb
> index d2214f6..29dd815 100644
> --- c/lib/rubygems.rb
> +++ i/lib/rubygems.rb
> @@ -1020,8 +1020,6 @@ end
>=20
> module Kernel
>=20
> -  undef gem if respond_to? :gem # defined in gem_prelude.rb on 1.9
> -
>   ##
>   # Use Kernel#gem to activate a specific version of +gem_name+.
>   #
> @@ -1098,13 +1096,34 @@ end
>=20
> require 'rubygems/config_file'
>=20
> +class << Gem
> +  verbose, debug =3D $VERBOSE, $DEBUG
> +  $VERBOSE =3D $DEBUG =3D nil

Why is setting $VERBOSE and $DEBUG needed?

> +
> +  ##
> +  #
> +  # Called from the custom_require to attempt to activate +path+
> +  # Internal use only.
> +
> +  def try_activate(path) # :doc:
> +    spec =3D Gem.searcher.find(path)
> +    return false unless spec
> +
> +    Gem.activate(spec.name, "=3D #{spec.version}")
> +    return true
> +  end
> +
> +ensure
> +  $VERBOSE, $DEBUG =3D verbose, debug
> +end
> +
> ##
> # Enables the require hook for RubyGems.
> #
> # Ruby 1.9 allows --disable-gems, so we require it when we didn't =
detect a Gem
> # constant at rubygems.rb load time.
>=20
> -require 'rubygems/custom_require' if gem_disabled or RUBY_VERSION < =
'1.9'
> +require 'rubygems/custom_require'
>=20
> Gem.clear_paths
>=20
> diff --git c/lib/rubygems/custom_require.rb =
i/lib/rubygems/custom_require.rb
> index 43b3136..343ad31 100644
> --- c/lib/rubygems/custom_require.rb
> +++ i/lib/rubygems/custom_require.rb
> @@ -4,8 +4,6 @@
> # See LICENSE.txt for permissions.
> #++
>=20
> -require 'rubygems'
> -
> module Kernel
>=20
>   ##
> @@ -31,8 +29,7 @@ module Kernel
>     gem_original_require path
>   rescue LoadError =3D> load_error
>     if load_error.message.end_with?(path) and
> -       spec =3D Gem.searcher.find(path) then
> -      Gem.activate(spec.name, "=3D #{spec.version}")
> +       Gem.try_activate(path) then
>       gem_original_require path
>     else
>       raise load_error
> @@ -42,5 +39,5 @@ module Kernel
>   private :require
>   private :gem_original_require
>=20
> -end
> +end unless Kernel.private_method_defined?(:gem_original_require)
>=20
>=20
>=20
> --=20
> Nobu Nakada
>=20
>=20