From 4c84eeeb696f54179d99601e2850f0205557a34b Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 28 May 2026 05:56:54 +0000 Subject: [PATCH 1/2] refactor: simplify pnpm install/update arg construction Pull the duplicated install/update bodies into shared _build_mutation_args / _apply_min_version / _install_args_have_option helpers, matching the npm provider's pattern. Also drops the dead '--loglevel=error' entry that was being prepended to install_args inside arg-presence checks but could never match the searched flag. --- abxpkg/binprovider_pnpm.py | 173 +++++++++++++++++++------------------ 1 file changed, 88 insertions(+), 85 deletions(-) diff --git a/abxpkg/binprovider_pnpm.py b/abxpkg/binprovider_pnpm.py index 151a26a..cc6c3cf 100755 --- a/abxpkg/binprovider_pnpm.py +++ b/abxpkg/binprovider_pnpm.py @@ -125,6 +125,57 @@ def supports_min_release_age(self, action, no_cache: bool = False) -> bool: def supports_postinstall_disable(self, action, no_cache: bool = False) -> bool: return action in ("install", "update") + @staticmethod + def _install_args_have_option(args: InstallArgs, *options: str) -> bool: + """Return True when install_args already contains any of the requested options.""" + return any( + arg == option or arg.startswith(f"{option}=") + for arg in args + for option in options + ) + + def _apply_min_version( + self, + install_args: InstallArgs, + min_version: SemVer | None, + ) -> list[str]: + """Rewrite unpinned package specs to ``pkg@>=`` when requested.""" + if not min_version: + return list(install_args) + return [ + f"{arg}@>={min_version}" + if arg + and not arg.startswith(("-", ".", "/")) + and ":" not in arg.split("/")[0] + and "@" not in arg.split("/")[-1] + else arg + for arg in install_args + ] + + def _build_mutation_args( + self, + install_args: InstallArgs, + *, + postinstall_scripts: bool, + min_release_age: float, + ) -> list[str]: + """Shared ``add``/``update`` CLI args (security flags + prefix).""" + args: list[str] = ["--loglevel=error", f"--store-dir={self.cache_dir}"] + if not postinstall_scripts: + args.append("--ignore-scripts") + else: + # pnpm 10+ blocks ALL postinstall scripts unless explicitly allowed. + args.append("--config.dangerouslyAllowAllBuilds=true") + if min_release_age > 0 and not self._install_args_have_option( + install_args, + "--config.minimumReleaseAge", + ): + args.append( + f"--config.minimumReleaseAge={max(int(min_release_age * 24 * 60), 1)}", + ) + args.append(f"--dir={self.install_root}" if self.install_root else "--global") + return args + def default_install_args_handler( self, bin_name: BinName, @@ -336,52 +387,27 @@ def default_install_handler( ) -> str: installer_bin = self.INSTALLER_BINARY(no_cache=no_cache).loaded_abspath assert installer_bin - postinstall_scripts = ( - False if postinstall_scripts is None else postinstall_scripts + install_args = self._apply_min_version( + install_args or self.get_install_args(bin_name), + min_version, ) - min_release_age = 7.0 if min_release_age is None else min_release_age - install_args = install_args or self.get_install_args(bin_name) - if min_version: - install_args = [ - f"{arg}@>={min_version}" - if arg - and not arg.startswith(("-", ".", "/")) - and ":" not in arg.split("/")[0] - and "@" not in arg.split("/")[-1] - else arg - for arg in install_args - ] - if any( - arg == "--ignore-scripts" for arg in ("--loglevel=error", *install_args) - ): + if postinstall_scripts is None: postinstall_scripts = False + if self._install_args_have_option(install_args, "--ignore-scripts"): + postinstall_scripts = False + if min_release_age is None: + min_release_age = 7.0 - cmd: list[str] = [ - "add", - "--loglevel=error", - f"--store-dir={self.cache_dir}", - ] - if not postinstall_scripts: - cmd.append("--ignore-scripts") - else: - # pnpm 10+ blocks ALL postinstall scripts unless explicitly allowed. - cmd.append("--config.dangerouslyAllowAllBuilds=true") - if ( - min_release_age is not None - and min_release_age > 0 - and not any( - arg == "--config.minimumReleaseAge" - or arg.startswith("--config.minimumReleaseAge=") - for arg in ("--loglevel=error", *install_args) - ) - ): - cmd.append( - f"--config.minimumReleaseAge={max(int(min_release_age * 24 * 60), 1)}", - ) - cmd.append(f"--dir={self.install_root}" if self.install_root else "--global") - cmd.extend(install_args) - - proc = self.exec(bin_name=installer_bin, cmd=cmd, timeout=timeout) + mutation_args = self._build_mutation_args( + install_args, + postinstall_scripts=postinstall_scripts, + min_release_age=min_release_age, + ) + proc = self.exec( + bin_name=installer_bin, + cmd=["add", *mutation_args, *install_args], + timeout=timeout, + ) if proc.returncode != 0: self._raise_proc_error("install", install_args, proc) return format_subprocess_output(proc.stdout, proc.stderr) @@ -399,51 +425,28 @@ def default_update_handler( ) -> str: installer_bin = self.INSTALLER_BINARY(no_cache=no_cache).loaded_abspath assert installer_bin - postinstall_scripts = ( - False if postinstall_scripts is None else postinstall_scripts + install_args = self._apply_min_version( + install_args or self.get_install_args(bin_name), + min_version, ) - min_release_age = 7.0 if min_release_age is None else min_release_age - install_args = install_args or self.get_install_args(bin_name) - if min_version: - install_args = [ - f"{arg}@>={min_version}" - if arg - and not arg.startswith(("-", ".", "/")) - and ":" not in arg.split("/")[0] - and "@" not in arg.split("/")[-1] - else arg - for arg in install_args - ] - if any( - arg == "--ignore-scripts" for arg in ("--loglevel=error", *install_args) - ): + if postinstall_scripts is None: postinstall_scripts = False + if self._install_args_have_option(install_args, "--ignore-scripts"): + postinstall_scripts = False + if min_release_age is None: + min_release_age = 7.0 - cmd: list[str] = [ - "add" if min_version is not None else "update", - "--loglevel=error", - f"--store-dir={self.cache_dir}", - ] - if not postinstall_scripts: - cmd.append("--ignore-scripts") - else: - cmd.append("--config.dangerouslyAllowAllBuilds=true") - if ( - min_release_age is not None - and min_release_age > 0 - and not any( - arg == "--config.minimumReleaseAge" - or arg.startswith("--config.minimumReleaseAge=") - for arg in ("--loglevel=error", *install_args) - ) - ): - cmd.append( - f"--config.minimumReleaseAge={max(int(min_release_age * 24 * 60), 1)}", - ) - cmd.append(f"--dir={self.install_root}" if self.install_root else "--global") - cmd.extend(install_args) - - proc = self.exec(bin_name=installer_bin, cmd=cmd, timeout=timeout) + mutation_args = self._build_mutation_args( + install_args, + postinstall_scripts=postinstall_scripts, + min_release_age=min_release_age, + ) + verb = "add" if min_version is not None else "update" + proc = self.exec( + bin_name=installer_bin, + cmd=[verb, *mutation_args, *install_args], + timeout=timeout, + ) if proc.returncode != 0: self._raise_proc_error("update", install_args, proc) return format_subprocess_output(proc.stdout, proc.stderr) From a4a7e9c30d78e5c5fef3e13157e465c0f8233444 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 28 May 2026 06:25:18 +0000 Subject: [PATCH 2/2] refactor: inline pnpm install_args logic, drop shared helpers Fold the _build_mutation_args / _apply_min_version / _install_args_have_option helpers back into the install and update handlers directly. They were only used in two call sites each, and the indirection didn't pay for itself. --- abxpkg/binprovider_pnpm.py | 140 ++++++++++++++++--------------------- 1 file changed, 59 insertions(+), 81 deletions(-) diff --git a/abxpkg/binprovider_pnpm.py b/abxpkg/binprovider_pnpm.py index cc6c3cf..0b0fed4 100755 --- a/abxpkg/binprovider_pnpm.py +++ b/abxpkg/binprovider_pnpm.py @@ -125,57 +125,6 @@ def supports_min_release_age(self, action, no_cache: bool = False) -> bool: def supports_postinstall_disable(self, action, no_cache: bool = False) -> bool: return action in ("install", "update") - @staticmethod - def _install_args_have_option(args: InstallArgs, *options: str) -> bool: - """Return True when install_args already contains any of the requested options.""" - return any( - arg == option or arg.startswith(f"{option}=") - for arg in args - for option in options - ) - - def _apply_min_version( - self, - install_args: InstallArgs, - min_version: SemVer | None, - ) -> list[str]: - """Rewrite unpinned package specs to ``pkg@>=`` when requested.""" - if not min_version: - return list(install_args) - return [ - f"{arg}@>={min_version}" - if arg - and not arg.startswith(("-", ".", "/")) - and ":" not in arg.split("/")[0] - and "@" not in arg.split("/")[-1] - else arg - for arg in install_args - ] - - def _build_mutation_args( - self, - install_args: InstallArgs, - *, - postinstall_scripts: bool, - min_release_age: float, - ) -> list[str]: - """Shared ``add``/``update`` CLI args (security flags + prefix).""" - args: list[str] = ["--loglevel=error", f"--store-dir={self.cache_dir}"] - if not postinstall_scripts: - args.append("--ignore-scripts") - else: - # pnpm 10+ blocks ALL postinstall scripts unless explicitly allowed. - args.append("--config.dangerouslyAllowAllBuilds=true") - if min_release_age > 0 and not self._install_args_have_option( - install_args, - "--config.minimumReleaseAge", - ): - args.append( - f"--config.minimumReleaseAge={max(int(min_release_age * 24 * 60), 1)}", - ) - args.append(f"--dir={self.install_root}" if self.install_root else "--global") - return args - def default_install_args_handler( self, bin_name: BinName, @@ -387,27 +336,42 @@ def default_install_handler( ) -> str: installer_bin = self.INSTALLER_BINARY(no_cache=no_cache).loaded_abspath assert installer_bin - install_args = self._apply_min_version( - install_args or self.get_install_args(bin_name), - min_version, - ) + install_args = install_args or self.get_install_args(bin_name) + if min_version: + install_args = [ + f"{arg}@>={min_version}" + if arg + and not arg.startswith(("-", ".", "/")) + and ":" not in arg.split("/")[0] + and "@" not in arg.split("/")[-1] + else arg + for arg in install_args + ] if postinstall_scripts is None: postinstall_scripts = False - if self._install_args_have_option(install_args, "--ignore-scripts"): + if any(arg == "--ignore-scripts" for arg in install_args): postinstall_scripts = False if min_release_age is None: min_release_age = 7.0 - mutation_args = self._build_mutation_args( - install_args, - postinstall_scripts=postinstall_scripts, - min_release_age=min_release_age, - ) - proc = self.exec( - bin_name=installer_bin, - cmd=["add", *mutation_args, *install_args], - timeout=timeout, - ) + cmd: list[str] = ["add", "--loglevel=error", f"--store-dir={self.cache_dir}"] + if not postinstall_scripts: + cmd.append("--ignore-scripts") + else: + # pnpm 10+ blocks ALL postinstall scripts unless explicitly allowed. + cmd.append("--config.dangerouslyAllowAllBuilds=true") + if min_release_age > 0 and not any( + arg == "--config.minimumReleaseAge" + or arg.startswith("--config.minimumReleaseAge=") + for arg in install_args + ): + cmd.append( + f"--config.minimumReleaseAge={max(int(min_release_age * 24 * 60), 1)}", + ) + cmd.append(f"--dir={self.install_root}" if self.install_root else "--global") + cmd.extend(install_args) + + proc = self.exec(bin_name=installer_bin, cmd=cmd, timeout=timeout) if proc.returncode != 0: self._raise_proc_error("install", install_args, proc) return format_subprocess_output(proc.stdout, proc.stderr) @@ -425,28 +389,42 @@ def default_update_handler( ) -> str: installer_bin = self.INSTALLER_BINARY(no_cache=no_cache).loaded_abspath assert installer_bin - install_args = self._apply_min_version( - install_args or self.get_install_args(bin_name), - min_version, - ) + install_args = install_args or self.get_install_args(bin_name) + if min_version: + install_args = [ + f"{arg}@>={min_version}" + if arg + and not arg.startswith(("-", ".", "/")) + and ":" not in arg.split("/")[0] + and "@" not in arg.split("/")[-1] + else arg + for arg in install_args + ] if postinstall_scripts is None: postinstall_scripts = False - if self._install_args_have_option(install_args, "--ignore-scripts"): + if any(arg == "--ignore-scripts" for arg in install_args): postinstall_scripts = False if min_release_age is None: min_release_age = 7.0 - mutation_args = self._build_mutation_args( - install_args, - postinstall_scripts=postinstall_scripts, - min_release_age=min_release_age, - ) verb = "add" if min_version is not None else "update" - proc = self.exec( - bin_name=installer_bin, - cmd=[verb, *mutation_args, *install_args], - timeout=timeout, - ) + cmd: list[str] = [verb, "--loglevel=error", f"--store-dir={self.cache_dir}"] + if not postinstall_scripts: + cmd.append("--ignore-scripts") + else: + cmd.append("--config.dangerouslyAllowAllBuilds=true") + if min_release_age > 0 and not any( + arg == "--config.minimumReleaseAge" + or arg.startswith("--config.minimumReleaseAge=") + for arg in install_args + ): + cmd.append( + f"--config.minimumReleaseAge={max(int(min_release_age * 24 * 60), 1)}", + ) + cmd.append(f"--dir={self.install_root}" if self.install_root else "--global") + cmd.extend(install_args) + + proc = self.exec(bin_name=installer_bin, cmd=cmd, timeout=timeout) if proc.returncode != 0: self._raise_proc_error("update", install_args, proc) return format_subprocess_output(proc.stdout, proc.stderr)