homebrew: fix incorrect handling of aliases (#9255)

* Add failing test (See commit description)

Second assert returns this:

changed: [localhost] => changed=true
  changed_pkgs:
  - sqlite3
  msg: 'Changed: 1, Unchanged: 1'
  unchanged_pkgs:
  - sqlite

* Extract proper package_name from brew info using alisases

* Add changelog fragment

* Fix pep8

* Make sure sqlite is uninstalled beforehand

* Use `package_result is (not) changed` syntax in assertions

* Register more explicit names

* Fix handling of casks

changelogs/fragments/9255-fix-handling-of-aliased-homebrew-packages.yml

@@ -0,0 +1,2 @@
+bugfixes:
+  - homebrew - fix incorrect handling of aliased homebrew modules when the alias is requested (https://github.com/ansible-collections/community.general/pull/9255, https://github.com/ansible-collections/community.general/issues/9240).

plugins/modules/homebrew.py

@@ -379,6 +379,20 @@ class Homebrew(object):
             )
             raise HomebrewException(self.message)
 
+    def _save_package_info(self, package_detail, package_name):
+        if bool(package_detail.get("installed")):
+            self.installed_packages.add(package_name)
+        if bool(package_detail.get("outdated")):
+            self.outdated_packages.add(package_name)
+
+    def _extract_package_name(self, package_detail, is_cask):
+        canonical_name = package_detail["token"] if is_cask else package_detail["name"]  # For ex: 'sqlite'
+        all_valid_names = set(package_detail.get("aliases", []))  # For ex: {'sqlite3'}
+        all_valid_names.add(canonical_name)
+
+        # Then make sure the user provided name resurface.
+        return (all_valid_names & set(self.packages)).pop()
+
     def _get_packages_info(self):
         cmd = [
             "{brew_path}".format(brew_path=self.brew_path),
@@ -397,16 +411,13 @@ class Homebrew(object):
 
         data = json.loads(out)
         for package_detail in data.get("formulae", []):
-            if bool(package_detail.get("installed")):
+            package_name = self._extract_package_name(package_detail, is_cask=False)
-                self.installed_packages.add(package_detail["name"])
+            self._save_package_info(package_detail, package_name)
-            if bool(package_detail.get("outdated")):
-                self.outdated_packages.add(package_detail["name"])
 
         for package_detail in data.get("casks", []):
-            if bool(package_detail.get("installed")):
+            package_name = self._extract_package_name(package_detail, is_cask=True)
-                self.installed_packages.add(package_detail["token"])
+            self._save_package_info(package_detail, package_name)
-            if bool(package_detail.get("outdated")):
+
-                self.outdated_packages.add(package_detail["token"])
     # /prep -------------------------------------------------------- }}}
 
     def run(self):

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

@@ -55,7 +55,7 @@
 
   - assert:
       that:
-        - package_result.changed
+        - package_result is changed
 
   - name: Again install {{ package_name }} package using homebrew
     homebrew:
@@ -68,7 +68,7 @@
 
   - assert:
       that:
-        - not package_result.changed
+        - package_result is not changed
 
   - name: Uninstall {{ package_name }} package using homebrew
     homebrew:
@@ -81,7 +81,7 @@
 
   - assert:
       that:
-        - package_result.changed
+        - package_result is changed
 
   - name: Again uninstall {{ package_name }} package using homebrew
     homebrew:
@@ -94,4 +94,4 @@
 
   - assert:
       that:
-        - not package_result.changed
+        - package_result is not changed

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

@@ -55,7 +55,7 @@
 
   - assert:
       that:
-        - package_result.changed
+        - package_result is changed
         - "package_result.msg == 'Package installed: gnu-tar'"
         - "package_result.changed_pkgs == ['gnu-tar']"
         - "package_result.unchanged_pkgs == []"
@@ -71,7 +71,7 @@
 
   - assert:
       that:
-        - not package_result.changed
+        - package_result is not changed
         - "package_result.msg == 'Package already installed: gnu-tar'"
         - "package_result.changed_pkgs == []"
         - "package_result.unchanged_pkgs == ['gnu-tar']"
@@ -87,7 +87,7 @@
 
   - assert:
       that:
-        - package_result.changed
+        - package_result is changed
         - "package_result.msg == 'Package unlinked: gnu-tar'"
         - "package_result.changed_pkgs == ['gnu-tar']"
         - "package_result.unchanged_pkgs == []"
@@ -103,7 +103,7 @@
 
   - assert:
       that:
-        - package_result.changed
+        - package_result is changed
         - "package_result.msg == 'Package linked: gnu-tar'"
         - "package_result.changed_pkgs == ['gnu-tar']"
         - "package_result.unchanged_pkgs == []"
@@ -119,7 +119,7 @@
 
   - assert:
       that:
-        - package_result.changed
+        - package_result is changed
         - "package_result.msg == 'Package uninstalled: gnu-tar'"
         - "package_result.changed_pkgs == ['gnu-tar']"
         - "package_result.unchanged_pkgs == []"
@@ -135,7 +135,7 @@
 
   - assert:
       that:
-        - not package_result.changed
+        - package_result is not changed
         - "package_result.msg == 'Package already uninstalled: gnu-tar'"
         - "package_result.changed_pkgs == []"
         - "package_result.unchanged_pkgs == ['gnu-tar']"
@@ -151,7 +151,7 @@
 
   - assert:
       that:
-        - package_result.changed
+        - package_result is changed
         - "package_result.msg == 'Package upgraded: gnu-tar'"
         - "package_result.changed_pkgs == ['gnu-tar']"
         - "package_result.unchanged_pkgs == []"
@@ -167,7 +167,7 @@
 
   - assert:
       that:
-        - not package_result.changed
+        - package_result is not changed
         - "package_result.msg == 'Package already upgraded: gnu-tar'"
         - "package_result.changed_pkgs == []"
         - "package_result.unchanged_pkgs == ['gnu-tar']"
@@ -205,7 +205,7 @@
 
   - assert:
       that:
-        - package_result.changed
+        - package_result is changed
         - "package_result.msg == 'Changed: 1, Unchanged: 1'"
         - "package_result.changed_pkgs == ['gnu-time']"
         - "package_result.unchanged_pkgs == ['gnu-tar']"
@@ -221,7 +221,7 @@
 
   - assert:
       that:
-        - not package_result.changed
+        - package_result is not changed
         - "package_result.msg == 'Changed: 0, Unchanged: 2'"
         - "package_result.changed_pkgs == []"
         - "package_result.unchanged_pkgs | sort == ['gnu-tar', 'gnu-time']"
@@ -237,7 +237,7 @@
 
   - assert:
       that:
-        - package_result.changed
+        - package_result is changed
         - "package_result.msg == 'Changed: 2, Unchanged: 0'"
         - "package_result.changed_pkgs | sort == ['gnu-tar', 'gnu-time']"
         - "package_result.unchanged_pkgs == []"
@@ -253,7 +253,7 @@
 
   - assert:
       that:
-        - package_result.changed
+        - package_result is changed
         - "package_result.msg == 'Changed: 2, Unchanged: 0'"
         - "package_result.changed_pkgs | sort == ['gnu-tar', 'gnu-time']"
         - "package_result.unchanged_pkgs == []"
@@ -269,7 +269,7 @@
 
   - assert:
       that:
-        - package_result.changed
+        - package_result is changed
         - "package_result.msg == 'Changed: 2, Unchanged: 0'"
         - "package_result.changed_pkgs | sort == ['gnu-tar', 'gnu-time']"
         - "package_result.unchanged_pkgs == []"
@@ -285,7 +285,7 @@
 
   - assert:
       that:
-        - not package_result.changed
+        - package_result is not changed
         - "package_result.msg == 'Changed: 0, Unchanged: 2'"
         - "package_result.changed_pkgs == []"
         - "package_result.unchanged_pkgs | sort == ['gnu-tar', 'gnu-time']"
@@ -301,7 +301,7 @@
 
   - assert:
       that:
-        - package_result.changed
+        - package_result is changed
         - "package_result.msg == 'Changed: 2, Unchanged: 0'"
         - "package_result.changed_pkgs | sort == ['gnu-tar', 'gnu-time']"
         - "package_result.unchanged_pkgs == []"
@@ -317,7 +317,49 @@
 
   - assert:
       that:
-        - not package_result.changed
+        - package_result is not changed
         - "package_result.msg == 'Changed: 0, Unchanged: 2'"
         - "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
+      update_homebrew: false
+    become: true
+    become_user: "{{ brew_stat.stat.pw_name }}"
+
+  - name: Install sqlite package using alias (sqlite3)
+    homebrew:
+      name: "sqlite3"
+      state: present
+      update_homebrew: false
+    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
+      update_homebrew: false
+    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']"