Revert "Refactor SVN and cURL download strategies."

12 files changed, 96 insertions, 74 deletions

diff --git a/Library/Homebrew/cask/lib/hbc/audit.rb b/Library/Homebrew/cask/lib/hbc/audit.rb
index b8bb6ab81..cee1fe807 100644
--- a/Library/Homebrew/cask/lib/hbc/audit.rb
+++ b/Library/Homebrew/cask/lib/hbc/audit.rb
@@ -143,15 +143,7 @@ module Hbc
 
     def check_appcast_http_code
       odebug "Verifying appcast returns 200 HTTP response code"
-
-      curl_executable, *args = curl_args(
-        "--compressed", "--location", "--fail",
-        "--write-out", "%{http_code}",
-        "--output", "/dev/null",
-        cask.appcast,
-        user_agent: :fake
-      )
-      result = @command.run(curl_executable, args: args, print_stderr: false)
+      result = @command.run("/usr/bin/curl", args: ["--compressed", "--location", "--user-agent", URL::FAKE_USER_AGENT, "--output", "/dev/null", "--write-out", "%{http_code}", cask.appcast], print_stderr: false)
       if result.success?
         http_code = result.stdout.chomp
         add_warning "unexpected HTTP response code retrieving appcast: #{http_code}" unless http_code == "200"
diff --git a/Library/Homebrew/cask/lib/hbc/cask_loader.rb b/Library/Homebrew/cask/lib/hbc/cask_loader.rb
index dd9c61089..500314671 100644
--- a/Library/Homebrew/cask/lib/hbc/cask_loader.rb
+++ b/Library/Homebrew/cask/lib/hbc/cask_loader.rb
@@ -71,7 +71,7 @@ module Hbc
 
         begin
           ohai "Downloading #{url}."
-          curl_download url, to: path
+          curl url, "-o", path
         rescue ErrorDuringExecution
           raise CaskUnavailableError.new(token, "Failed to download #{Formatter.url(url)}.")
         end
diff --git a/Library/Homebrew/cask/lib/hbc/container.rb b/Library/Homebrew/cask/lib/hbc/container.rb
index fab3a3c1c..93e825e03 100644
--- a/Library/Homebrew/cask/lib/hbc/container.rb
+++ b/Library/Homebrew/cask/lib/hbc/container.rb
@@ -4,7 +4,6 @@ require "hbc/container/bzip2"
 require "hbc/container/cab"
 require "hbc/container/criteria"
 require "hbc/container/dmg"
-require "hbc/container/directory"
 require "hbc/container/executable"
 require "hbc/container/generic_unar"
 require "hbc/container/gpg"
@@ -15,7 +14,6 @@ require "hbc/container/otf"
 require "hbc/container/pkg"
 require "hbc/container/seven_zip"
 require "hbc/container/sit"
-require "hbc/container/svn_repository"
 require "hbc/container/tar"
 require "hbc/container/ttf"
 require "hbc/container/rar"
@@ -45,7 +43,6 @@ module Hbc
         Xz,    # pure xz
         Gpg,   # GnuPG signed data
         Executable,
-        SvnRepository,
       ]
       # for explicit use only (never autodetected):
       # Hbc::Container::Naked
diff --git a/Library/Homebrew/cask/lib/hbc/container/criteria.rb b/Library/Homebrew/cask/lib/hbc/container/criteria.rb
index 52f171d6a..66ecb8c87 100644
--- a/Library/Homebrew/cask/lib/hbc/container/criteria.rb
+++ b/Library/Homebrew/cask/lib/hbc/container/criteria.rb
@@ -13,11 +13,9 @@ module Hbc
       end
 
       def magic_number(regex)
-        return false if path.directory?
-
         # 262: length of the longest regex (currently: Hbc::Container::Tar)
-        @magic_number ||= File.open(path, "rb") { |f| f.read(262) }
-        @magic_number.match?(regex)
+        @magic_number ||= File.open(@path, "rb") { |f| f.read(262) }
+        @magic_number =~ regex
       end
     end
   end
