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 <felix@fontein.de>

* Update tests/integration/targets/homebrew/tasks/casks.yml

Co-authored-by: Felix Fontein <felix@fontein.de>

* Update tests/integration/targets/homebrew/tasks/package_names_item.yml

Co-authored-by: Felix Fontein <felix@fontein.de>

* Update tests/integration/targets/homebrew/tasks/formulae.yml

Co-authored-by: Felix Fontein <felix@fontein.de>

* 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 <felix@fontein.de>

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).

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.

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 }}"

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

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'

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

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]'