From 833e6e36de75b71beb41347c756100445a12f6db Mon Sep 17 00:00:00 2001 From: brad2014 Date: Mon, 15 Sep 2025 18:26:01 +0200 Subject: [PATCH] homebrew: Support old_tokens and oldnames in homebrew package data (#10805) * homebrew: Support old_tokens and oldnames in homebrew package data Fixes #10804 Since brew info will accept old_tokens (for casks) and oldnames (for formulae) when provided by the homebrew module "name" argument, the module also needs to consider thes old names as valid for the given package. This commit updates _extract_package_name to do that. All existing package name tests, including existing tests for name aliases and tap prefixing, have been consolidated with new name tests into package_names.yml. * Added changelog fragment. * homebrew: replace non-py2 compliant f-string usage * code formatting lint, and py2 compatibility fixes * homebrew: added licenses to new files, nox lint * Update plugins/modules/homebrew.py use str.format() instead of string addition Co-authored-by: Felix Fontein * Update tests/integration/targets/homebrew/tasks/casks.yml Co-authored-by: Felix Fontein * Update tests/integration/targets/homebrew/tasks/package_names_item.yml Co-authored-by: Felix Fontein * Update tests/integration/targets/homebrew/tasks/formulae.yml Co-authored-by: Felix Fontein * Fixes for performance concerns on new homebrew tests. 1) tests for alternate package names are commented out in main.yml. 2) the "install via alternate name, uninstall via base name" test case was deemed duplicative, and has been deleted . 3) minor fixes to use jinja2 "~" for string concat instead of "+" * Fix nox lint --------- Co-authored-by: Felix Fontein --- .../10805-homebrew-support-old-names.yml | 2 + plugins/modules/homebrew.py | 32 +++++++--- .../targets/homebrew/tasks/casks.yml | 9 --- .../targets/homebrew/tasks/formulae.yml | 39 ------------ .../targets/homebrew/tasks/main.yml | 5 ++ .../targets/homebrew/tasks/package_names.yml | 55 ++++++++++++++++ .../homebrew/tasks/package_names_item.yml | 62 +++++++++++++++++++ 7 files changed, 146 insertions(+), 58 deletions(-) create mode 100644 changelogs/fragments/10805-homebrew-support-old-names.yml create mode 100644 tests/integration/targets/homebrew/tasks/package_names.yml create mode 100644 tests/integration/targets/homebrew/tasks/package_names_item.yml 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]'