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

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

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

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

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

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* homebrew_tap: add changelog fragment for #11848

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* remove hombrew_tap from PR #11783 changelog - change reverted here

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>

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

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

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: