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: