Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make google-cloud-sdk versioned to 165.0.0 #37317

Closed
wants to merge 2 commits into from

Conversation

cglong
Copy link
Contributor

@cglong cglong commented Aug 3, 2017

After making all changes to the cask:

  • brew cask audit --download {{cask_file}} is error-free.
  • brew cask style --fix {{cask_file}} reports no offenses.
  • The commit message includes the cask’s name and version.

Additionally, if updating a cask:

  • [If the sha256 changed but the version didn’t][version-checksum],
    provide public confirmation by the developer: {{link}}

@commitay
Copy link
Contributor

commitay commented Aug 3, 2017

@cglong This will break the completions and PATH. See #32124

@cglong
Copy link
Contributor Author

cglong commented Aug 3, 2017

@commitay Sorry, what do you mean about the PATH? And re: completions, is there a way to still support those (e.g. symlinking or something)? The main benefit I personally receive from Caskroom is auto-updating and asking users to instead manually enter gcloud components update seems counter to that goal.

@commitay
Copy link
Contributor

commitay commented Aug 3, 2017

Sorry, what do you mean about the PATH?

      for bash users
        source '#{staged_path}/#{token}/path.bash.inc'
        source '#{staged_path}/#{token}/completion.bash.inc'

      for zsh users
        source '#{staged_path}/#{token}/path.zsh.inc'
        source '#{staged_path}/#{token}/completion.zsh.inc'

The main benefit I personally receive from Caskroom is auto-updating and asking users to instead manually enter gcloud components update seems counter to that goal.

Caskroom doesn't auto-update.

And re: completions, is there a way to still support those (e.g. symlinking or something)?

Yes, using artifact or postflight and uninstall_preflight. We don't have support for installing completions like Homebrew.

It was suggested a while ago that this could be moved to Homebrew-core (Homebrew/homebrew-core#583 (comment)) which would auto-update and properly support installing completions.

@cglong
Copy link
Contributor Author

cglong commented Aug 3, 2017

FWIW, it looks like the PATHs are resolving correctly, albeit inconveniently as you mentioned:

  for bash users
    source '/usr/local/Caskroom/google-cloud-sdk/165.0.0/google-cloud-sdk/path.bash.inc'
    source '/usr/local/Caskroom/google-cloud-sdk/165.0.0/google-cloud-sdk/completion.bash.inc'

  for zsh users
    source '/usr/local/Caskroom/google-cloud-sdk/165.0.0/google-cloud-sdk/path.zsh.inc'
    source '/usr/local/Caskroom/google-cloud-sdk/165.0.0/google-cloud-sdk/completion.zsh.inc'

Caskroom doesn't auto-update.

Oh, I never noticed this (I'm pretty new to Caskroom tbh)! Either way, I do like having a centralized place for managing my command line tools, rather than having to memorize vendor-specific commands.

Yes, using artifact or postflight and uninstall_preflight.

Awesome! I'll toy around with these and see if I can get something working 🙂

@commitay
Copy link
Contributor

commitay commented Aug 3, 2017

it looks like the PATHs are resolving correctly, albeit inconveniently as you mentioned

Yes, I meant they will break for everyone currently using them (if they update the cask).


I'm not sure I understand how making this versioned will avoid having to run gcloud components update? Only bq, core, gsutil and gcloud are installed by default.

Updating the Cask (running brew cask reinstall google-cloud-sdk) will remove any installed components.

@cglong
Copy link
Contributor Author

cglong commented Aug 3, 2017

Updating the Cask (running brew cask reinstall google-cloud-sdk) will remove any installed components.

Ah yeah, that's true. It could be interesting to explore making additional Casks for auxiliary components that depends_on this one, but I'm not sure how easy it would be to replicate what was done for the .debs/.rpms corresponding to those components.

I could take this either way and will let you decide whether it should be merged in or not 🙂

@commitay
Copy link
Contributor

commitay commented Aug 3, 2017

It could be interesting to explore making additional Casks for auxiliary components that depends_on this one, but I'm not sure how easy it would be to replicate what was done for the .debs/.rpms corresponding to those components.

This would also work better as a Homebrew formula which would allow install options and resource blocks.

Whilst we do support depends_on cask until we have a brew cask upgrade (#29301) it isn't going to work very well and it could have issues if the casks are not updated at the same time.


Going to pass on this as versioning this cask will mean downloaded components will be overwritten.

Thank you anyway for the PR @cglong!

@commitay commitay closed this Aug 3, 2017
@commitay commitay removed the on hold label Aug 3, 2017
@cglong
Copy link
Contributor Author

cglong commented Aug 3, 2017

@commitay I just noticed install.sh has --additional-components and --override-components flags where we can modify the list of what gets installed. I'm wondering if it'd make sense to just install all components by default? It would also help with situations like the datalab component which can be optionally installed but then isn't symlinked.

@commitay
Copy link
Contributor

commitay commented Aug 3, 2017

We could install everything but this will mean downloading an additional 340Mb of components (installed size is 800Mb larger than the default of 250Mb) that users may not need or want.

gcloud components update would update all installed components so users would have to uninstall what they don't use.

@Homebrew Homebrew locked and limited conversation to collaborators May 9, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants