Add GCSDownloadStrategy #5070

andres-lowrie · 2018-10-08T22:40:29Z

Have you followed the guidelines in our Contributing document?
Have you checked to ensure there aren't other open Pull Requests for the same change?
Have you added an explanation of what your changes do and why you'd like us to include them?
Have you written new tests for your changes? Here's an example.
Have you successfully run brew style with your changes locally?
Have you successfully run brew tests with your changes locally?

Hello, I based this off of the S3DownloadStrategy as it most closely matched the steps that needed to be done.

I'm still running tests on a dummy formula to make sure the google cloud gem get's installed and all that as I was having issues on my other laptop but I want to make sure it's not my ruby setup before filing a bug.

This being the case, I don't know if I should label this PR or do something more since I didn't see anything in the contributing guide in terms of adding new download strategies.

Thanks in advance and thanks for this tool... life saver 🙏

andres-lowrie · 2018-10-08T23:15:27Z

Okay so I'm getting an install error when trying to --build-from-source:

/usr/local/Homebrew/Library/Homebrew/os/mac/xcode.rb:1:in `<top (required)>': OS is not a module (TypeError)
	from /System/Library/Frameworks/Ruby.framework/Versions/2.3/usr/lib/ruby/2.3.0/rubygems/core_ext/kernel_require.rb:55:in `require'
	from /System/Library/Frameworks/Ruby.framework/Versions/2.3/usr/lib/ruby/2.3.0/rubygems/core_ext/kernel_require.rb:55:in `require'
	from /usr/local/Homebrew/Library/Homebrew/extend/os/mac/development_tools.rb:1:in `<top (required)>'
	from /System/Library/Frameworks/Ruby.framework/Versions/2.3/usr/lib/ruby/2.3.0/rubygems/core_ext/kernel_require.rb:55:in `require'
	from /System/Library/Frameworks/Ruby.framework/Versions/2.3/usr/lib/ruby/2.3.0/rubygems/core_ext/kernel_require.rb:55:in `require'
	from /usr/local/Homebrew/Library/Homebrew/extend/os/development_tools.rb:1:in `<top (required)>'
	from /System/Library/Frameworks/Ruby.framework/Versions/2.3/usr/lib/ruby/2.3.0/rubygems/core_ext/kernel_require.rb:55:in `require'
	from /System/Library/Frameworks/Ruby.framework/Versions/2.3/usr/lib/ruby/2.3.0/rubygems/core_ext/kernel_require.rb:55:in `require'
	from /usr/local/Homebrew/Library/Homebrew/development_tools.rb:127:in `<top (required)>'
	from /System/Library/Frameworks/Ruby.framework/Versions/2.3/usr/lib/ruby/2.3.0/rubygems/core_ext/kernel_require.rb:55:in `require'
	from /System/Library/Frameworks/Ruby.framework/Versions/2.3/usr/lib/ruby/2.3.0/rubygems/core_ext/kernel_require.rb:55:in `require'
	from /usr/local/Homebrew/Library/Homebrew/tab.rb:5:in `<top (required)>'
	from /System/Library/Frameworks/Ruby.framework/Versions/2.3/usr/lib/ruby/2.3.0/rubygems/core_ext/kernel_require.rb:55:in `require'
	from /System/Library/Frameworks/Ruby.framework/Versions/2.3/usr/lib/ruby/2.3.0/rubygems/core_ext/kernel_require.rb:55:in `require'
	from /usr/local/Homebrew/Library/Homebrew/utils/bottles.rb:1:in `<top (required)>'
	from /System/Library/Frameworks/Ruby.framework/Versions/2.3/usr/lib/ruby/2.3.0/rubygems/core_ext/kernel_require.rb:55:in `require'
	from /System/Library/Frameworks/Ruby.framework/Versions/2.3/usr/lib/ruby/2.3.0/rubygems/core_ext/kernel_require.rb:55:in `require'
	from /usr/local/Homebrew/Library/Homebrew/formula.rb:6:in `<top (required)>'
	from /System/Library/Frameworks/Ruby.framework/Versions/2.3/usr/lib/ruby/2.3.0/rubygems/core_ext/kernel_require.rb:55:in `require'
	from /System/Library/Frameworks/Ruby.framework/Versions/2.3/usr/lib/ruby/2.3.0/rubygems/core_ext/kernel_require.rb:55:in `require'
	from /usr/local/Homebrew/Library/Homebrew/readall.rb:1:in `<top (required)>'
	from /System/Library/Frameworks/Ruby.framework/Versions/2.3/usr/lib/ruby/2.3.0/rubygems/core_ext/kernel_require.rb:55:in `require'
	from /System/Library/Frameworks/Ruby.framework/Versions/2.3/usr/lib/ruby/2.3.0/rubygems/core_ext/kernel_require.rb:55:in `require'
	from /usr/local/Homebrew/Library/Homebrew/tap.rb:2:in `<top (required)>'
	from /System/Library/Frameworks/Ruby.framework/Versions/2.3/usr/lib/ruby/2.3.0/rubygems/core_ext/kernel_require.rb:55:in `require'
	from /System/Library/Frameworks/Ruby.framework/Versions/2.3/usr/lib/ruby/2.3.0/rubygems/core_ext/kernel_require.rb:55:in `require'
	from /usr/local/Homebrew/Library/Homebrew/global.rb:142:in `<top (required)>'
	from /System/Library/Frameworks/Ruby.framework/Versions/2.3/usr/lib/ruby/2.3.0/rubygems/core_ext/kernel_require.rb:55:in `require'
	from /System/Library/Frameworks/Ruby.framework/Versions/2.3/usr/lib/ruby/2.3.0/rubygems/core_ext/kernel_require.rb:55:in `require'
	from /usr/local/Homebrew/Library/Homebrew/build.rb:6:in `<main>'
Error: Failure while executing; `/usr/bin/sandbox-exec -f /private/tmp/homebrew20181008-17955-hwoj4s.sb nice /System/Library/Frameworks/Ruby.framework/Versions/2.3/usr/bin/ruby -W0 -I /usr/local/Homebrew/Library/Homebrew/vendor/bundle-standalone/bundler/../ruby/2.3.0/gems/ruby-macho-2.0.0/lib:/usr/local/Homebrew/Library/Homebrew/vendor/bundle-standalone/bundler/../ruby/2.3.0/gems/plist-3.4.0/lib:/usr/local/Homebrew/Library/Homebrew/vendor/bundle-standalone/bundler/:/usr/local/Homebrew/Library/Homebrew/vendor/bundle-standalone/bundler/../ruby/2.3.0/gems/backports-3.11.4/lib:/usr/local/Homebrew/Library/Homebrew/vendor/bundle-standalone/bundler/../ruby/2.3.0/gems/activesupport-5.2.1/lib:/usr/local/Homebrew/Library/Homebrew/vendor/bundle-standalone/bundler/../ruby/2.3.0/gems/tzinfo-1.2.5/lib:/usr/local/Homebrew/Library/Homebrew/vendor/bundle-standalone/bundler/../ruby/2.3.0/gems/thread_safe-0.3.6/lib:/usr/local/Homebrew/Library/Homebrew/vendor/bundle-standalone/bundler/../ruby/2.3.0/gems/minitest-5.11.3/lib:/usr/local/Homebrew/Library/Homebrew/vendor/bundle-standalone/bundler/../ruby/2.3.0/gems/i18n-1.1.0/lib:/usr/local/Homebrew/Library/Homebrew/vendor/bundle-standalone/bundler/../ruby/2.3.0/gems/concurrent-ruby-1.0.5/lib:/Library/Ruby/Gems/2.3.0/gems/did_you_mean-1.0.0/lib:/usr/local/Homebrew/Library/Homebrew/vendor/bundle/ruby/2.3.0/gems/multipart-post-2.0.0/lib:/usr/local/Homebrew/Library/Homebrew/vendor/bundle/ruby/2.3.0/gems/faraday-0.15.3/lib:/usr/local/Homebrew/Library/Homebrew/vendor/bundle/ruby/2.3.0/gems/google-cloud-env-1.0.5/lib:/usr/local/Homebrew/Library/Homebrew/vendor/bundle/ruby/2.3.0/gems/google-cloud-core-1.2.7/lib:/usr/local/Homebrew/Library/Homebrew/vendor/bundle/ruby/2.3.0/gems/uber-0.1.0/lib:/usr/local/Homebrew/Library/Homebrew/vendor/bundle/ruby/2.3.0/gems/declarative-0.0.10/lib:/usr/local/Homebrew/Library/Homebrew/vendor/bundle/ruby/2.3.0/gems/declarative-option-0.1.0/lib:/usr/local/Homebrew/Library/Homebrew/vendor/bundle/ruby/2.3.0/gems/representable-3.0.4/lib:/usr/local/Homebrew/Library/Homebrew/vendor/bundle/ruby/2.3.0/gems/retriable-3.1.2/lib:/usr/local/Homebrew/Library/Homebrew/vendor/bundle/ruby/2.3.0/gems/public_suffix-3.0.3/lib:/usr/local/Homebrew/Library/Homebrew/vendor/bundle/ruby/2.3.0/gems/addressable-2.5.2/lib:/usr/local/Homebrew/Library/Homebrew/vendor/bundle/ruby/2.3.0/gems/mime-types-data-3.2018.0812/lib:/usr/local/Homebrew/Library/Homebrew/vendor/bundle/ruby/2.3.0/gems/mime-types-3.2.2/lib:/usr/local/Homebrew/Library/Homebrew/vendor/bundle/ruby/2.3.0/gems/multi_json-1.13.1/lib:/usr/local/Homebrew/Library/Homebrew/vendor/bundle/ruby/2.3.0/gems/jwt-2.1.0/lib:/usr/local/Homebrew/Library/Homebrew/vendor/bundle/ruby/2.3.0/gems/signet-0.10.0/lib:/usr/local/Homebrew/Library/Homebrew/vendor/bundle/ruby/2.3.0/gems/memoist-0.16.0/lib:/usr/local/Homebrew/Library/Homebrew/vendor/bundle/ruby/2.3.0/gems/os-1.0.0/lib:/usr/local/Homebrew/Library/Homebrew/vendor/bundle/ruby/2.3.0/gems/googleauth-0.6.6/lib:/usr/local/Homebrew/Library/Homebrew/vendor/bundle/ruby/2.3.0/gems/httpclient-2.8.3/lib:/usr/local/Homebrew/Library/Homebrew/vendor/bundle/ruby/2.3.0/gems/google-api-client-0.24.2/lib:/usr/local/Homebrew/Library/Homebrew/vendor/bundle/ruby/2.3.0/gems/google-api-client-0.24.2/generated:/usr/local/Homebrew/Library/Homebrew/vendor/bundle/ruby/2.3.0/gems/google-api-client-0.24.2/third_party:/usr/local/Homebrew/Library/Homebrew/vendor/bundle/ruby/2.3.0/gems/digest-crc-0.4.1/lib:/usr/local/Homebrew/Library/Homebrew/vendor/bundle/ruby/2.3.0/gems/google-cloud-storage-1.15.0/lib:/Library/Ruby/Site/2.3.0:/Library/Ruby/Site/2.3.0/x86_64-darwin17:/Library/Ruby/Site/2.3.0/universal-darwin17:/Library/Ruby/Site:/System/Library/Frameworks/Ruby.framework/Versions/2.3/usr/lib/ruby/vendor_ruby/2.3.0:/System/Library/Frameworks/Ruby.framework/Versions/2.3/usr/lib/ruby/vendor_ruby/2.3.0/x86_64-darwin17:/System/Library/Frameworks/Ruby.framework/Versions/2.3/usr/lib/ruby/vendor_ruby/2.3.0/universal-darwin17:/System/Library/Frameworks/Ruby.framework/Versions/2.3/usr/lib/ruby/vendor_ruby:/System/Library/Frameworks/Ruby.framework/Versions/2.3/usr/lib/ruby/2.3.0:/System/Library/Frameworks/Ruby.framework/Versions/2.3/usr/lib/ruby/2.3.0/x86_64-darwin17:/System/Library/Frameworks/Ruby.framework/Versions/2.3/usr/lib/ruby/2.3.0/universal-darwin17:/usr/local/Homebrew/Library/Homebrew -- /usr/local/Homebrew/Library/Homebrew/build.rb /usr/local/Homebrew/Library/Taps/homebrew/homebrew-core/Formula/dummy.rb` exited with 1.

My config looks like this:

HOMEBREW_VERSION: 1.7.6-12-gc23e352
ORIGIN: https://github.com/Homebrew/brew
HEAD: c23e3525045987b3041ddc7ab0947af61b558223
Last commit: 41 minutes ago
Core tap ORIGIN: https://github.com/Homebrew/homebrew-core
Core tap HEAD: 45d56094d180dd29857ecf9d6b14436d155291c5
Core tap last commit: 35 minutes ago
HOMEBREW_PREFIX: /usr/local
HOMEBREW_DEV_CMD_RUN: 1
HOMEBREW_GCS_CREDENTIALS: /Users/andreslowrie/Creds/andres-on-mine.json
HOMEBREW_GCS_PROJECT: mine-218822
CPU: dodeca-core 64-bit kabylake
Homebrew Ruby: 2.3.7 => /System/Library/Frameworks/Ruby.framework/Versions/2.3/usr/bin/ruby
Clang: 10.0 build 1000
Git: 2.19.0 => /usr/local/bin/git
Curl: 7.54.0 => /usr/bin/curl
Java: N/A
macOS: 10.13.6-x86_64
CLT: 10.0.0.0.1.1535735448
CLT headers: N/A
Xcode: N/A
XQuartz: N/A

I'm guessing this is something I did wrong in the download strat?

colindean

I'm assuming that this is meant to be used with private formulae.

What does the GCS gem provide that accessing a URL cannot? There is the precedent of the S3 gem being installed…

.gitignore

colindean · 2018-10-08T23:12:39Z

Library/Homebrew/test/download_strategies_spec.rb

@@ -218,6 +218,24 @@ def setup_git_repo
  end
 end

+describe GCSDownloadStrategy do


Needs more substantial tests.

Can you point me to some examples so I can get a better sense of what to include...

I'm a noob with rspec and ruby 😂

Take a look at some of the other download strategies, e.g. GitHubGitDownloadStrategy and see what state you can inspect.

Library/Homebrew/download_strategy.rb

colindean · 2018-10-08T23:16:43Z

Library/Homebrew/download_strategy.rb

@@ -1200,6 +1200,43 @@ def update
  end
 end

+# GCSDownloadStrategy downloads files from Google Cloud Storage


This documentation will have to be elsewhere, as well…

I'm not sure where but wait to do so until some project members take a look at this.

andres-lowrie · 2018-10-08T23:40:05Z

Regarding the gem,

I based this on the S3DownloadStrategy and noticed that it went through some iterations where it went from accessing the url to using the aws gem ... so I thought that was preferred

Yes you're correct in your assumption, the idea is to be able to download private resources; I'm reading the documentation (Google's) now again to see how to do this without the gem.

I looked at Google's documentation and their gem it should be doable to do it without the gem, there're straight forward instructions here:

https://cloud.google.com/storage/docs/access-control/create-signed-urls-program

and it looks like the openssl module is part of the std lib so it should be doable without requiring any gem.

☝️ is this preferred w/o gem ?

MikeMcQuaid · 2018-10-09T09:03:28Z

I do not want to include any more download strategies that will not be used by Homebrew/homebrew-core. Instead I'm interested in allowing taps/private taps formulae to have ways to specify download strategies within their formulae.

MikeMcQuaid · 2018-10-09T09:13:15Z

I've opened #5074 with the proposed new approach.

colindean reviewed Oct 8, 2018

View reviewed changes

Add GCSDownloadStrategy

b3e270b

andres-lowrie force-pushed the gcs_download_strategy branch from c23e352 to b3e270b Compare October 9, 2018 00:43

MikeMcQuaid added the do not merge label Oct 9, 2018

MikeMcQuaid mentioned this pull request Oct 9, 2018

Download strategies should be able to be embedded in formulae #5074

Closed

MikeMcQuaid closed this Oct 9, 2018

MikeMcQuaid added the invalid Not eligible for e.g. Hacktoberfest label Oct 10, 2018

lock bot added the outdated PR was locked due to age label Nov 9, 2018

lock bot locked as resolved and limited conversation to collaborators Nov 9, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add GCSDownloadStrategy #5070

Add GCSDownloadStrategy #5070

andres-lowrie commented Oct 8, 2018

andres-lowrie commented Oct 8, 2018

colindean left a comment

colindean Oct 8, 2018

andres-lowrie Oct 8, 2018

colindean Oct 8, 2018

colindean Oct 8, 2018

andres-lowrie commented Oct 8, 2018 •

edited

MikeMcQuaid commented Oct 9, 2018

MikeMcQuaid commented Oct 9, 2018

Add GCSDownloadStrategy #5070

Add GCSDownloadStrategy #5070

Conversation

andres-lowrie commented Oct 8, 2018

andres-lowrie commented Oct 8, 2018

colindean left a comment

Choose a reason for hiding this comment

colindean Oct 8, 2018

Choose a reason for hiding this comment

andres-lowrie Oct 8, 2018

Choose a reason for hiding this comment

colindean Oct 8, 2018

Choose a reason for hiding this comment

colindean Oct 8, 2018

Choose a reason for hiding this comment

andres-lowrie commented Oct 8, 2018 • edited

MikeMcQuaid commented Oct 9, 2018

MikeMcQuaid commented Oct 9, 2018

andres-lowrie commented Oct 8, 2018 •

edited