diff --git a/Library/Homebrew/cask/lib/hbc/container/directory.rb b/Library/Homebrew/cask/lib/hbc/container/directory.rb
deleted file mode 100644
index e4bb1095b..000000000
--- a/Library/Homebrew/cask/lib/hbc/container/directory.rb
+++ /dev/null
@@ -1,24 +0,0 @@
-require "hbc/container/base"
-
-module Hbc
-  class Container
-    class Directory < Base
-      def self.me?(*)
-        false
-      end
-
-      def extract
-        @path.children.each do |child|
-          next if skip_path?(child)
-          FileUtils.cp child, @cask.staged_path
-        end
-      end
-
-      private
-
-      def skip_path?(*)
-        false
-      end
-    end
-  end
-end
diff --git a/Library/Homebrew/cask/lib/hbc/container/executable.rb b/Library/Homebrew/cask/lib/hbc/container/executable.rb
index af3b36fd1..848f6d4be 100644
--- a/Library/Homebrew/cask/lib/hbc/container/executable.rb
+++ b/Library/Homebrew/cask/lib/hbc/container/executable.rb
@@ -8,7 +8,7 @@ module Hbc
         return true if criteria.magic_number(/^#!\s*\S+/)
 
         begin
-          criteria.path.file? && MachO.open(criteria.path).header.executable?
+          MachO.open(criteria.path).header.executable?
         rescue MachO::MagicError
           false
         end
diff --git a/Library/Homebrew/cask/lib/hbc/container/svn_repository.rb b/Library/Homebrew/cask/lib/hbc/container/svn_repository.rb
deleted file mode 100644
index cae613b2d..000000000
--- a/Library/Homebrew/cask/lib/hbc/container/svn_repository.rb
+++ /dev/null
@@ -1,15 +0,0 @@
-require "hbc/container/directory"
-
-module Hbc
-  class Container
-    class SvnRepository < Directory
-      def self.me?(criteria)
-        criteria.path.join(".svn").directory?
-      end
-
-      def skip_path?(path)
-        path.basename.to_s == ".svn"
-      end
-    end
-  end
-end
diff --git a/Library/Homebrew/cask/lib/hbc/download_strategy.rb b/Library/Homebrew/cask/lib/hbc/download_strategy.rb
index 245ad4ade..016bb66e6 100644
--- a/Library/Homebrew/cask/lib/hbc/download_strategy.rb
+++ b/Library/Homebrew/cask/lib/hbc/download_strategy.rb
@@ -10,7 +10,7 @@ module Hbc
   class AbstractDownloadStrategy
     attr_reader :cask, :name, :url, :uri_object, :version
 
-    def initialize(cask, command: SystemCommand)
+    def initialize(cask, command = SystemCommand)
       @cask       = cask
       @command    = command
       # TODO: this excess of attributes is a function of integrating
@@ -33,8 +33,8 @@ module Hbc
   class HbVCSDownloadStrategy < AbstractDownloadStrategy
     REF_TYPES = [:branch, :revision, :revisions, :tag].freeze
 
-    def initialize(*args, **options)
-      super(*args, **options)
+    def initialize(cask, command = SystemCommand)
+      super
       @ref_type, @ref = extract_ref
       @clone = Hbc.cache.join(cache_filename)
     end
@@ -64,6 +64,11 @@ module Hbc
   end
 
   class CurlDownloadStrategy < AbstractDownloadStrategy
+    # TODO: should be part of url object
+    def mirrors
+      @mirrors ||= []
+    end
+
     def tarball_path
       @tarball_path ||= Hbc.cache.join("#{name}--#{version}#{ext}")
     end
@@ -90,8 +95,13 @@ module Hbc
       end
     end
 
+    def downloaded_size
+      temporary_path.size? || 0
+    end
+
     def _fetch
-      curl_download url, *cask_curl_args, to: temporary_path, user_agent: uri_object.user_agent
+      odebug "Calling curl with args #{cask_curl_args}"
+      curl(*cask_curl_args)
     end
 
     def fetch
@@ -121,12 +131,33 @@ module Hbc
         ignore_interrupts { temporary_path.rename(tarball_path) }
       end
       tarball_path
+    rescue CurlDownloadStrategyError
+      raise if mirrors.empty?
+      puts "Trying a mirror..."
+      @url = mirrors.shift
+      retry
     end
 
     private
 
     def cask_curl_args
-      cookies_args + referer_args
+      default_curl_args.tap do |args|
+        args.concat(user_agent_args)
+        args.concat(cookies_args)
+        args.concat(referer_args)
+      end
+    end
+
+    def default_curl_args
+      [url, "-C", downloaded_size, "-o", temporary_path]
+    end
+
+    def user_agent_args
+      if uri_object.user_agent
+        ["-A", uri_object.user_agent]
+      else
+        []
+      end
     end
 
     def cookies_args
@@ -160,7 +191,8 @@ module Hbc
 
   class CurlPostDownloadStrategy < CurlDownloadStrategy
     def cask_curl_args
-      super.concat(post_args)
+      super
+      default_curl_args.concat(post_args)
     end
 
     def post_args
@@ -193,8 +225,8 @@ module Hbc
 
     # super does not provide checks for already-existing downloads
     def fetch
-      if cached_location.directory?
-        puts "Already downloaded: #{cached_location}"
+      if tarball_path.exist?
+        puts "Already downloaded: #{tarball_path}"
       else
         @url = @url.sub(/^svn\+/, "") if @url =~ %r{^svn\+http://}
         ohai "Checking out #{@url}"
@@ -220,8 +252,9 @@ module Hbc
         else
           fetch_repo @clone, @url
         end
+        compress
       end
-      cached_location
+      tarball_path
     end
 
     # This primary reason for redefining this method is the trust_cert
@@ -255,6 +288,10 @@ module Hbc
                     print_stderr: false)
     end
 
+    def tarball_path
+      @tarball_path ||= cached_location.dirname.join(cached_location.basename.to_s + "-#{@cask.version}.tar")
+    end
+
     def shell_quote(str)
       # Oh god escaping shell args.
       # See http://notetoself.vrensk.com/2008/08/escaping-single-quotes-in-ruby-harder-than-expected/
@@ -267,5 +304,35 @@ module Hbc
         yield name, url
       end
     end
+
+    private
+
+    # TODO/UPDATE: the tar approach explained below is fragile
+    # against challenges such as case-sensitive filesystems,
+    # and must be re-implemented.
+    #
+    # Seems nutty: we "download" the contents into a tape archive.
+    # Why?
+    # * A single file is tractable to the rest of the Cask toolchain,
+    # * An alternative would be to create a Directory container type.
+    #   However, some type of file-serialization trick would still be
+    #   needed in order to enable calculating a single checksum over
+    #   a directory.  So, in that alternative implementation, the
+    #   special cases would propagate outside this class, including
+    #   the use of tar or equivalent.
+    # * SubversionDownloadStrategy.cached_location is not versioned
+    # * tarball_path provides a needed return value for our overridden
+    #   fetch method.
+    # * We can also take this private opportunity to strip files from
+    #   the download which are protocol-specific.
+
+    def compress
+      Dir.chdir(cached_location) do
+        @command.run!("/usr/bin/tar",
+                      args:         ['-s/^\.//', "--exclude", ".svn", "-cf", Pathname.new(tarball_path), "--", "."],
+                      print_stderr: false)
+      end
+      clear_cache
+    end
   end
 end
diff --git a/Library/Homebrew/cask/lib/hbc/dsl/appcast.rb b/Library/Homebrew/cask/lib/hbc/dsl/appcast.rb
index fc7e83a20..d302d0946 100644
--- a/Library/Homebrew/cask/lib/hbc/dsl/appcast.rb
+++ b/Library/Homebrew/cask/lib/hbc/dsl/appcast.rb
@@ -12,11 +12,7 @@ module Hbc
       end
 
       def calculate_checkpoint
-        curl_executable, *args = curl_args(
-          "--compressed", "--location", "--fail", @uri,
-          user_agent: :fake
-        )
-        result = SystemCommand.run(curl_executable, args: args, print_stderr: false)
+        result = SystemCommand.run("/usr/bin/curl", args: ["--compressed", "--location", "--user-agent", URL::FAKE_USER_AGENT, "--fail", @uri], print_stderr: false)
 
         checkpoint = if result.success?
           processed_appcast_text = result.stdout.gsub(%r{<pubDate>[^<]*</pubDate>}m, "")
diff --git a/Library/Homebrew/cask/lib/hbc/system_command.rb b/Library/Homebrew/cask/lib/hbc/system_command.rb
index b735ae4f9..901617b71 100644
--- a/Library/Homebrew/cask/lib/hbc/system_command.rb
+++ b/Library/Homebrew/cask/lib/hbc/system_command.rb
@@ -112,7 +112,11 @@ module Hbc
                  processed_output[:stderr],
                  processed_status.exitstatus)
     end
