[Rubygems-developers] [patch] use open-uri in remote_installer.rb

[George Marrows]

> Nice patch. It's great to see all of those "-" symbols in the patch. ;)
>
> It has been applied.
>
> Thanks,
> Chad 


Thanks Chad - sorry you had to battle with the formatting :(

To avoid all such problems, I've attached a new version of the patch. 
This version:
- should actually delete the now unused methods :)
- restores the --http-proxy command line option to its previous glory
- moves all open-uri dependencies off to a fetch method

As before, use of open-uri ensures HTTP_PROXY, http_proxy, NO_PROXY etc 
are all honoured.

-- George
-------------- next part --------------
? cvs
? patch-use-open-uri
? patch-use-open-uri-2
? readme
? todo
? bin/cvs
? doc/cvs
? example/cvs
? example/lib/cvs
? example/lib/test/cvs
? examples/cvs
? examples/application/cvs
? examples/application/bin/cvs
? examples/application/lib/cvs
? gemspecs/cvs
? gemspecs/readme
? lib/cvs
? lib/rubygems/cvs
? lib/rubygems/remote_installer.rb.my
? lib/rubygems/remote_installer.rb.my2
? packages/cvs
? packages/sources/cvs
? packages/sources/sources-0.0.1.gem
? packages/sources/lib/cvs
? test/cvs
Index: bin/gem
===================================================================
RCS file: /var/cvs/rubygems/rubygems/bin/gem,v
retrieving revision 1.42
diff -u -r1.42 gem
--- bin/gem	27 Mar 2004 13:23:56 -0000	1.42
+++ bin/gem	27 Mar 2004 21:21:50 -0000
@@ -192,7 +192,7 @@
       op.on(      '--force',           "Force gem to intall, bypassing dependency checks") {|o.force|}
       op.on(      '--gen-rdoc',        "Generate RDoc documentation for the gem on install") {|o.gen_rdoc|}
       op.on(      '--run-tests',       "Run unit tests prior to installation") {|o.run_tests|}
-      op.on(      '--http-proxy', "Use HTTP Proxy for remote operations. Proxy must be specified in http_proxy or HTTP_PROXY environment variable") {|o.http_proxy|}
+      op.on(      '--http-proxy PROXY', "Use HTTP proxy for remote operations. Specify as full http:// URL.") { |o.http_proxy| }
       op.on(      '--version VERSION', "Specify version of gem to perform operation on") {|o.gem_version|}
       op.on('-L', '--local',           "Restrict operations to the LOCAL domain") { o.domain = :local }
       op.on('-R', '--remote',          "Restrict operations to the REMOTE domain") { o.domain = :remote }
@@ -504,7 +504,7 @@
   end
 
   def _get_remote_installer
-    Gem::RemoteInstaller.new(@options.http_proxy)
+    Gem::RemoteInstaller.new(@options.http_proxy || true)
   end
 
   # List all gems, local and/or remote, that match the given pattern.  This provides the
Index: lib/rubygems/remote_installer.rb
===================================================================
RCS file: /var/cvs/rubygems/rubygems/lib/rubygems/remote_installer.rb,v
retrieving revision 1.14
diff -u -r1.14 remote_installer.rb
--- lib/rubygems/remote_installer.rb	27 Mar 2004 13:17:53 -0000	1.14
+++ lib/rubygems/remote_installer.rb	27 Mar 2004 21:21:51 -0000
@@ -5,7 +5,8 @@
 
     ##
     # http_proxy:: [String] URL of http proxy.  Will override any environment 
-    #   variable setting
+    # variable setting. Defaults of true causes open-uri to use
+    # environment variables.
     def initialize(http_proxy=true)
       require 'open-uri'
 
@@ -66,14 +67,12 @@
 
       caches = {}
       sources.each do |source|
-    open(source + "/yaml",
-         :proxy => @http_proxy) do |yaml_source|
+        yaml = fetch(source + "/yaml")
 
-      spec = YAML.load(yaml_source.read)
+        spec = YAML.load(yaml)
 
-      raise "Didn't get a valid YAML document" if not spec
-      caches[source] = spec
-    end
+        raise "Didn't get a valid YAML document" if not spec
+        caches[source] = spec
       end
       return caches
     end
@@ -118,40 +117,8 @@
     def download_gem(destination_file, source, spec)
       # TODO
       uri = source + "/gems/#{spec.full_name}.gem"
-      response = fetch(uri)
-      write_gem_to_file(response.body, destination_file)
-    end
-    ##
-    # Returns HTTP proxy info if specified
-    # (code adapted/borrowed from raa-install)
-    def get_proxy
-      name = 'http_proxy'
-      if proxy_uri = @http_proxy || ENV[name] || ENV[name.upcase]
-        proxy_uri = URI.parse(proxy_uri)
-        name = 'no_proxy'
-        if no_proxy = ENV[name] || ENV[name.upcase]
-          no_proxy.scan(/([^:,]*)(?::(\d+))?/) {|host, port|
-            if /(\A|\.)#{Regexp.quote host}\z/i =~ proxy_uri.host && (!port || 80 == port.to_i)
-              proxy_uri = nil
-              break
-            end                
-          }
-        end
-        proxy_uri
-      else
-        nil
-      end
-    end
-
-    ##
-    # Returns the class to use for downloading files via http.  This method exists sole so that it can be overridden in the derived class in a unit test to return a mock http class.  Note that we could override the fetch method in the derived class instead, but then we wouldn't be able to test it.
-    def http_class
-      require 'net/http'
-      if proxy_uri = get_proxy
-        Net::HTTP.Proxy(proxy_uri.host, proxy_uri.port)
-      else
-        Net::HTTP
-      end
+      gem = fetch(uri)
+      write_gem_to_file(gem, destination_file)
     end
 
     def write_gem_to_file(body, destination_file)
@@ -160,20 +127,10 @@
       end
     end
 
-    def fetch( uri_str, limit = 10 )
-      require 'uri'
-      require 'net/http'
-
-      # You should choose better exception. 
-      raise ArgumentError, 'http redirect too deep' if limit == 0
-
-      response = http_class().get_response(URI.parse(uri_str))
-      case response
-      when Net::HTTPSuccess     then response
-      when Net::HTTPRedirection then fetch(response['location'], limit - 1)
-      else
-        $stderr.puts "Error downloading #{uri_str}"
-        response.error!
+    def fetch(uri)
+      open(uri, :proxy => @http_proxy) do 
+	|input|
+	input.read
       end
     end