diff --git a/CHANGELOG.md b/CHANGELOG.md index 89add60d..9e822701 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ## [Unreleased] +### Fixed + +- `cloudsmith credential-helper list` now finds a launcher that `install` wrote to the default directory, `~/.cloudsmith/bin`. Before, `list` looked in the directory of the `cloudsmith` executable and reported the helper as not installed. + ## [1.29.0] - 2026-10-15 ### Added diff --git a/cloudsmith_cli/cli/commands/credential_helper/manage.py b/cloudsmith_cli/cli/commands/credential_helper/manage.py index ad980d0e..56713e90 100644 --- a/cloudsmith_cli/cli/commands/credential_helper/manage.py +++ b/cloudsmith_cli/cli/commands/credential_helper/manage.py @@ -9,7 +9,6 @@ from __future__ import annotations import sys -from pathlib import Path import click @@ -47,17 +46,6 @@ } -def _resolve_bin_dir(helper: str, bin_dir: str | None) -> str | None: - """Return *bin_dir*, defaulting to ``~/.cloudsmith/bin`` when unset. - - Terraform is left as ``None`` so its installer uses Terraform's plugin - directory (Terraform does not search PATH for credentials helpers). - """ - if bin_dir is not None or helper == "terraform": - return bin_dir - return str(Path.home() / ".cloudsmith" / "bin") - - def _get_installer(name: str): """Return an instantiated installer for *name*, or exit with a clear error. @@ -251,7 +239,7 @@ def install_cmd( installer = _get_installer(helper) install_kwargs = { - "bin_dir": _resolve_bin_dir(helper, bin_dir), + "bin_dir": bin_dir, "domains": domains, "dry_run": dry_run, "discover": not no_discover, @@ -355,9 +343,7 @@ def uninstall_cmd(ctx, opts, helper: str, bin_dir: str | None, dry_run: bool) -> """ installer = _get_installer(helper) try: - actions = installer.uninstall( - bin_dir=_resolve_bin_dir(helper, bin_dir), dry_run=dry_run - ) + actions = installer.uninstall(bin_dir=bin_dir, dry_run=dry_run) except OSError as exc: raise click.ClickException( f"Failed to uninstall {helper!r} credential helper: {exc}" diff --git a/cloudsmith_cli/cli/tests/commands/test_credential_helper_install.py b/cloudsmith_cli/cli/tests/commands/test_credential_helper_install.py index 9723032d..d6815891 100644 --- a/cloudsmith_cli/cli/tests/commands/test_credential_helper_install.py +++ b/cloudsmith_cli/cli/tests/commands/test_credential_helper_install.py @@ -23,7 +23,6 @@ from ....credential_helpers.launchers import ( _launcher_content, _launcher_filename, - _user_bin_dir, is_on_path, remove_launcher, resolve_bin_dir, @@ -209,23 +208,33 @@ def test_resolve_bin_dir_override_relative_current_dir(tmp_path, monkeypatch): assert result == expected -@pytest.mark.parametrize( - "windows,expected_suffix", - [ - (False, (".local", "bin")), - (True, ("Cloudsmith", "bin")), - ], -) -def test_user_bin_dir(tmp_path, monkeypatch, windows, expected_suffix): - """_user_bin_dir returns the per-platform user bin location. +def test_resolve_bin_dir_defaults_to_cloudsmith_bin(tmp_path, monkeypatch): + """Without an override the launcher goes to ``~/.cloudsmith/bin``.""" + monkeypatch.setattr(Path, "home", staticmethod(lambda: tmp_path)) + assert resolve_bin_dir() == tmp_path / ".cloudsmith" / "bin" + + +def test_list_finds_a_launcher_installed_to_the_default_dir( + runner, tmp_path, monkeypatch +): + """`list` looks for launchers where `install` puts them by default.""" + from ....cli.commands.credential_helper.manage import install_cmd, list_cmd - Parameterised on ``windows`` (not ``os.name``) to avoid building a - ``WindowsPath`` on a posix host. - """ monkeypatch.setattr(Path, "home", staticmethod(lambda: tmp_path)) - monkeypatch.setenv("LOCALAPPDATA", str(tmp_path)) - result = _user_bin_dir(windows) - assert result.parts[-2:] == expected_suffix + monkeypatch.setenv("DOCKER_CONFIG", str(tmp_path / ".docker")) + result = runner.invoke( + install_cmd, ["docker", "--no-discover"], catch_exceptions=False + ) + assert result.exit_code == 0, result.output + + result = runner.invoke(list_cmd, ["-F", "json"], catch_exceptions=False) + + docker = next( + e for e in json.loads(result.stdout)["data"] if e["helper"] == "docker" + ) + assert docker["launcher"] == str( + tmp_path / ".cloudsmith" / "bin" / "docker-credential-cloudsmith" + ) # --------------------------------------------------------------------------- diff --git a/cloudsmith_cli/credential_helpers/launchers.py b/cloudsmith_cli/credential_helpers/launchers.py index 26fa967b..7ba168ee 100644 --- a/cloudsmith_cli/credential_helpers/launchers.py +++ b/cloudsmith_cli/credential_helpers/launchers.py @@ -5,7 +5,7 @@ ``docker-credential-cloudsmith`` (or similar) that forwards every call to the single ``cloudsmith`` binary already installed on the user's PATH. -The platform-specific bits (file name, script body, user bin directory) live in +The platform-specific bits (file name, script body) live in small pure helpers parameterised on ``windows`` so they can be unit-tested without monkeypatching ``os.name`` — faking ``os.name`` on a posix host makes ``pathlib.Path`` raise ``NotImplementedError`` on Python < 3.12. @@ -14,8 +14,6 @@ from __future__ import annotations import os -import shutil -import sys from pathlib import Path @@ -41,19 +39,6 @@ def _launcher_content(target_cmd: str, *, windows: bool) -> str: return f'#!/bin/sh\nexec {target_cmd} "$@"\n' -def _user_bin_dir(windows: bool) -> Path: - """Return the user-local bin directory for the platform. - - Unix → ``~/.local/bin``; Windows → ``%LOCALAPPDATA%\\Cloudsmith\\bin`` - (falling back to the home directory when ``LOCALAPPDATA`` is unset). - """ - if windows: - localappdata = os.environ.get("LOCALAPPDATA") - base = Path(localappdata) if localappdata else Path.home() - return base / "Cloudsmith" / "bin" - return Path.home() / ".local" / "bin" - - def write_launcher(bin_dir: Path, name: str, target_cmd: str, dry_run=False) -> Path: """Write a launcher script for *name* in *bin_dir* that execs *target_cmd*. @@ -122,9 +107,7 @@ def resolve_bin_dir(override: str | None = None) -> Path: ---------------- 1. *override* → resolved to an absolute path (relative paths are resolved against the current working directory). - 2. The directory of the running ``cloudsmith`` executable — if that - directory is writable. - 3. The user-local bin directory (see :func:`_user_bin_dir`). + 2. ``~/.cloudsmith/bin``, which a package manager upgrade never replaces. The chosen directory is **not** created here; that happens when the launcher is written via :func:`write_launcher`. @@ -145,18 +128,7 @@ def resolve_bin_dir(override: str | None = None) -> Path: if override is not None: return Path(override).resolve() - # Option 2: beside the running cloudsmith binary (if writable) - cloudsmith_path = shutil.which("cloudsmith") - if cloudsmith_path: - candidate = Path(os.path.dirname(os.path.realpath(cloudsmith_path))) - else: - candidate = Path(os.path.dirname(os.path.realpath(sys.argv[0]))) - - if os.access(candidate, os.W_OK | os.X_OK): - return candidate.resolve() - - # Option 3: user-local bin - return _user_bin_dir(is_windows()).resolve() + return (Path.home() / ".cloudsmith" / "bin").resolve() def is_on_path(directory: Path) -> bool: