diff --git a/cloudsmith_cli/cli/commands/credential_helper/manage.py b/cloudsmith_cli/cli/commands/credential_helper/manage.py index 6935d64e..ad980d0e 100644 --- a/cloudsmith_cli/cli/commands/credential_helper/manage.py +++ b/cloudsmith_cli/cli/commands/credential_helper/manage.py @@ -9,6 +9,7 @@ from __future__ import annotations import sys +from pathlib import Path import click @@ -46,6 +47,17 @@ } +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. @@ -149,7 +161,10 @@ def _terraform_next_steps(helper_args: tuple[str, ...]) -> list[str]: @click.command("install") @click.argument("helper") @click.option( - "--bin-dir", default=None, help="Directory to install the launcher binary." + "--bin-dir", + default=None, + help="Directory to install the launcher binary. " + "[default: ~/.cloudsmith/bin, or Terraform's plugin directory]", ) @click.option( "--domain", @@ -236,7 +251,7 @@ def install_cmd( installer = _get_installer(helper) install_kwargs = { - "bin_dir": bin_dir, + "bin_dir": _resolve_bin_dir(helper, bin_dir), "domains": domains, "dry_run": dry_run, "discover": not no_discover, @@ -309,7 +324,10 @@ def install_cmd( @click.command("uninstall") @click.argument("helper") @click.option( - "--bin-dir", default=None, help="Directory where the launcher binary was installed." + "--bin-dir", + default=None, + help="Directory where the launcher binary was installed. " + "[default: ~/.cloudsmith/bin, or Terraform's plugin directory]", ) @click.option( "--dry-run", @@ -337,7 +355,9 @@ def uninstall_cmd(ctx, opts, helper: str, bin_dir: str | None, dry_run: bool) -> """ installer = _get_installer(helper) try: - actions = installer.uninstall(bin_dir=bin_dir, dry_run=dry_run) + actions = installer.uninstall( + bin_dir=_resolve_bin_dir(helper, 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/test_decorators.py b/cloudsmith_cli/cli/tests/test_decorators.py index 14950bce..7e714db2 100644 --- a/cloudsmith_cli/cli/tests/test_decorators.py +++ b/cloudsmith_cli/cli/tests/test_decorators.py @@ -32,7 +32,6 @@ def test_rejected_sso_session_continues_without_early_exception(): def reject(context): context.keyring_refresh_failed = True context.keyring_refresh_rejected = True - return None with patch( "cloudsmith_cli.cli.decorators.CredentialProviderChain.resolve", @@ -50,7 +49,6 @@ def reject(context): def test_auth_commands_skip_automatic_keyring_refresh(command_name): def resolve(context): assert context.skip_keyring_refresh is True - return None with patch( "cloudsmith_cli.cli.decorators.CredentialProviderChain.resolve", diff --git a/cloudsmith_cli/cli/tests/test_org_option.py b/cloudsmith_cli/cli/tests/test_org_option.py index 470d51ef..ce2c9187 100644 --- a/cloudsmith_cli/cli/tests/test_org_option.py +++ b/cloudsmith_cli/cli/tests/test_org_option.py @@ -139,7 +139,6 @@ def test_workspace_environment_takes_precedence(org_reporting_command, monkeypat def test_credential_warning_writer_uses_stderr_without_debug(org_reporting_command): def emit_warning(context): context.warning_writer("OIDC diagnostic") - return None with patch( "cloudsmith_cli.cli.decorators.CredentialProviderChain.resolve", diff --git a/cloudsmith_cli/credential_helpers/cargo/installer.py b/cloudsmith_cli/credential_helpers/cargo/installer.py index 18d7bc50..32893505 100644 --- a/cloudsmith_cli/credential_helpers/cargo/installer.py +++ b/cloudsmith_cli/credential_helpers/cargo/installer.py @@ -27,9 +27,15 @@ from ...core.cache_utils import merge_config_file from ..backends import BackendKind -from ..common import extract_hostname +from ..common import extract_hostname, path_warning from ..custom_domains import get_format_domains -from ..launchers import is_on_path, remove_launcher, resolve_bin_dir, write_launcher +from ..launchers import ( + is_on_path, + is_windows, + remove_launcher, + resolve_bin_dir, + write_launcher, +) if TYPE_CHECKING: from ...core.credentials.models import CredentialResult @@ -347,8 +353,7 @@ def mutate(config: dict) -> None: if not is_on_path(target_dir): actions.append( - f"WARNING: {target_dir} is not on PATH — " - f"add it to your PATH so Cargo can find {self.LAUNCHER_NAME}" + path_warning(target_dir, "Cargo", self.LAUNCHER_NAME, is_windows()) ) return actions diff --git a/cloudsmith_cli/credential_helpers/common.py b/cloudsmith_cli/credential_helpers/common.py index d79bd0ab..bc8dd54c 100644 --- a/cloudsmith_cli/credential_helpers/common.py +++ b/cloudsmith_cli/credential_helpers/common.py @@ -6,12 +6,28 @@ """ import logging +from pathlib import Path from .custom_domains import get_custom_domains, get_format_domains logger = logging.getLogger(__name__) +def path_warning( + target_dir: Path, helper_format: str, launcher: str, windows: bool +) -> str: + # if not windows, assume POSIX + if windows: + hint = f'$env:Path += ";{target_dir}"' + else: + hint = f'export PATH="$PATH:{target_dir}"' + return ( + f"WARNING: {target_dir} is not on PATH\n" + f"add it to your PATH so {helper_format} can find {launcher}:\n\n" + f" {hint}" + ) + + def extract_hostname(url): """ Extract bare hostname from any URL format. diff --git a/cloudsmith_cli/credential_helpers/docker/installer.py b/cloudsmith_cli/credential_helpers/docker/installer.py index e4aa4df7..0382b9a9 100644 --- a/cloudsmith_cli/credential_helpers/docker/installer.py +++ b/cloudsmith_cli/credential_helpers/docker/installer.py @@ -17,8 +17,15 @@ from ...core.cache_utils import merge_config_file from ..backends import BackendKind +from ..common import path_warning from ..custom_domains import get_format_domains -from ..launchers import is_on_path, remove_launcher, resolve_bin_dir, write_launcher +from ..launchers import ( + is_on_path, + is_windows, + remove_launcher, + resolve_bin_dir, + write_launcher, +) if TYPE_CHECKING: from ...core.credentials.models import CredentialResult @@ -226,8 +233,7 @@ def mutate(config: dict) -> None: if not is_on_path(target_dir): actions.append( - f"WARNING: {target_dir} is not on PATH — " - "add it to your PATH so Docker can find docker-credential-cloudsmith" + path_warning(target_dir, "Docker", self.LAUNCHER_NAME, is_windows()) ) return actions diff --git a/cloudsmith_cli/credential_helpers/launchers.py b/cloudsmith_cli/credential_helpers/launchers.py index 2886879e..26fa967b 100644 --- a/cloudsmith_cli/credential_helpers/launchers.py +++ b/cloudsmith_cli/credential_helpers/launchers.py @@ -19,7 +19,7 @@ from pathlib import Path -def _is_windows() -> bool: +def is_windows() -> bool: """Return True when running on Windows.""" return os.name == "nt" @@ -72,7 +72,7 @@ def write_launcher(bin_dir: Path, name: str, target_cmd: str, dry_run=False) -> Path The path of the written file. """ - windows = _is_windows() + windows = is_windows() if dry_run: if windows: return bin_dir / f"{name}.cmd" @@ -106,7 +106,7 @@ def remove_launcher(bin_dir: Path, name: str, dry_run=False) -> bool: bool ``True`` if a file was removed, ``False`` if no file was found. """ - target = Path(bin_dir) / _launcher_filename(name, windows=_is_windows()) + target = Path(bin_dir) / _launcher_filename(name, windows=is_windows()) if target.exists(): if not dry_run: @@ -156,7 +156,7 @@ def resolve_bin_dir(override: str | None = None) -> Path: return candidate.resolve() # Option 3: user-local bin - return _user_bin_dir(_is_windows()).resolve() + return _user_bin_dir(is_windows()).resolve() def is_on_path(directory: Path) -> bool: