fixing the cli option retry_download_count to support simulator

lib/xcode/install.rb

@@ -25,18 +25,63 @@ class Curl
     # @param progress: parse and show the progress?
     # @param progress_block: A block that's called whenever we have an updated progress %
     #                        the parameter is a single number that's literally percent (e.g. 1, 50, 80 or 100)
-    # @param retry_download_count: A count to retry the downloading Xcode dmg/xip
-    # rubocop:disable Metrics/AbcSize
+    # @param number_of_try: How many times try to download DMG file if downloading fails. Default is 3.
+    def curl_file(command_string,
+                  progress,
+                  progress_block,
+                  progress_log_file,
+                  number_of_try)
+      # Run the curl command in a loop, retry when curl exit status is 18
+      # "Partial file. Only a part of the file was transferred."
+      # https://curl.haxx.se/mail/archive-2008-07/0098.html
+      # https://github.com/KrauseFx/xcode-install/issues/210
+      number_of_try.times do
+        # Non-blocking call of Open3
+        # We're not using the block based syntax, as the bacon testing
+        # library doesn't seem to support writing tests for it
+        stdin, stdout, stderr, wait_thr = Open3.popen3(command_string)
+
+        # Poll the file and see if we're done yet
+        while wait_thr.alive?
+          sleep(0.5) # it's not critical for this to be real-time
+          next unless File.exist?(progress_log_file) # it might take longer for it to be created
+
+          progress_content = File.read(progress_log_file).split("\r").last
+
+          # Print out the progress for the CLI
+          if progress
+            print "\r#{progress_content}%"
+            $stdout.flush
+          end
+
+          # Call back the block for other processes that might be interested
+          matched = progress_content.match(/^\s*(\d+)/)
+          next unless matched && matched.length == 2
+          percent = matched[1].to_i
+          progress_block.call(percent) if progress_block
+        end
+
+        # as we're not making use of the block-based syntax
+        # we need to manually close those
+        stdin.close
+        stdout.close
+        stderr.close
+
+        return wait_thr.value.success? if wait_thr.value.success?
+      end
+    end
+
     def fetch(url: nil,
               directory: nil,
               cookies: nil,
               output: nil,
               progress: nil,
               progress_block: nil,
-              retry_download_count: 3)
+              number_of_try: 3)
       options = cookies.nil? ? [] : ['--cookie', cookies, '--cookie-jar', COOKIES_PATH]
 
       uri = URI.parse(url)
+
       output ||= File.basename(uri.path)
       output = (Pathname.new(directory) + Pathname.new(output)) if directory
 