+  end
+end
 
+module Hbc
+  class SystemCommand
     class Result
       attr_accessor :command, :stdout, :stderr, :exit_status
 
diff --git a/Library/Homebrew/cask/lib/hbc/url.rb b/Library/Homebrew/cask/lib/hbc/url.rb
index 8c652657b..15da2ced2 100644
--- a/Library/Homebrew/cask/lib/hbc/url.rb
+++ b/Library/Homebrew/cask/lib/hbc/url.rb
@@ -1,6 +1,8 @@
 module Hbc
   class URL
-    attr_reader :using, :revision, :trust_cert, :uri, :cookies, :referer, :data, :user_agent
+    FAKE_USER_AGENT = "Mozilla/5.0 (Macintosh; Intel Mac OS X 10) https://caskroom.github.io".freeze
+
+    attr_reader :using, :revision, :trust_cert, :uri, :cookies, :referer, :data
 
     extend Forwardable
     def_delegators :uri, :path, :scheme, :to_s
@@ -15,7 +17,7 @@ module Hbc
 
     def initialize(uri, options = {})
       @uri        = Hbc::UnderscoreSupportingURI.parse(uri)
-      @user_agent = options.fetch(:user_agent, :default)
+      @user_agent = options[:user_agent]
       @cookies    = options[:cookies]
       @referer    = options[:referer]
       @using      = options[:using]
@@ -23,5 +25,10 @@ module Hbc
       @trust_cert = options[:trust_cert]
       @data       = options[:data]
     end
+
+    def user_agent
+      return FAKE_USER_AGENT if @user_agent == :fake
+      @user_agent
+    end
   end
 end
diff --git a/Library/Homebrew/cask/lib/hbc/verify/gpg.rb b/Library/Homebrew/cask/lib/hbc/verify/gpg.rb
index f4996a5b5..dbb537756 100644
--- a/Library/Homebrew/cask/lib/hbc/verify/gpg.rb
+++ b/Library/Homebrew/cask/lib/hbc/verify/gpg.rb
@@ -33,7 +33,7 @@ module Hbc
         meta_dir = cached || cask.metadata_subdir("gpg", :now, true)
         sig_path = meta_dir.join("signature.asc")
 
-        curl_download cask.gpg.signature, to: sig_path unless cached || force
+        curl(cask.gpg.signature, "-o", sig_path.to_s) unless cached || force
 
         sig_path
       end