From 9452a2c8ac06cd12446d5fe500d6c24506f62075 Mon Sep 17 00:00:00 2001 From: Thibaut Decombe <68703331+UnknownPlatypus@users.noreply.github.com> Date: Fri, 20 Dec 2024 22:53:41 +0100 Subject: [PATCH] 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 --- ...-handling-of-aliased-homebrew-packages.yml | 2 + plugins/modules/homebrew.py | 27 +++++-- .../targets/homebrew/tasks/casks.yml | 8 +- .../targets/homebrew/tasks/formulae.yml | 74 +++++++++++++++---- 4 files changed, 83 insertions(+), 28 deletions(-) create mode 100644 changelogs/fragments/9255-fix-handling-of-aliased-homebrew-packages.yml diff --git a/changelogs/fragments/9255-fix-handling-of-aliased-homebrew-packages.yml b/changelogs/fragments/9255-fix-handling-of-aliased-homebrew-packages.yml new file mode 100644 index 0000000000..350e81af8e --- /dev/null +++ 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). \ No newline at end of file diff --git a/plugins/modules/homebrew.py b/plugins/modules/homebrew.py index 8eb1b9d689..980b5cf656 100644 --- a/plugins/modules/homebrew.py +++ b/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")): - 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 index ffbe67d158..50824a9e9f 100644 --- 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 index 21276e3a2e..1559ba5dd8 100644 --- 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']"