search

xcpretty / xcode-install

🔽 Install and update your Xcodes
https://fastlane.tools
MIT License
2.58k stars 237 forks source link

fixing the cli option retry_download_count to support simulator #405

Open gunesmes opened 3 years ago

gunesmes commented 3 years ago

While I was submitting my PR https://github.com/xcpretty/xcode-install/pull/404, it was already solved by @freddi-kit with https://github.com/xcpretty/xcode-install/pull/400. However this PR includes an update for downloading simulators but the corresponding part was not implemented to simulators.rb so this will cause a bug for downloading simulators, as following:

bash-3.2$ xcversion simulators --install='iOS 11.3 Simulator' --retry-download-count=5

simulators --install=iOS 11.3 Simulator --retry-download-count=5
[!] Unknown option: `--retry-download-count=5`
Did you mean: --force?

Usage:

    $ xcversion simulators

      List or install iOS simulators.

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.
    --verbose        Show more debugging information
    --no-ansi        Show output without ANSI codes
    --help           Show help banner of specified command
bash-3.2$

or

bash-3.2$ xcversion simulators --install='iOS 11.3 Simulator'

simulators --install=iOS 11.3 Simulator
Installing iOS 11.3 Simulator for Xcode 11.5.0...
Traceback (most recent call last):
    6: from /Users/mesut/.rbenv/versions/2.7.1/bin/xcversion:23:in `<main>'
    5: from /Users/mesut/.rbenv/versions/2.7.1/bin/xcversion:23:in `load'
    4: from /Users/mesut/.rbenv/versions/2.7.1/lib/ruby/gems/2.7.0/gems/xcode-install-2.6.6/bin/xcversion:12:in `<top (required)>'
    3: from /Users/mesut/.rbenv/versions/2.7.1/lib/ruby/gems/2.7.0/gems/claide-1.0.3/lib/claide/command.rb:334:in `run'
    2: from /Users/mesut/.rbenv/versions/2.7.1/lib/ruby/gems/2.7.0/gems/xcode-install-2.6.6/lib/xcode/install/simulators.rb:26:in `run'
    1: from /Users/mesut/.rbenv/versions/2.7.1/lib/ruby/gems/2.7.0/gems/xcode-install-2.6.6/lib/xcode/install/simulators.rb:45:in `install'
