From 3919c340dbd10e09ab0d9f0d12fe1e1137ea0589 Mon Sep 17 00:00:00 2001 From: Ian Duffy Date: Thu, 22 Oct 2026 17:46:25 +0100 Subject: [PATCH 01/27] feat(no-ticket): install the Cloudsmith Maven tools for Maven, Gradle, and sbt `credential-helper install maven|gradle|sbt` sets up the Maven extension and the Gradle and sbt plugins, which get repository credentials from `cloudsmith credential-helper generic`. Each install takes the newest release from the public cloudsmith/maven-tools repository. - maven: puts the extension in ~/.m2/repository and adds it to .mvn/extensions.xml of the project in the current directory. Maven 3.9 has no user-wide extensions, and loads a build extension only from Maven Central or the local repository. - gradle: writes an init script to ~/.gradle/init.d that applies the plugin to every build. - sbt: writes a global plugin to ~/.sbt/1.0/plugins for every sbt 1 build. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 7 + .../cli/commands/credential_helper/manage.py | 12 + .../test_credential_helper_maven_tools.py | 411 ++++++++++++++++++ .../credential_helpers/gradle/__init__.py | 2 + .../credential_helpers/gradle/installer.py | 39 ++ .../credential_helpers/maven/__init__.py | 2 + .../credential_helpers/maven/extensions.py | 86 ++++ .../credential_helpers/maven/installer.py | 107 +++++ .../credential_helpers/maven_tools.py | 86 ++++ .../credential_helpers/sbt/__init__.py | 2 + .../credential_helpers/sbt/installer.py | 26 ++ 11 files changed, 780 insertions(+) create mode 100644 cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py create mode 100644 cloudsmith_cli/credential_helpers/gradle/__init__.py create mode 100644 cloudsmith_cli/credential_helpers/gradle/installer.py create mode 100644 cloudsmith_cli/credential_helpers/maven/__init__.py create mode 100644 cloudsmith_cli/credential_helpers/maven/extensions.py create mode 100644 cloudsmith_cli/credential_helpers/maven/installer.py create mode 100644 cloudsmith_cli/credential_helpers/maven_tools.py create mode 100644 cloudsmith_cli/credential_helpers/sbt/__init__.py create mode 100644 cloudsmith_cli/credential_helpers/sbt/installer.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 02b6919f..c47c7ad4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,13 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ## [Unreleased] +### Added + +- Added `cloudsmith credential-helper install maven`, `install gradle`, and `install sbt`. They set up the Cloudsmith Maven tools, which get repository credentials from `cloudsmith credential-helper generic`. Each command installs the newest release from the public `cloudsmith/maven-tools` repository. + - `maven` puts the extension in `~/.m2/repository` and adds it to `.mvn/extensions.xml` of the Maven project in the current directory. + - `gradle` writes an init script to `~/.gradle/init.d` (or `$GRADLE_USER_HOME/init.d`), which applies the plugin to every Gradle build. + - `sbt` writes a global plugin to `~/.sbt/1.0/plugins`, which adds the plugin to every sbt 1 build. + ## [1.30.0] - 2026-10-22 ### Added diff --git a/cloudsmith_cli/cli/commands/credential_helper/manage.py b/cloudsmith_cli/cli/commands/credential_helper/manage.py index da5d690c..f6b5da1d 100644 --- a/cloudsmith_cli/cli/commands/credential_helper/manage.py +++ b/cloudsmith_cli/cli/commands/credential_helper/manage.py @@ -15,7 +15,14 @@ from cloudsmith_cli.credential_helpers.cargo.installer import CargoInstaller from cloudsmith_cli.credential_helpers.generic import PartialInstallError +from cloudsmith_cli.credential_helpers.gradle.installer import GradleInstaller +from cloudsmith_cli.credential_helpers.maven.extensions import ExtensionsFileError +from cloudsmith_cli.credential_helpers.maven.installer import ( + MavenInstaller, + MavenProjectNotFound, +) from cloudsmith_cli.credential_helpers.pnpm.installer import PNPMInstaller +from cloudsmith_cli.credential_helpers.sbt.installer import SbtInstaller from cloudsmith_cli.credential_helpers.terraform.installer import ( TerraformHelperExeNotFound, TerraformInstaller, @@ -44,6 +51,9 @@ "pnpm": PNPMInstaller, "cargo": CargoInstaller, "terraform": TerraformInstaller, + "maven": MavenInstaller, + "gradle": GradleInstaller, + "sbt": SbtInstaller, } @@ -316,6 +326,8 @@ def install_cmd( TerraformrcConflictError, TerraformHelperExeNotFound, TerraformPluginDirError, + MavenProjectNotFound, + ExtensionsFileError, ) as exc: raise click.ClickException(str(exc)) except OSError as exc: diff --git a/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py b/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py new file mode 100644 index 00000000..631b6dcf --- /dev/null +++ b/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py @@ -0,0 +1,411 @@ +# Copyright 2026 Cloudsmith Ltd +"""Tests for the Maven, Gradle, and sbt installers of the Cloudsmith Maven tools. + +The installers configure each build tool to load the Cloudsmith extension or +plugin, which gets its credential from `cloudsmith credential-helper generic`. +""" + +from __future__ import annotations + +from pathlib import Path + +import click.testing +import httpretty +import httpretty.core +import pytest + +from ...credential_helpers import maven_tools +from ...credential_helpers.gradle.installer import GradleInstaller +from ...credential_helpers.maven import extensions +from ...credential_helpers.maven.installer import MavenInstaller, MavenProjectNotFound +from ...credential_helpers.sbt.installer import SbtInstaller + +EXTENSION_PATH = "io/cloudsmith/cloudsmith-maven-extension" +METADATA = """ + + io.cloudsmith + cloudsmith-maven-extension + + 0.3.0-SNAPSHOT + 0.2.0 + 0.1.00.2.0 + + +""" +OUR_EXTENSION = """ + io.cloudsmith + cloudsmith-maven-extension + 0.1.0 + +""" +OTHER_EXTENSION = """ + kr.motd.maven + os-maven-plugin + 1.7.1 + +""" + + +@pytest.fixture(autouse=True) +def _fake_socket_shutdown(monkeypatch): + monkeypatch.setattr( + httpretty.core.fakesock.socket, + "shutdown", + lambda self, how: None, + raising=False, + ) + + +@pytest.fixture() +def home(tmp_path, monkeypatch): + """Point the home directory at an empty directory.""" + home_dir = tmp_path / "home" + home_dir.mkdir() + monkeypatch.setattr(Path, "home", staticmethod(lambda: home_dir)) + monkeypatch.delenv("GRADLE_USER_HOME", raising=False) + return home_dir + + +@pytest.fixture() +def fake_repository(monkeypatch): + """Serve version 0.2.0 of every artifact without network access.""" + downloads = [] + + def fake_download(artifact_path, destination): + downloads.append(artifact_path) + destination.parent.mkdir(parents=True, exist_ok=True) + destination.write_text(f"content of {artifact_path}", encoding="utf-8") + + monkeypatch.setattr(maven_tools, "latest_version", lambda artifact_path: "0.2.0") + monkeypatch.setattr(maven_tools, "download", fake_download) + return downloads + + +@pytest.fixture() +def maven_project(tmp_path, monkeypatch): + """Make a Maven project and run the test from its root directory.""" + project = tmp_path / "project" + project.mkdir() + (project / "pom.xml").write_text("", encoding="utf-8") + monkeypatch.chdir(project) + return project + + +# --------------------------------------------------------------------------- +# 1. The public maven-tools repository +# --------------------------------------------------------------------------- + + +@httpretty.activate(allow_net_connect=False) +def test_latest_version_reads_the_release_from_the_metadata(): + """The release version is used, not a newer snapshot in .""" + httpretty.register_uri( + httpretty.GET, + f"{maven_tools.REPOSITORY_URL}{EXTENSION_PATH}/maven-metadata.xml", + body=METADATA, + ) + + assert maven_tools.latest_version(EXTENSION_PATH) == "0.2.0" + + +@httpretty.activate(allow_net_connect=False) +def test_latest_version_fails_with_an_os_error_when_the_artifact_is_missing(): + """A missing artifact is an OSError, which the install command reports.""" + httpretty.register_uri( + httpretty.GET, + f"{maven_tools.REPOSITORY_URL}{EXTENSION_PATH}/maven-metadata.xml", + status=404, + ) + + with pytest.raises(OSError): + maven_tools.latest_version(EXTENSION_PATH) + + +@httpretty.activate(allow_net_connect=False) +def test_download_writes_the_file(tmp_path): + """The file is written to the destination, and missing directories are made.""" + httpretty.register_uri( + httpretty.GET, + f"{maven_tools.REPOSITORY_URL}{EXTENSION_PATH}/0.2.0/extension.jar", + body=b"jar bytes", + ) + destination = tmp_path / "a" / "b" / "extension.jar" + + maven_tools.download(f"{EXTENSION_PATH}/0.2.0/extension.jar", destination) + + assert destination.read_bytes() == b"jar bytes" + + +# --------------------------------------------------------------------------- +# 2. .mvn/extensions.xml +# --------------------------------------------------------------------------- + + +def test_add_extension_makes_a_new_file(): + """Without a file, the result lists only the Cloudsmith extension.""" + text, changed = extensions.add_extension(None, "0.1.0") + + assert changed is True + assert text == f"\n{OUR_EXTENSION}\n" + + +def test_add_extension_keeps_other_extensions(): + """The Cloudsmith extension is added after the extensions already listed.""" + existing = f'\n\n{OTHER_EXTENSION}\n' + + text, changed = extensions.add_extension(existing, "0.1.0") + + assert changed is True + assert text == ( + f'\n\n{OTHER_EXTENSION}{OUR_EXTENSION}' + "\n" + ) + + +def test_add_extension_to_an_empty_element(): + """A self-closing element is opened to hold the extension.""" + text, changed = extensions.add_extension("\n", "0.1.0") + + assert changed is True + assert text == f"\n{OUR_EXTENSION}\n" + + +def test_add_extension_updates_the_version(): + """An older version of the Cloudsmith extension is changed in place.""" + existing = f"\n{OUR_EXTENSION}{OTHER_EXTENSION}\n" + + text, changed = extensions.add_extension(existing, "0.2.0") + + assert changed is True + assert text == existing.replace( + "0.1.0", "0.2.0" + ) + + +def test_add_extension_with_the_same_version_changes_nothing(): + """Installing the same version again leaves the file as it is.""" + existing = f"\n{OUR_EXTENSION}\n" + + assert extensions.add_extension(existing, "0.1.0") == (existing, False) + + +def test_remove_extension_keeps_other_extensions(): + """Only the Cloudsmith extension is removed.""" + existing = f"\n{OTHER_EXTENSION}{OUR_EXTENSION}\n" + + text, changed = extensions.remove_extension(existing) + + assert changed is True + assert text == f"\n{OTHER_EXTENSION}\n" + + +def test_remove_the_only_extension_returns_no_text(): + """When nothing else is listed, the file is not needed.""" + assert extensions.remove_extension( + f"\n{OUR_EXTENSION}\n" + ) == (None, True) + + +def test_remove_extension_without_the_extension_changes_nothing(): + """A file without the Cloudsmith extension stays as it is.""" + existing = f"\n{OTHER_EXTENSION}\n" + + assert extensions.remove_extension(existing) == (existing, False) + + +# --------------------------------------------------------------------------- +# 3. MavenInstaller +# --------------------------------------------------------------------------- + + +def _local_extension_dir(home: Path, version: str) -> Path: + return home / ".m2" / "repository" / EXTENSION_PATH / version + + +def test_maven_install_puts_the_extension_in_the_local_repository( + home, fake_repository, maven_project +): + """Maven loads build extensions only from Maven Central or the local repository.""" + actions = MavenInstaller().install() + + version_dir = _local_extension_dir(home, "0.2.0") + assert (version_dir / "cloudsmith-maven-extension-0.2.0.jar").exists() + assert (version_dir / "cloudsmith-maven-extension-0.2.0.pom").exists() + assert any(str(version_dir) in action for action in actions) + + +def test_maven_install_turns_on_the_extension_in_the_project( + home, fake_repository, maven_project +): + """The project's .mvn/extensions.xml lists the installed version.""" + actions = MavenInstaller().install() + + extensions_file = maven_project / ".mvn" / "extensions.xml" + assert "0.2.0" in extensions_file.read_text(encoding="utf-8") + assert any(str(extensions_file) in action for action in actions) + + +def test_maven_install_from_a_module_uses_the_root_project( + home, fake_repository, maven_project, monkeypatch +): + """Maven reads .mvn from the nearest directory above that has one.""" + (maven_project / ".mvn").mkdir() + module = maven_project / "module" + module.mkdir() + (module / "pom.xml").write_text("", encoding="utf-8") + monkeypatch.chdir(module) + + MavenInstaller().install() + + assert (maven_project / ".mvn" / "extensions.xml").exists() + assert not (module / ".mvn").exists() + + +def test_maven_install_outside_a_project_fails( + home, fake_repository, tmp_path, monkeypatch +): + """Without a pom.xml, the install stops before it changes anything.""" + monkeypatch.chdir(tmp_path) + + with pytest.raises(MavenProjectNotFound): + MavenInstaller().install() + + assert not (tmp_path / ".mvn").exists() + assert not fake_repository + + +def test_maven_install_dry_run_changes_nothing(home, fake_repository, maven_project): + """A dry run reports the planned changes and writes no files.""" + actions = MavenInstaller().install(dry_run=True) + + assert not (maven_project / ".mvn").exists() + assert not (home / ".m2").exists() + assert all(action.startswith("would ") for action in actions) + + +def test_maven_uninstall_removes_the_extension_from_the_project( + home, fake_repository, maven_project +): + """Uninstall removes the file it made, and keeps the shared local repository.""" + MavenInstaller().install() + + MavenInstaller().uninstall() + + assert not (maven_project / ".mvn" / "extensions.xml").exists() + assert _local_extension_dir(home, "0.2.0").exists() + + +def test_maven_status_names_the_project_file(home, fake_repository, maven_project): + """list shows the extensions.xml of the current project when it is set up.""" + assert MavenInstaller().status() == {"launcher": None, "hosts": []} + + MavenInstaller().install() + + extensions_file = maven_project / ".mvn" / "extensions.xml" + assert MavenInstaller().status() == {"launcher": str(extensions_file), "hosts": []} + + +# --------------------------------------------------------------------------- +# 4. GradleInstaller +# --------------------------------------------------------------------------- + + +def test_gradle_install_writes_an_init_script(home, fake_repository): + """The init script applies the plugin to the settings of every build.""" + actions = GradleInstaller().install() + + script = home / ".gradle" / "init.d" / "cloudsmith.init.gradle.kts" + text = script.read_text(encoding="utf-8") + assert f'maven {{ url = uri("{maven_tools.REPOSITORY_URL}") }}' in text + assert 'classpath("io.cloudsmith:cloudsmith-gradle-plugin:0.2.0")' in text + assert "pluginManager.apply(io.cloudsmith.gradle.CloudsmithPlugin::class.java)" in ( + text + ) + assert any(str(script) in action for action in actions) + + +def test_gradle_install_uses_gradle_user_home( + home, fake_repository, tmp_path, monkeypatch +): + """GRADLE_USER_HOME moves the init.d directory.""" + monkeypatch.setenv("GRADLE_USER_HOME", str(tmp_path / "gradle-home")) + + GradleInstaller().install() + + assert (tmp_path / "gradle-home" / "init.d" / "cloudsmith.init.gradle.kts").exists() + + +def test_gradle_uninstall_and_status(home, fake_repository): + """Uninstall removes the init script, and status follows it.""" + script = home / ".gradle" / "init.d" / "cloudsmith.init.gradle.kts" + GradleInstaller().install() + assert GradleInstaller().status() == {"launcher": str(script), "hosts": []} + + GradleInstaller().uninstall() + + assert not script.exists() + assert GradleInstaller().status() == {"launcher": None, "hosts": []} + + +# --------------------------------------------------------------------------- +# 5. SbtInstaller +# --------------------------------------------------------------------------- + + +def test_sbt_install_writes_a_global_plugin(home, fake_repository): + """sbt 1 loads global plugins for every build from ~/.sbt/1.0/plugins.""" + actions = SbtInstaller().install() + + plugin_file = home / ".sbt" / "1.0" / "plugins" / "cloudsmith.sbt" + text = plugin_file.read_text(encoding="utf-8") + assert ( + f'resolvers += "cloudsmith-maven-tools" at "{maven_tools.REPOSITORY_URL}"' + in (text) + ) + assert 'addSbtPlugin("io.cloudsmith" % "sbt-cloudsmith" % "0.2.0")' in text + assert any(str(plugin_file) in action for action in actions) + + +def test_sbt_uninstall_and_status(home, fake_repository): + """Uninstall removes the global plugin, and status follows it.""" + plugin_file = home / ".sbt" / "1.0" / "plugins" / "cloudsmith.sbt" + SbtInstaller().install() + assert SbtInstaller().status() == {"launcher": str(plugin_file), "hosts": []} + + SbtInstaller().uninstall() + + assert not plugin_file.exists() + assert SbtInstaller().status() == {"launcher": None, "hosts": []} + + +# --------------------------------------------------------------------------- +# 6. CLI wiring +# --------------------------------------------------------------------------- + + +@pytest.mark.parametrize("helper", ["maven", "gradle", "sbt"]) +def test_cli_install_runs_the_installer(helper, home, fake_repository, maven_project): + """`credential-helper install ` writes the config without credentials.""" + from ...cli.commands.credential_helper.manage import install_cmd + + result = click.testing.CliRunner().invoke( + install_cmd, [helper, "--no-discover"], catch_exceptions=False + ) + + assert result.exit_code == 0, result.output + assert "0.2.0" in result.output + + +def test_cli_install_maven_outside_a_project_fails_cleanly( + home, fake_repository, tmp_path, monkeypatch +): + """The error names the directory and the fix, without a traceback.""" + from ...cli.commands.credential_helper.manage import install_cmd + + monkeypatch.chdir(tmp_path) + result = click.testing.CliRunner().invoke( + install_cmd, ["maven", "--no-discover"], catch_exceptions=False + ) + + assert result.exit_code != 0 + assert "pom.xml" in result.output diff --git a/cloudsmith_cli/credential_helpers/gradle/__init__.py b/cloudsmith_cli/credential_helpers/gradle/__init__.py new file mode 100644 index 00000000..b84b292e --- /dev/null +++ b/cloudsmith_cli/credential_helpers/gradle/__init__.py @@ -0,0 +1,2 @@ +# Copyright 2026 Cloudsmith Ltd +"""Installer for the Cloudsmith Gradle plugin.""" diff --git a/cloudsmith_cli/credential_helpers/gradle/installer.py b/cloudsmith_cli/credential_helpers/gradle/installer.py new file mode 100644 index 00000000..ec362119 --- /dev/null +++ b/cloudsmith_cli/credential_helpers/gradle/installer.py @@ -0,0 +1,39 @@ +# Copyright 2026 Cloudsmith Ltd +"""Apply the Cloudsmith Gradle plugin to every Gradle build of the user.""" + +from __future__ import annotations + +import os +from pathlib import Path + +from ..maven_tools import REPOSITORY_URL, ConfigFileInstaller + +INIT_SCRIPT = """\ +// Written by `cloudsmith credential-helper install gradle`. +initscript {{ + repositories {{ + maven {{ url = uri("{repository_url}") }} + }} + dependencies {{ + classpath("io.cloudsmith:cloudsmith-gradle-plugin:{version}") + }} +}} + +beforeSettings {{ + pluginManager.apply(io.cloudsmith.gradle.CloudsmithPlugin::class.java) +}} +""" + + +class GradleInstaller(ConfigFileInstaller): + name = "gradle" + summary = "Gradle plugin that gets credentials from the Cloudsmith CLI" + artifact_path = "io/cloudsmith/cloudsmith-gradle-plugin" + + def path(self) -> Path: + gradle_user_home = os.environ.get("GRADLE_USER_HOME") + base = Path(gradle_user_home) if gradle_user_home else Path.home() / ".gradle" + return base / "init.d" / "cloudsmith.init.gradle.kts" + + def render(self, version: str) -> str: + return INIT_SCRIPT.format(repository_url=REPOSITORY_URL, version=version) diff --git a/cloudsmith_cli/credential_helpers/maven/__init__.py b/cloudsmith_cli/credential_helpers/maven/__init__.py new file mode 100644 index 00000000..e00ccd62 --- /dev/null +++ b/cloudsmith_cli/credential_helpers/maven/__init__.py @@ -0,0 +1,2 @@ +# Copyright 2026 Cloudsmith Ltd +"""Installer for the Cloudsmith Maven extension.""" diff --git a/cloudsmith_cli/credential_helpers/maven/extensions.py b/cloudsmith_cli/credential_helpers/maven/extensions.py new file mode 100644 index 00000000..c3ce1f31 --- /dev/null +++ b/cloudsmith_cli/credential_helpers/maven/extensions.py @@ -0,0 +1,86 @@ +# Copyright 2026 Cloudsmith Ltd +"""Add and remove the Cloudsmith extension in a Maven ``.mvn/extensions.xml`` file. + +The functions edit the text, so the rest of the file keeps its formatting. +""" + +from __future__ import annotations + +import re + +GROUP_ID = "io.cloudsmith" +ARTIFACT_ID = "cloudsmith-maven-extension" + +_EXTENSION_BLOCK = re.compile(r"[ \t]*.*?[ \t]*\n?", re.DOTALL) +_EMPTY_EXTENSIONS = re.compile(r"]*?)\s*/>") +_VERSION = re.compile(r"[^<]*") + + +class ExtensionsFileError(Exception): + """The extensions.xml file has no element to add to.""" + + +def _render(version: str) -> str: + return ( + " \n" + f" {GROUP_ID}\n" + f" {ARTIFACT_ID}\n" + f" {version}\n" + " \n" + ) + + +def _find_extension(text: str) -> re.Match | None: + for block in _EXTENSION_BLOCK.finditer(text): + content = block.group(0) + if f"{GROUP_ID}" in content and ( + f"{ARTIFACT_ID}" in content + ): + return block + return None + + +def has_extension(text: str) -> bool: + """Return whether *text* lists the Cloudsmith extension.""" + return _find_extension(text) is not None + + +def add_extension(text: str | None, version: str) -> tuple[str, bool]: + """Return *text* with the Cloudsmith extension at *version*, and whether it changed. + + A *text* of ``None`` means that there is no file yet. + """ + if text is None: + return f"\n{_render(version)}\n", True + block = _find_extension(text) + if block: + updated = _VERSION.sub(f"{version}", block.group(0), count=1) + new_text = text[: block.start()] + updated + text[block.end() :] + return new_text, new_text != text + if _EMPTY_EXTENSIONS.search(text): + new_text = _EMPTY_EXTENSIONS.sub( + lambda empty: ( + f"\n{_render(version)}" + ), + text, + count=1, + ) + return new_text, True + closing = text.rfind("") + if closing == -1: + raise ExtensionsFileError("The file has no element.") + return text[:closing] + _render(version) + text[closing:], True + + +def remove_extension(text: str) -> tuple[str | None, bool]: + """Return *text* without the Cloudsmith extension, and whether it changed. + + The text is ``None`` when no other extension is left, so the file is not needed. + """ + block = _find_extension(text) + if not block: + return text, False + new_text = text[: block.start()] + text[block.end() :] + if not _EXTENSION_BLOCK.search(new_text): + return None, True + return new_text, True diff --git a/cloudsmith_cli/credential_helpers/maven/installer.py b/cloudsmith_cli/credential_helpers/maven/installer.py new file mode 100644 index 00000000..ca604837 --- /dev/null +++ b/cloudsmith_cli/credential_helpers/maven/installer.py @@ -0,0 +1,107 @@ +# Copyright 2026 Cloudsmith Ltd +"""Turn on the Cloudsmith Maven extension in the Maven project of the current directory. + +Maven 3.9 has no user-wide extensions, and Maven loads a build extension only +from Maven Central or the local repository. So the installer puts the extension +in the local repository and lists it in the project's ``.mvn/extensions.xml``. +""" + +from __future__ import annotations + +from pathlib import Path + +from .. import maven_tools +from . import extensions + +EXTENSION_PATH = f"io/cloudsmith/{extensions.ARTIFACT_ID}" + + +class MavenProjectNotFound(Exception): + """The current directory is not in a Maven project.""" + + +def _project_dir() -> Path: + """Return the directory where Maven reads ``.mvn``: the nearest one that has it.""" + cwd = Path.cwd() + for directory in (cwd, *cwd.parents): + if (directory / ".mvn").is_dir(): + return directory + return cwd + + +def _extensions_file() -> Path: + return _project_dir() / ".mvn" / "extensions.xml" + + +def _read(path: Path) -> str | None: + return path.read_text(encoding="utf-8") if path.exists() else None + + +class MavenInstaller: + name = "maven" + summary = "Maven extension that gets credentials from the Cloudsmith CLI" + + def install(self, *, dry_run: bool = False, **_options) -> list[str]: + """Put the extension in ``~/.m2/repository`` and list it in ``.mvn/extensions.xml``.""" + project_dir = _project_dir() + if not (project_dir / "pom.xml").is_file(): + raise MavenProjectNotFound( + f"No pom.xml in {project_dir}. " + "Run the command in the root directory of a Maven project." + ) + version = maven_tools.latest_version(EXTENSION_PATH) + return self._download(version, dry_run) + self._turn_on(version, dry_run) + + def _download(self, version: str, dry_run: bool) -> list[str]: + version_dir = Path.home() / ".m2" / "repository" / EXTENSION_PATH / version + actions = [] + for file_type in ("jar", "pom"): + file_name = f"{extensions.ARTIFACT_ID}-{version}.{file_type}" + destination = version_dir / file_name + if destination.exists(): + actions.append(f"{file_name} already in {version_dir} (no change)") + elif dry_run: + actions.append(f"would download {file_name} to {version_dir}") + else: + maven_tools.download( + f"{EXTENSION_PATH}/{version}/{file_name}", destination + ) + actions.append(f"downloaded {file_name} to {version_dir}") + return actions + + def _turn_on(self, version: str, dry_run: bool) -> list[str]: + path = _extensions_file() + text, changed = extensions.add_extension(_read(path), version) + if not changed: + return [f"{path} already lists version {version} (no change)"] + if dry_run: + return [f"would add version {version} to {path}"] + path.parent.mkdir(exist_ok=True) + path.write_text(text, encoding="utf-8") + return [f"added version {version} to {path}"] + + def uninstall(self, *, dry_run: bool = False, **_options) -> list[str]: + """Remove the extension from ``.mvn/extensions.xml``. + + The local repository is shared with other projects, so it stays as it is. + """ + path = _extensions_file() + current = _read(path) + if current is None: + return [f"{path} not found (nothing to remove)"] + text, changed = extensions.remove_extension(current) + if not changed: + return [f"{path} does not list the extension (nothing to remove)"] + if dry_run: + return [f"would remove the extension from {path}"] + if text is None: + path.unlink() + return [f"removed {path}"] + path.write_text(text, encoding="utf-8") + return [f"removed the extension from {path}"] + + def status(self) -> dict: + path = _extensions_file() + current = _read(path) + configured = current is not None and extensions.has_extension(current) + return {"launcher": str(path) if configured else None, "hosts": []} diff --git a/cloudsmith_cli/credential_helpers/maven_tools.py b/cloudsmith_cli/credential_helpers/maven_tools.py new file mode 100644 index 00000000..7e199a7b --- /dev/null +++ b/cloudsmith_cli/credential_helpers/maven_tools.py @@ -0,0 +1,86 @@ +# Copyright 2026 Cloudsmith Ltd +""" +Shared code for the installers of the Cloudsmith Maven tools. + +The Maven extension and the Gradle and sbt plugins get their credential from +`cloudsmith credential-helper generic`. They are published to a public +Cloudsmith repository. +""" + +from __future__ import annotations + +from typing import TYPE_CHECKING +from xml.etree import ElementTree + +from ..core.session import create_requests_session + +if TYPE_CHECKING: + from pathlib import Path + +REPOSITORY_URL = "https://dl.cloudsmith.io/public/cloudsmith/maven-tools/maven/" +TIMEOUT_SECONDS = 30 + + +def _get(artifact_path: str) -> bytes: + response = create_requests_session().get( + REPOSITORY_URL + artifact_path, timeout=TIMEOUT_SECONDS + ) + response.raise_for_status() + return response.content + + +def latest_version(artifact_path: str) -> str: + """Return the newest release of the artifact at *artifact_path*.""" + metadata = ElementTree.fromstring(_get(f"{artifact_path}/maven-metadata.xml")) + release = metadata.findtext("versioning/release") + if not release: + raise OSError(f"{REPOSITORY_URL}{artifact_path} has no release") + return release + + +def download(artifact_path: str, destination: Path) -> None: + """Download the file at *artifact_path* to *destination*.""" + content = _get(artifact_path) + destination.parent.mkdir(parents=True, exist_ok=True) + destination.write_bytes(content) + + +class ConfigFileInstaller: + """Install a plugin with one config file that names its latest version.""" + + name = "" + summary = "" + artifact_path = "" + + def path(self) -> Path: + """Return the config file that loads the plugin.""" + raise NotImplementedError + + def render(self, version: str) -> str: + """Return the content of the config file for *version*.""" + raise NotImplementedError + + def install(self, *, dry_run: bool = False, **_options) -> list[str]: + version = latest_version(self.artifact_path) + path = self.path() + text = self.render(version) + if path.exists() and path.read_text(encoding="utf-8") == text: + return [f"{path} already uses version {version} (no change)"] + if dry_run: + return [f"would write {path} for version {version}"] + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(text, encoding="utf-8") + return [f"wrote {path} for version {version}"] + + def uninstall(self, *, dry_run: bool = False, **_options) -> list[str]: + path = self.path() + if not path.exists(): + return [f"{path} not found (nothing to remove)"] + if dry_run: + return [f"would remove {path}"] + path.unlink() + return [f"removed {path}"] + + def status(self) -> dict: + path = self.path() + return {"launcher": str(path) if path.exists() else None, "hosts": []} diff --git a/cloudsmith_cli/credential_helpers/sbt/__init__.py b/cloudsmith_cli/credential_helpers/sbt/__init__.py new file mode 100644 index 00000000..77e03164 --- /dev/null +++ b/cloudsmith_cli/credential_helpers/sbt/__init__.py @@ -0,0 +1,2 @@ +# Copyright 2026 Cloudsmith Ltd +"""Installer for the Cloudsmith sbt plugin.""" diff --git a/cloudsmith_cli/credential_helpers/sbt/installer.py b/cloudsmith_cli/credential_helpers/sbt/installer.py new file mode 100644 index 00000000..ade02ae0 --- /dev/null +++ b/cloudsmith_cli/credential_helpers/sbt/installer.py @@ -0,0 +1,26 @@ +# Copyright 2026 Cloudsmith Ltd +"""Add the Cloudsmith sbt plugin to every sbt 1 build of the user.""" + +from __future__ import annotations + +from pathlib import Path + +from ..maven_tools import REPOSITORY_URL, ConfigFileInstaller + +GLOBAL_PLUGIN = """\ +// Written by `cloudsmith credential-helper install sbt`. +resolvers += "cloudsmith-maven-tools" at "{repository_url}" +addSbtPlugin("io.cloudsmith" % "sbt-cloudsmith" % "{version}") +""" + + +class SbtInstaller(ConfigFileInstaller): + name = "sbt" + summary = "sbt plugin that gets credentials from the Cloudsmith CLI" + artifact_path = "io/cloudsmith/sbt-cloudsmith_2.12_1.0" + + def path(self) -> Path: + return Path.home() / ".sbt" / "1.0" / "plugins" / "cloudsmith.sbt" + + def render(self, version: str) -> str: + return GLOBAL_PLUGIN.format(repository_url=REPOSITORY_URL, version=version) From c44465a73ba6ee6e0f17f6f2539d1cc3bd0e9c7e Mon Sep 17 00:00:00 2001 From: Ian Duffy Date: Thu, 22 Oct 2026 17:49:20 +0100 Subject: [PATCH 02/27] fix(no-ticket): ignore comments and tag spaces in .mvn/extensions.xml `install maven` found the Cloudsmith extension inside an XML comment and changed the version there. The file then had no active entry, but the command reported success. A groupId or artifactId with spaces inside the tag was not found, so the command added a second entry. The search now replaces comments with spaces before it looks for the extension, its , and the element. The offsets do not change, so the edit still keeps the rest of the file as it is. The groupId and artifactId match with spaces around the value. Co-Authored-By: Claude Opus 5.5 --- .../test_credential_helper_maven_tools.py | 54 ++++++++++++++++++ .../credential_helpers/maven/extensions.py | 55 +++++++++++-------- 2 files changed, 85 insertions(+), 24 deletions(-) diff --git a/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py b/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py index 631b6dcf..58a591d9 100644 --- a/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py +++ b/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py @@ -189,6 +189,60 @@ def test_add_extension_with_the_same_version_changes_nothing(): assert extensions.add_extension(existing, "0.1.0") == (existing, False) +def test_add_extension_ignores_a_commented_out_extension(): + """A Cloudsmith extension in a comment is not active, so a new entry is added.""" + comment = f" \n" + existing = f"\n{comment}\n" + + text, changed = extensions.add_extension(existing, "0.2.0") + + assert changed is True + assert text == ( + f"\n{comment}" + f"{OUR_EXTENSION.replace('0.1.0', '0.2.0')}\n" + ) + + +def test_add_extension_finds_the_extension_with_spaces_in_the_tags(): + """Maven trims the values, so spaces inside a tag do not make a second entry.""" + spaced = OUR_EXTENSION.replace( + "io.cloudsmith", " io.cloudsmith " + ) + existing = f"\n{spaced}\n" + + text, changed = extensions.add_extension(existing, "0.2.0") + + assert changed is True + assert text == existing.replace( + "0.1.0", "0.2.0" + ) + + +def test_add_extension_ignores_a_version_in_a_comment(): + """Only the element of the extension changes.""" + existing = f"\n{OUR_EXTENSION}\n".replace( + " \n", " \n \n" + ) + + text, _ = extensions.add_extension(existing, "0.2.0") + + assert "" in text + assert "0.2.0" in text + + +def test_remove_extension_ignores_a_commented_out_extension(): + """A commented-out entry is not removed, and does not count as another extension.""" + comment = f" \n" + + assert extensions.remove_extension( + f"\n{comment}{OUR_EXTENSION}\n" + ) == (None, True) + assert extensions.remove_extension(f"\n{comment}\n") == ( + f"\n{comment}\n", + False, + ) + + def test_remove_extension_keeps_other_extensions(): """Only the Cloudsmith extension is removed.""" existing = f"\n{OTHER_EXTENSION}{OUR_EXTENSION}\n" diff --git a/cloudsmith_cli/credential_helpers/maven/extensions.py b/cloudsmith_cli/credential_helpers/maven/extensions.py index c3ce1f31..74982417 100644 --- a/cloudsmith_cli/credential_helpers/maven/extensions.py +++ b/cloudsmith_cli/credential_helpers/maven/extensions.py @@ -14,6 +14,9 @@ _EXTENSION_BLOCK = re.compile(r"[ \t]*.*?[ \t]*\n?", re.DOTALL) _EMPTY_EXTENSIONS = re.compile(r"]*?)\s*/>") _VERSION = re.compile(r"[^<]*") +_COMMENT = re.compile(r"", re.DOTALL) +_GROUP_ID = re.compile(rf"\s*{re.escape(GROUP_ID)}\s*") +_ARTIFACT_ID = re.compile(rf"\s*{re.escape(ARTIFACT_ID)}\s*") class ExtensionsFileError(Exception): @@ -30,19 +33,21 @@ def _render(version: str) -> str: ) -def _find_extension(text: str) -> re.Match | None: - for block in _EXTENSION_BLOCK.finditer(text): - content = block.group(0) - if f"{GROUP_ID}" in content and ( - f"{ARTIFACT_ID}" in content - ): +def _without_comments(text: str) -> str: + """Return *text* with each comment replaced by spaces, so the offsets stay the same.""" + return _COMMENT.sub(lambda comment: " " * len(comment.group(0)), text) + + +def _find_extension(masked: str) -> re.Match | None: + for block in _EXTENSION_BLOCK.finditer(masked): + if _GROUP_ID.search(block.group(0)) and _ARTIFACT_ID.search(block.group(0)): return block return None def has_extension(text: str) -> bool: """Return whether *text* lists the Cloudsmith extension.""" - return _find_extension(text) is not None + return _find_extension(_without_comments(text)) is not None def add_extension(text: str | None, version: str) -> tuple[str, bool]: @@ -52,21 +57,23 @@ def add_extension(text: str | None, version: str) -> tuple[str, bool]: """ if text is None: return f"\n{_render(version)}\n", True - block = _find_extension(text) + masked = _without_comments(text) + block = _find_extension(masked) if block: - updated = _VERSION.sub(f"{version}", block.group(0), count=1) - new_text = text[: block.start()] + updated + text[block.end() :] - return new_text, new_text != text - if _EMPTY_EXTENSIONS.search(text): - new_text = _EMPTY_EXTENSIONS.sub( - lambda empty: ( - f"\n{_render(version)}" - ), - text, - count=1, + old_version = _VERSION.search(masked, block.start(), block.end()) + if old_version is None: + raise ExtensionsFileError("The Cloudsmith extension has no .") + new_text = ( + text[: old_version.start()] + + f"{version}" + + text[old_version.end() :] ) - return new_text, True - closing = text.rfind("") + return new_text, new_text != text + empty = _EMPTY_EXTENSIONS.search(masked) + if empty: + opened = f"\n{_render(version)}" + return text[: empty.start()] + opened + text[empty.end() :], True + closing = masked.rfind("") if closing == -1: raise ExtensionsFileError("The file has no element.") return text[:closing] + _render(version) + text[closing:], True @@ -77,10 +84,10 @@ def remove_extension(text: str) -> tuple[str | None, bool]: The text is ``None`` when no other extension is left, so the file is not needed. """ - block = _find_extension(text) + masked = _without_comments(text) + block = _find_extension(masked) if not block: return text, False - new_text = text[: block.start()] + text[block.end() :] - if not _EXTENSION_BLOCK.search(new_text): + if not _EXTENSION_BLOCK.search(masked[: block.start()] + masked[block.end() :]): return None, True - return new_text, True + return text[: block.start()] + text[block.end() :], True From d993893b98b0db0a0d293ee2279e5b62da6b220a Mon Sep 17 00:00:00 2001 From: Ian Duffy Date: Thu, 22 Oct 2026 17:59:56 +0100 Subject: [PATCH 03/27] feat(no-ticket): set up the Maven tools for every build, with --local and --mavenrc `install maven`, `install gradle`, and `install sbt` now set up every build of the user by default. - maven: lists the extension in ~/.m2/extensions.xml, which Maven 3.10 and later read. When `mvn -v` reports an earlier Maven, install shows a warning that names the two options below. - maven --local: lists the extension in .mvn/extensions.xml of the project, as before. - maven --mavenrc: adds a marked block to ~/.mavenrc that sets maven.ext.class.path. The mvn and mvnw scripts of every Maven version read it. The other lines of the file stay, and uninstall removes only the block. - sbt --local: writes project/cloudsmith.sbt, so the build's own files stay as they are. - gradle --local: fails with the settings.gradle.kts plugin block to add. - Other helpers reject --local and --mavenrc with a usage error. Install also marks the extension files as installed in _remote.repositories. Maven does not load a build extension that it records as downloaded from a repository, so a copy that Maven downloaded earlier stopped every build in the project. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 7 +- .../cli/commands/credential_helper/manage.py | 55 ++- .../test_credential_helper_maven_tools.py | 344 +++++++++++++++--- .../credential_helpers/gradle/installer.py | 33 +- .../credential_helpers/maven/installer.py | 232 +++++++++--- .../credential_helpers/maven/mavenrc.py | 60 +++ .../credential_helpers/maven_tools.py | 27 +- .../credential_helpers/sbt/installer.py | 12 +- 8 files changed, 658 insertions(+), 112 deletions(-) create mode 100644 cloudsmith_cli/credential_helpers/maven/mavenrc.py diff --git a/CHANGELOG.md b/CHANGELOG.md index c47c7ad4..6e723c13 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,10 +9,11 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ### Added -- Added `cloudsmith credential-helper install maven`, `install gradle`, and `install sbt`. They set up the Cloudsmith Maven tools, which get repository credentials from `cloudsmith credential-helper generic`. Each command installs the newest release from the public `cloudsmith/maven-tools` repository. - - `maven` puts the extension in `~/.m2/repository` and adds it to `.mvn/extensions.xml` of the Maven project in the current directory. +- Added `cloudsmith credential-helper install maven`, `install gradle`, and `install sbt`. They set up the Cloudsmith Maven tools, which get repository credentials from `cloudsmith credential-helper generic`. Each command installs the newest release from the public `cloudsmith/maven-tools` repository, for every build of the user. + - `maven` puts the extension in `~/.m2/repository` and adds it to `~/.m2/extensions.xml`, which Maven 3.10 and later read. When `mvn -v` reports an earlier Maven, the command shows a warning. With `--local`, the command adds the extension to `.mvn/extensions.xml` of the Maven project in the current directory instead. With `--mavenrc`, it adds a block to `~/.mavenrc` instead, which the `mvn` and `mvnw` scripts of all Maven versions read. - `gradle` writes an init script to `~/.gradle/init.d` (or `$GRADLE_USER_HOME/init.d`), which applies the plugin to every Gradle build. - - `sbt` writes a global plugin to `~/.sbt/1.0/plugins`, which adds the plugin to every sbt 1 build. + - `sbt` writes a global plugin to `~/.sbt/1.0/plugins`, which adds the plugin to every sbt 1 build. With `--local`, it writes `project/cloudsmith.sbt` of the sbt project in the current directory instead. + - The commands add to existing files and keep their other content. `uninstall` removes only what `install` added. ## [1.30.0] - 2026-10-22 diff --git a/cloudsmith_cli/cli/commands/credential_helper/manage.py b/cloudsmith_cli/cli/commands/credential_helper/manage.py index f6b5da1d..1cb16ebf 100644 --- a/cloudsmith_cli/cli/commands/credential_helper/manage.py +++ b/cloudsmith_cli/cli/commands/credential_helper/manage.py @@ -17,9 +17,10 @@ from cloudsmith_cli.credential_helpers.generic import PartialInstallError from cloudsmith_cli.credential_helpers.gradle.installer import GradleInstaller from cloudsmith_cli.credential_helpers.maven.extensions import ExtensionsFileError -from cloudsmith_cli.credential_helpers.maven.installer import ( - MavenInstaller, - MavenProjectNotFound, +from cloudsmith_cli.credential_helpers.maven.installer import MavenInstaller +from cloudsmith_cli.credential_helpers.maven_tools import ( + InstallOptionError, + ProjectNotFound, ) from cloudsmith_cli.credential_helpers.pnpm.installer import PNPMInstaller from cloudsmith_cli.credential_helpers.sbt.installer import SbtInstaller @@ -86,6 +87,15 @@ def _get_installer(name: str): return cls() +def _scope_options(installer, helper: str, **requested: bool) -> dict[str, bool]: + """Return the scope options that were given, or fail if *helper* has no such option.""" + supported = getattr(installer, "install_options", ()) + for option, given in requested.items(): + if given and option not in supported: + raise click.UsageError(f"--{option} is not supported for {helper}.") + return {option: True for option, given in requested.items() if given} + + def _terraform_helper_args(ctx, opts, repo: str | None = None) -> tuple[str, ...]: """Build the terraformrc ``args`` list from the resolved config. @@ -251,6 +261,20 @@ def _terraform_workspace_next_steps(helper_args: tuple[str, ...]) -> list[str]: "list (as `-r `) so `terraform init` needs no CLOUDSMITH_REPO env " "var. Terraform does not pass the repository to a credentials helper.", ) +@click.option( + "--local", + is_flag=True, + default=False, + help="Maven and sbt only: set up the project in the current directory " + "instead of every build of the user.", +) +@click.option( + "--mavenrc", + is_flag=True, + default=False, + help="Maven only: load the extension through ~/.mavenrc instead of " + "~/.m2/extensions.xml. Use it for Maven 3.9 and earlier.", +) @common_cli_config_options @common_cli_output_options @common_api_auth_options @@ -266,6 +290,8 @@ def install_cmd( no_discover: bool, refresh: bool, repo: str | None, + local: bool, + mavenrc: bool, ) -> None: """Install a credential helper launcher and configure the package manager. @@ -310,6 +336,7 @@ def install_cmd( "org": opts.org, "credential": opts.credential, "api_host": opts.api_host, + **_scope_options(installer, helper, local=local, mavenrc=mavenrc), } # Terraform bakes the resolved org/profile into the terraformrc `args` list @@ -326,7 +353,8 @@ def install_cmd( TerraformrcConflictError, TerraformHelperExeNotFound, TerraformPluginDirError, - MavenProjectNotFound, + ProjectNotFound, + InstallOptionError, ExtensionsFileError, ) as exc: raise click.ClickException(str(exc)) @@ -392,10 +420,19 @@ def install_cmd( default=False, help="Show what would be done without making any changes.", ) +@click.option( + "--local", + is_flag=True, + default=False, + help="Maven and sbt only: remove the setup of the project in the current " + "directory.", +) @common_cli_config_options @common_cli_output_options @click.pass_context -def uninstall_cmd(ctx, opts, helper: str, bin_dir: str | None, dry_run: bool) -> None: +def uninstall_cmd( + ctx, opts, helper: str, bin_dir: str | None, dry_run: bool, local: bool +) -> None: """Uninstall a credential helper launcher and remove its config entries. HELPER is the name of the credential helper to uninstall (e.g. ``docker``). @@ -412,7 +449,13 @@ 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=bin_dir, + dry_run=dry_run, + **_scope_options(installer, helper, local=local), + ) + except (ProjectNotFound, InstallOptionError) as exc: + raise click.ClickException(str(exc)) except OSError as exc: raise click.ClickException( f"Failed to uninstall {helper!r} credential helper: {exc}" diff --git a/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py b/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py index 58a591d9..9fd9e9c2 100644 --- a/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py +++ b/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py @@ -16,8 +16,10 @@ from ...credential_helpers import maven_tools from ...credential_helpers.gradle.installer import GradleInstaller -from ...credential_helpers.maven import extensions -from ...credential_helpers.maven.installer import MavenInstaller, MavenProjectNotFound +from ...credential_helpers.maven import extensions, mavenrc +from ...credential_helpers.maven import installer as maven_installer +from ...credential_helpers.maven.installer import MavenInstaller +from ...credential_helpers.maven_tools import InstallOptionError, ProjectNotFound from ...credential_helpers.sbt.installer import SbtInstaller EXTENSION_PATH = "io/cloudsmith/cloudsmith-maven-extension" @@ -81,6 +83,21 @@ def fake_download(artifact_path, destination): return downloads +@pytest.fixture(autouse=True) +def _maven_3_10(monkeypatch): + """Run as if `mvn -v` reports Maven 3.10.0.""" + monkeypatch.setattr(maven_installer, "_maven_version", lambda: "3.10.0") + + +@pytest.fixture() +def outside_projects(tmp_path, monkeypatch): + """Run the test from a directory that is not in a project.""" + directory = tmp_path / "work" + directory.mkdir() + monkeypatch.chdir(directory) + return directory + + @pytest.fixture() def maven_project(tmp_path, monkeypatch): """Make a Maven project and run the test from its root directory.""" @@ -268,7 +285,66 @@ def test_remove_extension_without_the_extension_changes_nothing(): # --------------------------------------------------------------------------- -# 3. MavenInstaller +# 3. ~/.mavenrc +# --------------------------------------------------------------------------- + +JAR = Path("/home/me/.m2/repository/cloudsmith-maven-extension-0.2.0.jar") +BLOCK = ( + "# >>> cloudsmith credential-helper install maven >>>\n" + f'MAVEN_OPTS="$MAVEN_OPTS -Dmaven.ext.class.path={JAR}"\n' + "# <<< cloudsmith credential-helper install maven <<<\n" +) + + +def test_add_block_makes_a_new_file(): + """Without a file, the result is only the block.""" + assert mavenrc.add_block(None, JAR) == (BLOCK, True) + + +def test_add_block_keeps_the_existing_lines(): + """The block goes after the lines of the user, after a blank line.""" + existing = 'export JAVA_HOME="/opt/java"' + + assert mavenrc.add_block(existing, JAR) == (f"{existing}\n\n{BLOCK}", True) + + +def test_add_block_replaces_an_older_block_in_place(): + """An upgrade changes the block, and the lines around it stay as they are.""" + older = BLOCK.replace("0.2.0", "0.1.0") + + assert mavenrc.add_block(f"export A=1\n\n{older}export B=2\n", JAR) == ( + f"export A=1\n\n{BLOCK}export B=2\n", + True, + ) + + +def test_add_block_with_the_same_jar_changes_nothing(): + """Installing the same version again leaves the file as it is.""" + existing = f"export A=1\n\n{BLOCK}" + + assert mavenrc.add_block(existing, JAR) == (existing, False) + + +def test_remove_block_keeps_the_other_lines(): + """Only the block, and the blank line that install added before it, go.""" + assert mavenrc.remove_block(f"export A=1\n\n{BLOCK}export B=2\n") == ( + "export A=1\nexport B=2\n", + True, + ) + + +def test_remove_the_only_block_returns_no_text(): + """When nothing else is in the file, the file is not needed.""" + assert mavenrc.remove_block(BLOCK) == (None, True) + + +def test_remove_block_without_a_block_changes_nothing(): + """A file without the block stays as it is.""" + assert mavenrc.remove_block("export A=1\n") == ("export A=1\n", False) + + +# --------------------------------------------------------------------------- +# 4. MavenInstaller # --------------------------------------------------------------------------- @@ -276,8 +352,12 @@ def _local_extension_dir(home: Path, version: str) -> Path: return home / ".m2" / "repository" / EXTENSION_PATH / version +def _user_extensions(home: Path) -> Path: + return home / ".m2" / "extensions.xml" + + def test_maven_install_puts_the_extension_in_the_local_repository( - home, fake_repository, maven_project + home, fake_repository, outside_projects ): """Maven loads build extensions only from Maven Central or the local repository.""" actions = MavenInstaller().install() @@ -288,18 +368,73 @@ def test_maven_install_puts_the_extension_in_the_local_repository( assert any(str(version_dir) in action for action in actions) -def test_maven_install_turns_on_the_extension_in_the_project( - home, fake_repository, maven_project +def test_maven_install_turns_on_the_extension_for_every_build( + home, fake_repository, outside_projects +): + """By default, ~/.m2/extensions.xml lists the extension for Maven 3.10 and later.""" + actions = MavenInstaller().install() + + text = _user_extensions(home).read_text(encoding="utf-8") + assert "0.2.0" in text + assert not (outside_projects / ".mvn").exists() + assert not [action for action in actions if action.startswith("WARNING")] + + +def test_maven_install_warns_when_maven_ignores_user_extensions( + home, fake_repository, outside_projects, monkeypatch ): - """The project's .mvn/extensions.xml lists the installed version.""" + """Maven 3.9 and earlier ignore ~/.m2/extensions.xml, so install names the options.""" + monkeypatch.setattr(maven_installer, "_maven_version", lambda: "3.9.11") + actions = MavenInstaller().install() + warnings = [action for action in actions if action.startswith("WARNING")] + assert len(warnings) == 1 + assert "3.9.11" in warnings[0] + assert "--local" in warnings[0] + assert "--mavenrc" in warnings[0] + assert _user_extensions(home).exists() + + +def test_maven_install_marks_the_files_as_installed( + home, fake_repository, outside_projects +): + """Maven does not load an extension that it records as downloaded from a repository.""" + version_dir = _local_extension_dir(home, "0.2.0") + version_dir.mkdir(parents=True) + for file_type in ("jar", "pom"): + file_name = f"cloudsmith-maven-extension-0.2.0.{file_type}" + (version_dir / file_name).write_text("from a repository", encoding="utf-8") + (version_dir / "_remote.repositories").write_text( + "#NOTE: a comment\n" + "cloudsmith-maven-extension-0.2.0.jar>cloudsmith=\n" + "cloudsmith-maven-extension-0.2.0-sources.jar>cloudsmith=\n", + encoding="utf-8", + ) + + MavenInstaller().install() + + assert (version_dir / "_remote.repositories").read_text(encoding="utf-8") == ( + "#NOTE: a comment\n" + "cloudsmith-maven-extension-0.2.0.jar>=\n" + "cloudsmith-maven-extension-0.2.0-sources.jar>cloudsmith=\n" + "cloudsmith-maven-extension-0.2.0.pom>=\n" + ) + + +def test_maven_install_local_turns_on_the_extension_in_the_project( + home, fake_repository, maven_project +): + """With local, the project's .mvn/extensions.xml lists the extension.""" + actions = MavenInstaller().install(local=True) + extensions_file = maven_project / ".mvn" / "extensions.xml" assert "0.2.0" in extensions_file.read_text(encoding="utf-8") assert any(str(extensions_file) in action for action in actions) + assert not _user_extensions(home).exists() -def test_maven_install_from_a_module_uses_the_root_project( +def test_maven_install_local_from_a_module_uses_the_root_project( home, fake_repository, maven_project, monkeypatch ): """Maven reads .mvn from the nearest directory above that has one.""" @@ -309,58 +444,112 @@ def test_maven_install_from_a_module_uses_the_root_project( (module / "pom.xml").write_text("", encoding="utf-8") monkeypatch.chdir(module) - MavenInstaller().install() + MavenInstaller().install(local=True) assert (maven_project / ".mvn" / "extensions.xml").exists() assert not (module / ".mvn").exists() -def test_maven_install_outside_a_project_fails( - home, fake_repository, tmp_path, monkeypatch +def test_maven_install_local_outside_a_project_fails( + home, fake_repository, outside_projects ): """Without a pom.xml, the install stops before it changes anything.""" - monkeypatch.chdir(tmp_path) + with pytest.raises(ProjectNotFound, match="pom.xml"): + MavenInstaller().install(local=True) - with pytest.raises(MavenProjectNotFound): - MavenInstaller().install() - - assert not (tmp_path / ".mvn").exists() + assert not (outside_projects / ".mvn").exists() assert not fake_repository +def test_maven_install_with_mavenrc_adds_a_block( + home, fake_repository, outside_projects +): + """--mavenrc loads the extension in Maven 3.9 and earlier, and keeps the lines there.""" + (home / ".mavenrc").write_text('export JAVA_HOME="/opt/java"\n', encoding="utf-8") + + MavenInstaller().install(mavenrc=True) + + jar = _local_extension_dir(home, "0.2.0") / "cloudsmith-maven-extension-0.2.0.jar" + text = (home / ".mavenrc").read_text(encoding="utf-8") + assert text.startswith('export JAVA_HOME="/opt/java"\n') + assert f"-Dmaven.ext.class.path={jar}" in text + # Maven 3.10 would load the extension twice from both files. + assert not _user_extensions(home).exists() + + +def test_maven_install_with_local_and_mavenrc_fails( + home, fake_repository, maven_project +): + """The two options set up different scopes, so only one is allowed.""" + with pytest.raises(InstallOptionError): + MavenInstaller().install(local=True, mavenrc=True) + + def test_maven_install_dry_run_changes_nothing(home, fake_repository, maven_project): """A dry run reports the planned changes and writes no files.""" - actions = MavenInstaller().install(dry_run=True) + actions = [ + *MavenInstaller().install(dry_run=True), + *MavenInstaller().install(dry_run=True, local=True), + *MavenInstaller().install(dry_run=True, mavenrc=True), + ] assert not (maven_project / ".mvn").exists() assert not (home / ".m2").exists() + assert not (home / ".mavenrc").exists() assert all(action.startswith("would ") for action in actions) -def test_maven_uninstall_removes_the_extension_from_the_project( - home, fake_repository, maven_project +def test_maven_uninstall_removes_only_what_install_added( + home, fake_repository, outside_projects ): - """Uninstall removes the file it made, and keeps the shared local repository.""" + """Other extensions and other .mavenrc lines stay, and so does the local repository.""" + _user_extensions(home).parent.mkdir() + _user_extensions(home).write_text( + f"\n{OTHER_EXTENSION}\n", encoding="utf-8" + ) + (home / ".mavenrc").write_text("export A=1\n", encoding="utf-8") MavenInstaller().install() + MavenInstaller().install(mavenrc=True) MavenInstaller().uninstall() - assert not (maven_project / ".mvn" / "extensions.xml").exists() + assert _user_extensions(home).read_text(encoding="utf-8") == ( + f"\n{OTHER_EXTENSION}\n" + ) + assert (home / ".mavenrc").read_text(encoding="utf-8") == "export A=1\n" assert _local_extension_dir(home, "0.2.0").exists() -def test_maven_status_names_the_project_file(home, fake_repository, maven_project): - """list shows the extensions.xml of the current project when it is set up.""" +def test_maven_uninstall_local_removes_the_extension_from_the_project( + home, fake_repository, maven_project +): + """Uninstall with local removes the file that install with local made.""" + MavenInstaller().install(local=True) + + MavenInstaller().uninstall(local=True) + + assert not (maven_project / ".mvn" / "extensions.xml").exists() + + +def test_maven_status_names_the_file_that_loads_the_extension( + home, fake_repository, maven_project +): + """list shows the user-wide file first, then the file of the current project.""" assert MavenInstaller().status() == {"launcher": None, "hosts": []} - MavenInstaller().install() + MavenInstaller().install(local=True) + project_file = maven_project / ".mvn" / "extensions.xml" + assert MavenInstaller().status() == {"launcher": str(project_file), "hosts": []} - extensions_file = maven_project / ".mvn" / "extensions.xml" - assert MavenInstaller().status() == {"launcher": str(extensions_file), "hosts": []} + MavenInstaller().install() + assert MavenInstaller().status() == { + "launcher": str(_user_extensions(home)), + "hosts": [], + } # --------------------------------------------------------------------------- -# 4. GradleInstaller +# 5. GradleInstaller # --------------------------------------------------------------------------- @@ -389,6 +578,14 @@ def test_gradle_install_uses_gradle_user_home( assert (tmp_path / "gradle-home" / "init.d" / "cloudsmith.init.gradle.kts").exists() +def test_gradle_install_local_explains_the_settings_plugin(home, fake_repository): + """Gradle has no project init scripts, so the error shows what to add instead.""" + with pytest.raises(InstallOptionError, match="settings.gradle.kts"): + GradleInstaller().install(local=True) + + assert not (home / ".gradle").exists() + + def test_gradle_uninstall_and_status(home, fake_repository): """Uninstall removes the init script, and status follows it.""" script = home / ".gradle" / "init.d" / "cloudsmith.init.gradle.kts" @@ -402,7 +599,7 @@ def test_gradle_uninstall_and_status(home, fake_repository): # --------------------------------------------------------------------------- -# 5. SbtInstaller +# 6. SbtInstaller # --------------------------------------------------------------------------- @@ -420,6 +617,34 @@ def test_sbt_install_writes_a_global_plugin(home, fake_repository): assert any(str(plugin_file) in action for action in actions) +def test_sbt_install_local_writes_a_project_plugin( + home, fake_repository, outside_projects +): + """sbt loads every .sbt file in project/, so the build's own files stay as they are.""" + (outside_projects / "build.sbt").write_text("", encoding="utf-8") + + SbtInstaller().install(local=True) + + plugin_file = outside_projects / "project" / "cloudsmith.sbt" + text = plugin_file.read_text(encoding="utf-8") + assert 'addSbtPlugin("io.cloudsmith" % "sbt-cloudsmith" % "0.2.0")' in text + assert not (home / ".sbt").exists() + + SbtInstaller().uninstall(local=True) + + assert not plugin_file.exists() + + +def test_sbt_install_local_outside_a_project_fails( + home, fake_repository, outside_projects +): + """Without a build.sbt, the install stops before it changes anything.""" + with pytest.raises(ProjectNotFound, match="build.sbt"): + SbtInstaller().install(local=True) + + assert not (outside_projects / "project").exists() + + def test_sbt_uninstall_and_status(home, fake_repository): """Uninstall removes the global plugin, and status follows it.""" plugin_file = home / ".sbt" / "1.0" / "plugins" / "cloudsmith.sbt" @@ -433,33 +658,68 @@ def test_sbt_uninstall_and_status(home, fake_repository): # --------------------------------------------------------------------------- -# 6. CLI wiring +# 7. CLI wiring # --------------------------------------------------------------------------- +def _invoke(command_name, args): + from ...cli.commands.credential_helper import manage + + command = {"install": manage.install_cmd, "uninstall": manage.uninstall_cmd} + return click.testing.CliRunner().invoke( + command[command_name], args, catch_exceptions=False + ) + + @pytest.mark.parametrize("helper", ["maven", "gradle", "sbt"]) def test_cli_install_runs_the_installer(helper, home, fake_repository, maven_project): """`credential-helper install ` writes the config without credentials.""" - from ...cli.commands.credential_helper.manage import install_cmd - - result = click.testing.CliRunner().invoke( - install_cmd, [helper, "--no-discover"], catch_exceptions=False - ) + result = _invoke("install", [helper, "--no-discover"]) assert result.exit_code == 0, result.output assert "0.2.0" in result.output -def test_cli_install_maven_outside_a_project_fails_cleanly( - home, fake_repository, tmp_path, monkeypatch +def test_cli_install_and_uninstall_maven_local(home, fake_repository, maven_project): + """--local reaches the installer for install and for uninstall.""" + extensions_file = maven_project / ".mvn" / "extensions.xml" + + assert _invoke("install", ["maven", "--local", "--no-discover"]).exit_code == 0 + assert extensions_file.exists() + assert _invoke("uninstall", ["maven", "--local"]).exit_code == 0 + assert not extensions_file.exists() + + +def test_cli_install_maven_with_mavenrc(home, fake_repository, outside_projects): + """--mavenrc reaches the Maven installer.""" + result = _invoke("install", ["maven", "--mavenrc", "--no-discover"]) + + assert result.exit_code == 0, result.output + assert (home / ".mavenrc").exists() + + +def test_cli_install_maven_local_outside_a_project_fails_cleanly( + home, fake_repository, outside_projects ): """The error names the directory and the fix, without a traceback.""" - from ...cli.commands.credential_helper.manage import install_cmd - - monkeypatch.chdir(tmp_path) - result = click.testing.CliRunner().invoke( - install_cmd, ["maven", "--no-discover"], catch_exceptions=False - ) + result = _invoke("install", ["maven", "--local", "--no-discover"]) assert result.exit_code != 0 assert "pom.xml" in result.output + + +def test_cli_install_gradle_local_shows_the_settings_plugin(home, fake_repository): + """The error shows the plugins block to add to settings.gradle.kts.""" + result = _invoke("install", ["gradle", "--local", "--no-discover"]) + + assert result.exit_code != 0 + assert 'id("io.cloudsmith.gradle-plugin")' in result.output + + +@pytest.mark.parametrize("option", ["--local", "--mavenrc"]) +def test_cli_install_rejects_an_option_that_the_helper_does_not_have(option, home): + """Other helpers fail with a clear error, not with a TypeError.""" + result = _invoke("install", ["docker", option, "--no-discover"]) + + assert result.exit_code != 0 + assert f"{option} is not supported for docker" in result.output diff --git a/cloudsmith_cli/credential_helpers/gradle/installer.py b/cloudsmith_cli/credential_helpers/gradle/installer.py index ec362119..41a0b4aa 100644 --- a/cloudsmith_cli/credential_helpers/gradle/installer.py +++ b/cloudsmith_cli/credential_helpers/gradle/installer.py @@ -6,7 +6,12 @@ import os from pathlib import Path -from ..maven_tools import REPOSITORY_URL, ConfigFileInstaller +from ..maven_tools import ( + REPOSITORY_URL, + ConfigFileInstaller, + InstallOptionError, + latest_version, +) INIT_SCRIPT = """\ // Written by `cloudsmith credential-helper install gradle`. @@ -24,6 +29,22 @@ }} """ +SETTINGS_PLUGIN = """\ +--local is not supported for gradle. To set up one build, add the plugin to \ +settings.gradle.kts: + +pluginManagement {{ + repositories {{ + maven {{ url = uri("{repository_url}") }} + gradlePluginPortal() + }} +}} + +plugins {{ + id("io.cloudsmith.gradle-plugin") version "{version}" +}} +""" + class GradleInstaller(ConfigFileInstaller): name = "gradle" @@ -35,5 +56,15 @@ def path(self) -> Path: base = Path(gradle_user_home) if gradle_user_home else Path.home() / ".gradle" return base / "init.d" / "cloudsmith.init.gradle.kts" + def local_path(self) -> Path: + # Gradle has no init scripts for one build, and a script edit of the + # build's settings file could break it. + raise InstallOptionError( + SETTINGS_PLUGIN.format( + repository_url=REPOSITORY_URL, + version=latest_version(self.artifact_path), + ) + ) + def render(self, version: str) -> str: return INIT_SCRIPT.format(repository_url=REPOSITORY_URL, version=version) diff --git a/cloudsmith_cli/credential_helpers/maven/installer.py b/cloudsmith_cli/credential_helpers/maven/installer.py index ca604837..4ec025f7 100644 --- a/cloudsmith_cli/credential_helpers/maven/installer.py +++ b/cloudsmith_cli/credential_helpers/maven/installer.py @@ -1,23 +1,36 @@ # Copyright 2026 Cloudsmith Ltd -"""Turn on the Cloudsmith Maven extension in the Maven project of the current directory. +"""Turn on the Cloudsmith Maven extension for every Maven build, or for one project. -Maven 3.9 has no user-wide extensions, and Maven loads a build extension only -from Maven Central or the local repository. So the installer puts the extension -in the local repository and lists it in the project's ``.mvn/extensions.xml``. +Maven loads a build extension only from Maven Central or the local repository, +so the installer puts the extension in the local repository first. Then it lists +the extension in one of these files: + +- ``~/.m2/extensions.xml`` (the default), which Maven 3.10 and later read. +- ``.mvn/extensions.xml`` of the current project (``local``), which Maven 3.8 + and later read. +- ``~/.mavenrc`` (``mavenrc``), which the ``mvn`` and ``mvnw`` scripts read. """ from __future__ import annotations +import re +import shutil +import subprocess +from functools import partial from pathlib import Path +from typing import TYPE_CHECKING from .. import maven_tools +from ..maven_tools import InstallOptionError, ProjectNotFound from . import extensions +from .mavenrc import add_block, has_block, remove_block -EXTENSION_PATH = f"io/cloudsmith/{extensions.ARTIFACT_ID}" - +if TYPE_CHECKING: + from collections.abc import Callable -class MavenProjectNotFound(Exception): - """The current directory is not in a Maven project.""" +EXTENSION_PATH = f"io/cloudsmith/{extensions.ARTIFACT_ID}" +FIRST_MAVEN_WITH_USER_EXTENSIONS = (3, 10) +MAVEN_VERSION_TIMEOUT_SECONDS = 60 def _project_dir() -> Path: @@ -29,34 +42,153 @@ def _project_dir() -> Path: return cwd -def _extensions_file() -> Path: - return _project_dir() / ".mvn" / "extensions.xml" +def _project_extensions_file() -> Path: + project_dir = _project_dir() + if not (project_dir / "pom.xml").is_file(): + raise ProjectNotFound( + f"No pom.xml in {project_dir}. " + "Run the command in the root directory of a Maven project." + ) + return project_dir / ".mvn" / "extensions.xml" + + +def _user_extensions_file() -> Path: + return Path.home() / ".m2" / "extensions.xml" + + +def _mavenrc_file() -> Path: + return Path.home() / ".mavenrc" + + +def _version_dir(version: str) -> Path: + return Path.home() / ".m2" / "repository" / EXTENSION_PATH / version def _read(path: Path) -> str | None: return path.read_text(encoding="utf-8") if path.exists() else None +def _maven_version() -> str | None: + """Return the version that ``mvn -v`` reports, or ``None`` when there is none.""" + mvn = shutil.which("mvn") + if not mvn: + return None + try: + result = subprocess.run( + [mvn, "-v"], + capture_output=True, + text=True, + timeout=MAVEN_VERSION_TIMEOUT_SECONDS, + check=False, + ) + except (OSError, subprocess.TimeoutExpired): + return None + match = re.search(r"Apache Maven (\d+(?:\.\d+)+)", result.stdout) + return match.group(1) if match else None + + +def _old_maven_warning() -> list[str]: + version = _maven_version() + if version is None: + return [] + major_minor = tuple(int(part) for part in version.split(".")[:2]) + if major_minor >= FIRST_MAVEN_WITH_USER_EXTENSIONS: + return [] + warning = ( + f"WARNING: Maven {version} ignores {_user_extensions_file()}. " + "Maven 3.10 and later read it. For this Maven, run " + "`cloudsmith credential-helper install maven --local` in each project, " + "or run `cloudsmith credential-helper install maven --mavenrc` once." + ) + return [warning] + + +def _mark_installed(version_dir: Path, file_names: list[str]) -> None: + """Record the files as installed, not downloaded, in ``_remote.repositories``. + + Maven does not load a build extension that this file records as downloaded + from a repository other than Maven Central. A missing file means installed. + """ + tracking_file = version_dir / "_remote.repositories" + current = _read(tracking_file) + if current is None: + return + lines = [] + for line in current.splitlines(): + name = line.split(">", 1)[0] + lines.append(f"{name}>=" if name in file_names else line) + lines += [f"{name}>=" for name in file_names if f"{name}>=" not in lines] + tracking_file.write_text("\n".join(lines) + "\n", encoding="utf-8") + + +def _add(path: Path, edit: Callable, version: str, dry_run: bool) -> list[str]: + text, changed = edit(_read(path)) + if not changed: + return [f"{path} already loads version {version} (no change)"] + if dry_run: + return [f"would add version {version} to {path}"] + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(text, encoding="utf-8") + return [f"added version {version} to {path}"] + + +def _remove(path: Path, edit: Callable, dry_run: bool) -> list[str]: + current = _read(path) + if current is None: + return [f"{path} not found (nothing to remove)"] + text, changed = edit(current) + if not changed: + return [f"{path} does not load the extension (nothing to remove)"] + if dry_run: + return [f"would remove the extension from {path}"] + if text is None: + path.unlink() + return [f"removed {path}"] + path.write_text(text, encoding="utf-8") + return [f"removed the extension from {path}"] + + class MavenInstaller: name = "maven" summary = "Maven extension that gets credentials from the Cloudsmith CLI" - - def install(self, *, dry_run: bool = False, **_options) -> list[str]: - """Put the extension in ``~/.m2/repository`` and list it in ``.mvn/extensions.xml``.""" - project_dir = _project_dir() - if not (project_dir / "pom.xml").is_file(): - raise MavenProjectNotFound( - f"No pom.xml in {project_dir}. " - "Run the command in the root directory of a Maven project." - ) + install_options = ("local", "mavenrc") + + def install( + self, + *, + dry_run: bool = False, + local: bool = False, + mavenrc: bool = False, + **_options, + ) -> list[str]: + """Put the extension in ``~/.m2/repository`` and turn it on.""" + if local and mavenrc: + raise InstallOptionError("Use --local or --mavenrc, not both.") + project_file = _project_extensions_file() if local else None version = maven_tools.latest_version(EXTENSION_PATH) - return self._download(version, dry_run) + self._turn_on(version, dry_run) + actions = self._download(version, dry_run) + if mavenrc: + jar = _version_dir(version) / f"{extensions.ARTIFACT_ID}-{version}.jar" + return actions + _add( + _mavenrc_file(), partial(add_block, jar=jar), version, dry_run + ) + add_extension = partial(extensions.add_extension, version=version) + if project_file: + return actions + _add(project_file, add_extension, version, dry_run) + return ( + actions + + _add(_user_extensions_file(), add_extension, version, dry_run) + + _old_maven_warning() + ) def _download(self, version: str, dry_run: bool) -> list[str]: - version_dir = Path.home() / ".m2" / "repository" / EXTENSION_PATH / version + version_dir = _version_dir(version) + file_names = [ + f"{extensions.ARTIFACT_ID}-{version}.{file_type}" + for file_type in ("jar", "pom") + ] actions = [] - for file_type in ("jar", "pom"): - file_name = f"{extensions.ARTIFACT_ID}-{version}.{file_type}" + for file_name in file_names: destination = version_dir / file_name if destination.exists(): actions.append(f"{file_name} already in {version_dir} (no change)") @@ -67,41 +199,33 @@ def _download(self, version: str, dry_run: bool) -> list[str]: f"{EXTENSION_PATH}/{version}/{file_name}", destination ) actions.append(f"downloaded {file_name} to {version_dir}") + if not dry_run: + _mark_installed(version_dir, file_names) return actions - def _turn_on(self, version: str, dry_run: bool) -> list[str]: - path = _extensions_file() - text, changed = extensions.add_extension(_read(path), version) - if not changed: - return [f"{path} already lists version {version} (no change)"] - if dry_run: - return [f"would add version {version} to {path}"] - path.parent.mkdir(exist_ok=True) - path.write_text(text, encoding="utf-8") - return [f"added version {version} to {path}"] - - def uninstall(self, *, dry_run: bool = False, **_options) -> list[str]: - """Remove the extension from ``.mvn/extensions.xml``. + def uninstall( + self, *, dry_run: bool = False, local: bool = False, **_options + ) -> list[str]: + """Remove the extension from the files that install changes. The local repository is shared with other projects, so it stays as it is. """ - path = _extensions_file() - current = _read(path) - if current is None: - return [f"{path} not found (nothing to remove)"] - text, changed = extensions.remove_extension(current) - if not changed: - return [f"{path} does not list the extension (nothing to remove)"] - if dry_run: - return [f"would remove the extension from {path}"] - if text is None: - path.unlink() - return [f"removed {path}"] - path.write_text(text, encoding="utf-8") - return [f"removed the extension from {path}"] + if local: + return _remove( + _project_extensions_file(), extensions.remove_extension, dry_run + ) + return _remove( + _user_extensions_file(), extensions.remove_extension, dry_run + ) + _remove(_mavenrc_file(), remove_block, dry_run) def status(self) -> dict: - path = _extensions_file() - current = _read(path) - configured = current is not None and extensions.has_extension(current) - return {"launcher": str(path) if configured else None, "hosts": []} + candidates = ( + (_user_extensions_file(), extensions.has_extension), + (_mavenrc_file(), has_block), + (_project_dir() / ".mvn" / "extensions.xml", extensions.has_extension), + ) + for path, loads_extension in candidates: + current = _read(path) + if current is not None and loads_extension(current): + return {"launcher": str(path), "hosts": []} + return {"launcher": None, "hosts": []} diff --git a/cloudsmith_cli/credential_helpers/maven/mavenrc.py b/cloudsmith_cli/credential_helpers/maven/mavenrc.py new file mode 100644 index 00000000..58da8f2e --- /dev/null +++ b/cloudsmith_cli/credential_helpers/maven/mavenrc.py @@ -0,0 +1,60 @@ +# Copyright 2026 Cloudsmith Ltd +"""Add and remove the block that loads the Cloudsmith extension in ``~/.mavenrc``. + +The ``mvn`` and ``mvnw`` scripts read ``~/.mavenrc`` before they start Maven. The +block adds the extension to ``maven.ext.class.path``, which every Maven version +reads. The functions change only the block, so the other lines stay as they are. +""" + +from __future__ import annotations + +import re +from typing import TYPE_CHECKING + +if TYPE_CHECKING: + from pathlib import Path + +BEGIN = "# >>> cloudsmith credential-helper install maven >>>" +END = "# <<< cloudsmith credential-helper install maven <<<" + +_BLOCK_PATTERN = rf"^{re.escape(BEGIN)}\n.*?^{re.escape(END)}\n?" +_BLOCK = re.compile(_BLOCK_PATTERN, re.DOTALL | re.MULTILINE) +# Also matches the newline that add_block puts before the block. +_BLOCK_WITH_SEPARATOR = re.compile(rf"\n?{_BLOCK_PATTERN}", re.DOTALL | re.MULTILINE) + + +def render_block(jar: Path) -> str: + """Return the block that adds *jar* to the Maven extension class path.""" + return f'{BEGIN}\nMAVEN_OPTS="$MAVEN_OPTS -Dmaven.ext.class.path={jar}"\n{END}\n' + + +def has_block(text: str) -> bool: + """Return whether *text* has the block.""" + return _BLOCK.search(text) is not None + + +def add_block(text: str | None, jar: Path) -> tuple[str, bool]: + """Return *text* with the block for *jar*, and whether it changed. + + A *text* of ``None`` means that there is no file yet. + """ + block = render_block(jar) + if not text: + return block, True + existing = _BLOCK.search(text) + if existing: + new_text = text[: existing.start()] + block + text[existing.end() :] + return new_text, new_text != text + separator = "\n" if text.endswith("\n") else "\n\n" + return text + separator + block, True + + +def remove_block(text: str) -> tuple[str | None, bool]: + """Return *text* without the block, and whether it changed. + + The text is ``None`` when nothing else is left, so the file is not needed. + """ + if not has_block(text): + return text, False + new_text = _BLOCK_WITH_SEPARATOR.sub("", text, count=1) + return (new_text if new_text.strip() else None), True diff --git a/cloudsmith_cli/credential_helpers/maven_tools.py b/cloudsmith_cli/credential_helpers/maven_tools.py index 7e199a7b..f19dec99 100644 --- a/cloudsmith_cli/credential_helpers/maven_tools.py +++ b/cloudsmith_cli/credential_helpers/maven_tools.py @@ -21,6 +21,14 @@ TIMEOUT_SECONDS = 30 +class ProjectNotFound(Exception): + """The current directory is not the root directory of a project.""" + + +class InstallOptionError(Exception): + """The helper cannot install with the given options.""" + + def _get(artifact_path: str) -> bytes: response = create_requests_session().get( REPOSITORY_URL + artifact_path, timeout=TIMEOUT_SECONDS @@ -51,18 +59,25 @@ class ConfigFileInstaller: name = "" summary = "" artifact_path = "" + install_options = ("local",) def path(self) -> Path: - """Return the config file that loads the plugin.""" + """Return the config file that loads the plugin for every build.""" + raise NotImplementedError + + def local_path(self) -> Path: + """Return the config file that loads the plugin for the current project.""" raise NotImplementedError def render(self, version: str) -> str: """Return the content of the config file for *version*.""" raise NotImplementedError - def install(self, *, dry_run: bool = False, **_options) -> list[str]: + def install( + self, *, dry_run: bool = False, local: bool = False, **_options + ) -> list[str]: + path = self.local_path() if local else self.path() version = latest_version(self.artifact_path) - path = self.path() text = self.render(version) if path.exists() and path.read_text(encoding="utf-8") == text: return [f"{path} already uses version {version} (no change)"] @@ -72,8 +87,10 @@ def install(self, *, dry_run: bool = False, **_options) -> list[str]: path.write_text(text, encoding="utf-8") return [f"wrote {path} for version {version}"] - def uninstall(self, *, dry_run: bool = False, **_options) -> list[str]: - path = self.path() + def uninstall( + self, *, dry_run: bool = False, local: bool = False, **_options + ) -> list[str]: + path = self.local_path() if local else self.path() if not path.exists(): return [f"{path} not found (nothing to remove)"] if dry_run: diff --git a/cloudsmith_cli/credential_helpers/sbt/installer.py b/cloudsmith_cli/credential_helpers/sbt/installer.py index ade02ae0..7b1fa8a6 100644 --- a/cloudsmith_cli/credential_helpers/sbt/installer.py +++ b/cloudsmith_cli/credential_helpers/sbt/installer.py @@ -5,7 +5,7 @@ from pathlib import Path -from ..maven_tools import REPOSITORY_URL, ConfigFileInstaller +from ..maven_tools import REPOSITORY_URL, ConfigFileInstaller, ProjectNotFound GLOBAL_PLUGIN = """\ // Written by `cloudsmith credential-helper install sbt`. @@ -22,5 +22,15 @@ class SbtInstaller(ConfigFileInstaller): def path(self) -> Path: return Path.home() / ".sbt" / "1.0" / "plugins" / "cloudsmith.sbt" + def local_path(self) -> Path: + # sbt loads every .sbt file in project/, so the build's own files stay as they are. + project_dir = Path.cwd() + if not (project_dir / "build.sbt").is_file(): + raise ProjectNotFound( + f"No build.sbt in {project_dir}. " + "Run the command in the root directory of an sbt project." + ) + return project_dir / "project" / "cloudsmith.sbt" + def render(self, version: str) -> str: return GLOBAL_PLUGIN.format(repository_url=REPOSITORY_URL, version=version) From c8817f8050c874411c079afb781920448341738b Mon Sep 17 00:00:00 2001 From: Ian Duffy Date: Thu, 22 Oct 2026 19:14:31 +0100 Subject: [PATCH 04/27] fix(no-ticket): keep other Maven extensions and use the command's TLS settings Addresses the review of #520. - Uninstall deleted .mvn/extensions.xml when the other extensions had spaces before the > of a tag, such as ``. The block pattern now allows these spaces. Before uninstall deletes the file, it parses what is left, and keeps the file if an element remains or if the rest is not valid XML. - The downloads of the Maven tools used a default session, so --proxy, --without-ssl-verify, and --without-ssl-verify-strict did not apply to them. `install` now passes a session with these settings to the installers that download files. The session does not get the --api-headers, because the public repository is not the API host. Co-Authored-By: Claude Opus 5.5 --- .../cli/commands/credential_helper/manage.py | 17 ++++ .../test_credential_helper_maven_tools.py | 95 ++++++++++++++++++- .../credential_helpers/gradle/installer.py | 31 ++++-- .../credential_helpers/maven/extensions.py | 21 +++- .../credential_helpers/maven/installer.py | 15 ++- .../credential_helpers/maven_tools.py | 29 ++++-- 6 files changed, 183 insertions(+), 25 deletions(-) diff --git a/cloudsmith_cli/cli/commands/credential_helper/manage.py b/cloudsmith_cli/cli/commands/credential_helper/manage.py index 1cb16ebf..d7803ae2 100644 --- a/cloudsmith_cli/cli/commands/credential_helper/manage.py +++ b/cloudsmith_cli/cli/commands/credential_helper/manage.py @@ -34,6 +34,7 @@ TerraformrcConflictError, ) +from ....core.session import create_requests_session from ....credential_helpers.docker.installer import DockerInstaller from ... import utils from ...decorators import ( @@ -96,6 +97,20 @@ def _scope_options(installer, helper: str, **requested: bool) -> dict[str, bool] return {option: True for option, given in requested.items() if given} +def _download_session(opts): + """Return a session with the proxy and TLS settings of the command. + + The session goes to the public maven-tools repository, so it does not get + the --api-headers that are for the API host. + """ + return create_requests_session( + proxy=opts.proxy, + ssl_verify=opts.ssl_verify, + ssl_verify_strict=opts.ssl_verify_strict, + user_agent=opts.api_user_agent, + ) + + def _terraform_helper_args(ctx, opts, repo: str | None = None) -> tuple[str, ...]: """Build the terraformrc ``args`` list from the resolved config. @@ -338,6 +353,8 @@ def install_cmd( "api_host": opts.api_host, **_scope_options(installer, helper, local=local, mavenrc=mavenrc), } + if getattr(installer, "downloads_files", False): + install_kwargs["session"] = _download_session(opts) # Terraform bakes the resolved org/profile into the terraformrc `args` list # so `terraform init` needs neither env vars nor a hand-edited config. The diff --git a/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py b/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py index 9fd9e9c2..f86521e2 100644 --- a/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py +++ b/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py @@ -8,6 +8,7 @@ from __future__ import annotations from pathlib import Path +from unittest.mock import Mock, patch import click.testing import httpretty @@ -73,12 +74,14 @@ def fake_repository(monkeypatch): """Serve version 0.2.0 of every artifact without network access.""" downloads = [] - def fake_download(artifact_path, destination): + def fake_download(artifact_path, destination, session=None): downloads.append(artifact_path) destination.parent.mkdir(parents=True, exist_ok=True) destination.write_text(f"content of {artifact_path}", encoding="utf-8") - monkeypatch.setattr(maven_tools, "latest_version", lambda artifact_path: "0.2.0") + monkeypatch.setattr( + maven_tools, "latest_version", lambda artifact_path, session=None: "0.2.0" + ) monkeypatch.setattr(maven_tools, "download", fake_download) return downloads @@ -125,6 +128,18 @@ def test_latest_version_reads_the_release_from_the_metadata(): assert maven_tools.latest_version(EXTENSION_PATH) == "0.2.0" +def test_latest_version_uses_the_given_session(): + """The install command's proxy and TLS settings reach the download.""" + session = Mock() + session.get.return_value.content = METADATA.encode() + + assert maven_tools.latest_version(EXTENSION_PATH, session) == "0.2.0" + session.get.assert_called_once_with( + f"{maven_tools.REPOSITORY_URL}{EXTENSION_PATH}/maven-metadata.xml", + timeout=maven_tools.TIMEOUT_SECONDS, + ) + + @httpretty.activate(allow_net_connect=False) def test_latest_version_fails_with_an_os_error_when_the_artifact_is_missing(): """A missing artifact is an OSError, which the install command reports.""" @@ -284,6 +299,44 @@ def test_remove_extension_without_the_extension_changes_nothing(): assert extensions.remove_extension(existing) == (existing, False) +def _spaced_tags(extension: str) -> str: + return extension.replace("", "").replace( + "", "" + ) + + +def test_remove_extension_keeps_an_extension_with_spaces_in_its_tags(): + """XML allows spaces before the > of a tag, so the other extension still counts.""" + other = _spaced_tags(OTHER_EXTENSION) + + assert extensions.remove_extension( + f"\n{OUR_EXTENSION}{other}\n" + ) == (f"\n{other}\n", True) + + +def test_remove_extension_keeps_a_file_that_cannot_be_parsed(): + """When the rest of the file is not valid XML, the file stays.""" + broken = " \n kr.motd.maven\n" + existing = f"\n{OUR_EXTENSION}{broken}\n" + + assert extensions.remove_extension(existing) == ( + f"\n{broken}\n", + True, + ) + + +def test_add_extension_finds_the_extension_with_spaces_in_its_tags(): + """An entry with spaces in its tags is updated, not listed a second time.""" + existing = f"\n{_spaced_tags(OUR_EXTENSION)}\n" + + text, changed = extensions.add_extension(existing, "0.2.0") + + assert changed is True + assert text == existing.replace( + "0.1.0", "0.2.0" + ) + + # --------------------------------------------------------------------------- # 3. ~/.mavenrc # --------------------------------------------------------------------------- @@ -723,3 +776,41 @@ def test_cli_install_rejects_an_option_that_the_helper_does_not_have(option, hom assert result.exit_code != 0 assert f"{option} is not supported for docker" in result.output + + +@pytest.mark.parametrize("helper", ["maven", "gradle", "sbt"]) +def test_cli_install_downloads_with_the_tls_settings_of_the_command( + helper, home, outside_projects +): + """--without-ssl-verify-strict also applies to the downloads of the Maven tools.""" + from ...cli.commands.credential_helper import manage + + download_session = Mock() + sessions = [] + + def record_latest_version(artifact_path, session=None): + sessions.append(session) + return "0.2.0" + + def record_download(artifact_path, destination, session=None): + sessions.append(session) + destination.parent.mkdir(parents=True, exist_ok=True) + destination.write_text("content", encoding="utf-8") + + with ( + patch.object(maven_tools, "latest_version", record_latest_version), + patch.object(maven_tools, "download", record_download), + patch.object( + manage, "create_requests_session", return_value=download_session + ) as create_session, + ): + result = _invoke( + "install", [helper, "--without-ssl-verify-strict", "--no-discover"] + ) + + assert result.exit_code == 0, result.output + assert create_session.call_args.kwargs["ssl_verify_strict"] is False + # The session for the public repository gets no --api-headers. + assert "headers" not in create_session.call_args.kwargs + assert sessions + assert all(session is download_session for session in sessions) diff --git a/cloudsmith_cli/credential_helpers/gradle/installer.py b/cloudsmith_cli/credential_helpers/gradle/installer.py index 41a0b4aa..7fcb282c 100644 --- a/cloudsmith_cli/credential_helpers/gradle/installer.py +++ b/cloudsmith_cli/credential_helpers/gradle/installer.py @@ -5,6 +5,7 @@ import os from pathlib import Path +from typing import TYPE_CHECKING from ..maven_tools import ( REPOSITORY_URL, @@ -13,6 +14,9 @@ latest_version, ) +if TYPE_CHECKING: + import requests + INIT_SCRIPT = """\ // Written by `cloudsmith credential-helper install gradle`. initscript {{ @@ -56,15 +60,26 @@ def path(self) -> Path: base = Path(gradle_user_home) if gradle_user_home else Path.home() / ".gradle" return base / "init.d" / "cloudsmith.init.gradle.kts" - def local_path(self) -> Path: - # Gradle has no init scripts for one build, and a script edit of the - # build's settings file could break it. - raise InstallOptionError( - SETTINGS_PLUGIN.format( - repository_url=REPOSITORY_URL, - version=latest_version(self.artifact_path), + def install( + self, + *, + local: bool = False, + session: requests.Session | None = None, + **options, + ) -> list[str]: + if local: + # Gradle has no init scripts for one build, and a script edit of the + # build's settings file could break it. + raise InstallOptionError( + SETTINGS_PLUGIN.format( + repository_url=REPOSITORY_URL, + version=latest_version(self.artifact_path, session), + ) ) - ) + return super().install(session=session, **options) + + def local_path(self) -> Path: + raise InstallOptionError("--local is not supported for gradle.") def render(self, version: str) -> str: return INIT_SCRIPT.format(repository_url=REPOSITORY_URL, version=version) diff --git a/cloudsmith_cli/credential_helpers/maven/extensions.py b/cloudsmith_cli/credential_helpers/maven/extensions.py index 74982417..e0cb11eb 100644 --- a/cloudsmith_cli/credential_helpers/maven/extensions.py +++ b/cloudsmith_cli/credential_helpers/maven/extensions.py @@ -7,11 +7,15 @@ from __future__ import annotations import re +from xml.etree import ElementTree GROUP_ID = "io.cloudsmith" ARTIFACT_ID = "cloudsmith-maven-extension" -_EXTENSION_BLOCK = re.compile(r"[ \t]*.*?[ \t]*\n?", re.DOTALL) +# XML allows spaces and new lines before the > of a tag. +_EXTENSION_BLOCK = re.compile( + r"[ \t]*.*?[ \t]*\n?", re.DOTALL +) _EMPTY_EXTENSIONS = re.compile(r"]*?)\s*/>") _VERSION = re.compile(r"[^<]*") _COMMENT = re.compile(r"", re.DOTALL) @@ -45,6 +49,15 @@ def _find_extension(masked: str) -> re.Match | None: return None +def _has_extension_element(text: str) -> bool: + """Return whether *text* has an element, or is not valid XML.""" + try: + root = ElementTree.fromstring(text) + except ElementTree.ParseError: + return True + return any(element.tag.rpartition("}")[2] == "extension" for element in root.iter()) + + def has_extension(text: str) -> bool: """Return whether *text* lists the Cloudsmith extension.""" return _find_extension(_without_comments(text)) is not None @@ -83,11 +96,13 @@ def remove_extension(text: str) -> tuple[str | None, bool]: """Return *text* without the Cloudsmith extension, and whether it changed. The text is ``None`` when no other extension is left, so the file is not needed. + A file that is not valid XML without the extension is kept. """ masked = _without_comments(text) block = _find_extension(masked) if not block: return text, False - if not _EXTENSION_BLOCK.search(masked[: block.start()] + masked[block.end() :]): + new_text = text[: block.start()] + text[block.end() :] + if not _has_extension_element(new_text): return None, True - return text[: block.start()] + text[block.end() :], True + return new_text, True diff --git a/cloudsmith_cli/credential_helpers/maven/installer.py b/cloudsmith_cli/credential_helpers/maven/installer.py index 4ec025f7..5abc6274 100644 --- a/cloudsmith_cli/credential_helpers/maven/installer.py +++ b/cloudsmith_cli/credential_helpers/maven/installer.py @@ -28,6 +28,8 @@ if TYPE_CHECKING: from collections.abc import Callable + import requests + EXTENSION_PATH = f"io/cloudsmith/{extensions.ARTIFACT_ID}" FIRST_MAVEN_WITH_USER_EXTENSIONS = (3, 10) MAVEN_VERSION_TIMEOUT_SECONDS = 60 @@ -152,6 +154,8 @@ class MavenInstaller: name = "maven" summary = "Maven extension that gets credentials from the Cloudsmith CLI" install_options = ("local", "mavenrc") + # The install command passes a session with its proxy and TLS settings. + downloads_files = True def install( self, @@ -159,14 +163,15 @@ def install( dry_run: bool = False, local: bool = False, mavenrc: bool = False, + session: requests.Session | None = None, **_options, ) -> list[str]: """Put the extension in ``~/.m2/repository`` and turn it on.""" if local and mavenrc: raise InstallOptionError("Use --local or --mavenrc, not both.") project_file = _project_extensions_file() if local else None - version = maven_tools.latest_version(EXTENSION_PATH) - actions = self._download(version, dry_run) + version = maven_tools.latest_version(EXTENSION_PATH, session) + actions = self._download(version, dry_run, session) if mavenrc: jar = _version_dir(version) / f"{extensions.ARTIFACT_ID}-{version}.jar" return actions + _add( @@ -181,7 +186,9 @@ def install( + _old_maven_warning() ) - def _download(self, version: str, dry_run: bool) -> list[str]: + def _download( + self, version: str, dry_run: bool, session: requests.Session | None + ) -> list[str]: version_dir = _version_dir(version) file_names = [ f"{extensions.ARTIFACT_ID}-{version}.{file_type}" @@ -196,7 +203,7 @@ def _download(self, version: str, dry_run: bool) -> list[str]: actions.append(f"would download {file_name} to {version_dir}") else: maven_tools.download( - f"{EXTENSION_PATH}/{version}/{file_name}", destination + f"{EXTENSION_PATH}/{version}/{file_name}", destination, session ) actions.append(f"downloaded {file_name} to {version_dir}") if not dry_run: diff --git a/cloudsmith_cli/credential_helpers/maven_tools.py b/cloudsmith_cli/credential_helpers/maven_tools.py index f19dec99..98a5f590 100644 --- a/cloudsmith_cli/credential_helpers/maven_tools.py +++ b/cloudsmith_cli/credential_helpers/maven_tools.py @@ -17,6 +17,8 @@ if TYPE_CHECKING: from pathlib import Path + import requests + REPOSITORY_URL = "https://dl.cloudsmith.io/public/cloudsmith/maven-tools/maven/" TIMEOUT_SECONDS = 30 @@ -29,26 +31,30 @@ class InstallOptionError(Exception): """The helper cannot install with the given options.""" -def _get(artifact_path: str) -> bytes: - response = create_requests_session().get( +def _get(artifact_path: str, session: requests.Session | None) -> bytes: + response = (session or create_requests_session()).get( REPOSITORY_URL + artifact_path, timeout=TIMEOUT_SECONDS ) response.raise_for_status() return response.content -def latest_version(artifact_path: str) -> str: +def latest_version(artifact_path: str, session: requests.Session | None = None) -> str: """Return the newest release of the artifact at *artifact_path*.""" - metadata = ElementTree.fromstring(_get(f"{artifact_path}/maven-metadata.xml")) + metadata = ElementTree.fromstring( + _get(f"{artifact_path}/maven-metadata.xml", session) + ) release = metadata.findtext("versioning/release") if not release: raise OSError(f"{REPOSITORY_URL}{artifact_path} has no release") return release -def download(artifact_path: str, destination: Path) -> None: +def download( + artifact_path: str, destination: Path, session: requests.Session | None = None +) -> None: """Download the file at *artifact_path* to *destination*.""" - content = _get(artifact_path) + content = _get(artifact_path, session) destination.parent.mkdir(parents=True, exist_ok=True) destination.write_bytes(content) @@ -60,6 +66,8 @@ class ConfigFileInstaller: summary = "" artifact_path = "" install_options = ("local",) + # The install command passes a session with its proxy and TLS settings. + downloads_files = True def path(self) -> Path: """Return the config file that loads the plugin for every build.""" @@ -74,10 +82,15 @@ def render(self, version: str) -> str: raise NotImplementedError def install( - self, *, dry_run: bool = False, local: bool = False, **_options + self, + *, + dry_run: bool = False, + local: bool = False, + session: requests.Session | None = None, + **_options, ) -> list[str]: path = self.local_path() if local else self.path() - version = latest_version(self.artifact_path) + version = latest_version(self.artifact_path, session) text = self.render(version) if path.exists() and path.read_text(encoding="utf-8") == text: return [f"{path} already uses version {version} (no change)"] From 60aa3fe4311350874ed86d7d19d8d42eadf24760 Mon Sep 17 00:00:00 2001 From: Ian Duffy Date: Wed, 28 Oct 2026 12:04:27 +0000 Subject: [PATCH 05/27] refactor(no-ticket): let Click check the Maven and sbt options --local and --mavenrc now name the helpers that accept them. The install and uninstall commands show a usage error for another helper before the installer runs. Click also rejects --local together with --mavenrc. `install gradle --local` is now a usage error. It no longer gets the latest version only to show a settings.gradle.kts block. Co-Authored-By: Claude Opus 5.5 --- .../cli/commands/credential_helper/manage.py | 48 +++++++---- .../test_credential_helper_maven_tools.py | 84 ++++++------------- .../credential_helpers/gradle/installer.py | 48 +---------- .../credential_helpers/maven/installer.py | 5 +- .../credential_helpers/maven_tools.py | 5 -- 5 files changed, 62 insertions(+), 128 deletions(-) diff --git a/cloudsmith_cli/cli/commands/credential_helper/manage.py b/cloudsmith_cli/cli/commands/credential_helper/manage.py index 190f2744..bb99e20e 100644 --- a/cloudsmith_cli/cli/commands/credential_helper/manage.py +++ b/cloudsmith_cli/cli/commands/credential_helper/manage.py @@ -18,10 +18,7 @@ from cloudsmith_cli.credential_helpers.gradle.installer import GradleInstaller from cloudsmith_cli.credential_helpers.maven.extensions import ExtensionsFileError from cloudsmith_cli.credential_helpers.maven.installer import MavenInstaller -from cloudsmith_cli.credential_helpers.maven_tools import ( - InstallOptionError, - ProjectNotFound, -) +from cloudsmith_cli.credential_helpers.maven_tools import ProjectNotFound from cloudsmith_cli.credential_helpers.pnpm.installer import PNPMInstaller from cloudsmith_cli.credential_helpers.sbt.installer import SbtInstaller from cloudsmith_cli.credential_helpers.terraform.installer import ( @@ -88,13 +85,26 @@ def _get_installer(name: str): return cls() -def _scope_options(installer, helper: str, **requested: bool) -> dict[str, bool]: - """Return the scope options that were given, or fail if *helper* has no such option.""" - supported = getattr(installer, "install_options", ()) - for option, given in requested.items(): - if given and option not in supported: - raise click.UsageError(f"--{option} is not supported for {helper}.") - return {option: True for option, given in requested.items() if given} +class HelperOption(click.Option): + """A flag that only the helpers in *helpers* accept.""" + + def __init__(self, *args, helpers: tuple[str, ...], **kwargs): + super().__init__(*args, **kwargs) + self.helpers = helpers + + +def _helper_options(ctx: click.Context, helper: str) -> dict[str, bool]: + """Return the helper flags that were given, or fail if *helper* does not accept one.""" + given = {} + for param in ctx.command.params: + if not isinstance(param, HelperOption) or not ctx.params[param.name]: + continue + if helper not in param.helpers: + raise click.UsageError( + f"{param.opts[0]} applies only to {' and '.join(param.helpers)}.", ctx + ) + given[param.name] = True + return given def _download_session(opts): @@ -278,6 +288,8 @@ def _terraform_workspace_next_steps(helper_args: tuple[str, ...]) -> list[str]: ) @click.option( "--local", + cls=HelperOption, + helpers=("maven", "sbt"), is_flag=True, default=False, help="Maven and sbt only: set up the project in the current directory " @@ -285,6 +297,8 @@ def _terraform_workspace_next_steps(helper_args: tuple[str, ...]) -> list[str]: ) @click.option( "--mavenrc", + cls=HelperOption, + helpers=("maven",), is_flag=True, default=False, help="Maven only: load the extension through ~/.mavenrc instead of " @@ -341,6 +355,9 @@ def install_cmd( $ cloudsmith credential-helper install terraform -w acme --repo my-repo """ installer = _get_installer(helper) + helper_options = _helper_options(ctx, helper) + if local and mavenrc: + raise click.UsageError("Use --local or --mavenrc, not both.", ctx) install_kwargs = { "bin_dir": bin_dir, @@ -351,7 +368,7 @@ def install_cmd( "org": opts.org, "credential": opts.credential, "api_host": opts.api_host, - **_scope_options(installer, helper, local=local, mavenrc=mavenrc), + **helper_options, } if getattr(installer, "downloads_files", False): install_kwargs["session"] = _download_session(opts) @@ -371,7 +388,6 @@ def install_cmd( TerraformHelperExeNotFound, TerraformPluginDirError, ProjectNotFound, - InstallOptionError, ExtensionsFileError, ) as exc: raise click.ClickException(str(exc)) @@ -439,6 +455,8 @@ def install_cmd( ) @click.option( "--local", + cls=HelperOption, + helpers=("maven", "sbt"), is_flag=True, default=False, help="Maven and sbt only: remove the setup of the project in the current " @@ -469,9 +487,9 @@ def uninstall_cmd( actions = installer.uninstall( bin_dir=bin_dir, dry_run=dry_run, - **_scope_options(installer, helper, local=local), + **_helper_options(ctx, helper), ) - except (ProjectNotFound, InstallOptionError) as exc: + except ProjectNotFound as exc: raise click.ClickException(str(exc)) except OSError as exc: raise click.ClickException( diff --git a/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py b/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py index f86521e2..2fb3bbd0 100644 --- a/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py +++ b/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py @@ -20,7 +20,7 @@ from ...credential_helpers.maven import extensions, mavenrc from ...credential_helpers.maven import installer as maven_installer from ...credential_helpers.maven.installer import MavenInstaller -from ...credential_helpers.maven_tools import InstallOptionError, ProjectNotFound +from ...credential_helpers.maven_tools import ProjectNotFound from ...credential_helpers.sbt.installer import SbtInstaller EXTENSION_PATH = "io/cloudsmith/cloudsmith-maven-extension" @@ -128,18 +128,6 @@ def test_latest_version_reads_the_release_from_the_metadata(): assert maven_tools.latest_version(EXTENSION_PATH) == "0.2.0" -def test_latest_version_uses_the_given_session(): - """The install command's proxy and TLS settings reach the download.""" - session = Mock() - session.get.return_value.content = METADATA.encode() - - assert maven_tools.latest_version(EXTENSION_PATH, session) == "0.2.0" - session.get.assert_called_once_with( - f"{maven_tools.REPOSITORY_URL}{EXTENSION_PATH}/maven-metadata.xml", - timeout=maven_tools.TIMEOUT_SECONDS, - ) - - @httpretty.activate(allow_net_connect=False) def test_latest_version_fails_with_an_os_error_when_the_artifact_is_missing(): """A missing artifact is an OSError, which the install command reports.""" @@ -235,21 +223,6 @@ def test_add_extension_ignores_a_commented_out_extension(): ) -def test_add_extension_finds_the_extension_with_spaces_in_the_tags(): - """Maven trims the values, so spaces inside a tag do not make a second entry.""" - spaced = OUR_EXTENSION.replace( - "io.cloudsmith", " io.cloudsmith " - ) - existing = f"\n{spaced}\n" - - text, changed = extensions.add_extension(existing, "0.2.0") - - assert changed is True - assert text == existing.replace( - "0.1.0", "0.2.0" - ) - - def test_add_extension_ignores_a_version_in_a_comment(): """Only the element of the extension changes.""" existing = f"\n{OUR_EXTENSION}\n".replace( @@ -326,8 +299,11 @@ def test_remove_extension_keeps_a_file_that_cannot_be_parsed(): def test_add_extension_finds_the_extension_with_spaces_in_its_tags(): - """An entry with spaces in its tags is updated, not listed a second time.""" - existing = f"\n{_spaced_tags(OUR_EXTENSION)}\n" + """An entry with spaces in its tags or values is updated, not listed a second time.""" + spaced = _spaced_tags(OUR_EXTENSION).replace( + "io.cloudsmith", " io.cloudsmith " + ) + existing = f"\n{spaced}\n" text, changed = extensions.add_extension(existing, "0.2.0") @@ -530,14 +506,6 @@ def test_maven_install_with_mavenrc_adds_a_block( assert not _user_extensions(home).exists() -def test_maven_install_with_local_and_mavenrc_fails( - home, fake_repository, maven_project -): - """The two options set up different scopes, so only one is allowed.""" - with pytest.raises(InstallOptionError): - MavenInstaller().install(local=True, mavenrc=True) - - def test_maven_install_dry_run_changes_nothing(home, fake_repository, maven_project): """A dry run reports the planned changes and writes no files.""" actions = [ @@ -631,14 +599,6 @@ def test_gradle_install_uses_gradle_user_home( assert (tmp_path / "gradle-home" / "init.d" / "cloudsmith.init.gradle.kts").exists() -def test_gradle_install_local_explains_the_settings_plugin(home, fake_repository): - """Gradle has no project init scripts, so the error shows what to add instead.""" - with pytest.raises(InstallOptionError, match="settings.gradle.kts"): - GradleInstaller().install(local=True) - - assert not (home / ".gradle").exists() - - def test_gradle_uninstall_and_status(home, fake_repository): """Uninstall removes the init script, and status follows it.""" script = home / ".gradle" / "init.d" / "cloudsmith.init.gradle.kts" @@ -761,21 +721,31 @@ def test_cli_install_maven_local_outside_a_project_fails_cleanly( assert "pom.xml" in result.output -def test_cli_install_gradle_local_shows_the_settings_plugin(home, fake_repository): - """The error shows the plugins block to add to settings.gradle.kts.""" - result = _invoke("install", ["gradle", "--local", "--no-discover"]) +@pytest.mark.parametrize( + ("command_name", "helper", "option"), + [ + ("install", "docker", "--local"), + ("install", "gradle", "--local"), + ("install", "sbt", "--mavenrc"), + ("uninstall", "gradle", "--local"), + ], +) +def test_cli_rejects_an_option_that_the_helper_does_not_have( + command_name, helper, option, home +): + """Click stops the command before the installer runs.""" + result = _invoke(command_name, [helper, option]) - assert result.exit_code != 0 - assert 'id("io.cloudsmith.gradle-plugin")' in result.output + assert result.exit_code == 2 + assert f"{option} applies only to" in result.output -@pytest.mark.parametrize("option", ["--local", "--mavenrc"]) -def test_cli_install_rejects_an_option_that_the_helper_does_not_have(option, home): - """Other helpers fail with a clear error, not with a TypeError.""" - result = _invoke("install", ["docker", option, "--no-discover"]) +def test_cli_install_rejects_local_with_mavenrc(home): + """The two options set up different scopes, so only one is allowed.""" + result = _invoke("install", ["maven", "--local", "--mavenrc"]) - assert result.exit_code != 0 - assert f"{option} is not supported for docker" in result.output + assert result.exit_code == 2 + assert "--local or --mavenrc, not both" in result.output @pytest.mark.parametrize("helper", ["maven", "gradle", "sbt"]) diff --git a/cloudsmith_cli/credential_helpers/gradle/installer.py b/cloudsmith_cli/credential_helpers/gradle/installer.py index 7fcb282c..ec362119 100644 --- a/cloudsmith_cli/credential_helpers/gradle/installer.py +++ b/cloudsmith_cli/credential_helpers/gradle/installer.py @@ -5,17 +5,8 @@ import os from pathlib import Path -from typing import TYPE_CHECKING -from ..maven_tools import ( - REPOSITORY_URL, - ConfigFileInstaller, - InstallOptionError, - latest_version, -) - -if TYPE_CHECKING: - import requests +from ..maven_tools import REPOSITORY_URL, ConfigFileInstaller INIT_SCRIPT = """\ // Written by `cloudsmith credential-helper install gradle`. @@ -33,22 +24,6 @@ }} """ -SETTINGS_PLUGIN = """\ ---local is not supported for gradle. To set up one build, add the plugin to \ -settings.gradle.kts: - -pluginManagement {{ - repositories {{ - maven {{ url = uri("{repository_url}") }} - gradlePluginPortal() - }} -}} - -plugins {{ - id("io.cloudsmith.gradle-plugin") version "{version}" -}} -""" - class GradleInstaller(ConfigFileInstaller): name = "gradle" @@ -60,26 +35,5 @@ def path(self) -> Path: base = Path(gradle_user_home) if gradle_user_home else Path.home() / ".gradle" return base / "init.d" / "cloudsmith.init.gradle.kts" - def install( - self, - *, - local: bool = False, - session: requests.Session | None = None, - **options, - ) -> list[str]: - if local: - # Gradle has no init scripts for one build, and a script edit of the - # build's settings file could break it. - raise InstallOptionError( - SETTINGS_PLUGIN.format( - repository_url=REPOSITORY_URL, - version=latest_version(self.artifact_path, session), - ) - ) - return super().install(session=session, **options) - - def local_path(self) -> Path: - raise InstallOptionError("--local is not supported for gradle.") - def render(self, version: str) -> str: return INIT_SCRIPT.format(repository_url=REPOSITORY_URL, version=version) diff --git a/cloudsmith_cli/credential_helpers/maven/installer.py b/cloudsmith_cli/credential_helpers/maven/installer.py index 5abc6274..71d591a5 100644 --- a/cloudsmith_cli/credential_helpers/maven/installer.py +++ b/cloudsmith_cli/credential_helpers/maven/installer.py @@ -21,7 +21,7 @@ from typing import TYPE_CHECKING from .. import maven_tools -from ..maven_tools import InstallOptionError, ProjectNotFound +from ..maven_tools import ProjectNotFound from . import extensions from .mavenrc import add_block, has_block, remove_block @@ -153,7 +153,6 @@ def _remove(path: Path, edit: Callable, dry_run: bool) -> list[str]: class MavenInstaller: name = "maven" summary = "Maven extension that gets credentials from the Cloudsmith CLI" - install_options = ("local", "mavenrc") # The install command passes a session with its proxy and TLS settings. downloads_files = True @@ -167,8 +166,6 @@ def install( **_options, ) -> list[str]: """Put the extension in ``~/.m2/repository`` and turn it on.""" - if local and mavenrc: - raise InstallOptionError("Use --local or --mavenrc, not both.") project_file = _project_extensions_file() if local else None version = maven_tools.latest_version(EXTENSION_PATH, session) actions = self._download(version, dry_run, session) diff --git a/cloudsmith_cli/credential_helpers/maven_tools.py b/cloudsmith_cli/credential_helpers/maven_tools.py index 98a5f590..1882ced7 100644 --- a/cloudsmith_cli/credential_helpers/maven_tools.py +++ b/cloudsmith_cli/credential_helpers/maven_tools.py @@ -27,10 +27,6 @@ class ProjectNotFound(Exception): """The current directory is not the root directory of a project.""" -class InstallOptionError(Exception): - """The helper cannot install with the given options.""" - - def _get(artifact_path: str, session: requests.Session | None) -> bytes: response = (session or create_requests_session()).get( REPOSITORY_URL + artifact_path, timeout=TIMEOUT_SECONDS @@ -65,7 +61,6 @@ class ConfigFileInstaller: name = "" summary = "" artifact_path = "" - install_options = ("local",) # The install command passes a session with its proxy and TLS settings. downloads_files = True From a821cf52bee5757b2e250eb7c6cd8c68d2106fe9 Mon Sep 17 00:00:00 2001 From: Ian Duffy Date: Tue, 3 Nov 2026 10:43:41 +0000 Subject: [PATCH 06/27] Keep other extensions in the Maven extension class path The JVM uses only the last -Dmaven.ext.class.path value. The ~/.mavenrc block now adds the JAR to that value instead of a second option. Co-Authored-By: Claude Opus 5.5 --- .../test_credential_helper_maven_tools.py | 49 +++++++++++++++++-- .../credential_helpers/maven/mavenrc.py | 15 +++++- 2 files changed, 58 insertions(+), 6 deletions(-) diff --git a/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py b/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py index 2fb3bbd0..23e042a9 100644 --- a/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py +++ b/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py @@ -7,6 +7,8 @@ from __future__ import annotations +import os +import subprocess from pathlib import Path from unittest.mock import Mock, patch @@ -318,11 +320,7 @@ def test_add_extension_finds_the_extension_with_spaces_in_its_tags(): # --------------------------------------------------------------------------- JAR = Path("/home/me/.m2/repository/cloudsmith-maven-extension-0.2.0.jar") -BLOCK = ( - "# >>> cloudsmith credential-helper install maven >>>\n" - f'MAVEN_OPTS="$MAVEN_OPTS -Dmaven.ext.class.path={JAR}"\n' - "# <<< cloudsmith credential-helper install maven <<<\n" -) +BLOCK = mavenrc.render_block(JAR) def test_add_block_makes_a_new_file(): @@ -372,6 +370,47 @@ def test_remove_block_without_a_block_changes_nothing(): assert mavenrc.remove_block("export A=1\n") == ("export A=1\n", False) +def _maven_opts_after_block(maven_opts: str) -> str: + """Run the block in the shell that the mvn script uses, and return MAVEN_OPTS.""" + script = f'MAVEN_OPTS="$1"\n{BLOCK}printf %s "$MAVEN_OPTS"\n' + result = subprocess.run( + ["sh", "-c", script, "sh", maven_opts], + capture_output=True, + text=True, + check=True, + ) + return result.stdout + + +POSIX_SHELL_ONLY = pytest.mark.skipif( + os.name == "nt", reason="The mvn script that reads ~/.mavenrc is a POSIX script." +) + + +@POSIX_SHELL_ONLY +def test_block_adds_the_class_path_to_maven_opts(): + """Without a class path in MAVEN_OPTS, the block adds one with only the JAR.""" + assert _maven_opts_after_block("-Xmx1g") == f"-Xmx1g -Dmaven.ext.class.path={JAR}" + + +@POSIX_SHELL_ONLY +def test_block_keeps_the_other_extensions_in_the_class_path(): + """Java uses only the last -D value, so the block adds the JAR to that value.""" + maven_opts = "-Xmx1g -Dmaven.ext.class.path=/opt/other.jar -Dquiet=true" + + assert _maven_opts_after_block(maven_opts) == ( + f"-Xmx1g -Dmaven.ext.class.path={JAR}{os.pathsep}/opt/other.jar -Dquiet=true" + ) + + +@POSIX_SHELL_ONLY +def test_block_does_not_add_the_jar_twice(): + """A nested mvn gets MAVEN_OPTS with the JAR, so the block leaves it as it is.""" + maven_opts = f"-Dmaven.ext.class.path={JAR}{os.pathsep}/opt/other.jar" + + assert _maven_opts_after_block(maven_opts) == maven_opts + + # --------------------------------------------------------------------------- # 4. MavenInstaller # --------------------------------------------------------------------------- diff --git a/cloudsmith_cli/credential_helpers/maven/mavenrc.py b/cloudsmith_cli/credential_helpers/maven/mavenrc.py index 58da8f2e..d9bbbf1f 100644 --- a/cloudsmith_cli/credential_helpers/maven/mavenrc.py +++ b/cloudsmith_cli/credential_helpers/maven/mavenrc.py @@ -8,6 +8,7 @@ from __future__ import annotations +import os import re from typing import TYPE_CHECKING @@ -25,7 +26,19 @@ def render_block(jar: Path) -> str: """Return the block that adds *jar* to the Maven extension class path.""" - return f'{BEGIN}\nMAVEN_OPTS="$MAVEN_OPTS -Dmaven.ext.class.path={jar}"\n{END}\n' + option = "-Dmaven.ext.class.path=" + return ( + f"{BEGIN}\n" + "# Java uses only the last -D value, so add the JAR to that value.\n" + 'case "$MAVEN_OPTS" in\n' + f' *"{jar}"*) ;;\n' + f" *{option}*)\n" + f' MAVEN_OPTS="${{MAVEN_OPTS%{option}*}}{option}{jar}{os.pathsep}' + f'${{MAVEN_OPTS##*{option}}}" ;;\n' + f' *) MAVEN_OPTS="$MAVEN_OPTS {option}{jar}" ;;\n' + "esac\n" + f"{END}\n" + ) def has_block(text: str) -> bool: From 9d6bb256e959ce06b7f4dd8b5d964d44a93ab4b9 Mon Sep 17 00:00:00 2001 From: Ian Duffy Date: Tue, 3 Nov 2026 10:44:54 +0000 Subject: [PATCH 07/27] Keep Gradle and sbt settings that install did not write Install and uninstall now stop with an error when the config file has content other than the file that install writes. Co-Authored-By: Claude Opus 5.5 --- .../cli/commands/credential_helper/manage.py | 8 +- .../test_credential_helper_maven_tools.py | 73 ++++++++++++++++++- .../credential_helpers/maven_tools.py | 28 ++++++- 3 files changed, 104 insertions(+), 5 deletions(-) diff --git a/cloudsmith_cli/cli/commands/credential_helper/manage.py b/cloudsmith_cli/cli/commands/credential_helper/manage.py index 805ad61e..e36c35f8 100644 --- a/cloudsmith_cli/cli/commands/credential_helper/manage.py +++ b/cloudsmith_cli/cli/commands/credential_helper/manage.py @@ -19,7 +19,10 @@ from cloudsmith_cli.credential_helpers.gradle.installer import GradleInstaller from cloudsmith_cli.credential_helpers.maven.extensions import ExtensionsFileError from cloudsmith_cli.credential_helpers.maven.installer import MavenInstaller -from cloudsmith_cli.credential_helpers.maven_tools import ProjectNotFound +from cloudsmith_cli.credential_helpers.maven_tools import ( + ConfigFileConflict, + ProjectNotFound, +) from cloudsmith_cli.credential_helpers.pnpm.installer import PNPMInstaller from cloudsmith_cli.credential_helpers.sbt.installer import SbtInstaller from cloudsmith_cli.credential_helpers.terraform.installer import ( @@ -391,6 +394,7 @@ def install_cmd( TerraformPluginDirError, ProjectNotFound, ExtensionsFileError, + ConfigFileConflict, ) as exc: raise click.ClickException(str(exc)) except OSError as exc: @@ -491,7 +495,7 @@ def uninstall_cmd( dry_run=dry_run, **_helper_options(ctx, helper), ) - except (ConfigFileParseError, ProjectNotFound) as exc: + except (ConfigFileParseError, ProjectNotFound, ConfigFileConflict) as exc: raise click.ClickException(str(exc)) except OSError as exc: raise click.ClickException( diff --git a/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py b/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py index 23e042a9..a208149f 100644 --- a/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py +++ b/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py @@ -22,7 +22,7 @@ from ...credential_helpers.maven import extensions, mavenrc from ...credential_helpers.maven import installer as maven_installer from ...credential_helpers.maven.installer import MavenInstaller -from ...credential_helpers.maven_tools import ProjectNotFound +from ...credential_helpers.maven_tools import ConfigFileConflict, ProjectNotFound from ...credential_helpers.sbt.installer import SbtInstaller EXTENSION_PATH = "io/cloudsmith/cloudsmith-maven-extension" @@ -650,6 +650,31 @@ def test_gradle_uninstall_and_status(home, fake_repository): assert GradleInstaller().status() == {"launcher": None, "hosts": []} +def test_gradle_install_upgrades_an_init_script_that_it_wrote(home, fake_repository): + """An init script for an older version gets the new version.""" + script = home / ".gradle" / "init.d" / "cloudsmith.init.gradle.kts" + script.parent.mkdir(parents=True) + script.write_text(GradleInstaller().render("0.1.0"), encoding="utf-8") + + GradleInstaller().install() + + assert script.read_text(encoding="utf-8") == GradleInstaller().render("0.2.0") + + +def test_gradle_uninstall_keeps_an_init_script_with_other_settings( + home, fake_repository +): + """Uninstall stops before it removes settings that install did not write.""" + script = home / ".gradle" / "init.d" / "cloudsmith.init.gradle.kts" + script.parent.mkdir(parents=True) + script.write_text('println("hello")\n', encoding="utf-8") + + with pytest.raises(ConfigFileConflict, match="did not write"): + GradleInstaller().uninstall() + + assert script.read_text(encoding="utf-8") == 'println("hello")\n' + + # --------------------------------------------------------------------------- # 6. SbtInstaller # --------------------------------------------------------------------------- @@ -697,6 +722,37 @@ def test_sbt_install_local_outside_a_project_fails( assert not (outside_projects / "project").exists() +def test_sbt_install_keeps_a_plugin_file_with_other_settings( + home, fake_repository, outside_projects +): + """Install stops before it replaces settings that it did not write.""" + (outside_projects / "build.sbt").write_text("", encoding="utf-8") + plugin_file = outside_projects / "project" / "cloudsmith.sbt" + plugin_file.parent.mkdir() + other_plugin = 'addSbtPlugin("com.example" % "sbt-other" % "1.0.0")\n' + plugin_file.write_text(other_plugin, encoding="utf-8") + + with pytest.raises(ConfigFileConflict, match="did not write"): + SbtInstaller().install(local=True) + + assert plugin_file.read_text(encoding="utf-8") == other_plugin + + +def test_sbt_install_keeps_lines_added_to_its_plugin_file(home, fake_repository): + """A line that the user adds to the file stops a later install and uninstall.""" + plugin_file = home / ".sbt" / "1.0" / "plugins" / "cloudsmith.sbt" + plugin_file.parent.mkdir(parents=True) + edited = SbtInstaller().render("0.1.0") + 'addSbtPlugin("a" % "b" % "1")\n' + plugin_file.write_text(edited, encoding="utf-8") + + with pytest.raises(ConfigFileConflict): + SbtInstaller().install() + with pytest.raises(ConfigFileConflict): + SbtInstaller().uninstall() + + assert plugin_file.read_text(encoding="utf-8") == edited + + def test_sbt_uninstall_and_status(home, fake_repository): """Uninstall removes the global plugin, and status follows it.""" plugin_file = home / ".sbt" / "1.0" / "plugins" / "cloudsmith.sbt" @@ -760,6 +816,21 @@ def test_cli_install_maven_local_outside_a_project_fails_cleanly( assert "pom.xml" in result.output +@pytest.mark.parametrize("command_name", ["install", "uninstall"]) +def test_cli_with_a_config_file_that_install_did_not_write_fails_cleanly( + command_name, home, fake_repository, outside_projects +): + """The error names the file, without a traceback.""" + plugin_file = home / ".sbt" / "1.0" / "plugins" / "cloudsmith.sbt" + plugin_file.parent.mkdir(parents=True) + plugin_file.write_text("// my settings\n", encoding="utf-8") + + result = _invoke(command_name, ["sbt"]) + + assert result.exit_code == 1 + assert str(plugin_file) in result.output + + @pytest.mark.parametrize( ("command_name", "helper", "option"), [ diff --git a/cloudsmith_cli/credential_helpers/maven_tools.py b/cloudsmith_cli/credential_helpers/maven_tools.py index 1882ced7..7e06a437 100644 --- a/cloudsmith_cli/credential_helpers/maven_tools.py +++ b/cloudsmith_cli/credential_helpers/maven_tools.py @@ -9,6 +9,7 @@ from __future__ import annotations +import re from typing import TYPE_CHECKING from xml.etree import ElementTree @@ -27,6 +28,10 @@ class ProjectNotFound(Exception): """The current directory is not the root directory of a project.""" +class ConfigFileConflict(Exception): + """The config file has content that the installer did not write.""" + + def _get(artifact_path: str, session: requests.Session | None) -> bytes: response = (session or create_requests_session()).get( REPOSITORY_URL + artifact_path, timeout=TIMEOUT_SECONDS @@ -76,6 +81,25 @@ def render(self, version: str) -> str: """Return the content of the config file for *version*.""" raise NotImplementedError + def _written_by_install(self, text: str) -> bool: + placeholder = "\0" + template = self.render(placeholder) + before, after = (re.escape(part) for part in template.split(placeholder)) + return re.fullmatch(rf"{before}[^\s\"]+{after}", text) is not None + + def _read_own_file(self, path: Path) -> str | None: + """Return the text of *path*, or ``None`` when there is no file.""" + if not path.exists(): + return None + text = path.read_text(encoding="utf-8") + if not self._written_by_install(text): + raise ConfigFileConflict( + f"{path} has settings that `cloudsmith credential-helper install " + f"{self.name}` did not write. Move them to a different file, " + "and then run the command again." + ) + return text + def install( self, *, @@ -87,7 +111,7 @@ def install( path = self.local_path() if local else self.path() version = latest_version(self.artifact_path, session) text = self.render(version) - if path.exists() and path.read_text(encoding="utf-8") == text: + if self._read_own_file(path) == text: return [f"{path} already uses version {version} (no change)"] if dry_run: return [f"would write {path} for version {version}"] @@ -99,7 +123,7 @@ def uninstall( self, *, dry_run: bool = False, local: bool = False, **_options ) -> list[str]: path = self.local_path() if local else self.path() - if not path.exists(): + if self._read_own_file(path) is None: return [f"{path} not found (nothing to remove)"] if dry_run: return [f"would remove {path}"] From 7e9456a515f51a93fa7a49116d371c7aea744e15 Mon Sep 17 00:00:00 2001 From: Ian Duffy Date: Tue, 3 Nov 2026 10:45:22 +0000 Subject: [PATCH 08/27] List the sbt plugin file of the current project Status now also checks project/cloudsmith.sbt, so list reports an install that used --local. Co-Authored-By: Claude Opus 5.5 --- .../tests/test_credential_helper_maven_tools.py | 14 ++++++++++++++ cloudsmith_cli/credential_helpers/maven_tools.py | 9 +++++++-- 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py b/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py index a208149f..b0c8bee2 100644 --- a/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py +++ b/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py @@ -765,6 +765,20 @@ def test_sbt_uninstall_and_status(home, fake_repository): assert SbtInstaller().status() == {"launcher": None, "hosts": []} +def test_sbt_status_names_the_plugin_file_of_the_project( + home, fake_repository, outside_projects +): + """After install --local, status names project/cloudsmith.sbt.""" + assert SbtInstaller().status() == {"launcher": None, "hosts": []} + (outside_projects / "build.sbt").write_text("", encoding="utf-8") + SbtInstaller().install(local=True) + + assert SbtInstaller().status() == { + "launcher": str(outside_projects / "project" / "cloudsmith.sbt"), + "hosts": [], + } + + # --------------------------------------------------------------------------- # 7. CLI wiring # --------------------------------------------------------------------------- diff --git a/cloudsmith_cli/credential_helpers/maven_tools.py b/cloudsmith_cli/credential_helpers/maven_tools.py index 7e06a437..3bf19e16 100644 --- a/cloudsmith_cli/credential_helpers/maven_tools.py +++ b/cloudsmith_cli/credential_helpers/maven_tools.py @@ -9,6 +9,7 @@ from __future__ import annotations +import contextlib import re from typing import TYPE_CHECKING from xml.etree import ElementTree @@ -131,5 +132,9 @@ def uninstall( return [f"removed {path}"] def status(self) -> dict: - path = self.path() - return {"launcher": str(path) if path.exists() else None, "hosts": []} + paths = [self.path()] + # Gradle has no local config, and the current directory is often not a project. + with contextlib.suppress(NotImplementedError, ProjectNotFound): + paths.append(self.local_path()) + path = next((path for path in paths if path.exists()), None) + return {"launcher": str(path) if path else None, "hosts": []} From 771b1687a322c9ee6e3be5f1fac986a1ce00d2dd Mon Sep 17 00:00:00 2001 From: Ian Duffy Date: Tue, 3 Nov 2026 10:55:22 +0000 Subject: [PATCH 09/27] Look for the JAR only in the last Maven class path value Java uses only the last -Dmaven.ext.class.path value. A JAR in an earlier value or in a different option does not load the extension. Co-Authored-By: Claude Opus 5.5 --- .../test_credential_helper_maven_tools.py | 24 +++++++++++++++++++ .../credential_helpers/maven/mavenrc.py | 11 ++++++--- 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py b/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py index b0c8bee2..16149c76 100644 --- a/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py +++ b/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py @@ -403,6 +403,30 @@ def test_block_keeps_the_other_extensions_in_the_class_path(): ) +@POSIX_SHELL_ONLY +@pytest.mark.parametrize( + ("maven_opts", "expected"), + [ + ( + f"-Dmaven.ext.class.path={JAR} -Dmaven.ext.class.path=/opt/other.jar", + f"-Dmaven.ext.class.path={JAR} " + f"-Dmaven.ext.class.path={JAR}{os.pathsep}/opt/other.jar", + ), + ( + f"-Dmaven.ext.class.path=/opt/other.jar -Dnote={JAR}", + f"-Dmaven.ext.class.path={JAR}{os.pathsep}/opt/other.jar -Dnote={JAR}", + ), + ( + f"-Dmaven.ext.class.path={JAR}.old", + f"-Dmaven.ext.class.path={JAR}{os.pathsep}{JAR}.old", + ), + ], +) +def test_block_looks_for_the_jar_only_in_the_last_class_path(maven_opts, expected): + """Java uses only the last -D value, so the JAR in other text does not count.""" + assert _maven_opts_after_block(maven_opts) == expected + + @POSIX_SHELL_ONLY def test_block_does_not_add_the_jar_twice(): """A nested mvn gets MAVEN_OPTS with the JAR, so the block leaves it as it is.""" diff --git a/cloudsmith_cli/credential_helpers/maven/mavenrc.py b/cloudsmith_cli/credential_helpers/maven/mavenrc.py index d9bbbf1f..dddf11b4 100644 --- a/cloudsmith_cli/credential_helpers/maven/mavenrc.py +++ b/cloudsmith_cli/credential_helpers/maven/mavenrc.py @@ -27,14 +27,19 @@ def render_block(jar: Path) -> str: """Return the block that adds *jar* to the Maven extension class path.""" option = "-Dmaven.ext.class.path=" + after_last = "cloudsmith_after_class_path" return ( f"{BEGIN}\n" "# Java uses only the last -D value, so add the JAR to that value.\n" 'case "$MAVEN_OPTS" in\n' - f' *"{jar}"*) ;;\n' f" *{option}*)\n" - f' MAVEN_OPTS="${{MAVEN_OPTS%{option}*}}{option}{jar}{os.pathsep}' - f'${{MAVEN_OPTS##*{option}}}" ;;\n' + f' {after_last}="${{MAVEN_OPTS##*{option}}}"\n' + f' case "{os.pathsep}${{{after_last}%% *}}{os.pathsep}" in\n' + f' *"{os.pathsep}{jar}{os.pathsep}"*) ;;\n' + f' *) MAVEN_OPTS="${{MAVEN_OPTS%{option}*}}{option}{jar}{os.pathsep}' + f'${after_last}" ;;\n' + " esac\n" + f" unset {after_last} ;;\n" f' *) MAVEN_OPTS="$MAVEN_OPTS {option}{jar}" ;;\n' "esac\n" f"{END}\n" From 36c60c74bb48508d12233a13be5be38cea536de6 Mon Sep 17 00:00:00 2001 From: Ian Duffy Date: Tue, 3 Nov 2026 10:56:04 +0000 Subject: [PATCH 10/27] Find the Maven extension with spaces in its child tags XML allows spaces before the > of a tag. Install, uninstall, and status now find groupId, artifactId, and version tags with these spaces. Co-Authored-By: Claude Opus 5.5 --- .../test_credential_helper_maven_tools.py | 25 +++++++++++++++++++ .../credential_helpers/maven/extensions.py | 14 +++++------ 2 files changed, 31 insertions(+), 8 deletions(-) diff --git a/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py b/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py index 16149c76..f9f7b32b 100644 --- a/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py +++ b/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py @@ -315,6 +315,31 @@ def test_add_extension_finds_the_extension_with_spaces_in_its_tags(): ) +def _spaced_child_tags(extension: str) -> str: + for name in ("groupId", "artifactId", "version"): + extension = extension.replace(f"<{name}>", f"<{name} >") + extension = extension.replace(f"", f"") + return extension + + +def test_add_extension_finds_the_extension_with_spaces_in_its_child_tags(): + """XML allows spaces before the > of each child tag, so only the version changes.""" + existing = f"\n{_spaced_child_tags(OUR_EXTENSION)}\n" + + assert extensions.add_extension(existing, "0.2.0") == ( + existing.replace(">0.1.0<", ">0.2.0<"), + True, + ) + + +def test_remove_extension_finds_the_extension_with_spaces_in_its_child_tags(): + """Status and uninstall find the entry with spaces in its child tags.""" + existing = f"\n{_spaced_child_tags(OUR_EXTENSION)}\n" + + assert extensions.has_extension(existing) + assert extensions.remove_extension(existing) == (None, True) + + # --------------------------------------------------------------------------- # 3. ~/.mavenrc # --------------------------------------------------------------------------- diff --git a/cloudsmith_cli/credential_helpers/maven/extensions.py b/cloudsmith_cli/credential_helpers/maven/extensions.py index e0cb11eb..e31a65ab 100644 --- a/cloudsmith_cli/credential_helpers/maven/extensions.py +++ b/cloudsmith_cli/credential_helpers/maven/extensions.py @@ -17,10 +17,12 @@ r"[ \t]*.*?[ \t]*\n?", re.DOTALL ) _EMPTY_EXTENSIONS = re.compile(r"]*?)\s*/>") -_VERSION = re.compile(r"[^<]*") +_VERSION = re.compile(r"([^<]*)") _COMMENT = re.compile(r"", re.DOTALL) -_GROUP_ID = re.compile(rf"\s*{re.escape(GROUP_ID)}\s*") -_ARTIFACT_ID = re.compile(rf"\s*{re.escape(ARTIFACT_ID)}\s*") +_GROUP_ID = re.compile(rf"\s*{re.escape(GROUP_ID)}\s*") +_ARTIFACT_ID = re.compile( + rf"\s*{re.escape(ARTIFACT_ID)}\s*" +) class ExtensionsFileError(Exception): @@ -76,11 +78,7 @@ def add_extension(text: str | None, version: str) -> tuple[str, bool]: old_version = _VERSION.search(masked, block.start(), block.end()) if old_version is None: raise ExtensionsFileError("The Cloudsmith extension has no .") - new_text = ( - text[: old_version.start()] - + f"{version}" - + text[old_version.end() :] - ) + new_text = text[: old_version.start(1)] + version + text[old_version.end(1) :] return new_text, new_text != text empty = _EMPTY_EXTENSIONS.search(masked) if empty: From 528a8c9919f78275b2da0a3b86879e9563fac35f Mon Sep 17 00:00:00 2001 From: Ian Duffy Date: Tue, 3 Nov 2026 10:56:27 +0000 Subject: [PATCH 11/27] Find the closing extensions tag with spaces before its > Install now adds the extension to a valid extensions.xml that ends in or has a new line before the >. Co-Authored-By: Claude Opus 5.5 --- .../cli/tests/test_credential_helper_maven_tools.py | 11 +++++++++++ cloudsmith_cli/credential_helpers/maven/extensions.py | 7 ++++--- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py b/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py index f9f7b32b..93472a85 100644 --- a/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py +++ b/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py @@ -184,6 +184,17 @@ def test_add_extension_keeps_other_extensions(): ) +@pytest.mark.parametrize("closing_tag", ["", ""]) +def test_add_extension_finds_a_closing_tag_with_spaces(closing_tag): + """XML allows spaces before the > of the closing tag.""" + existing = f"\n{OTHER_EXTENSION}{closing_tag}\n" + + assert extensions.add_extension(existing, "0.1.0") == ( + f"\n{OTHER_EXTENSION}{OUR_EXTENSION}{closing_tag}\n", + True, + ) + + def test_add_extension_to_an_empty_element(): """A self-closing element is opened to hold the extension.""" text, changed = extensions.add_extension("\n", "0.1.0") diff --git a/cloudsmith_cli/credential_helpers/maven/extensions.py b/cloudsmith_cli/credential_helpers/maven/extensions.py index e31a65ab..4b909a86 100644 --- a/cloudsmith_cli/credential_helpers/maven/extensions.py +++ b/cloudsmith_cli/credential_helpers/maven/extensions.py @@ -17,6 +17,7 @@ r"[ \t]*.*?[ \t]*\n?", re.DOTALL ) _EMPTY_EXTENSIONS = re.compile(r"]*?)\s*/>") +_CLOSING_EXTENSIONS = re.compile(r"") _VERSION = re.compile(r"([^<]*)") _COMMENT = re.compile(r"", re.DOTALL) _GROUP_ID = re.compile(rf"\s*{re.escape(GROUP_ID)}\s*") @@ -84,10 +85,10 @@ def add_extension(text: str | None, version: str) -> tuple[str, bool]: if empty: opened = f"\n{_render(version)}" return text[: empty.start()] + opened + text[empty.end() :], True - closing = masked.rfind("") - if closing == -1: + closing = _CLOSING_EXTENSIONS.search(masked) + if closing is None: raise ExtensionsFileError("The file has no element.") - return text[:closing] + _render(version) + text[closing:], True + return text[: closing.start()] + _render(version) + text[closing.start() :], True def remove_extension(text: str) -> tuple[str | None, bool]: From 87ece2890f3f5bd999e8bee0c08b581e9c3d93d5 Mon Sep 17 00:00:00 2001 From: Ian Duffy Date: Tue, 3 Nov 2026 11:04:27 +0000 Subject: [PATCH 12/27] Keep comments next to the Maven extension on uninstall Co-Authored-By: Claude Opus 5.5 --- .../tests/test_credential_helper_maven_tools.py | 16 ++++++++++++++++ .../credential_helpers/maven/extensions.py | 11 +++++++---- 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py b/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py index 93472a85..d9527453 100644 --- a/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py +++ b/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py @@ -271,6 +271,22 @@ def test_remove_extension_keeps_other_extensions(): assert text == f"\n{OTHER_EXTENSION}\n" +def test_remove_extension_keeps_comments_on_the_same_lines(): + """A comment next to the Cloudsmith extension is not part of it.""" + ours = OUR_EXTENSION.rstrip("\n") + existing = ( + f"\n{ours}\n" + f"{OTHER_EXTENSION}\n" + ) + + text, changed = extensions.remove_extension(existing) + + assert changed is True + assert text == ( + f"\n\n{OTHER_EXTENSION}\n" + ) + + def test_remove_the_only_extension_returns_no_text(): """When nothing else is listed, the file is not needed.""" assert extensions.remove_extension( diff --git a/cloudsmith_cli/credential_helpers/maven/extensions.py b/cloudsmith_cli/credential_helpers/maven/extensions.py index 4b909a86..2a9dc819 100644 --- a/cloudsmith_cli/credential_helpers/maven/extensions.py +++ b/cloudsmith_cli/credential_helpers/maven/extensions.py @@ -13,9 +13,7 @@ ARTIFACT_ID = "cloudsmith-maven-extension" # XML allows spaces and new lines before the > of a tag. -_EXTENSION_BLOCK = re.compile( - r"[ \t]*.*?[ \t]*\n?", re.DOTALL -) +_EXTENSION_BLOCK = re.compile(r".*?", re.DOTALL) _EMPTY_EXTENSIONS = re.compile(r"]*?)\s*/>") _CLOSING_EXTENSIONS = re.compile(r"") _VERSION = re.compile(r"([^<]*)") @@ -101,7 +99,12 @@ def remove_extension(text: str) -> tuple[str | None, bool]: block = _find_extension(masked) if not block: return text, False - new_text = text[: block.start()] + text[block.end() :] + # Read the blanks around the block from the real text, because the masked + # text shows a comment as blanks. + start = len(text[: block.start()].rstrip(" \t")) + end = len(text) - len(text[block.end() :].lstrip(" \t")) + end += text.startswith("\n", end) + new_text = text[:start] + text[end:] if not _has_extension_element(new_text): return None, True return new_text, True From 7a5d820ed28721d59974b98b27382b8c2eee6c44 Mon Sep 17 00:00:00 2001 From: Ian Duffy Date: Tue, 3 Nov 2026 11:11:41 +0000 Subject: [PATCH 13/27] Skip unreadable Maven config files in status credential-helper list calls the status of each helper. A ~/.mavenrc that is not UTF-8, or a file that cannot be read, no longer stops it. Co-Authored-By: Claude Opus 5.5 --- .../test_credential_helper_maven_tools.py | 20 +++++++++++++++++++ .../credential_helpers/maven/installer.py | 5 ++++- 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py b/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py index d9527453..cc253410 100644 --- a/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py +++ b/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py @@ -684,6 +684,26 @@ def test_maven_status_names_the_file_that_loads_the_extension( } +def test_maven_status_skips_a_file_that_cannot_be_decoded(home, outside_projects): + """A ~/.mavenrc that is not UTF-8 does not stop `credential-helper list`.""" + (home / ".mavenrc").write_bytes(b"# \xe9\nexport A=1\n") + + assert MavenInstaller().status() == {"launcher": None, "hosts": []} + + +def test_maven_status_skips_a_file_that_cannot_be_read( + home, fake_repository, outside_projects +): + """An unreadable candidate does not hide the next file that loads the extension.""" + MavenInstaller().install(mavenrc=True) + _user_extensions(home).mkdir(parents=True) + + assert MavenInstaller().status() == { + "launcher": str(home / ".mavenrc"), + "hosts": [], + } + + # --------------------------------------------------------------------------- # 5. GradleInstaller # --------------------------------------------------------------------------- diff --git a/cloudsmith_cli/credential_helpers/maven/installer.py b/cloudsmith_cli/credential_helpers/maven/installer.py index 71d591a5..c1009f11 100644 --- a/cloudsmith_cli/credential_helpers/maven/installer.py +++ b/cloudsmith_cli/credential_helpers/maven/installer.py @@ -229,7 +229,10 @@ def status(self) -> dict: (_project_dir() / ".mvn" / "extensions.xml", extensions.has_extension), ) for path, loads_extension in candidates: - current = _read(path) + try: + current = _read(path) + except (ValueError, OSError): + continue if current is not None and loads_extension(current): return {"launcher": str(path), "hosts": []} return {"launcher": None, "hosts": []} From 326d636463a98c4d9efa58284baf8e048c208b4e Mon Sep 17 00:00:00 2001 From: Ian Duffy Date: Tue, 3 Nov 2026 11:12:24 +0000 Subject: [PATCH 14/27] Keep config file bytes that are not UTF-8 Install and uninstall for Maven now keep these bytes in ~/.mavenrc and extensions.xml. Gradle and sbt report such a file as a conflict. Co-Authored-By: Claude Opus 5.5 --- .../test_credential_helper_maven_tools.py | 26 +++++++++++++++++++ .../credential_helpers/maven/installer.py | 9 ++++--- .../credential_helpers/maven_tools.py | 2 +- 3 files changed, 33 insertions(+), 4 deletions(-) diff --git a/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py b/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py index cc253410..074321e2 100644 --- a/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py +++ b/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py @@ -656,6 +656,20 @@ def test_maven_uninstall_removes_only_what_install_added( assert _local_extension_dir(home, "0.2.0").exists() +def test_maven_install_and_uninstall_keep_a_mavenrc_that_is_not_utf_8( + home, fake_repository, outside_projects +): + """The block is added and removed, and the other bytes stay as they are.""" + original = b"# \xe9\nexport A=1\n" + (home / ".mavenrc").write_bytes(original) + + MavenInstaller().install(mavenrc=True) + assert (home / ".mavenrc").read_bytes().startswith(original) + + MavenInstaller().uninstall() + assert (home / ".mavenrc").read_bytes() == original + + def test_maven_uninstall_local_removes_the_extension_from_the_project( home, fake_repository, maven_project ): @@ -757,6 +771,18 @@ def test_gradle_install_upgrades_an_init_script_that_it_wrote(home, fake_reposit assert script.read_text(encoding="utf-8") == GradleInstaller().render("0.2.0") +def test_gradle_install_keeps_an_init_script_that_is_not_utf_8(home, fake_repository): + """A file that is not UTF-8 is not a file that install wrote.""" + script = home / ".gradle" / "init.d" / "cloudsmith.init.gradle.kts" + script.parent.mkdir(parents=True) + script.write_bytes(b"// \xe9\n") + + with pytest.raises(ConfigFileConflict): + GradleInstaller().install() + + assert script.read_bytes() == b"// \xe9\n" + + def test_gradle_uninstall_keeps_an_init_script_with_other_settings( home, fake_repository ): diff --git a/cloudsmith_cli/credential_helpers/maven/installer.py b/cloudsmith_cli/credential_helpers/maven/installer.py index c1009f11..c5673c3b 100644 --- a/cloudsmith_cli/credential_helpers/maven/installer.py +++ b/cloudsmith_cli/credential_helpers/maven/installer.py @@ -67,7 +67,10 @@ def _version_dir(version: str) -> Path: def _read(path: Path) -> str | None: - return path.read_text(encoding="utf-8") if path.exists() else None + # Keep bytes that are not UTF-8, for example comments in ~/.mavenrc. + if not path.exists(): + return None + return path.read_text(encoding="utf-8", errors="surrogateescape") def _maven_version() -> str | None: @@ -130,7 +133,7 @@ def _add(path: Path, edit: Callable, version: str, dry_run: bool) -> list[str]: if dry_run: return [f"would add version {version} to {path}"] path.parent.mkdir(parents=True, exist_ok=True) - path.write_text(text, encoding="utf-8") + path.write_text(text, encoding="utf-8", errors="surrogateescape") return [f"added version {version} to {path}"] @@ -146,7 +149,7 @@ def _remove(path: Path, edit: Callable, dry_run: bool) -> list[str]: if text is None: path.unlink() return [f"removed {path}"] - path.write_text(text, encoding="utf-8") + path.write_text(text, encoding="utf-8", errors="surrogateescape") return [f"removed the extension from {path}"] diff --git a/cloudsmith_cli/credential_helpers/maven_tools.py b/cloudsmith_cli/credential_helpers/maven_tools.py index 3bf19e16..9435db20 100644 --- a/cloudsmith_cli/credential_helpers/maven_tools.py +++ b/cloudsmith_cli/credential_helpers/maven_tools.py @@ -92,7 +92,7 @@ def _read_own_file(self, path: Path) -> str | None: """Return the text of *path*, or ``None`` when there is no file.""" if not path.exists(): return None - text = path.read_text(encoding="utf-8") + text = path.read_text(encoding="utf-8", errors="surrogateescape") if not self._written_by_install(text): raise ConfigFileConflict( f"{path} has settings that `cloudsmith credential-helper install " From 37d2a183bace52a964ac35bbfbddb41e48c7dcb9 Mon Sep 17 00:00:00 2001 From: Ian Duffy Date: Tue, 3 Nov 2026 11:12:52 +0000 Subject: [PATCH 15/27] Replace a downloaded Maven tools file only after the write succeeds The download now writes a temporary file next to the destination and then replaces the destination. A failed write leaves no partial file, so the next install downloads the file again. Co-Authored-By: Claude Opus 5.5 --- .../test_credential_helper_maven_tools.py | 24 +++++++++++++++++++ .../credential_helpers/maven_tools.py | 10 +++++++- 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py b/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py index 074321e2..16218d1e 100644 --- a/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py +++ b/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py @@ -7,6 +7,7 @@ from __future__ import annotations +import errno import os import subprocess from pathlib import Path @@ -158,6 +159,29 @@ def test_download_writes_the_file(tmp_path): assert destination.read_bytes() == b"jar bytes" +@httpretty.activate(allow_net_connect=False) +def test_download_leaves_no_partial_file_when_the_write_fails(tmp_path, monkeypatch): + """A later install finds no file, so it downloads the file again.""" + httpretty.register_uri( + httpretty.GET, + f"{maven_tools.REPOSITORY_URL}{EXTENSION_PATH}/0.2.0/extension.jar", + body=b"jar bytes", + ) + + def write_half_and_fail(path, data): + with open(path, "wb") as partial_file: + partial_file.write(data[: len(data) // 2]) + raise OSError(errno.ENOSPC, "No space left on device") + + monkeypatch.setattr(Path, "write_bytes", write_half_and_fail) + destination = tmp_path / "extension.jar" + + with pytest.raises(OSError, match="No space left"): + maven_tools.download(f"{EXTENSION_PATH}/0.2.0/extension.jar", destination) + + assert list(tmp_path.iterdir()) == [] + + # --------------------------------------------------------------------------- # 2. .mvn/extensions.xml # --------------------------------------------------------------------------- diff --git a/cloudsmith_cli/credential_helpers/maven_tools.py b/cloudsmith_cli/credential_helpers/maven_tools.py index 9435db20..33cf07df 100644 --- a/cloudsmith_cli/credential_helpers/maven_tools.py +++ b/cloudsmith_cli/credential_helpers/maven_tools.py @@ -10,6 +10,7 @@ from __future__ import annotations import contextlib +import os import re from typing import TYPE_CHECKING from xml.etree import ElementTree @@ -58,7 +59,14 @@ def download( """Download the file at *artifact_path* to *destination*.""" content = _get(artifact_path, session) destination.parent.mkdir(parents=True, exist_ok=True) - destination.write_bytes(content) + # Install skips a file that exists, so a failed write must not leave a partial file. + partial = destination.with_name(f"{destination.name}.{os.getpid()}.part") + try: + partial.write_bytes(content) + partial.replace(destination) + except OSError: + partial.unlink(missing_ok=True) + raise class ConfigFileInstaller: From b9872d400c5f9f361310b28bea0f581ea08d9a9f Mon Sep 17 00:00:00 2001 From: Ian Duffy Date: Tue, 3 Nov 2026 11:21:28 +0000 Subject: [PATCH 16/27] Report only owned Gradle and sbt files, and accept non-UTF-8 XML Co-Authored-By: Claude Opus 5.5 --- .../test_credential_helper_maven_tools.py | 19 +++++++++++++++++++ .../credential_helpers/maven/extensions.py | 3 ++- .../credential_helpers/maven_tools.py | 9 ++++++++- 3 files changed, 29 insertions(+), 2 deletions(-) diff --git a/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py b/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py index 16218d1e..036ed35d 100644 --- a/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py +++ b/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py @@ -311,6 +311,16 @@ def test_remove_extension_keeps_comments_on_the_same_lines(): ) +def test_remove_extension_keeps_a_file_that_is_not_utf_8(): + """A byte that is not UTF-8 stops the XML check, so the file stays.""" + existing = f"\n\n{OUR_EXTENSION}{OTHER_EXTENSION}\n" + + assert extensions.remove_extension(existing) == ( + f"\n\n{OTHER_EXTENSION}\n", + True, + ) + + def test_remove_the_only_extension_returns_no_text(): """When nothing else is listed, the file is not needed.""" assert extensions.remove_extension( @@ -784,6 +794,15 @@ def test_gradle_uninstall_and_status(home, fake_repository): assert GradleInstaller().status() == {"launcher": None, "hosts": []} +def test_gradle_status_ignores_an_init_script_that_install_did_not_write(home): + """Status reports only a file that uninstall can remove.""" + script = home / ".gradle" / "init.d" / "cloudsmith.init.gradle.kts" + script.parent.mkdir(parents=True) + script.write_text("// my own settings\n", encoding="utf-8") + + assert GradleInstaller().status() == {"launcher": None, "hosts": []} + + def test_gradle_install_upgrades_an_init_script_that_it_wrote(home, fake_repository): """An init script for an older version gets the new version.""" script = home / ".gradle" / "init.d" / "cloudsmith.init.gradle.kts" diff --git a/cloudsmith_cli/credential_helpers/maven/extensions.py b/cloudsmith_cli/credential_helpers/maven/extensions.py index 2a9dc819..81c65b96 100644 --- a/cloudsmith_cli/credential_helpers/maven/extensions.py +++ b/cloudsmith_cli/credential_helpers/maven/extensions.py @@ -54,7 +54,8 @@ def _has_extension_element(text: str) -> bool: """Return whether *text* has an element, or is not valid XML.""" try: root = ElementTree.fromstring(text) - except ElementTree.ParseError: + # A byte that is not UTF-8 gives a UnicodeEncodeError, which is a ValueError. + except (ElementTree.ParseError, ValueError): return True return any(element.tag.rpartition("}")[2] == "extension" for element in root.iter()) diff --git a/cloudsmith_cli/credential_helpers/maven_tools.py b/cloudsmith_cli/credential_helpers/maven_tools.py index 33cf07df..79e01bd3 100644 --- a/cloudsmith_cli/credential_helpers/maven_tools.py +++ b/cloudsmith_cli/credential_helpers/maven_tools.py @@ -144,5 +144,12 @@ def status(self) -> dict: # Gradle has no local config, and the current directory is often not a project. with contextlib.suppress(NotImplementedError, ProjectNotFound): paths.append(self.local_path()) - path = next((path for path in paths if path.exists()), None) + path = next((path for path in paths if self._is_own_file(path)), None) return {"launcher": str(path) if path else None, "hosts": []} + + def _is_own_file(self, path: Path) -> bool: + """Return whether *path* is a file that uninstall can remove.""" + try: + return self._read_own_file(path) is not None + except (ConfigFileConflict, OSError): + return False From d821e327e588b5b297547819db12f33c0c4faac2 Mon Sep 17 00:00:00 2001 From: Ian Duffy Date: Tue, 3 Nov 2026 11:29:05 +0000 Subject: [PATCH 17/27] Escape shell characters in the JAR path of the .mavenrc block Co-Authored-By: Claude Opus 5.5 --- .../tests/test_credential_helper_maven_tools.py | 17 +++++++++++++++-- .../credential_helpers/maven/mavenrc.py | 8 +++++--- 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py b/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py index 036ed35d..4aad8090 100644 --- a/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py +++ b/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py @@ -456,9 +456,9 @@ def test_remove_block_without_a_block_changes_nothing(): assert mavenrc.remove_block("export A=1\n") == ("export A=1\n", False) -def _maven_opts_after_block(maven_opts: str) -> str: +def _maven_opts_after_block(maven_opts: str, block: str = BLOCK) -> str: """Run the block in the shell that the mvn script uses, and return MAVEN_OPTS.""" - script = f'MAVEN_OPTS="$1"\n{BLOCK}printf %s "$MAVEN_OPTS"\n' + script = f'MAVEN_OPTS="$1"\n{block}printf %s "$MAVEN_OPTS"\n' result = subprocess.run( ["sh", "-c", script, "sh", maven_opts], capture_output=True, @@ -479,6 +479,19 @@ def test_block_adds_the_class_path_to_maven_opts(): assert _maven_opts_after_block("-Xmx1g") == f"-Xmx1g -Dmaven.ext.class.path={JAR}" +@POSIX_SHELL_ONLY +@pytest.mark.parametrize("maven_opts", ["", "-Dmaven.ext.class.path=/opt/other.jar"]) +def test_block_reads_shell_characters_in_the_jar_path_as_text(maven_opts): + """The shell does not expand or run the characters of the JAR path.""" + jar = Path('/home/a$(id)`id`"d\\e/extension.jar') + block = mavenrc.render_block(jar) + + once = _maven_opts_after_block(maven_opts, block) + + assert f"-Dmaven.ext.class.path={jar}" in once + assert _maven_opts_after_block(once, block) == once + + @POSIX_SHELL_ONLY def test_block_keeps_the_other_extensions_in_the_class_path(): """Java uses only the last -D value, so the block adds the JAR to that value.""" diff --git a/cloudsmith_cli/credential_helpers/maven/mavenrc.py b/cloudsmith_cli/credential_helpers/maven/mavenrc.py index dddf11b4..c3b91082 100644 --- a/cloudsmith_cli/credential_helpers/maven/mavenrc.py +++ b/cloudsmith_cli/credential_helpers/maven/mavenrc.py @@ -28,6 +28,8 @@ def render_block(jar: Path) -> str: """Return the block that adds *jar* to the Maven extension class path.""" option = "-Dmaven.ext.class.path=" after_last = "cloudsmith_after_class_path" + # The shell expands \ " $ and ` in double quotes, so the path escapes them. + jar_text = re.sub(r'([\\"$`])', r"\\\1", str(jar)) return ( f"{BEGIN}\n" "# Java uses only the last -D value, so add the JAR to that value.\n" @@ -35,12 +37,12 @@ def render_block(jar: Path) -> str: f" *{option}*)\n" f' {after_last}="${{MAVEN_OPTS##*{option}}}"\n' f' case "{os.pathsep}${{{after_last}%% *}}{os.pathsep}" in\n' - f' *"{os.pathsep}{jar}{os.pathsep}"*) ;;\n' - f' *) MAVEN_OPTS="${{MAVEN_OPTS%{option}*}}{option}{jar}{os.pathsep}' + f' *"{os.pathsep}{jar_text}{os.pathsep}"*) ;;\n' + f' *) MAVEN_OPTS="${{MAVEN_OPTS%{option}*}}{option}{jar_text}{os.pathsep}' f'${after_last}" ;;\n' " esac\n" f" unset {after_last} ;;\n" - f' *) MAVEN_OPTS="$MAVEN_OPTS {option}{jar}" ;;\n' + f' *) MAVEN_OPTS="$MAVEN_OPTS {option}{jar_text}" ;;\n' "esac\n" f"{END}\n" ) From 5100305ab6b2118e1140aed3d8c43a1c449fb57d Mon Sep 17 00:00:00 2001 From: Ian Duffy Date: Tue, 3 Nov 2026 11:36:31 +0000 Subject: [PATCH 18/27] Write Maven tools config files atomically and keep lines apart on removal Co-Authored-By: Claude Opus 5.5 --- .../test_credential_helper_maven_tools.py | 46 +++++++++++++++++++ .../credential_helpers/maven/installer.py | 4 +- .../credential_helpers/maven/mavenrc.py | 6 ++- .../credential_helpers/maven_tools.py | 18 ++++++-- 4 files changed, 66 insertions(+), 8 deletions(-) diff --git a/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py b/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py index 4aad8090..e690cd32 100644 --- a/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py +++ b/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py @@ -182,6 +182,44 @@ def write_half_and_fail(path, data): assert list(tmp_path.iterdir()) == [] +def _write_half_and_fail(path, data): + with open(path, "wb") as partial_file: + partial_file.write(data[: len(data) // 2]) + raise OSError(errno.ENOSPC, "No space left on device") + + +def test_write_atomically_keeps_the_old_file_when_the_write_fails( + tmp_path, monkeypatch +): + """A full disk does not leave a config file with only part of its text.""" + path = tmp_path / ".mavenrc" + path.write_bytes(b"export A=1\n") + monkeypatch.setattr(Path, "write_bytes", _write_half_and_fail) + + with pytest.raises(OSError, match="No space left"): + maven_tools.write_atomically(path, b"export A=1\nexport B=2\n") + + assert path.read_bytes() == b"export A=1\n" + assert [entry.name for entry in tmp_path.iterdir()] == [".mavenrc"] + + +@pytest.mark.skipif(os.name == "nt", reason="Symbolic links need extra rights.") +def test_write_atomically_keeps_a_symbolic_link_and_the_file_mode(tmp_path): + """A dotfile that links to another file stays a link, with the same mode.""" + target = tmp_path / "dotfiles" / "mavenrc" + target.parent.mkdir() + target.write_bytes(b"export A=1\n") + target.chmod(0o600) + link = tmp_path / ".mavenrc" + link.symlink_to(target) + + maven_tools.write_atomically(link, b"export B=2\n") + + assert link.is_symlink() + assert target.read_bytes() == b"export B=2\n" + assert target.stat().st_mode & 0o777 == 0o600 + + # --------------------------------------------------------------------------- # 2. .mvn/extensions.xml # --------------------------------------------------------------------------- @@ -446,6 +484,14 @@ def test_remove_block_keeps_the_other_lines(): ) +def test_remove_block_keeps_the_lines_around_it_apart(): + """Without a blank line before the block, the line before it keeps its newline.""" + assert mavenrc.remove_block(f"export A=1\n{BLOCK}export B=2\n") == ( + "export A=1\nexport B=2\n", + True, + ) + + def test_remove_the_only_block_returns_no_text(): """When nothing else is in the file, the file is not needed.""" assert mavenrc.remove_block(BLOCK) == (None, True) diff --git a/cloudsmith_cli/credential_helpers/maven/installer.py b/cloudsmith_cli/credential_helpers/maven/installer.py index c5673c3b..2a176838 100644 --- a/cloudsmith_cli/credential_helpers/maven/installer.py +++ b/cloudsmith_cli/credential_helpers/maven/installer.py @@ -133,7 +133,7 @@ def _add(path: Path, edit: Callable, version: str, dry_run: bool) -> list[str]: if dry_run: return [f"would add version {version} to {path}"] path.parent.mkdir(parents=True, exist_ok=True) - path.write_text(text, encoding="utf-8", errors="surrogateescape") + maven_tools.write_atomically(path, text.encode("utf-8", "surrogateescape")) return [f"added version {version} to {path}"] @@ -149,7 +149,7 @@ def _remove(path: Path, edit: Callable, dry_run: bool) -> list[str]: if text is None: path.unlink() return [f"removed {path}"] - path.write_text(text, encoding="utf-8", errors="surrogateescape") + maven_tools.write_atomically(path, text.encode("utf-8", "surrogateescape")) return [f"removed the extension from {path}"] diff --git a/cloudsmith_cli/credential_helpers/maven/mavenrc.py b/cloudsmith_cli/credential_helpers/maven/mavenrc.py index c3b91082..0a0dfeb1 100644 --- a/cloudsmith_cli/credential_helpers/maven/mavenrc.py +++ b/cloudsmith_cli/credential_helpers/maven/mavenrc.py @@ -20,8 +20,10 @@ _BLOCK_PATTERN = rf"^{re.escape(BEGIN)}\n.*?^{re.escape(END)}\n?" _BLOCK = re.compile(_BLOCK_PATTERN, re.DOTALL | re.MULTILINE) -# Also matches the newline that add_block puts before the block. -_BLOCK_WITH_SEPARATOR = re.compile(rf"\n?{_BLOCK_PATTERN}", re.DOTALL | re.MULTILINE) +# Also matches the blank line that add_block puts before the block. +_BLOCK_WITH_SEPARATOR = re.compile( + rf"(?:(?<=\n)\n)?{_BLOCK_PATTERN}", re.DOTALL | re.MULTILINE +) def render_block(jar: Path) -> str: diff --git a/cloudsmith_cli/credential_helpers/maven_tools.py b/cloudsmith_cli/credential_helpers/maven_tools.py index 79e01bd3..5243995a 100644 --- a/cloudsmith_cli/credential_helpers/maven_tools.py +++ b/cloudsmith_cli/credential_helpers/maven_tools.py @@ -12,6 +12,7 @@ import contextlib import os import re +import shutil from typing import TYPE_CHECKING from xml.etree import ElementTree @@ -60,10 +61,19 @@ def download( content = _get(artifact_path, session) destination.parent.mkdir(parents=True, exist_ok=True) # Install skips a file that exists, so a failed write must not leave a partial file. - partial = destination.with_name(f"{destination.name}.{os.getpid()}.part") + write_atomically(destination, content) + + +def write_atomically(path: Path, data: bytes) -> None: + """Write *data* to *path*, so that a failed write leaves the old file as it was.""" + # Write through a symbolic link, such as a dotfile, so that the link stays. + target = path.resolve() + partial = target.with_name(f"{target.name}.{os.getpid()}.part") try: - partial.write_bytes(content) - partial.replace(destination) + partial.write_bytes(data) + if target.exists(): + shutil.copymode(target, partial) + partial.replace(target) except OSError: partial.unlink(missing_ok=True) raise @@ -125,7 +135,7 @@ def install( if dry_run: return [f"would write {path} for version {version}"] path.parent.mkdir(parents=True, exist_ok=True) - path.write_text(text, encoding="utf-8") + write_atomically(path, text.encode("utf-8")) return [f"wrote {path} for version {version}"] def uninstall( From 14c3ccc65e465ca2237710e0c32159e9fb0132b7 Mon Sep 17 00:00:00 2001 From: Ian Duffy Date: Tue, 3 Nov 2026 11:43:00 +0000 Subject: [PATCH 19/27] Create the temporary config copy with the mode of the file it replaces Co-Authored-By: Claude Opus 5.5 --- .../test_credential_helper_maven_tools.py | 26 +++++++++++++++++++ .../credential_helpers/maven_tools.py | 4 +++ 2 files changed, 30 insertions(+) diff --git a/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py b/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py index e690cd32..f6a365d8 100644 --- a/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py +++ b/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py @@ -203,6 +203,32 @@ def test_write_atomically_keeps_the_old_file_when_the_write_fails( assert [entry.name for entry in tmp_path.iterdir()] == [".mavenrc"] +@pytest.mark.skipif(os.name == "nt", reason="Windows has no POSIX file modes.") +def test_write_atomically_never_widens_the_mode_of_a_private_file( + tmp_path, monkeypatch +): + """The temporary copy of a private file is never readable by other users.""" + path = tmp_path / ".mavenrc" + path.write_bytes(b"export SECRET=1\n") + path.chmod(0o600) + modes_while_writing = [] + write_bytes = Path.write_bytes + + def record_mode_and_write(partial, data): + if partial.exists(): + modes_while_writing.append(partial.stat().st_mode & 0o777) + else: + modes_while_writing.append(None) + return write_bytes(partial, data) + + monkeypatch.setattr(Path, "write_bytes", record_mode_and_write) + + maven_tools.write_atomically(path, b"export SECRET=2\n") + + assert modes_while_writing == [0o600] + assert path.stat().st_mode & 0o777 == 0o600 + + @pytest.mark.skipif(os.name == "nt", reason="Symbolic links need extra rights.") def test_write_atomically_keeps_a_symbolic_link_and_the_file_mode(tmp_path): """A dotfile that links to another file stays a link, with the same mode.""" diff --git a/cloudsmith_cli/credential_helpers/maven_tools.py b/cloudsmith_cli/credential_helpers/maven_tools.py index 5243995a..f4debfec 100644 --- a/cloudsmith_cli/credential_helpers/maven_tools.py +++ b/cloudsmith_cli/credential_helpers/maven_tools.py @@ -13,6 +13,7 @@ import os import re import shutil +import stat from typing import TYPE_CHECKING from xml.etree import ElementTree @@ -69,7 +70,10 @@ def write_atomically(path: Path, data: bytes) -> None: # Write through a symbolic link, such as a dotfile, so that the link stays. target = path.resolve() partial = target.with_name(f"{target.name}.{os.getpid()}.part") + # Make the copy before the data goes in, never with a wider mode than the file. + mode = stat.S_IMODE(target.stat().st_mode) if target.exists() else 0o666 try: + partial.touch(mode=mode, exist_ok=False) partial.write_bytes(data) if target.exists(): shutil.copymode(target, partial) From 46202532b374f32f702517c62a7acd60c393294f Mon Sep 17 00:00:00 2001 From: Ian Duffy Date: Tue, 3 Nov 2026 11:50:41 +0000 Subject: [PATCH 20/27] Find a Maven extension with namespace prefixes or attributes Co-Authored-By: Claude Opus 5.5 --- .../test_credential_helper_maven_tools.py | 33 +++++++++++++++++++ .../credential_helpers/maven/extensions.py | 26 ++++++++++++--- 2 files changed, 55 insertions(+), 4 deletions(-) diff --git a/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py b/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py index f6a365d8..5b19f43e 100644 --- a/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py +++ b/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py @@ -405,6 +405,39 @@ def _spaced_tags(extension: str) -> str: ) +NAMESPACED_EXTENSION = """ + io.cloudsmith + cloudsmith-maven-extension + 0.1.0 + +""" + + +def test_extension_with_namespaces_is_found_updated_and_removed(): + """A namespace prefix or attribute does not hide the Cloudsmith extension.""" + existing = f"\n{NAMESPACED_EXTENSION}{OTHER_EXTENSION}\n" + + updated, changed = extensions.add_extension(existing, "0.2.0") + + assert extensions.has_extension(existing) + assert changed is True + assert updated == existing.replace("0.1.0<", "0.2.0<") + assert extensions.remove_extension(existing) == ( + f"\n{OTHER_EXTENSION}\n", + True, + ) + + +def test_a_self_closing_extension_is_not_the_start_of_a_block(): + """An empty element does not join the next extension.""" + existing = f"\n \n{OUR_EXTENSION}\n" + + assert extensions.remove_extension(existing) == ( + "\n \n\n", + True, + ) + + def test_remove_extension_keeps_an_extension_with_spaces_in_its_tags(): """XML allows spaces before the > of a tag, so the other extension still counts.""" other = _spaced_tags(OTHER_EXTENSION) diff --git a/cloudsmith_cli/credential_helpers/maven/extensions.py b/cloudsmith_cli/credential_helpers/maven/extensions.py index 81c65b96..5c92ce50 100644 --- a/cloudsmith_cli/credential_helpers/maven/extensions.py +++ b/cloudsmith_cli/credential_helpers/maven/extensions.py @@ -12,15 +12,33 @@ GROUP_ID = "io.cloudsmith" ARTIFACT_ID = "cloudsmith-maven-extension" + +def _start_tag(name: str) -> str: + """Return a pattern for the start tag *name*, with a namespace prefix or attributes. + + The ``(?`` element. + """ + return rf"<(?:[\w.-]+:)?{name}(?:\s[^>]*)?(?" + + +def _end_tag(name: str) -> str: + """Return a pattern for the end tag *name*, with a namespace prefix.""" + return rf"" + + # XML allows spaces and new lines before the > of a tag. -_EXTENSION_BLOCK = re.compile(r".*?", re.DOTALL) +_EXTENSION_BLOCK = re.compile( + rf"{_start_tag('extension')}.*?{_end_tag('extension')}", re.DOTALL +) _EMPTY_EXTENSIONS = re.compile(r"]*?)\s*/>") _CLOSING_EXTENSIONS = re.compile(r"") -_VERSION = re.compile(r"([^<]*)") +_VERSION = re.compile(rf"{_start_tag('version')}([^<]*){_end_tag('version')}") _COMMENT = re.compile(r"", re.DOTALL) -_GROUP_ID = re.compile(rf"\s*{re.escape(GROUP_ID)}\s*") +_GROUP_ID = re.compile( + rf"{_start_tag('groupId')}\s*{re.escape(GROUP_ID)}\s*{_end_tag('groupId')}" +) _ARTIFACT_ID = re.compile( - rf"\s*{re.escape(ARTIFACT_ID)}\s*" + rf"{_start_tag('artifactId')}\s*{re.escape(ARTIFACT_ID)}\s*{_end_tag('artifactId')}" ) From 4dd8628815effcbe2a664e74a3763023a80a53cc Mon Sep 17 00:00:00 2001 From: Ian Duffy Date: Tue, 3 Nov 2026 11:56:12 +0000 Subject: [PATCH 21/27] Add the Maven extension with the prefix of a prefixed extensions element Co-Authored-By: Claude Opus 5.5 --- .../test_credential_helper_maven_tools.py | 25 ++++++++++++++++++ .../credential_helpers/maven/extensions.py | 26 ++++++++++++------- 2 files changed, 41 insertions(+), 10 deletions(-) diff --git a/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py b/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py index 5b19f43e..c51b6289 100644 --- a/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py +++ b/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py @@ -428,6 +428,31 @@ def test_extension_with_namespaces_is_found_updated_and_removed(): ) +@pytest.mark.parametrize( + "existing", + [ + '\n' + "\n", + '\n', + ], +) +def test_add_extension_uses_the_prefix_of_the_extensions_element(existing): + """The new extension is in the same namespace as the element.""" + text, changed = extensions.add_extension(existing, "0.2.0") + + assert changed is True + assert text == ( + '\n' + " \n" + " io.cloudsmith\n" + " cloudsmith-maven-extension\n" + " 0.2.0\n" + " \n" + "\n" + ) + assert extensions.has_extension(text) + + def test_a_self_closing_extension_is_not_the_start_of_a_block(): """An empty element does not join the next extension.""" existing = f"\n \n{OUR_EXTENSION}\n" diff --git a/cloudsmith_cli/credential_helpers/maven/extensions.py b/cloudsmith_cli/credential_helpers/maven/extensions.py index 5c92ce50..c5c8c133 100644 --- a/cloudsmith_cli/credential_helpers/maven/extensions.py +++ b/cloudsmith_cli/credential_helpers/maven/extensions.py @@ -30,8 +30,8 @@ def _end_tag(name: str) -> str: _EXTENSION_BLOCK = re.compile( rf"{_start_tag('extension')}.*?{_end_tag('extension')}", re.DOTALL ) -_EMPTY_EXTENSIONS = re.compile(r"]*?)\s*/>") -_CLOSING_EXTENSIONS = re.compile(r"") +_EMPTY_EXTENSIONS = re.compile(r"<((?:[\w.-]+:)?)extensions\b([^>]*?)\s*/>") +_CLOSING_EXTENSIONS = re.compile(r"") _VERSION = re.compile(rf"{_start_tag('version')}([^<]*){_end_tag('version')}") _COMMENT = re.compile(r"", re.DOTALL) _GROUP_ID = re.compile( @@ -46,13 +46,14 @@ class ExtensionsFileError(Exception): """The extensions.xml file has no element to add to.""" -def _render(version: str) -> str: +def _render(version: str, prefix: str = "") -> str: + """Return the extension element, with the namespace *prefix* of .""" return ( - " \n" - f" {GROUP_ID}\n" - f" {ARTIFACT_ID}\n" - f" {version}\n" - " \n" + f" <{prefix}extension>\n" + f" <{prefix}groupId>{GROUP_ID}\n" + f" <{prefix}artifactId>{ARTIFACT_ID}\n" + f" <{prefix}version>{version}\n" + f" \n" ) @@ -100,12 +101,17 @@ def add_extension(text: str | None, version: str) -> tuple[str, bool]: return new_text, new_text != text empty = _EMPTY_EXTENSIONS.search(masked) if empty: - opened = f"\n{_render(version)}" + prefix, attributes = empty.groups() + opened = ( + f"<{prefix}extensions{attributes}>\n" + f"{_render(version, prefix)}" + ) return text[: empty.start()] + opened + text[empty.end() :], True closing = _CLOSING_EXTENSIONS.search(masked) if closing is None: raise ExtensionsFileError("The file has no element.") - return text[: closing.start()] + _render(version) + text[closing.start() :], True + rendered = _render(version, closing.group(1)) + return text[: closing.start()] + rendered + text[closing.start() :], True def remove_extension(text: str) -> tuple[str | None, bool]: From cd7273c46529828e9430c35214c1b26ce2a01599 Mon Sep 17 00:00:00 2001 From: Ian Duffy Date: Tue, 3 Nov 2026 12:02:16 +0000 Subject: [PATCH 22/27] Find sbt and Maven projects the way sbt and Maven do sbt reads any .sbt file in the root and the project/ directory. Maven finds the project root by its .mvn directory, whatever the POM is called. Co-Authored-By: Claude Opus 5.5 --- .../test_credential_helper_maven_tools.py | 29 +++++++++++++++++++ .../credential_helpers/maven/installer.py | 3 +- .../credential_helpers/sbt/installer.py | 11 +++++-- 3 files changed, 40 insertions(+), 3 deletions(-) diff --git a/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py b/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py index c51b6289..30d61bee 100644 --- a/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py +++ b/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py @@ -782,6 +782,21 @@ def test_maven_install_local_outside_a_project_fails( assert not fake_repository +def test_maven_uninstall_local_finds_a_project_by_its_mvn_directory( + home, outside_projects +): + """Maven finds the project root by its .mvn directory, also without a pom.xml.""" + extensions_file = outside_projects / ".mvn" / "extensions.xml" + extensions_file.parent.mkdir() + extensions_file.write_text( + f"\n{OUR_EXTENSION}\n", encoding="utf-8" + ) + + MavenInstaller().uninstall(local=True) + + assert not extensions_file.exists() + + def test_maven_install_with_mavenrc_adds_a_block( home, fake_repository, outside_projects ): @@ -1030,6 +1045,20 @@ def test_sbt_install_local_outside_a_project_fails( assert not (outside_projects / "project").exists() +@pytest.mark.parametrize( + "build_file", ["main.sbt", "project/build.properties", "project/cloudsmith.sbt"] +) +def test_sbt_finds_a_project_without_a_build_sbt(outside_projects, build_file): + """sbt reads any .sbt file in the root and the project/ directory.""" + path = outside_projects / build_file + path.parent.mkdir(exist_ok=True) + path.write_text("", encoding="utf-8") + + assert SbtInstaller().local_path() == ( + outside_projects / "project" / "cloudsmith.sbt" + ) + + def test_sbt_install_keeps_a_plugin_file_with_other_settings( home, fake_repository, outside_projects ): diff --git a/cloudsmith_cli/credential_helpers/maven/installer.py b/cloudsmith_cli/credential_helpers/maven/installer.py index 2a176838..7e3e228a 100644 --- a/cloudsmith_cli/credential_helpers/maven/installer.py +++ b/cloudsmith_cli/credential_helpers/maven/installer.py @@ -46,7 +46,8 @@ def _project_dir() -> Path: def _project_extensions_file() -> Path: project_dir = _project_dir() - if not (project_dir / "pom.xml").is_file(): + # Maven finds the project root by its .mvn directory, whatever the POM is called. + if not (project_dir / "pom.xml").is_file() and not (project_dir / ".mvn").is_dir(): raise ProjectNotFound( f"No pom.xml in {project_dir}. " "Run the command in the root directory of a Maven project." diff --git a/cloudsmith_cli/credential_helpers/sbt/installer.py b/cloudsmith_cli/credential_helpers/sbt/installer.py index 7b1fa8a6..32645794 100644 --- a/cloudsmith_cli/credential_helpers/sbt/installer.py +++ b/cloudsmith_cli/credential_helpers/sbt/installer.py @@ -25,12 +25,19 @@ def path(self) -> Path: def local_path(self) -> Path: # sbt loads every .sbt file in project/, so the build's own files stay as they are. project_dir = Path.cwd() - if not (project_dir / "build.sbt").is_file(): + plugin_file = project_dir / "project" / "cloudsmith.sbt" + # sbt reads any .sbt file in the root, and a build can have only project/. + is_project = ( + any(path.is_file() for path in project_dir.glob("*.sbt")) + or (project_dir / "project" / "build.properties").is_file() + or plugin_file.is_file() + ) + if not is_project: raise ProjectNotFound( f"No build.sbt in {project_dir}. " "Run the command in the root directory of an sbt project." ) - return project_dir / "project" / "cloudsmith.sbt" + return plugin_file def render(self, version: str) -> str: return GLOBAL_PLUGIN.format(repository_url=REPOSITORY_URL, version=version) From df934d54d820feb431a287e23e9a159f7de98714 Mon Sep 17 00:00:00 2001 From: Ian Duffy Date: Tue, 3 Nov 2026 12:10:34 +0000 Subject: [PATCH 23/27] Change only a class path option that starts a word of MAVEN_OPTS Co-Authored-By: Claude Opus 5.5 --- .../test_credential_helper_maven_tools.py | 10 ++++++++++ .../credential_helpers/maven/mavenrc.py | 18 +++++++++++------- 2 files changed, 21 insertions(+), 7 deletions(-) diff --git a/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py b/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py index 30d61bee..8e07761f 100644 --- a/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py +++ b/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py @@ -622,6 +622,16 @@ def test_block_reads_shell_characters_in_the_jar_path_as_text(maven_opts): assert _maven_opts_after_block(once, block) == once +@POSIX_SHELL_ONLY +def test_block_ignores_the_class_path_option_inside_another_value(): + """Java reads only an option that starts a word of MAVEN_OPTS.""" + maven_opts = "-DargLine=-Dmaven.ext.class.path=/opt/test.jar -Xmx1g" + + assert _maven_opts_after_block(maven_opts) == ( + f"{maven_opts} -Dmaven.ext.class.path={JAR}" + ) + + @POSIX_SHELL_ONLY def test_block_keeps_the_other_extensions_in_the_class_path(): """Java uses only the last -D value, so the block adds the JAR to that value.""" diff --git a/cloudsmith_cli/credential_helpers/maven/mavenrc.py b/cloudsmith_cli/credential_helpers/maven/mavenrc.py index 0a0dfeb1..43af5299 100644 --- a/cloudsmith_cli/credential_helpers/maven/mavenrc.py +++ b/cloudsmith_cli/credential_helpers/maven/mavenrc.py @@ -32,20 +32,24 @@ def render_block(jar: Path) -> str: after_last = "cloudsmith_after_class_path" # The shell expands \ " $ and ` in double quotes, so the path escapes them. jar_text = re.sub(r'([\\"$`])', r"\\\1", str(jar)) + # mvn splits MAVEN_OPTS at spaces, so an option starts after a space. + words = "cloudsmith_maven_opts" return ( f"{BEGIN}\n" "# Java uses only the last -D value, so add the JAR to that value.\n" - 'case "$MAVEN_OPTS" in\n' - f" *{option}*)\n" - f' {after_last}="${{MAVEN_OPTS##*{option}}}"\n' + f'{words}=" $MAVEN_OPTS"\n' + f'case "${words}" in\n' + f' *" {option}"*)\n' + f' {after_last}="${{{words}##* {option}}}"\n' f' case "{os.pathsep}${{{after_last}%% *}}{os.pathsep}" in\n' f' *"{os.pathsep}{jar_text}{os.pathsep}"*) ;;\n' - f' *) MAVEN_OPTS="${{MAVEN_OPTS%{option}*}}{option}{jar_text}{os.pathsep}' - f'${after_last}" ;;\n' - " esac\n" - f" unset {after_last} ;;\n" + f' *) MAVEN_OPTS="${{{words}% {option}*}} {option}{jar_text}{os.pathsep}' + f'${after_last}"\n' + ' MAVEN_OPTS="${MAVEN_OPTS# }" ;;\n' + " esac ;;\n" f' *) MAVEN_OPTS="$MAVEN_OPTS {option}{jar_text}" ;;\n' "esac\n" + f"unset {words} {after_last}\n" f"{END}\n" ) From cf5441e4d5013f5bb5e90e06d7f729bb94af218d Mon Sep 17 00:00:00 2001 From: Ian Duffy Date: Tue, 3 Nov 2026 12:18:26 +0000 Subject: [PATCH 24/27] Find the Maven class path option after a tab or a new line Co-Authored-By: Claude Opus 5.5 --- .../cli/tests/test_credential_helper_maven_tools.py | 11 +++++++++++ cloudsmith_cli/credential_helpers/maven/mavenrc.py | 4 ++-- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py b/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py index 8e07761f..d2d9eb0c 100644 --- a/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py +++ b/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py @@ -632,6 +632,17 @@ def test_block_ignores_the_class_path_option_inside_another_value(): ) +@POSIX_SHELL_ONLY +@pytest.mark.parametrize("separator", ["\t", "\n"]) +def test_block_finds_the_class_path_after_a_tab_or_a_new_line(separator): + """mvn splits MAVEN_OPTS at tabs and new lines too.""" + maven_opts = f"-Xmx1g{separator}-Dmaven.ext.class.path=/opt/other.jar" + + assert _maven_opts_after_block(maven_opts) == ( + f"-Xmx1g -Dmaven.ext.class.path={JAR}{os.pathsep}/opt/other.jar" + ) + + @POSIX_SHELL_ONLY def test_block_keeps_the_other_extensions_in_the_class_path(): """Java uses only the last -D value, so the block adds the JAR to that value.""" diff --git a/cloudsmith_cli/credential_helpers/maven/mavenrc.py b/cloudsmith_cli/credential_helpers/maven/mavenrc.py index 43af5299..bbc8778d 100644 --- a/cloudsmith_cli/credential_helpers/maven/mavenrc.py +++ b/cloudsmith_cli/credential_helpers/maven/mavenrc.py @@ -32,12 +32,12 @@ def render_block(jar: Path) -> str: after_last = "cloudsmith_after_class_path" # The shell expands \ " $ and ` in double quotes, so the path escapes them. jar_text = re.sub(r'([\\"$`])', r"\\\1", str(jar)) - # mvn splits MAVEN_OPTS at spaces, so an option starts after a space. + # mvn splits MAVEN_OPTS at spaces, tabs and new lines. The copy has only spaces. words = "cloudsmith_maven_opts" return ( f"{BEGIN}\n" "# Java uses only the last -D value, so add the JAR to that value.\n" - f'{words}=" $MAVEN_OPTS"\n' + f"{words}=\" $(printf %s \"$MAVEN_OPTS\" | tr '\\t\\n' ' ')\"\n" f'case "${words}" in\n' f' *" {option}"*)\n' f' {after_last}="${{{words}##* {option}}}"\n' From 738c9efe9df4b7a625a0f1f745942bc74303b4a6 Mon Sep 17 00:00:00 2001 From: Ian Duffy Date: Tue, 3 Nov 2026 12:24:06 +0000 Subject: [PATCH 25/27] Keep an extensions.xml with comments when uninstall removes the last extension Co-Authored-By: Claude Opus 5.5 --- .../cli/tests/test_credential_helper_maven_tools.py | 2 +- cloudsmith_cli/credential_helpers/maven/extensions.py | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py b/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py index d2d9eb0c..b514ae58 100644 --- a/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py +++ b/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py @@ -342,7 +342,7 @@ def test_remove_extension_ignores_a_commented_out_extension(): assert extensions.remove_extension( f"\n{comment}{OUR_EXTENSION}\n" - ) == (None, True) + ) == (f"\n{comment}\n", True) assert extensions.remove_extension(f"\n{comment}\n") == ( f"\n{comment}\n", False, diff --git a/cloudsmith_cli/credential_helpers/maven/extensions.py b/cloudsmith_cli/credential_helpers/maven/extensions.py index c5c8c133..31035127 100644 --- a/cloudsmith_cli/credential_helpers/maven/extensions.py +++ b/cloudsmith_cli/credential_helpers/maven/extensions.py @@ -117,7 +117,8 @@ def add_extension(text: str | None, version: str) -> tuple[str, bool]: def remove_extension(text: str) -> tuple[str | None, bool]: """Return *text* without the Cloudsmith extension, and whether it changed. - The text is ``None`` when no other extension is left, so the file is not needed. + The text is ``None`` when no other extension or comment is left, so the file is + not needed. A file that is not valid XML without the extension is kept. """ masked = _without_comments(text) @@ -130,6 +131,6 @@ def remove_extension(text: str) -> tuple[str | None, bool]: end = len(text) - len(text[block.end() :].lstrip(" \t")) end += text.startswith("\n", end) new_text = text[:start] + text[end:] - if not _has_extension_element(new_text): + if not _has_extension_element(new_text) and not _COMMENT.search(new_text): return None, True return new_text, True From 6356d43fdfe30b85f5ca32f220172a4c10dd37c9 Mon Sep 17 00:00:00 2001 From: Ian Duffy Date: Tue, 3 Nov 2026 12:29:46 +0000 Subject: [PATCH 26/27] Turn off the other user-level Maven activation on install Maven 3.10 reads both ~/.m2/extensions.xml and ~/.mavenrc, so only one of them loads the extension. Co-Authored-By: Claude Opus 5.5 --- .../test_credential_helper_maven_tools.py | 27 +++++++++++++++++++ .../credential_helpers/maven/installer.py | 17 ++++++++++-- 2 files changed, 42 insertions(+), 2 deletions(-) diff --git a/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py b/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py index b514ae58..c3cada20 100644 --- a/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py +++ b/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py @@ -834,6 +834,33 @@ def test_maven_install_with_mavenrc_adds_a_block( assert not _user_extensions(home).exists() +def test_maven_install_with_mavenrc_turns_off_the_user_extensions_file( + home, fake_repository, outside_projects +): + """Maven 3.10 loads both, so only one of them loads the extension.""" + MavenInstaller().install() + + MavenInstaller().install(mavenrc=True) + + assert not _user_extensions(home).exists() + assert mavenrc.has_block((home / ".mavenrc").read_text(encoding="utf-8")) + + +def test_maven_install_turns_off_the_mavenrc_block( + home, fake_repository, outside_projects +): + """Maven 3.10 loads both, so only one of them loads the extension.""" + (home / ".mavenrc").write_text('export JAVA_HOME="/opt/java"\n', encoding="utf-8") + MavenInstaller().install(mavenrc=True) + + MavenInstaller().install() + + assert extensions.has_extension(_user_extensions(home).read_text(encoding="utf-8")) + assert (home / ".mavenrc").read_text(encoding="utf-8") == ( + 'export JAVA_HOME="/opt/java"\n' + ) + + def test_maven_install_dry_run_changes_nothing(home, fake_repository, maven_project): """A dry run reports the planned changes and writes no files.""" actions = [ diff --git a/cloudsmith_cli/credential_helpers/maven/installer.py b/cloudsmith_cli/credential_helpers/maven/installer.py index 7e3e228a..e5a3886d 100644 --- a/cloudsmith_cli/credential_helpers/maven/installer.py +++ b/cloudsmith_cli/credential_helpers/maven/installer.py @@ -154,6 +154,14 @@ def _remove(path: Path, edit: Callable, dry_run: bool) -> list[str]: return [f"removed the extension from {path}"] +def _turn_off(path: Path, edit: Callable, dry_run: bool) -> list[str]: + """Remove the other user-level activation, because Maven 3.10 reads both files.""" + current = _read(path) + if current is None or not edit(current)[1]: + return [] + return _remove(path, edit, dry_run) + + class MavenInstaller: name = "maven" summary = "Maven extension that gets credentials from the Cloudsmith CLI" @@ -175,8 +183,12 @@ def install( actions = self._download(version, dry_run, session) if mavenrc: jar = _version_dir(version) / f"{extensions.ARTIFACT_ID}-{version}.jar" - return actions + _add( - _mavenrc_file(), partial(add_block, jar=jar), version, dry_run + return ( + actions + + _add(_mavenrc_file(), partial(add_block, jar=jar), version, dry_run) + + _turn_off( + _user_extensions_file(), extensions.remove_extension, dry_run + ) ) add_extension = partial(extensions.add_extension, version=version) if project_file: @@ -184,6 +196,7 @@ def install( return ( actions + _add(_user_extensions_file(), add_extension, version, dry_run) + + _turn_off(_mavenrc_file(), remove_block, dry_run) + _old_maven_warning() ) From a09bf0fa474db76b50c62c766b8d8ab1fdb2584a Mon Sep 17 00:00:00 2001 From: Ian Duffy Date: Tue, 3 Nov 2026 12:35:57 +0000 Subject: [PATCH 27/27] Find an sbt build that has only .sbt or .scala files in project/ Co-Authored-By: Claude Opus 5.5 --- .../tests/test_credential_helper_maven_tools.py | 9 ++++++++- .../credential_helpers/sbt/installer.py | 15 +++++++++------ 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py b/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py index c3cada20..6246a6e3 100644 --- a/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py +++ b/cloudsmith_cli/cli/tests/test_credential_helper_maven_tools.py @@ -1094,7 +1094,14 @@ def test_sbt_install_local_outside_a_project_fails( @pytest.mark.parametrize( - "build_file", ["main.sbt", "project/build.properties", "project/cloudsmith.sbt"] + "build_file", + [ + "main.sbt", + "project/build.properties", + "project/cloudsmith.sbt", + "project/plugins.sbt", + "project/Build.scala", + ], ) def test_sbt_finds_a_project_without_a_build_sbt(outside_projects, build_file): """sbt reads any .sbt file in the root and the project/ directory.""" diff --git a/cloudsmith_cli/credential_helpers/sbt/installer.py b/cloudsmith_cli/credential_helpers/sbt/installer.py index 32645794..c2d13010 100644 --- a/cloudsmith_cli/credential_helpers/sbt/installer.py +++ b/cloudsmith_cli/credential_helpers/sbt/installer.py @@ -25,13 +25,16 @@ def path(self) -> Path: def local_path(self) -> Path: # sbt loads every .sbt file in project/, so the build's own files stay as they are. project_dir = Path.cwd() - plugin_file = project_dir / "project" / "cloudsmith.sbt" + meta_dir = project_dir / "project" + plugin_file = meta_dir / "cloudsmith.sbt" # sbt reads any .sbt file in the root, and a build can have only project/. - is_project = ( - any(path.is_file() for path in project_dir.glob("*.sbt")) - or (project_dir / "project" / "build.properties").is_file() - or plugin_file.is_file() - ) + build_files = [ + *project_dir.glob("*.sbt"), + meta_dir / "build.properties", + *meta_dir.glob("*.sbt"), + *meta_dir.glob("*.scala"), + ] + is_project = any(path.is_file() for path in build_files) if not is_project: raise ProjectNotFound( f"No build.sbt in {project_dir}. "