From 67821c0c1211a094ddb31fe2f1284847795e29ca Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 16 Apr 2026 07:44:54 +0000 Subject: [PATCH 1/2] Fix dry_run bypass in install/update/uninstall and correct README docs Move dry_run checks before handler execution in BinProvider.install(), update(), and uninstall(). Previously, the dry_run check happened after calling _call_handler_for_action, which meant providers that call subprocess directly (like AnsibleProvider) would execute real commands even in dry_run mode, causing timeouts. Now dry_run short-circuits before any handler is invoked. Also fix README: replace non-existent field references in Ansible and Pyinfra provider sections with comments, and fix duplicate text in CargoProvider section. https://claude.ai/code/session_01RR9j9JutDnrVqB7qE7iPgS --- README.md | 10 +++--- abxpkg/binprovider.py | 80 ++++++++++++++++++++++++++----------------- 2 files changed, 54 insertions(+), 36 deletions(-) diff --git a/README.md b/README.md index afeb4538..04dd499c 100644 --- a/README.md +++ b/README.md @@ -946,7 +946,7 @@ PATH = "" # prepends cargo_root/bin and cargo_home/bi cargo_root = None # set this for hermetic installs ``` -- Install root: set `install_root=Path(...)` or `install_root=Path(...)` for isolated installs under `/bin`; otherwise installs go through `cargo_home`. +- Install root: set `install_root=Path(...)` (or the alias `cargo_root=Path(...)`) for isolated installs under `/bin`; otherwise installs go through `cargo_home`. - Auto-switching: none. - `dry_run`: shared behavior. - Security: `min_release_age` and `postinstall_scripts=False` are unsupported and are ignored with a warning if explicitly requested. @@ -1110,8 +1110,8 @@ Source: [`abxpkg/binprovider_pyinfra.py`](./abxpkg/binprovider_pyinfra.py) • T ```python INSTALLER_BIN = "pyinfra" PATH = os.environ.get("PATH", DEFAULT_PATH) -pyinfra_installer_module = "auto" -pyinfra_installer_kwargs = {} +# installer_module defaults to "auto" (operations.brew.packages on macOS, operations.server.packages on Linux) +# installer_kwargs defaults to {} ``` - Install root: **no hermetic prefix support**. It delegates to host package managers through pyinfra operations. @@ -1131,8 +1131,8 @@ Source: [`abxpkg/binprovider_ansible.py`](./abxpkg/binprovider_ansible.py) • T ```python INSTALLER_BIN = "ansible" PATH = os.environ.get("PATH", DEFAULT_PATH) -ansible_installer_module = "auto" -ansible_playbook_template = ANSIBLE_INSTALL_PLAYBOOK_TEMPLATE +# installer_module defaults to "auto" (community.general.homebrew on macOS, ansible.builtin.package on Linux) +# playbook_template defaults to ANSIBLE_INSTALL_PLAYBOOK_TEMPLATE ``` - Install root: **no hermetic prefix support**. It delegates to the host via `ansible-runner`. diff --git a/abxpkg/binprovider.py b/abxpkg/binprovider.py index b2b3179d..c9dea42c 100755 --- a/abxpkg/binprovider.py +++ b/abxpkg/binprovider.py @@ -2098,6 +2098,27 @@ def install( ) self.setup_PATH(no_cache=no_cache) + + if self.dry_run: + # return fake ShallowBinary without calling the handler + # no point running the real install if nothing should actually be installed + logger.info( + "DRY RUN (%s): skipping install of %s", + self.__class__.__name__, + bin_name, + ) + return ShallowBinary.model_construct( + name=bin_name, + description=bin_name, + loaded_binprovider=self, + loaded_abspath=UNKNOWN_ABSPATH, + loaded_version=UNKNOWN_VERSION, + loaded_sha256=UNKNOWN_SHA256, + loaded_mtime=UNKNOWN_MTIME, + loaded_euid=UNKNOWN_EUID, + binproviders=[self], + ) + install_log = None exec_log_prefix_token = ACTIVE_EXEC_LOG_PREFIX.set( f"⛟ Installing {bin_name} via {self.name}...", @@ -2125,21 +2146,6 @@ def install( finally: ACTIVE_EXEC_LOG_PREFIX.reset(exec_log_prefix_token) - if self.dry_run: - # return fake ShallowBinary if we're just doing a dry run - # no point trying to get real abspath or version if nothing was actually installed - return ShallowBinary.model_construct( - name=bin_name, - description=bin_name, - loaded_binprovider=self, - loaded_abspath=UNKNOWN_ABSPATH, - loaded_version=UNKNOWN_VERSION, - loaded_sha256=UNKNOWN_SHA256, - loaded_mtime=UNKNOWN_MTIME, - loaded_euid=UNKNOWN_EUID, - binproviders=[self], - ) - self.invalidate_cache(bin_name) result = self.load(bin_name, quiet=True, no_cache=no_cache) @@ -2275,6 +2281,25 @@ def update( ) self.setup_PATH(no_cache=no_cache) + + if self.dry_run: + logger.info( + "DRY RUN (%s): skipping update of %s", + self.__class__.__name__, + bin_name, + ) + return ShallowBinary.model_construct( + name=bin_name, + description=bin_name, + loaded_binprovider=self, + loaded_abspath=UNKNOWN_ABSPATH, + loaded_version=UNKNOWN_VERSION, + loaded_sha256=UNKNOWN_SHA256, + loaded_mtime=UNKNOWN_MTIME, + loaded_euid=UNKNOWN_EUID, + binproviders=[self], + ) + update_log = None exec_log_prefix_token = ACTIVE_EXEC_LOG_PREFIX.set( f"⬆ Updating {bin_name} via {self.name}...", @@ -2302,19 +2327,6 @@ def update( finally: ACTIVE_EXEC_LOG_PREFIX.reset(exec_log_prefix_token) - if self.dry_run: - return ShallowBinary.model_construct( - name=bin_name, - description=bin_name, - loaded_binprovider=self, - loaded_abspath=UNKNOWN_ABSPATH, - loaded_version=UNKNOWN_VERSION, - loaded_sha256=UNKNOWN_SHA256, - loaded_mtime=UNKNOWN_MTIME, - loaded_euid=UNKNOWN_EUID, - binproviders=[self], - ) - self.invalidate_cache(bin_name) result = self.load(bin_name, quiet=True, no_cache=no_cache) @@ -2380,6 +2392,15 @@ def uninstall( return False install_args = self.get_install_args(bin_name, quiet=quiet, no_cache=no_cache) self.setup_PATH(no_cache=no_cache) + + if self.dry_run: + logger.info( + "DRY RUN (%s): skipping uninstall of %s", + self.__class__.__name__, + bin_name, + ) + return True + uninstall_result = None exec_log_prefix_token = ACTIVE_EXEC_LOG_PREFIX.set( f"🗑️ Uninstalling {bin_name} via {self.name}...", @@ -2409,9 +2430,6 @@ def uninstall( self.invalidate_cache(bin_name) - if self.dry_run: - return True - if uninstall_result is not False: logger.info("🗑️ Uninstalled %s via %s", bin_name, self.name) return uninstall_result is not False From 7756837c33dd4ca9fc88d3077e3991b72fde463b Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 17 Apr 2026 22:32:32 +0000 Subject: [PATCH 2/2] Fix dry_run in Ansible/Pyinfra handlers instead of base class The previous approach short-circuited dry_run before calling handlers in the base BinProvider, which broke providers that use dry_run to log constructed commands (e.g. BunProvider's --trust flag test). The correct fix is to add dry_run checks in the AnsibleProvider and PyinfraProvider handlers that call subprocess.run directly, since _run_subprocess_as_provider already handles dry_run for all other providers. https://claude.ai/code/session_01RR9j9JutDnrVqB7qE7iPgS --- abxpkg/binprovider.py | 78 +++++++++++++---------------------- abxpkg/binprovider_ansible.py | 24 +++++++++++ abxpkg/binprovider_pyinfra.py | 24 +++++++++++ 3 files changed, 77 insertions(+), 49 deletions(-) diff --git a/abxpkg/binprovider.py b/abxpkg/binprovider.py index 8ecd23a6..7d185e40 100755 --- a/abxpkg/binprovider.py +++ b/abxpkg/binprovider.py @@ -2098,27 +2098,6 @@ def install( ) self.setup_PATH(no_cache=no_cache) - - if self.dry_run: - # return fake ShallowBinary without calling the handler - # no point running the real install if nothing should actually be installed - logger.info( - "DRY RUN (%s): skipping install of %s", - self.__class__.__name__, - bin_name, - ) - return ShallowBinary.model_construct( - name=bin_name, - description=bin_name, - loaded_binprovider=self, - loaded_abspath=UNKNOWN_ABSPATH, - loaded_version=UNKNOWN_VERSION, - loaded_sha256=UNKNOWN_SHA256, - loaded_mtime=UNKNOWN_MTIME, - loaded_euid=UNKNOWN_EUID, - binproviders=[self], - ) - install_log = None exec_log_prefix_token = ACTIVE_EXEC_LOG_PREFIX.set( f"⛟ Installing {bin_name} via {self.name}...", @@ -2146,6 +2125,19 @@ def install( finally: ACTIVE_EXEC_LOG_PREFIX.reset(exec_log_prefix_token) + if self.dry_run: + return ShallowBinary.model_construct( + name=bin_name, + description=bin_name, + loaded_binprovider=self, + loaded_abspath=UNKNOWN_ABSPATH, + loaded_version=UNKNOWN_VERSION, + loaded_sha256=UNKNOWN_SHA256, + loaded_mtime=UNKNOWN_MTIME, + loaded_euid=UNKNOWN_EUID, + binproviders=[self], + ) + self.invalidate_cache(bin_name) result = self.load(bin_name, quiet=True, no_cache=no_cache) @@ -2281,25 +2273,6 @@ def update( ) self.setup_PATH(no_cache=no_cache) - - if self.dry_run: - logger.info( - "DRY RUN (%s): skipping update of %s", - self.__class__.__name__, - bin_name, - ) - return ShallowBinary.model_construct( - name=bin_name, - description=bin_name, - loaded_binprovider=self, - loaded_abspath=UNKNOWN_ABSPATH, - loaded_version=UNKNOWN_VERSION, - loaded_sha256=UNKNOWN_SHA256, - loaded_mtime=UNKNOWN_MTIME, - loaded_euid=UNKNOWN_EUID, - binproviders=[self], - ) - update_log = None exec_log_prefix_token = ACTIVE_EXEC_LOG_PREFIX.set( f"⬆ Updating {bin_name} via {self.name}...", @@ -2327,6 +2300,19 @@ def update( finally: ACTIVE_EXEC_LOG_PREFIX.reset(exec_log_prefix_token) + if self.dry_run: + return ShallowBinary.model_construct( + name=bin_name, + description=bin_name, + loaded_binprovider=self, + loaded_abspath=UNKNOWN_ABSPATH, + loaded_version=UNKNOWN_VERSION, + loaded_sha256=UNKNOWN_SHA256, + loaded_mtime=UNKNOWN_MTIME, + loaded_euid=UNKNOWN_EUID, + binproviders=[self], + ) + self.invalidate_cache(bin_name) result = self.load(bin_name, quiet=True, no_cache=no_cache) @@ -2392,15 +2378,6 @@ def uninstall( return False install_args = self.get_install_args(bin_name, quiet=quiet, no_cache=no_cache) self.setup_PATH(no_cache=no_cache) - - if self.dry_run: - logger.info( - "DRY RUN (%s): skipping uninstall of %s", - self.__class__.__name__, - bin_name, - ) - return True - uninstall_result = None exec_log_prefix_token = ACTIVE_EXEC_LOG_PREFIX.set( f"🗑️ Uninstalling {bin_name} via {self.name}...", @@ -2430,6 +2407,9 @@ def uninstall( self.invalidate_cache(bin_name) + if self.dry_run: + return True + if uninstall_result is not False: logger.info("🗑️ Uninstalled %s via %s", bin_name, self.name) return uninstall_result is not False diff --git a/abxpkg/binprovider_ansible.py b/abxpkg/binprovider_ansible.py index 19e4f2d2..1a3d4bb3 100755 --- a/abxpkg/binprovider_ansible.py +++ b/abxpkg/binprovider_ansible.py @@ -373,6 +373,14 @@ def default_install_handler( ansible_playbook = self.INSTALLER_BINARY(no_cache=no_cache).loaded_abspath assert ansible_playbook + if self.dry_run: + logger.info( + "DRY RUN (%s): ansible-playbook install %s", + self.__class__.__name__, + " ".join(install_args), + ) + return f"DRY RUN: would install {install_args} via ansible" + module_extra_kwargs = self.get_ansible_module_extra_kwargs() return ansible_package_install( @@ -401,6 +409,14 @@ def default_update_handler( ansible_playbook = self.INSTALLER_BINARY(no_cache=no_cache).loaded_abspath assert ansible_playbook + if self.dry_run: + logger.info( + "DRY RUN (%s): ansible-playbook update %s", + self.__class__.__name__, + " ".join(install_args), + ) + return f"DRY RUN: would update {install_args} via ansible" + module_extra_kwargs = self.get_ansible_module_extra_kwargs() if module_extra_kwargs: return ansible_package_install( @@ -439,6 +455,14 @@ def default_uninstall_handler( ansible_playbook = self.INSTALLER_BINARY(no_cache=no_cache).loaded_abspath assert ansible_playbook + if self.dry_run: + logger.info( + "DRY RUN (%s): ansible-playbook uninstall %s", + self.__class__.__name__, + " ".join(install_args), + ) + return True + module_extra_kwargs = self.get_ansible_module_extra_kwargs() if module_extra_kwargs: ansible_package_install( diff --git a/abxpkg/binprovider_pyinfra.py b/abxpkg/binprovider_pyinfra.py index ee299405..c06ad106 100755 --- a/abxpkg/binprovider_pyinfra.py +++ b/abxpkg/binprovider_pyinfra.py @@ -294,6 +294,14 @@ def default_install_handler( pyinfra_abspath = self.INSTALLER_BINARY(no_cache=no_cache).loaded_abspath assert pyinfra_abspath + if self.dry_run: + logger.info( + "DRY RUN (%s): pyinfra install %s", + self.__class__.__name__, + " ".join(install_args), + ) + return f"DRY RUN: would install {install_args} via pyinfra" + return pyinfra_package_install( pkg_names=install_args, pyinfra_abspath=str(pyinfra_abspath), @@ -317,6 +325,14 @@ def default_update_handler( pyinfra_abspath = self.INSTALLER_BINARY(no_cache=no_cache).loaded_abspath assert pyinfra_abspath + if self.dry_run: + logger.info( + "DRY RUN (%s): pyinfra update %s", + self.__class__.__name__, + " ".join(install_args), + ) + return f"DRY RUN: would update {install_args} via pyinfra" + return pyinfra_package_install( pkg_names=install_args, pyinfra_abspath=str(pyinfra_abspath), @@ -342,6 +358,14 @@ def default_uninstall_handler( pyinfra_abspath = self.INSTALLER_BINARY(no_cache=no_cache).loaded_abspath assert pyinfra_abspath + if self.dry_run: + logger.info( + "DRY RUN (%s): pyinfra uninstall %s", + self.__class__.__name__, + " ".join(install_args), + ) + return True + pyinfra_package_install( pkg_names=install_args, pyinfra_abspath=str(pyinfra_abspath),