/Users/mesut/.rbenv/versions/2.7.1/lib/ruby/gems/2.7.0/gems/xcode-install-2.6.6/lib/xcode/install.rb:531:in `install': wrong number of arguments (given 2, expected 3) (ArgumentError)

This PR is fixing this

Simulator

without --retry_download_count, default is 3

bash-3.2$ xcversion simulators --install='iOS 11.0 Simulator'

simulators --install=iOS 11.0 Simulator
  9 1989M    9  189M    0     0  1677k      0  0:20:14  0:01:55  0:18:19 1805k%
curl: (18) transfer closed with 1886829804 bytes remaining to read
  5 1799M    5  105M    0     0  1697k      0  0:18:05  0:01:03  0:17:02 1811k%
curl: (18) transfer closed with 1776103408 bytes remaining to read
  9 1693M    9  163M    0     0  1590k      0  0:18:10  0:01:45  0:16:25 1541k%
curl: (18) transfer closed with 1604346613 bytes remaining to read
%[!] Failed to download iOS 11.0 Simulator.

with --retry-download-count=5 will try to dowload 5 times

bash-3.2$ xcversion simulators --install='iOS 10.3.1 Simulator' --retry=5

simulators --install=iOS 10.3.1 Simulator --retry=5
 13 1198M   13  164M    0     0   717k      0  0:28:30  0:03:54  0:24:36  861k%
curl: (18) transfer closed with 1084795514 bytes remaining to read
 34  865M   34  298M    0     0  1421k      0  0:10:23  0:03:34  0:06:49 1855k%
curl: (18) transfer closed with 595033804 bytes remaining to read
 31  528M   31  163M    0     0  1653k      0  0:05:27  0:01:41  0:03:46 1737k%
curl: (18) transfer closed with 381830355 bytes remaining to read
 45  364M   45  163M    0     0  1194k      0  0:05:12  0:02:20  0:02:52  354k%
curl: (18) transfer closed with 209942487 bytes remaining to read
 53  200M   53  107M    0     0  1510k      0  0:02:15  0:01:12  0:01:03 1760k%
curl: (18) transfer closed with 97659610 bytes remaining to read
%[!] Failed to download iOS 10.3.1 Simulator.
bash-3.2$
gunesmes commented 3 years ago

@jpsim @freddi-kit could you please review this?

freddi-kit commented 3 years ago

Thank you for fixing my bug! 👍 I did not notice

gunesmes commented 3 years ago

@freddi-kit could you please review changes?

freddi-kit commented 3 years ago

@gunesmes Could you help to run bundle exec rubocop -a again? robocop is reporting some code have to be fixed.

gunesmes commented 3 years ago

@gunesmes Could you help to run bundle exec rubocop -a again? robocop is reporting some code have to be fixed.

To solve all offenses, we should have made the number_of_try optional param. I have updated to fix this. dmg_path = download(progress, nil, number_of_try)

gunesmes commented 3 years ago

@jpsim fixed the issues, could you please accept the pr?

freddi-kit commented 3 years ago

@jpsim who can push to gem? it would be helpful the person who have network issue of curl.

traviswimer commented 3 years ago

Is there something holding up this pull-request? It sounds like the work has been completed.

I tried just cloning the repo and running from this branch, but I can't seem to get it to work. I think it's likely my lack of Ruby knowledge is the problem though. I get an error running bundle install that it needs an older version of bundler, but using the older version tells me it needs the newer version.

If this isn't ready to be merged, could someone offer some guidance on how I can get this branch working as a temporary fix?

gunesmes commented 3 years ago

@traviswimer someone who have write access should approve this PR and then push the gem for the new release. @joshdholtz @KrauseFx @orta or @steipete

joshdholtz commented 3 years ago

@gunesmes On it! And just for record... I’m the only one out of those 4 mentioned that are associated with this project ATM so no need to mention them in the future 😉 Just trying to save everyone from some extra GitHub notifications 🙃

gunesmes commented 3 years ago

@joshdholtz to fix the Assignment Branch Condition, separated the curl part of the function to new function as curl_file.

joshdholtz commented 3 years ago

@gunesmes Thank you! Testing this out one more time by downloading some Xcode but code is looking 💯

joshdholtz commented 3 years ago

@gunesmes What is this PR originally just for? Was this just for really adding the --retry-count to the simulators command? 😇

gunesmes commented 3 years ago

Yes this is fixing the simulator part for adding retry-count.

joshdholtz commented 3 years ago

@gunesmes Okay, thank you! I’ll dig in to see what’s going on. I might end up redoing some of the argument name changes to make it backwards compatible with the previous version but I will make sure that parameter works 😇 Hopefully we can get this merged and released for you tomorrow!

freddi-kit commented 3 years ago

How is this progressed?

gunesmes commented 3 years ago

@joshdholtz do you have any update for the your testing?

pronebird commented 3 years ago

Can we please bump the default retry count to 10 because there is no way this succeeds in three attempts.

But on the other side, it looks like xcversion does not really understand what's happening in curl. I still see this as a bug, because it's clear that curl knows that there is more data left to download, yet it stops for no apparent reason.

I think the code handling the retry count should be fixed, i.e:

let CURLE_PARTIAL_FILE = 18
if curl.exitCode == CURLE_PARTIAL_FILE, downloadFileSize < expectedFileSize {
  continueDownload()
} else {
  retryCount += 1
  if retryCount > maxRetryCount {
    reportFailure()
  } else {
    continueDownload()
  } 
}

That's the thing I continue witnessing for years now:

xcversion simulators --install="iOS 10.3.1"          

Installing iOS 10.3.1 Simulator for Xcode 12.4.0...
 13 1883M   13  249M    0     0  2882k      0  0:11:09  0:01:28  0:09:41 2562k%
curl: (18) transfer closed with 1713831788 bytes remaining to read
 22 1634M   22  361M    0     0  2475k      0  0:11:16  0:02:29  0:08:47 2399k%
curl: (18) transfer closed with 1334473327 bytes remaining to read
 12 1272M   12  165M    0     0  3026k      0  0:07:10  0:00:55  0:06:15 3755k%^R
 24 1272M   24  305M    0     0  2764k      0  0:07:51  0:01:53  0:05:58 2404k%
curl: (18) transfer closed with 1013687667 bytes remaining to read
%[!] Failed to download iOS 10.3.1 Simulator.
gunesmes commented 3 years ago

Update retrial count to 10

Onno113 commented 2 years ago

I am not sure what is missing to put the changes here to a release. It looks like it could be missing some kind of approval from @joshdholtz

I am trying to install simulators with xcversion, but it fails every time. I tried already around 10 times. I would like this to be more reliable, because we want to use this for the setup of our CI machines.

xcversion is an amazing tool and it helps us already a lot with installing different kind of Xcode versions, thanks a lot! :-)

gunesmes commented 1 year ago

@joshdholtz this is still waiting for your approval, any chance you can review it?