Progress Bar During Downloads#2
Conversation
|
I like it, however I'm not sure of including ProgressBar globally. Saying this because there is a progressbar gem: https://rubygems.org/gems/progressbar and it might clash. Rubinius guys did a simple implementation: http://github.com/evanphx/rubinius/blob/master/configure#L241-271 That did not depend on the the additional class. We also need to check how this could affect the automated output, specially for projects like Bundler and such. Guys? |
|
I added the progressbar class to the gem namespace, so it shouldn't conflict with anything now. |
|
Hello, I think the good idea will be remove ProgressBar at all. Also, all the remote operations are been handled by this, so fetching of specs and everything is streamed. There is also another problem, it doesn't work with 1.8.7: Tested with See my patch to only generate the progress report when a gem is being downloaded: http://gist.github.com/600349 |
|
I updated it to remove the progressbar class and to use a single line that gets redrawn as suggested in your gist. Also updated to work with 1.8.7. |
|
Thank you Ryan Posted about this to rubygems-devel: http://rubyforge.org/pipermail/rubygems-developers/2010-September/005573.html Waiting for feedback there before decide to integrate it. |
|
Hi Luis, I believe I addressed your issue of using print and flush directly by wrapping them in say-like statements. As no one has commented, can we get this incorporated? |
|
Hello Ryan. I bring this topic to RubyGems about making the progress bar something part of UI component to have better control of it. Also, this print/flush approach do not work nice with remote automations like Chef, so move of this to the UI and have an option to turn it of will be the best. See my thread and further comments there: http://rubyforge.org/pipermail/rubygems-developers/2010-September/005573.html This should be integrated, but to my bad, I'm not 100 percent sure on the approach and to my bad haven't received the feedback from other RubyGems maintainers Will keep you posted. |
|
Hello Ryan, I'm working today on extend UserInteraction to support a configurable Download reporter, in that way, it will make more easy for non-tty interfaces to work and not generate a bunch of output. Thank you again for your contribution. Will post a link to the commits once I'm done for you to test and review. |
|
Been wanting this forever. Any kind of feedback when |
|
I defer to Luis (who needs to stop deferring to the list). |
|
Hello guys, I've posted this to the list for review: http://rubyforge.org/pipermail/rubygems-developers/2010-December/005942.html I've implemented Ryan idea into a more modular and non-tty friendly version under download-reporter branch: https://github.com/rubygems/rubygems/tree/download-reporter Let me know your thoughts guys. |
|
Hello Guys, Wanted to let you know this has been merged into master (slightly different code, same idea) into master: Closing this out. Ryan, thank you for your contribution, patience and initial code for this implementation. |
Sometimes, as a result of parallel installation of gems reading & writing gemspecs concurrently during `bundle update`, some thread can end up printing warnings like the following: ``` [/home/runner/work/rubygems/rubygems/bundler/tmp/2/gems/system/specifications/zeitwerk-2.4.2.gemspec] isn't a Gem::Specification (NilClass instead). ``` This commit fixes the issue by completely overriding rubygems installer and removing all the unnecessary stuff. In particular, the `Gem::Specification.reset` call that was constantly invalidating gem specification caches and thus mutating shared state. As a results, `bundle install` time is noticiably faster. A quick test on my system (with ~500 gems installed) on an app with about ~100 locked gems (to avoid resolution overhead) without extensions (to avoid extension compilation overhead), with a fully loaded gem cache (to avoid network overhead), it results in ~15% speed up in `bundle install` time: ``` $ hyperfine -p 'rm -rf vendor/bundle' 'bundle install' 'bundle _2.2.11_ install' Benchmark #1: bundle install Time (mean ± σ): 5.713 s ± 0.387 s [User: 5.215 s, System: 2.163 s] Range (min … max): 5.443 s … 6.610 s 10 runs Benchmark #2: bundle _2.2.11_ install Time (mean ± σ): 6.549 s ± 0.106 s [User: 6.087 s, System: 2.353 s] Range (min … max): 6.412 s … 6.773 s 10 runs Summary 'bundle install' ran 1.15 ± 0.08 times faster than 'bundle _2.2.11_ install' ```
Sometimes, as a result of parallel installation of gems reading & writing gemspecs concurrently during `bundle update`, some thread can end up printing warnings like the following: ``` [/home/runner/work/rubygems/rubygems/bundler/tmp/2/gems/system/specifications/zeitwerk-2.4.2.gemspec] isn't a Gem::Specification (NilClass instead). ``` This commit fixes the issue by completely overriding rubygems installer and removing all the unnecessary stuff. In particular, the `Gem::Specification.reset` call that was constantly invalidating gem specification caches and thus mutating shared state. As a results, `bundle install` time is noticiably faster. A quick test on my system (with ~500 gems installed) on an app with about ~100 locked gems (to avoid resolution overhead) without extensions (to avoid extension compilation overhead), with a fully loaded gem cache (to avoid network overhead), it results in ~15% speed up in `bundle install` time: ``` $ hyperfine -p 'rm -rf vendor/bundle' 'bundle install' 'bundle _2.2.11_ install' Benchmark #1: bundle install Time (mean ± σ): 5.713 s ± 0.387 s [User: 5.215 s, System: 2.163 s] Range (min … max): 5.443 s … 6.610 s 10 runs Benchmark #2: bundle _2.2.11_ install Time (mean ± σ): 6.549 s ± 0.106 s [User: 6.087 s, System: 2.353 s] Range (min … max): 6.412 s … 6.773 s 10 runs Summary 'bundle install' ran 1.15 ± 0.08 times faster than 'bundle _2.2.11_ install' ```
Sometimes, as a result of parallel installation of gems reading & writing gemspecs concurrently during `bundle update`, some thread can end up printing warnings like the following: ``` [/home/runner/work/rubygems/rubygems/bundler/tmp/2/gems/system/specifications/zeitwerk-2.4.2.gemspec] isn't a Gem::Specification (NilClass instead). ``` This commit fixes the issue by completely overriding rubygems installer and removing all the unnecessary stuff. In particular, the `Gem::Specification.reset` call that was constantly invalidating gem specification caches and thus mutating shared state has been moved to the main thread, after after installation of all gems is finished. As a results, `bundle install` time is noticiably faster. A quick test on my system (with ~500 gems installed) on an app with about ~100 locked gems (to avoid resolution overhead) without extensions (to avoid extension compilation overhead), with a fully loaded gem cache (to avoid network overhead), it results in ~15% speed up in `bundle install` time: ``` $ hyperfine -p 'rm -rf vendor/bundle' 'bundle install' 'bundle _2.2.11_ install' Benchmark #1: bundle install Time (mean ± σ): 5.713 s ± 0.387 s [User: 5.215 s, System: 2.163 s] Range (min … max): 5.443 s … 6.610 s 10 runs Benchmark #2: bundle _2.2.11_ install Time (mean ± σ): 6.549 s ± 0.106 s [User: 6.087 s, System: 2.353 s] Range (min … max): 6.412 s … 6.773 s 10 runs Summary 'bundle install' ran 1.15 ± 0.08 times faster than 'bundle _2.2.11_ install' ```
Sometimes, as a result of parallel installation of gems reading & writing gemspecs concurrently during `bundle update`, some thread can end up printing warnings like the following: ``` [/home/runner/work/rubygems/rubygems/bundler/tmp/2/gems/system/specifications/zeitwerk-2.4.2.gemspec] isn't a Gem::Specification (NilClass instead). ``` The fix is to completely overriding rubygems installer to get rid of the places where it accessed and modified shared gem specification cache. In particular: * Stop calling `Gem::Specification.reset` after each gem installation and instead move it the main thread, after after installation of all gems has finished. * Stop using rubygems implementation of `Gem::Specification.latest_spec_for(name)` in favor of `Gem::Specification.stubs_for(name).first`, which does the same thing but without traversing and potentially loading all specifications. * Remove all the other code that's not actually necessary for bundler. Technically only the first point would be necessary to fix the race condition but I figured since I needed to overwrite the method I would only include what's needed, and make it faster. As a result, `bundle install` time is noticiably faster. A quick test on my system (with ~500 gems installed) on an app with about ~100 locked gems (to avoid resolution overhead) without extensions (to avoid extension compilation overhead), with a fully loaded gem cache (to avoid network overhead), it results in ~15% speed up in `bundle install` time: ``` $ hyperfine -p 'rm -rf vendor/bundle' 'bundle install' 'bundle _2.2.11_ install' Benchmark #1: bundle install Time (mean ± σ): 5.713 s ± 0.387 s [User: 5.215 s, System: 2.163 s] Range (min … max): 5.443 s … 6.610 s 10 runs Benchmark #2: bundle _2.2.11_ install Time (mean ± σ): 6.549 s ± 0.106 s [User: 6.087 s, System: 2.353 s] Range (min … max): 6.412 s … 6.773 s 10 runs Summary 'bundle install' ran 1.15 ± 0.08 times faster than 'bundle _2.2.11_ install' ```
When `bundle install` connects with a certification (CA) to a private RubyGems
HTTPS server emulated by WEBrick, the connection to the
https://localhost:18443/versions succeeded, but the connection to download gems
failed with the following error.
```
+ /home/jaruga/var/git/ruby/rubygems/bin/bundle config set --local ssl_ca_cert /home/jaruga/git/report-bundler-bundle-config-set-ssl_ca_cert/tmp/client/ssl/ca.crt
...
+ /home/jaruga/var/git/ruby/rubygems/bin/bundle install -V
Running `bundle install --verbose` with bundler 4.1.0.dev
Resolving dependencies because there's no lockfile
HTTP GET https://localhost:18443/versions
HTTP 206 Partial Content https://localhost:18443/versions
HTTP GET https://localhost:18443/versions
HTTP 200 OK https://localhost:18443/versions
Fetching gem metadata from https://localhost:18443/
Looking up gems ["hello"]
Resolving dependencies...
Using bundler 4.1.0.dev
1: bundler (4.1.0.dev) from /home/jaruga/var/git/ruby/rubygems/bundler/bundler.gemspec
Fetching hello 0.1.0
Retrying download gem from https://localhost:18443/ due to error (2/4): Gem::RemoteFetcher::FetchError SSL_connect returned=1 errno=0 peeraddr=127.0.0.1:18443 state=error: certificate verify failed (unable to get local issuer certificate) (https://localhost:18443/gems/hello-0.1.0.gem)
Sleeping for 1.22 seconds before retry
Retrying download gem from https://localhost:18443/ due to error (3/4): Gem::RemoteFetcher::FetchError SSL_connect returned=1 errno=0 peeraddr=127.0.0.1:18443 state=error: certificate verify failed (unable to get local issuer certificate) (https://localhost:18443/gems/hello-0.1.0.gem)
Sleeping for 2.26 seconds before retry
Retrying download gem from https://localhost:18443/ due to error (4/4): Gem::RemoteFetcher::FetchError SSL_connect returned=1 errno=0 peeraddr=127.0.0.1:18443 state=error: certificate verify failed (unable to get local issuer certificate) (https://localhost:18443/gems/hello-0.1.0.gem)
Sleeping for 4.02 seconds before retry
Bundler::InstallError: Bundler::HTTPError: Could not download gem from https://localhost:18443/ due to underlying error <SSL_connect returned=1 errno=0 peeraddr=127.0.0.1:18443 state=error: certificate verify failed (unable to get local issuer certificate) (https://localhost:18443/gems/hello-0.1.0.gem)>
/home/jaruga/var/git/ruby/rubygems/bundler/lib/bundler/rubygems_integration.rb:406:in 'Bundler::RubygemsIntegration#download_gem'
/home/jaruga/var/git/ruby/rubygems/bundler/lib/bundler/source/rubygems.rb:531:in 'block in Bundler::Source::Rubygems#download_gem'
/home/jaruga/.local/ruby-4.1.0-debug-3ef48ef9c8-openssl-4.1.0-7194354488/lib/ruby/4.1.0+1/rubygems.rb:1068:in 'Gem.time'
/home/jaruga/var/git/ruby/rubygems/bundler/lib/bundler/source/rubygems.rb:530:in 'Bundler::Source::Rubygems#download_gem'
/home/jaruga/var/git/ruby/rubygems/bundler/lib/bundler/source/rubygems.rb:459:in 'Bundler::Source::Rubygems#fetch_gem'
/home/jaruga/var/git/ruby/rubygems/bundler/lib/bundler/source/rubygems.rb:443:in 'Bundler::Source::Rubygems#fetch_gem_if_possible'
/home/jaruga/var/git/ruby/rubygems/bundler/lib/bundler/source/rubygems.rb:575:in 'Bundler::Source::Rubygems#rubygems_gem_installer'
/home/jaruga/var/git/ruby/rubygems/bundler/lib/bundler/source/rubygems.rb:184:in 'Bundler::Source::Rubygems#download'
/home/jaruga/var/git/ruby/rubygems/bundler/lib/bundler/installer/gem_installer.rb:29:in 'Bundler::GemInstaller#download'
/home/jaruga/var/git/ruby/rubygems/bundler/lib/bundler/installer/parallel_installer.rb:148:in 'Bundler::ParallelInstaller#do_download'
/home/jaruga/var/git/ruby/rubygems/bundler/lib/bundler/installer/parallel_installer.rb:132:in 'block in Bundler::ParallelInstaller#worker_pool'
/home/jaruga/var/git/ruby/rubygems/bundler/lib/bundler/worker.rb:70:in 'Bundler::Worker#apply_func'
/home/jaruga/var/git/ruby/rubygems/bundler/lib/bundler/worker.rb:65:in 'block in Bundler::Worker#process_queue'
/home/jaruga/var/git/ruby/rubygems/bundler/lib/bundler/worker.rb:56:in 'Kernel#loop'
/home/jaruga/var/git/ruby/rubygems/bundler/lib/bundler/worker.rb:56:in 'Bundler::Worker#process_queue'
/home/jaruga/var/git/ruby/rubygems/bundler/lib/bundler/worker.rb:98:in 'block (2 levels) in Bundler::Worker#create_threads'
...
```
`Bundler::Fetcher` has the `#connection` calling `#bundler_cert_store` storing
`Bundler.settings[:ssl_ca_cert]` in the following part. It is used in some parts.
bundler/lib/bundler/fetcher.rb
...
def bundler_cert_store
...
ssl_ca_cert = Bundler.settings[:ssl_ca_cert] ||
(Gem.configuration.ssl_ca_cert if
Gem.configuration.respond_to?(:ssl_ca_cert))
...
end
```
However in the case of downloading gems in Bundler,
Bundler calls `Bundler::Source::Rubygems#download_gem`
calling `Bundler::Fetcher#gem_remote_fetcher`
calling `Bundler::Fetcher::GemRemoteFetcher`
extending `Gem::RemoteFetcher` managing `@cert_files` for RubyGems.
Therefore, the `Bundler::Fetcher::GemRemoteFetcher` needs to update the `@cert_files`
by adding the value of the `Bundler.settings[:ssl_ca_cert]`.
As `Gem::Request.configure_connection_for_https` is called, and gets the value of
`Gem.configuration.ssl_ca_cert` in the proess of downloading gems,
we don't need to add the value in the `Bundler::Fetcher::GemRemoteFetcher`.
The backtrace is below.
```
+ rdbg -c \
-e 'b Gem::Request.configure_connection_for_https' \
-e c \
-- /home/jaruga/git/ruby/rubygems/bin/bundle install -V
...
(rdbg) bt # backtrace command
=>#0 Gem::Request.configure_connection_for_https(connection=#<Gem::Net::HTTP localhost:18443 open=fal..., cert_files=["/home/jaruga/.local/ruby-4.1.0-debug-3e...) at ~/.local/ruby-4.1.0-debug-3ef48ef9c8-openssl-4.1.0-7194354488/lib/ruby/4.1.0+1/rubygems/request.rb:71
ruby#1 Gem::Request::HTTPSPool#setup_connection(connection=#<Gem::Net::HTTP localhost:18443 open=fal...) at ~/.local/ruby-4.1.0-debug-3ef48ef9c8-openssl-4.1.0-7194354488/lib/ruby/4.1.0+1/rubygems/request/https_pool.rb:7
ruby#2 Gem::Request::HTTPPool#make_connection at ~/.local/ruby-4.1.0-debug-3ef48ef9c8-openssl-4.1.0-7194354488/lib/ruby/4.1.0+1/rubygems/request/http_pool.rb:43
ruby#3 Gem::Request::HTTPPool#checkout at ~/.local/ruby-4.1.0-debug-3ef48ef9c8-openssl-4.1.0-7194354488/lib/ruby/4.1.0+1/rubygems/request/http_pool.rb:23
ruby#4 Gem::Request#connection_for(uri=#<Gem::Uri:0x00007f4e473adda0 @parsed_uri...) at ~/.local/ruby-4.1.0-debug-3ef48ef9c8-openssl-4.1.0-7194354488/lib/ruby/4.1.0+1/rubygems/request.rb:136
ruby#5 Gem::Request#perform_request(request=#<Gem::Net::HTTP::Get GET path="/gems/hel...) at ~/.local/ruby-4.1.0-debug-3ef48ef9c8-openssl-4.1.0-7194354488/lib/ruby/4.1.0+1/rubygems/request.rb:194
ruby#6 Gem::Request#fetch at ~/.local/ruby-4.1.0-debug-3ef48ef9c8-openssl-4.1.0-7194354488/lib/ruby/4.1.0+1/rubygems/request.rb:161
ruby#7 Gem::RemoteFetcher#request(uri=#<Gem::Uri:0x00007f4e473adda0 @parsed_uri..., request_class=Gem::Net::HTTP::Get, last_modified=nil) at ~/.local/ruby-4.1.0-debug-3ef48ef9c8-openssl-4.1.0-7194354488/lib/ruby/4.1.0+1/rubygems/remote_fetcher.rb:326
ruby#8 Bundler::Fetcher::GemRemoteFetcher#request(args=[#<Gem::Uri:0x00007f4e473adda0 @parsed_ur...) at ~/var/git/ruby/rubygems/bundler/lib/bundler/fetcher/gem_remote_fetcher.rb:18
ruby#9 Gem::RemoteFetcher#fetch_http(uri=#<Gem::Uri:0x00007f4e473adda0 @parsed_uri..., last_modified=nil, head=false, depth=0) at ~/.local/ruby-4.1.0-debug-3ef48ef9c8-openssl-4.1.0-7194354488/lib/ruby/4.1.0+1/rubygems/remote_fetcher.rb:217
ruby#10 Gem::RemoteFetcher#fetch_path(uri=#<Gem::Uri:0x00007f4e473adda0 @parsed_uri..., mtime=nil, head=false) at ~/.local/ruby-4.1.0-debug-3ef48ef9c8-openssl-4.1.0-7194354488/lib/ruby/4.1.0+1/rubygems/remote_fetcher.rb:261
ruby#11 Gem::RemoteFetcher#cache_update_path(uri=#<Gem::URI::HTTPS https://localhost:18443..., path="/home/jaruga/var/git/report-bundler-bund..., update=true) at ~/.local/ruby-4.1.0-debug-3ef48ef9c8-openssl-4.1.0-7194354488/lib/ruby/4.1.0+1/rubygems/remote_fetcher.rb:302
ruby#12 block in Bundler::RubygemsIntegration#download_gem (2 levels) at ~/var/git/ruby/rubygems/bundler/lib/bundler/rubygems_integration.rb:393
ruby#13 Bundler::SharedHelpers#filesystem_access(path="/home/jaruga/var/git/report-bundler-bund..., action=:write, block=#<Proc:0x00007f4e1d7dfad8 /home/jaruga/va...) at ~/var/git/ruby/rubygems/bundler/lib/bundler/shared_helpers.rb:106
ruby#14 block in Bundler::RubygemsIntegration#download_gem at ~/var/git/ruby/rubygems/bundler/lib/bundler/rubygems_integration.rb:392
ruby#15 Bundler::Retry#run(block=#<Proc:0x00007f4e1d7df9c0 /home/jaruga/va...) at ~/var/git/ruby/rubygems/bundler/lib/bundler/retry.rb:48
ruby#16 Bundler::Retry#attempts(block=#<Proc:0x00007f4e1d7df880 /home/jaruga/va...) at ~/var/git/ruby/rubygems/bundler/lib/bundler/retry.rb:38
ruby#17 Bundler::RubygemsIntegration#download_gem(spec=Gem::Specification.new do |s| s.name = "h..., uri=#<Gem::URI::HTTPS https://localhost:18443..., cache_dir="/home/jaruga/var/git/report-bundler-bund..., fetcher=#<Bundler::Fetcher::GemRemoteFetcher:0x00...) at ~/var/git/ruby/rubygems/bundler/lib/bundler/rubygems_integration.rb:384
ruby#18 block in Bundler::Source::Rubygems#download_gem at ~/var/git/ruby/rubygems/bundler/lib/bundler/source/rubygems.rb:531
ruby#19 Gem.time(msg="Downloaded hello in", width=0, display=true) at ~/.local/ruby-4.1.0-debug-3ef48ef9c8-openssl-4.1.0-7194354488/lib/ruby/4.1.0+1/rubygems.rb:1068
ruby#20 Bundler::Source::Rubygems#download_gem(spec=Gem::Specification.new do |s| s.name = "h..., download_cache_path="/home/jaruga/var/git/report-bundler-bund..., previous_spec=nil) at ~/var/git/ruby/rubygems/bundler/lib/bundler/source/rubygems.rb:530
...
```
According to the following logic, `@cert_files` is not nil,
as `Dir.glob(patterns` returns Array.
lib/rubygems/remote_fetcher.rb
```
def initialize(proxy = nil, dns = nil, headers = {})
...
@cert_files = Gem::Request.get_cert_files
...
end
```
lib/rubygems/request.rb
```
def self.get_cert_files
pattern = File.expand_path("./ssl_certs/*/*.pem", __dir__)
Dir.glob(pattern)
end
```
Hello all,
This pull request brings in a progress bar that will be displayed while downloading gems. This is particularly useful when downloading large gems such as qtbindings on Windows. If the extra STDOUT output is undesirable, it would be easy to modify it to only output for large files (maybe > 1 Mb?). I like the feedback for all file sizes personally. Let me know what you think.
Thanks,
Ryan Melton