From 119623952d203d321c94aa5a3a896ac73ce619bf Mon Sep 17 00:00:00 2001 From: "patchback[bot]" <45432694+patchback[bot]@users.noreply.github.com> Date: Fri, 17 Apr 2026 18:32:10 +0200 Subject: [PATCH] [PR #11848/c4ed3467 backport][stable-12] homebrew_tap: fix None in command, redundant brew tap calls, format strings, and drop no-op locale vars (#11865) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit homebrew_tap: fix None in command, redundant brew tap calls, format strings, and drop no-op locale vars (#11848) * homebrew_tap: fix None in command list, redundant brew tap calls, and bad format strings - Fix None being injected into the run_command list when url is not provided to add_tap (filter with [opt for opt in [...] if opt]) - Reduce redundant `brew tap` calls: add_taps and remove_taps now fetch the tap list once upfront and pass it to the per-tap functions; already_tapped accepts an optional pre-fetched list to avoid re-running brew for every tap in a batch - Fix mixed f-string/%-formatting in error messages in add_taps and remove_taps, replaced with plain f-strings * homebrew_tap: simplify command construction in add_tap Replace the opaque list comprehension filter with an explicit conditional append — only url is ever optional, so testing the known-present items was misleading. * homebrew_tap: remove unnecessary locale env vars Homebrew has no i18n/l10n support — all output is hardcoded English. LANGUAGE=C and LC_ALL=C have no effect on brew output. * homebrew_tap: add changelog fragment for #11848 * remove hombrew_tap from PR #11783 changelog - change reverted here --------- (cherry picked from commit c4ed3467b607ee1240320d50d1fd0bf7cc88df8f) Co-authored-by: Alexei Znamensky <103110+russoz@users.noreply.github.com> Co-authored-by: Claude Sonnet 4.6 --- .../fragments/11783-group5-batch12-locale.yml | 3 - .../fragments/11848-homebrew-tap-fixes.yml | 7 +++ plugins/modules/homebrew_tap.py | 60 +++++++------------ 3 files changed, 30 insertions(+), 40 deletions(-) create mode 100644 changelogs/fragments/11848-homebrew-tap-fixes.yml diff --git a/changelogs/fragments/11783-group5-batch12-locale.yml b/changelogs/fragments/11783-group5-batch12-locale.yml index e7d883b889..8e2d1451d6 100644 --- a/changelogs/fragments/11783-group5-batch12-locale.yml +++ b/changelogs/fragments/11783-group5-batch12-locale.yml @@ -5,9 +5,6 @@ bugfixes: - bundler - set ``LANGUAGE`` and ``LC_ALL`` to ``C`` in ``run_command()`` calls to ensure locale-independent output parsing (https://github.com/ansible-collections/community.general/issues/11737, https://github.com/ansible-collections/community.general/pull/11783). - - homebrew_tap - set ``LANGUAGE`` and ``LC_ALL`` to ``C`` in ``run_command()`` calls to ensure locale-independent output parsing - (https://github.com/ansible-collections/community.general/issues/11737, - https://github.com/ansible-collections/community.general/pull/11783). - kibana_plugin - set ``LANGUAGE`` and ``LC_ALL`` to ``C`` in ``run_command()`` calls to ensure locale-independent output parsing (https://github.com/ansible-collections/community.general/issues/11737, https://github.com/ansible-collections/community.general/pull/11783). diff --git a/changelogs/fragments/11848-homebrew-tap-fixes.yml b/changelogs/fragments/11848-homebrew-tap-fixes.yml new file mode 100644 index 0000000000..014ca31159 --- /dev/null +++ b/changelogs/fragments/11848-homebrew-tap-fixes.yml @@ -0,0 +1,7 @@ +bugfixes: + - homebrew_tap - fix ``None`` being passed as a command argument when adding a tap without a URL + (https://github.com/ansible-collections/community.general/pull/11848). +minor_changes: + - homebrew_tap - avoid redundant ``brew tap`` calls when processing multiple taps by fetching + the tap list once upfront + (https://github.com/ansible-collections/community.general/pull/11848). diff --git a/plugins/modules/homebrew_tap.py b/plugins/modules/homebrew_tap.py index d543974554..1e9a5092ac 100644 --- a/plugins/modules/homebrew_tap.py +++ b/plugins/modules/homebrew_tap.py @@ -86,23 +86,16 @@ def a_valid_tap(tap): return regex.match(tap) -def already_tapped(module, brew_path, tap): +def already_tapped(module, brew_path, tap, taps=None): """Returns True if already tapped.""" - - rc, out, err = module.run_command( - [ - brew_path, - "tap", - ] - ) - - taps = [tap_.strip().lower() for tap_ in out.split("\n") if tap_] + if taps is None: + rc, out, err = module.run_command([brew_path, "tap"]) + taps = [tap_.strip().lower() for tap_ in out.split("\n") if tap_] tap_name = re.sub("homebrew-", "", tap.lower()) - return tap_name in taps -def add_tap(module, brew_path, tap, url=None): +def add_tap(module, brew_path, tap, url=None, taps=None): """Adds a single tap.""" failed, changed, msg = False, False, "" @@ -110,18 +103,14 @@ def add_tap(module, brew_path, tap, url=None): failed = True msg = f"not a valid tap: {tap}" - elif not already_tapped(module, brew_path, tap): + elif not already_tapped(module, brew_path, tap, taps): if module.check_mode: module.exit_json(changed=True) - rc, out, err = module.run_command( - [ - brew_path, - "tap", - tap, - url, - ] - ) + cmd = [brew_path, "tap", tap] + if url: + cmd.append(url) + rc, out, err = module.run_command(cmd) if rc == 0: changed = True msg = f"successfully tapped: {tap}" @@ -139,8 +128,11 @@ def add_taps(module, brew_path, taps): """Adds one or more taps.""" failed, changed, unchanged, added, msg = False, False, 0, 0, "" + rc, out, err = module.run_command([brew_path, "tap"]) + tapped = [t.strip().lower() for t in out.split("\n") if t] + for tap in taps: - (failed, changed, msg) = add_tap(module, brew_path, tap) + (failed, changed, msg) = add_tap(module, brew_path, tap, taps=tapped) if failed: break if changed: @@ -149,8 +141,7 @@ def add_taps(module, brew_path, taps): unchanged += 1 if failed: - msg = f"added: %d, unchanged: %d, error: {msg}" - msg = msg % (added, unchanged) + msg = f"added: {added}, unchanged: {unchanged}, error: {msg}" elif added: changed = True msg = f"added: {added}, unchanged: {unchanged}" @@ -160,7 +151,7 @@ def add_taps(module, brew_path, taps): return (failed, changed, msg) -def remove_tap(module, brew_path, tap): +def remove_tap(module, brew_path, tap, taps=None): """Removes a single tap.""" failed, changed, msg = False, False, "" @@ -168,17 +159,11 @@ def remove_tap(module, brew_path, tap): failed = True msg = f"not a valid tap: {tap}" - elif already_tapped(module, brew_path, tap): + elif already_tapped(module, brew_path, tap, taps): if module.check_mode: module.exit_json(changed=True) - rc, out, err = module.run_command( - [ - brew_path, - "untap", - tap, - ] - ) + rc, out, err = module.run_command([brew_path, "untap", tap]) if not already_tapped(module, brew_path, tap): changed = True msg = f"successfully untapped: {tap}" @@ -196,8 +181,11 @@ def remove_taps(module, brew_path, taps): """Removes one or more taps.""" failed, changed, unchanged, removed, msg = False, False, 0, 0, "" + rc, out, err = module.run_command([brew_path, "tap"]) + tapped = [t.strip().lower() for t in out.split("\n") if t] + for tap in taps: - (failed, changed, msg) = remove_tap(module, brew_path, tap) + (failed, changed, msg) = remove_tap(module, brew_path, tap, taps=tapped) if failed: break if changed: @@ -206,8 +194,7 @@ def remove_taps(module, brew_path, taps): unchanged += 1 if failed: - msg = f"removed: %d, unchanged: %d, error: {msg}" - msg = msg % (removed, unchanged) + msg = f"removed: {removed}, unchanged: {unchanged}, error: {msg}" elif removed: changed = True msg = f"removed: {removed}, unchanged: {unchanged}" @@ -230,7 +217,6 @@ def main(): ), supports_check_mode=True, ) - module.run_command_environ_update = {"LANGUAGE": "C", "LC_ALL": "C"} path = module.params["path"] if path: