[PR #9255/9452a2c8 backport][stable-10] homebrew: fix incorrect handl…

…ing of aliases (#9289) 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 (cherry picked from commit 9452a2c) Co-authored-by: Thibaut Decombe <[email protected]>
ansible-collections · Dec 20, 2024 · df58182 · df58182
1 parent 69aea38
commit df58182
Show file tree

Hide file tree

Showing 4 changed files with 83 additions and 28 deletions.
diff --git a/changelogs/fragments/9255-fix-handling-of-aliased-homebrew-packages.yml b/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).
diff --git a/plugins/modules/homebrew.py b/plugins/modules/homebrew.py
@@ -379,6 +379,20 @@ def _validate_packages_names(self):
             )
             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 @@ def _get_packages_info(self):
 
         data = json.loads(out)
         for package_detail in data.get("formulae", []):
-            if bool(package_detail.get("installed")):
-                self.installed_packages.add(package_detail["name"])
-            if bool(package_detail.get("outdated")):
-                self.outdated_packages.add(package_detail["name"])
+            package_name = self._extract_package_name(package_detail, is_cask=False)
+            self._save_package_info(package_detail, package_name)
 
         for package_detail in data.get("casks", []):
-            if bool(package_detail.get("installed")):
-                self.installed_packages.add(package_detail["token"])
-            if bool(package_detail.get("outdated")):
-                self.outdated_packages.add(package_detail["token"])
+            package_name = self._extract_package_name(package_detail, is_cask=True)
+            self._save_package_info(package_detail, package_name)
+
     # /prep -------------------------------------------------------- }}}
 
     def run(self):

diff --git a/tests/integration/targets/homebrew/tasks/casks.yml b/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
diff --git a/tests/integration/targets/homebrew/tasks/formulae.yml b/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']"
Original file line number	Diff line number	Diff line change
		@@ -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).