@@ -59,7 +104,7 @@ def fetch(url: nil,
       progress_log_file = File.join(CACHE_DIR, "progress.#{Time.now.to_i}.progress")
       FileUtils.rm_f(progress_log_file)
 
-      retry_options = ['--retry', '3']
+      retry_options = ['--retry', number_of_try]
       command = [
         'curl',
         '--disable',
@@ -76,44 +121,7 @@ def fetch(url: nil,
       command_string = command.collect(&:shellescape).join(' ')
       command_string += " 2> #{progress_log_file}" # to not run shellescape on the `2>`
 
-      # Run the curl command in a loop, retry when curl exit status is 18
-      # "Partial file. Only a part of the file was transferred."
-      # https://curl.haxx.se/mail/archive-2008-07/0098.html
-      # https://github.com/KrauseFx/xcode-install/issues/210
-      retry_download_count.times do
-        # Non-blocking call of Open3
-        # We're not using the block based syntax, as the bacon testing
-        # library doesn't seem to support writing tests for it
-        stdin, stdout, stderr, wait_thr = Open3.popen3(command_string)
-
-        # Poll the file and see if we're done yet
-        while wait_thr.alive?
-          sleep(0.5) # it's not critical for this to be real-time
-          next unless File.exist?(progress_log_file) # it might take longer for it to be created
-
-          progress_content = File.read(progress_log_file).split("\r").last
-
-          # Print out the progress for the CLI
-          if progress
-            print "\r#{progress_content}%"
-            $stdout.flush
-          end
-
-          # Call back the block for other processes that might be interested
-          matched = progress_content.match(/^\s*(\d+)/)
-          next unless matched && matched.length == 2
-          percent = matched[1].to_i
-          progress_block.call(percent) if progress_block
-        end
-
-        # as we're not making use of the block-based syntax
-        # we need to manually close those
-        stdin.close
-        stdout.close
-        stderr.close
-
-        return wait_thr.value.success? if wait_thr.value.success?
-      end
+      curl_file(command_string, progress, progress_block, progress_log_file, number_of_try)
       false
     ensure
       FileUtils.rm_f(COOKIES_PATH)
@@ -137,7 +145,7 @@ def current_symlink
       File.symlink?(SYMLINK_PATH) ? SYMLINK_PATH : nil
     end
 
-    def download(version, progress, url = nil, progress_block = nil, retry_download_count = 3)
+    def download(version, progress, url = nil, progress_block = nil, number_of_try = 3)
       xcode = find_xcode_version(version) if url.nil?
       return if url.nil? && xcode.nil?
 
@@ -150,7 +158,7 @@ def download(version, progress, url = nil, progress_block = nil, retry_download_
         output: dmg_file,
         progress: progress,
         progress_block: progress_block,
-        retry_download_count: retry_download_count
+        number_of_try: number_of_try
       )
       result ? CACHE_DIR + dmg_file : nil
     end
@@ -283,8 +291,8 @@ def install_dmg(dmg_path, suffix = '', switch = true, clean = true)
     end
 
     # rubocop:disable Metrics/ParameterLists
-    def install_version(version, switch = true, clean = true, install = true, progress = true, url = nil, show_release_notes = true, progress_block = nil, retry_download_count = 3)
-      dmg_path = get_dmg(version, progress, url, progress_block, retry_download_count)
+    def install_version(version, switch = true, clean = true, install = true, progress = true, url = nil, show_release_notes = true, progress_block = nil, number_of_try = 3)
+      dmg_path = get_dmg(version, progress, url, progress_block, number_of_try)
       fail Informative, "Failed to download Xcode #{version}." if dmg_path.nil?
 
       if install
@@ -373,7 +381,7 @@ def enable_developer_mode
       `sudo /usr/sbin/dseditgroup -o edit -t group -a staff _developer`
     end
 
-    def get_dmg(version, progress = true, url = nil, progress_block = nil, retry_download_count = 3)
+    def get_dmg(version, progress = true, url = nil, progress_block = nil, number_of_try = 3)
       if url
         path = Pathname.new(url)
         return path if path.exist?
@@ -384,7 +392,7 @@ def get_dmg(version, progress = true, url = nil, progress_block = nil, retry_dow
         end
       end
 
-      download(version, progress, url, progress_block, retry_download_count)
+      download(version, progress, url, progress_block, number_of_try)
     end
 
     def fetch_seedlist
@@ -515,19 +523,19 @@ def xcode
       end
     end
 
-    def download(progress, progress_block = nil, retry_download_count = 3)
+    def download(progress, progress_block = nil, number_of_try = 3)
       result = Curl.new.fetch(
         url: source,
         directory: CACHE_DIR,
         progress: progress,
         progress_block: progress_block,
-        retry_download_count: retry_download_count
+        number_of_try: number_of_try
       )
       result ? dmg_path : nil
     end
 
-    def install(progress, should_install)
-      dmg_path = download(progress)
+    def install(progress, should_install, number_of_try)
+      dmg_path = download(progress, nil, number_of_try)
       fail Informative, "Failed to download #{@name}." if dmg_path.nil?
 
       return unless should_install

lib/xcode/install/install.rb

@@ -18,7 +18,7 @@ def self.options
          ['--no-progress', 'Don’t show download progress.'],
          ['--no-clean', 'Don’t delete DMG after installation.'],
          ['--no-show-release-notes', 'Don’t open release notes in browser after installation.'],
-         ['--retry-download-count', 'Count of retrying download when curl is failed.']].concat(super)
+         ['--retry-count', 'How many times try to download DMG file if downloading fails. Default is 3.']].concat(super)
       end
 
       def initialize(argv)
@@ -32,7 +32,7 @@ def initialize(argv)
         @should_switch = argv.flag?('switch', true)
         @progress = argv.flag?('progress', true)
         @show_release_notes = argv.flag?('show-release-notes', true)
-        @retry_download_count = argv.option('retry-download-count', '3')
+        @number_of_try = argv.option('retry-count', '3')
         super
       end
 
@@ -46,12 +46,12 @@ def validate!
         end
         fail Informative, "Version #{@version} doesn't exist." unless @url || @installer.exist?(@version)
         fail Informative, "Invalid URL: `#{@url}`" unless !@url || @url =~ /\A#{URI.regexp}\z/
-        fail Informative, "Invalid Retry: `#{@retry_download_count} is not positive number.`" if (@retry_download_count =~ /\A[0-9]*\z/).nil?
+        fail Informative, "Invalid Retry: `#{@number_of_try} is not positive number.`" if (@number_of_try =~ /\A[0-9]*\z/).nil?
       end
 
       def run
         @installer.install_version(@version, @should_switch, @should_clean, @should_install,
-                                   @progress, @url, @show_release_notes, nil, @retry_download_count.to_i)
+                                   @progress, @url, @show_release_notes, nil, @number_of_try.to_i)
       end
     end
   end

lib/xcode/install/simulators.rb

@@ -10,7 +10,8 @@ def self.options
         [['--install=name', 'Install simulator beginning with name, e.g. \'iOS 8.4\', \'tvOS 9.0\'.'],
          ['--force', 'Install even if the same version is already installed.'],
          ['--no-install', 'Only download DMG, but do not install it.'],
-         ['--no-progress', 'Don’t show download progress.']].concat(super)
+         ['--no-progress', 'Don’t show download progress.'],
+         ['--retry-count', 'How many times try to download DMG file if downloading fails. Default is 3.']].concat(super)
       end
 
       def initialize(argv)
@@ -19,6 +20,7 @@ def initialize(argv)
         @force = argv.flag?('force', false)
         @should_install = argv.flag?('install', true)
         @progress = argv.flag?('progress', true)
+        @number_of_try = argv.option('retry-count', '3')
         super
       end
 
@@ -42,7 +44,8 @@ def install
         simulator = filtered_simulators.first
         fail Informative, "#{simulator.name} is already installed." if simulator.installed? && !@force
         puts "Installing #{simulator.name} for Xcode #{simulator.xcode.bundle_version}..."
-        simulator.install(@progress, @should_install)
+        fail Informative, "Invalid Retry: `#{@number_of_try} is not positive number.`" if (@number_of_try =~ /\A[0-9]*\z/).nil?
+        simulator.install(@progress, @should_install, @number_of_try.to_i)
       else
         puts "[!] More than one simulator matching #{@install} was found. Please specify the full version.".ansi.red
         filtered_simulators.each do |candidate|