diff --git a/changelogs/fragments/10805-homebrew-support-old-names.yml b/changelogs/fragments/10805-homebrew-support-old-names.yml new file mode 100644 index 0000000000..43d5a1c8bf --- /dev/null +++ b/changelogs/fragments/10805-homebrew-support-old-names.yml @@ -0,0 +1,2 @@ +bugfixes: + - homebrew - do not fail when cask or formula name has changed in homebrew repo (https://github.com/ansible-collections/community.general/issues/10804, https://github.com/ansible-collections/community.general/pull/10805). \ No newline at end of file diff --git a/plugins/modules/homebrew.py b/plugins/modules/homebrew.py index 021f990e67..661f7d4d7e 100644 --- a/plugins/modules/homebrew.py +++ b/plugins/modules/homebrew.py @@ -385,12 +385,13 @@ class Homebrew(object): self.outdated_packages.add(package_name) def _extract_package_name(self, package_detail, is_cask): - # "brew info" can lookup by name, full_name, token, full_token, or aliases - # In addition, any name can be prefixed by the tap. - # Any of these can be supplied by the user as the package name. In case - # of ambiguity, where a given name might match multiple packages, - # formulae are preferred over casks. For all other ambiguities, the - # results are an error. Note that in the homebrew/core and + # "brew info" can lookup by name, full_name, token, full_token, + # oldnames, old_tokens, or aliases. In addition, any of the + # above names can be prefixed by the tap. Any of these can be + # supplied by the user as the package name. In case of + # ambiguity, where a given name might match multiple packages, + # formulae are preferred over casks. For all other ambiguities, + # the results are an error. Note that in the homebrew/core and # homebrew/cask taps, there are no "other" ambiguities. if is_cask: # according to brew info name = package_detail["token"] @@ -405,15 +406,26 @@ class Homebrew(object): # # Issue https://github.com/ansible-collections/community.general/issues/10012: # package_detail["tap"] is None if package is no longer available. - tapped_name = [package_detail["tap"] + "/" + name] if package_detail["tap"] else [] - aliases = package_detail.get("aliases", []) - package_names = set([name, full_name] + tapped_name + aliases) + # + # Issue https://github.com/ansible-collections/community.general/issues/10804 + # name can be an alias, oldnames or old_tokens optionally prefixed by tap + package_names = {name, full_name} + package_names.update(package_detail.get("aliases", [])) + package_names.update(package_detail.get("oldnames", [])) + package_names.update(package_detail.get("old_tokens", [])) + if package_detail['tap']: + # names so far, with tap prefix added to each + tapped_names = {package_detail["tap"] + "/" + x for x in package_names} + package_names.update(tapped_names) # Finally, identify which of all those package names was the one supplied by the user. package_names = package_names & set(self.packages) if len(package_names) != 1: self.failed = True - self.message = "Package names are missing or ambiguous: " + ", ".join(str(p) for p in package_names) + self.message = "Package names for {name} are missing or ambiguous: {packages}".format( + name=name, + packages=", ".join(str(p) for p in package_names), + ) raise HomebrewException(self.message) # Then make sure the user provided name resurface. diff --git a/tests/integration/targets/homebrew/tasks/casks.yml b/tests/integration/targets/homebrew/tasks/casks.yml index 842d209f52..ebf8a2ee25 100644 --- a/tests/integration/targets/homebrew/tasks/casks.yml +++ b/tests/integration/targets/homebrew/tasks/casks.yml @@ -95,12 +95,3 @@ - assert: that: - package_result is not changed - - # This crashed on 4867eb4 - Ref: issue #9777 - - name: Install cask using homelab/cask syntax - homebrew: - package: "homebrew/cask/{{ package_name }}" - state: present - update_homebrew: false - become: true - become_user: "{{ brew_stat.stat.pw_name }}" \ No newline at end of file diff --git a/tests/integration/targets/homebrew/tasks/formulae.yml b/tests/integration/targets/homebrew/tasks/formulae.yml index 4cb4d5bd91..d2dd300fe6 100644 --- a/tests/integration/targets/homebrew/tasks/formulae.yml +++ b/tests/integration/targets/homebrew/tasks/formulae.yml @@ -303,45 +303,6 @@ - "package_result.changed_pkgs == []" - "package_result.unchanged_pkgs | sort == ['gnu-tar', 'gnu-time']" -# Test alias handling with sqlite (that is aliased to sqlite3) -- block: - - name: Make sure sqlite package is not installed - homebrew: - name: "sqlite" - state: absent - become: true - become_user: "{{ brew_stat.stat.pw_name }}" - - - name: Install sqlite package using alias (sqlite3) - homebrew: - name: "sqlite3" - state: present - become: true - become_user: "{{ brew_stat.stat.pw_name }}" - register: install_result - - - assert: - that: - - install_result is changed - - "install_result.msg == 'Package installed: sqlite3'" - - "install_result.changed_pkgs == ['sqlite3']" - - "install_result.unchanged_pkgs == []" - - - name: Again install sqlite package using alias (sqlite3) - homebrew: - name: "sqlite3" - state: present - become: true - become_user: "{{ brew_stat.stat.pw_name }}" - register: reinstall_result - - - assert: - that: - - reinstall_result is not changed - - "reinstall_result.msg == 'Package already installed: sqlite3'" - - "reinstall_result.changed_pkgs == []" - - "reinstall_result.unchanged_pkgs == ['sqlite3']" - # Test install from homebrew tap - block: - name: Tap hashicorp repository diff --git a/tests/integration/targets/homebrew/tasks/main.yml b/tests/integration/targets/homebrew/tasks/main.yml index 00d0bcf31c..31e6dc80eb 100644 --- a/tests/integration/targets/homebrew/tasks/main.yml +++ b/tests/integration/targets/homebrew/tasks/main.yml @@ -14,3 +14,8 @@ - include_tasks: 'formulae.yml' - include_tasks: 'casks.yml' - include_tasks: 'docker.yml' + + # Slow tests to exhaustively test install/uninstall on alternate package names, + # Commented out for normal use: + + # - include_tasks: 'package_names.yml' \ No newline at end of file diff --git a/tests/integration/targets/homebrew/tasks/package_names.yml b/tests/integration/targets/homebrew/tasks/package_names.yml new file mode 100644 index 0000000000..36da64f933 --- /dev/null +++ b/tests/integration/targets/homebrew/tasks/package_names.yml @@ -0,0 +1,55 @@ +# Copyright (c) Ansible Project +# GNU General Public License v3.0+ (see LICENSES/GPL-3.0-or-later.txt or https://www.gnu.org/licenses/gpl-3.0.txt) +# SPDX-License-Identifier: GPL-3.0-or-later + +# A series of package tests that check all the +# different ways that "brew info" can refer to a package. +# +# The homebrew module must mimic what brew info supports, +# or else errors will occur. + +- name: Find brew binary + command: which brew + register: brew_which + +- name: Get owner of brew binary + stat: + path: "{{ brew_which.stdout }}" + register: brew_stat + +# Note: We no longer use sqlite3 for "formula alias" tests, because mac +# ships with old python3 and if we use python3 from homebrew, sqlite3 +# cannot be uninstalled, which is part of the alias test. We use mandoc +# instead. + +# An example of a case were old_tokens support is important: User has an +# ansible task that installs homebrew/cask/docker (the tap prefix is +# required, or else one gets the formula ansible/core/docker). Homebrew +# recently changed the name of the cask to docker-desktop, moving +# "docker" into its old_tokens array. At that point +# community.general.homebrew stopped working, because it succeeds in +# running "brew info" on the name "homebrew/cask/docker", which adds +# docker-desktop to the list of affected packages. But, later, when it +# asks which user name entry corresponds to the affected cask, it +# failed, because the code didn't think to consider old_tokens as one of +# the valid ways of referring to a cask. That is issue #10804, herein +# fixed. + +- name: Test brew uninstall/install using various names accepted by "brew info" + vars: + packages: + # All packages are carefully selected to have no dependencies, to reduce install time + # homebrew/cask/kitty crashed on 4867eb4 - Ref: issue #9777 + - [kitty, homebrew/cask/kitty, "cask by name with tap prefix"] + # mdocml test replaces old sqlite3 test, see above. + - [mandoc, mdocml, "formula alias"] + - [mandoc, homebrew/core/mdocml, "formula alias with tap prefix"] + # These all crash on f772bcd - Ref: issue #10804 + - [cmark, commonmark, "formula with oldnames"] + - [cmark, homebrew/core/commonmark, "formula with oldnames and tap prefix"] + - [sipgate, clinq, "cask with old_tokens"] + - [sipgate, homebrew/cask/clinq, "cask with old_tokens and tap prefix"] + include_tasks: package_names_item.yml + loop: "{{ packages }}" + loop_control: + loop_var: package diff --git a/tests/integration/targets/homebrew/tasks/package_names_item.yml b/tests/integration/targets/homebrew/tasks/package_names_item.yml new file mode 100644 index 0000000000..86168b0b81 --- /dev/null +++ b/tests/integration/targets/homebrew/tasks/package_names_item.yml @@ -0,0 +1,62 @@ +# Copyright (c) Ansible Project +# GNU General Public License v3.0+ (see LICENSES/GPL-3.0-or-later.txt or https://www.gnu.org/licenses/gpl-3.0.txt) +# SPDX-License-Identifier: GPL-3.0-or-later + +# This task block is included by package_names.yml once for each test package, +# having set package variable to the name to test +- name: Test one package + block: + - debug: + var: package + + - name: Assure package is absent + homebrew: + package: "{{ package[0] }}" + state: absent + update_homebrew: false + become: true + become_user: "{{ brew_stat.stat.pw_name }}" + + - name: Install package by an alternative name + homebrew: + package: "{{ package[1] }}" + state: present + update_homebrew: false + become: true + become_user: "{{ brew_stat.stat.pw_name }}" + register: package_result + + - assert: + that: + - package_result is changed + - package_result.changed_pkgs[0] == package[1] + - package_result.unchanged_pkgs | length == 0 + - 'package_result.msg == "Package installed: " ~ package[1]' + + - name: Reinstall should do nothing + homebrew: + package: "{{ package[1] }}" + state: present + update_homebrew: false + become: true + become_user: "{{ brew_stat.stat.pw_name }}" + register: package_result + + - assert: + that: + - package_result is not changed + - 'package_result.msg == "Package already installed: " ~ package[1]' + + - name: Uninstall should work on alternate name + homebrew: + package: "{{ package[1] }}" + state: absent + update_homebrew: false + become: true + become_user: "{{ brew_stat.stat.pw_name }}" + register: package_result + + - assert: + that: + - package_result is changed + - 'package_result.msg == "Package uninstalled: " ~ package[1]'