Skip to content

Conversation

jaanhio
Copy link
Contributor

@jaanhio jaanhio commented Dec 13, 2020

SUMMARY

Replaced deprecated brew cask commands with --cask (e.g brew install --cask <app>)

See https://brew.sh/2020/12/01/homebrew-2.6.0/ & Homebrew/brew#8899

Fixes #1524.

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

plugins/modules/homebrew_cask.py

ADDITIONAL INFORMATION

@ansibullbot
Copy link
Collaborator

@ansibullbot ansibullbot added affects_2.10 community_review feature This issue/PR relates to a feature request module module needs_triage new_contributor Help guide this first time contributor os packaging plugins plugin (any type) labels Dec 13, 2020
@felixfontein
Copy link
Collaborator

Which versions support the new commands, and which ones do not? Maybe the code needs to be able to handle both cases.

Also, you need a changelog fragment.

@apiology
Copy link

It looks like Homebrew has removed the old syntax:

TASK [Install emacs] **************************************************************
fatal: [bigbookpro]: FAILED! => {"changed": false, "msg": "Updating Homebrew...\nError: Calling brew cask install is disabled! Use brew install [--cask] instead."}

Copy link
Collaborator

@felixfontein felixfontein left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Besides this, looks good. Cannot verify that it works since I'm not using Homebrew though.

@ansibullbot ansibullbot added the needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html label Dec 22, 2020
@jaanhio jaanhio force-pushed the update-deprecated-homebrew-cask-commands branch from e0d51da to e8ca4e1 Compare December 22, 2020 14:31
@ansibullbot ansibullbot added community_review and removed merge_commit This PR contains at least one merge commit. Please resolve! needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html labels Dec 22, 2020
jpraczyk added a commit to jpraczyk/community.general that referenced this pull request Dec 23, 2020
@felixfontein
Copy link
Collaborator

@dsnyder0pc can you test again?

f1sherman added a commit to f1sherman/new-machine-bootstrap that referenced this pull request Dec 29, 2020
Previous fix didn't do the trick. I'm going to wait for this to be fixed
upstream.

ansible-collections/community.general#1481
@apiology
Copy link

I used this command to use the code from this PR for the moment:

ansible-galaxy collection install --force git+https://github.com/jaanhio/community.general.git,update-deprecated-homebrew-cask-commands

@felixfontein felixfontein merged commit ed81317 into ansible-collections:main Dec 31, 2020
patchback bot pushed a commit that referenced this pull request Dec 31, 2020
* updated deprecated homebrew cask commands

* added methods for brew version deprecation check

* added comments and changelog fragment

* added unit test for version comparison

* switch to use disutils LooseVersion for version comparison

* updated changelog message and minor refactor for building brew command based on version

* added caching logic for retrieval of brew version and updated PR changelog yaml

* Update changelogs/fragments/1481-deprecated-brew-cask-command.yaml

Co-authored-by: Felix Fontein <[email protected]>

* Update plugins/modules/packaging/os/homebrew_cask.py

* Update plugins/modules/packaging/os/homebrew_cask.py

Co-authored-by: Felix Fontein <[email protected]>

* Update plugins/modules/packaging/os/homebrew_cask.py

Co-authored-by: Felix Fontein <[email protected]>

* switch to use subprocess.check_output instead of subprocess.run

* replace subprocess with run_command

* removed unused subprocess import

* removed error handling logic to depend on check_rc=True instead

Co-authored-by: Felix Fontein <[email protected]>
(cherry picked from commit ed81317)
@felixfontein
Copy link
Collaborator

@jaanhio thanks for fixing this!
@Akasurde @apiology @dsnyder0pc thanks for reviewing and testing!

@felixfontein felixfontein removed the check-before-release PR will be looked at again shortly before release and merged if possible. label Dec 31, 2020
@rounakdatta
Copy link

Thanks @jaanhio and the reviewers for this new year gift! 🙂

felixfontein pushed a commit that referenced this pull request Dec 31, 2020
* updated deprecated homebrew cask commands

* added methods for brew version deprecation check

* added comments and changelog fragment

* added unit test for version comparison

* switch to use disutils LooseVersion for version comparison

* updated changelog message and minor refactor for building brew command based on version

* added caching logic for retrieval of brew version and updated PR changelog yaml

* Update changelogs/fragments/1481-deprecated-brew-cask-command.yaml

Co-authored-by: Felix Fontein <[email protected]>

* Update plugins/modules/packaging/os/homebrew_cask.py

* Update plugins/modules/packaging/os/homebrew_cask.py

Co-authored-by: Felix Fontein <[email protected]>

* Update plugins/modules/packaging/os/homebrew_cask.py

Co-authored-by: Felix Fontein <[email protected]>

* switch to use subprocess.check_output instead of subprocess.run

* replace subprocess with run_command

* removed unused subprocess import

* removed error handling logic to depend on check_rc=True instead

Co-authored-by: Felix Fontein <[email protected]>
(cherry picked from commit ed81317)

Co-authored-by: Jianhao Tan <[email protected]>
@felixfontein
Copy link
Collaborator

Backport is merged and will be in the release on January 4th, which in turn will be included in the next Ansible 2.10.x release (planned for January 5th).

If someone wants to backport this to ansible/ansible's stable-2.9 branch, it can also appear in the next 2.9.x release.

@jaanhio
Copy link
Contributor Author

jaanhio commented Jan 1, 2021

thanks all for the guidance and advice provided! happy new year all!

@jorhett
Copy link

jorhett commented Mar 5, 2021

FYI the version detection fails fails when brew updates itself before running the command.

failed: [ansible-development] (item=docker) => {"ansible_loop_var": "item", "changed": false, "item": "docker", "msg": "Updating Homebrew...\n==> **SNIP** ==> Auto-updated Homebrew!\nUpdated 1 tap (homebrew/core).\n==> Updated Formulae\nUpdated 4647 formulae.\n\nError: Unknown command: cask"}
  1. Check version -- select older syntax
  2. Update homebrew
  3. Run homebrew older syntax which fails

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community_review feature This issue/PR relates to a feature request has_issue module module new_contributor Help guide this first time contributor os packaging plugins plugin (any type) tests tests unit tests/unit
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Calling brew cask install is disabled! Use brew install [--cask] instead.
9 participants