From d0bbee7c0c91416c7bc916bb165feb7c3bcc830c Mon Sep 17 00:00:00 2001 From: Michael Kriese Date: Tue, 22 Sep 2026 14:03:42 +0200 Subject: [PATCH 1/9] feat: convert the git tool to a typescript installer Replaces the last v1 shell tool with a `GitInstallService` and a `GitPrepareService`. The prepare step adds the `git-core` ppa keyring and source list, the install step runs apt through `AptService` and verifies the installed version is at least 2.33.0, and uninstall removes the package again via a new `AptService.remove`. git still requires root, which is enforced by a `needsRoot` flag on the install service: the cli now reports `NotRoot` with a clear message instead of failing somewhere inside apt. Co-Authored-By: Claude Opus 5 --- src/cli/install-tool/base-install.service.ts | 6 + src/cli/install-tool/index.ts | 2 + .../install-tool/install-tool.service.spec.ts | 54 +++++++- src/cli/install-tool/install-tool.service.ts | 17 ++- src/cli/prepare-tool/index.ts | 2 + src/cli/services/apt.service.spec.ts | 10 ++ src/cli/services/apt.service.ts | 6 + src/cli/tools/git/index.spec.ts | 129 ++++++++++++++++++ src/cli/tools/git/index.ts | 102 ++++++++++++++ src/cli/utils/codes.ts | 5 + src/usr/local/containerbase/tools/git.sh | 39 ------ 11 files changed, 330 insertions(+), 42 deletions(-) create mode 100644 src/cli/tools/git/index.spec.ts create mode 100644 src/cli/tools/git/index.ts delete mode 100644 src/usr/local/containerbase/tools/git.sh diff --git a/src/cli/install-tool/base-install.service.ts b/src/cli/install-tool/base-install.service.ts index feb3909207..fae7b218b8 100644 --- a/src/cli/install-tool/base-install.service.ts +++ b/src/cli/install-tool/base-install.service.ts @@ -48,6 +48,12 @@ export abstract class BaseInstallService { */ readonly parent?: string; + /** + * Some tools can only be installed as root, so they are only available at + * image build time. Eg. git is installed via apt. + */ + readonly needsRoot: boolean = false; + /** * Optional tool type for dynamic uninstallation support. * Currently `npm`, `gem` or `pip`. diff --git a/src/cli/install-tool/index.ts b/src/cli/install-tool/index.ts index 80ea5d39b7..6f9860e93f 100644 --- a/src/cli/install-tool/index.ts +++ b/src/cli/install-tool/index.ts @@ -31,6 +31,7 @@ import { ErlangInstallService } from '../tools/erlang/index.ts'; import { FlutterInstallService } from '../tools/flutter.ts'; import { FluxInstallService } from '../tools/flux.ts'; import { GhInstallService } from '../tools/gh.ts'; +import { GitInstallService } from '../tools/git/index.ts'; import { GitLfsInstallService } from '../tools/git/lfs.ts'; import { GleamInstallService } from '../tools/gleam.ts'; import { GolangInstallService } from '../tools/golang.ts'; @@ -156,6 +157,7 @@ async function prepareInstallContainer(): Promise { container.bind(INSTALL_TOOL_TOKEN).to(FlutterInstallService); container.bind(INSTALL_TOOL_TOKEN).to(FluxInstallService); container.bind(INSTALL_TOOL_TOKEN).to(GhInstallService); + container.bind(INSTALL_TOOL_TOKEN).to(GitInstallService); container.bind(INSTALL_TOOL_TOKEN).to(GitLfsInstallService); container.bind(INSTALL_TOOL_TOKEN).to(GhcInstallService); container.bind(INSTALL_TOOL_TOKEN).to(GleamInstallService); diff --git a/src/cli/install-tool/install-tool.service.spec.ts b/src/cli/install-tool/install-tool.service.spec.ts index 30096e57d8..2fd911e8f6 100644 --- a/src/cli/install-tool/install-tool.service.spec.ts +++ b/src/cli/install-tool/install-tool.service.spec.ts @@ -1,6 +1,6 @@ import fs from 'node:fs/promises'; import { execa } from 'execa'; -import type { Container } from 'inversify'; +import { type Container, injectFromHierarchy, injectable } from 'inversify'; import { beforeAll, beforeEach, describe, expect, test, vi } from 'vitest'; import { initializeTools, prepareTools } from '../prepare-tool/index.ts'; import { @@ -10,8 +10,9 @@ import { createContainer, } from '../services/index.ts'; import { BunInstallService } from '../tools/bun.ts'; -import { BlockingChild, NotSupported } from '../utils/codes.ts'; +import { BlockingChild, NotRoot, NotSupported } from '../utils/codes.ts'; import { isDockerBuild, logger } from '../utils/index.ts'; +import { BaseInstallService } from './base-install.service.ts'; import { V1ToolInstallService } from './install-legacy-tool.service.ts'; import { INSTALL_TOOL_TOKEN, @@ -29,12 +30,39 @@ vi.mock('../utils/index.ts', async (importActual) => ({ isDockerBuild: vi.fn(), })); +/** a tool which can only be installed at image build time, like `git` */ +@injectable() +@injectFromHierarchy() +class RootOnlyInstallService extends BaseInstallService { + override readonly name = 'root-only'; + + override readonly needsRoot = true; + + override install(_version: string): Promise { + return Promise.resolve(); + } + + override link(_version: string): Promise { + return Promise.resolve(); + } +} + describe('cli/install-tool/install-tool.service', () => { const parent = createContainer(); parent.bind(InstallToolService).toSelf(); parent.bind(V1ToolInstallService).toSelf(); parent.bind(INSTALL_TOOL_TOKEN).to(BunInstallService); + // a second container, so the tool lookups above stay unambiguous + const rootOnlyParent = createContainer(); + rootOnlyParent.bind(InstallToolService).toSelf(); + rootOnlyParent.bind(V1ToolInstallService).toSelf(); + rootOnlyParent.bind(INSTALL_TOOL_TOKEN).to(RootOnlyInstallService); + + function rootOnlyService(): Promise { + return createContainer(rootOnlyParent).getAsync(InstallToolService); + } + let child: Container; let install: InstallToolService; beforeAll(async () => { @@ -67,6 +95,16 @@ describe('cli/install-tool/install-tool.service', () => { }); }); + test('fails if the tool needs root', async () => { + const svc = await rootOnlyService(); + + expect(await svc.install('root-only', '1.0.0')).toBe(NotRoot); + expect(logger.fatal).toHaveBeenCalledExactlyOnceWith( + { tool: 'root-only' }, + 'tool must be installed as root', + ); + }); + test('writes version even if tool is installed', async () => { const ver = await child.getAsync(VersionService); const bun = await child.getAsync(INSTALL_TOOL_TOKEN); @@ -216,6 +254,18 @@ describe('cli/install-tool/install-tool.service', () => { ); }); + test('fails if the tool needs root', async () => { + const svc = await rootOnlyService(); + const ver = await child.getAsync(VersionService); + await ver.addInstalled({ name: 'root-only', version: '1.0.0' }); + + expect(await svc.uninstall('root-only', '1.0.0')).toBe(NotRoot); + expect(logger.fatal).toHaveBeenCalledExactlyOnceWith( + { tool: 'root-only' }, + 'tool must be uninstalled as root', + ); + }); + test('dry run', async () => { expect(await install.install('bun', '3.0.0')).toBeUndefined(); diff --git a/src/cli/install-tool/install-tool.service.ts b/src/cli/install-tool/install-tool.service.ts index 1e6dad6f3d..d8efaa18e2 100644 --- a/src/cli/install-tool/install-tool.service.ts +++ b/src/cli/install-tool/install-tool.service.ts @@ -12,7 +12,12 @@ import { VersionService, } from '../services/index.ts'; import type { ToolState } from '../services/version.service'; -import { BlockingChild, MissingParent, NotSupported } from '../utils/codes.ts'; +import { + BlockingChild, + MissingParent, + NotRoot, + NotSupported, +} from '../utils/codes.ts'; import { cleanAptFiles, cleanTmpFiles, @@ -62,6 +67,11 @@ export class InstallToolService { await this.ipc.start(); this._link.clear(); if (toolSvc) { + if (toolSvc.needsRoot && !this.envSvc.isRoot) { + logger.fatal({ tool }, 'tool must be installed as root'); + return NotRoot; + } + let parent: ToolState | null = null; if (toolSvc.parent) { @@ -237,6 +247,11 @@ export class InstallToolService { const toolSvc = this.toolSvcs.find((t) => t.name === tool); if (toolSvc) { + if (toolSvc.needsRoot && !this.envSvc.isRoot) { + logger.fatal({ tool }, 'tool must be uninstalled as root'); + return NotRoot; + } + logger.debug({ tool }, 'validate tool'); const childs = await this.versionSvc.getChilds({ name: tool, version }); if (childs.length) { diff --git a/src/cli/prepare-tool/index.ts b/src/cli/prepare-tool/index.ts index c2b49ddbea..e0aec9562d 100644 --- a/src/cli/prepare-tool/index.ts +++ b/src/cli/prepare-tool/index.ts @@ -8,6 +8,7 @@ import { PowershellPrepareService } from '../tools/dotnet/powershell.ts'; import { ElixirPrepareService } from '../tools/erlang/elixir.ts'; import { ErlangPrepareService } from '../tools/erlang/index.ts'; import { FlutterPrepareService } from '../tools/flutter.ts'; +import { GitPrepareService } from '../tools/git/index.ts'; import { GolangPrepareService } from '../tools/golang.ts'; import { CabalPrepareService } from '../tools/haskell/cabal.ts'; import { GhcPrepareService } from '../tools/haskell/ghc.ts'; @@ -61,6 +62,7 @@ async function prepareContainer(): Promise { container.bind(PREPARE_TOOL_TOKEN).to(ErlangPrepareService); container.bind(PREPARE_TOOL_TOKEN).to(FlutterPrepareService); container.bind(PREPARE_TOOL_TOKEN).to(GhcPrepareService); + container.bind(PREPARE_TOOL_TOKEN).to(GitPrepareService); container.bind(PREPARE_TOOL_TOKEN).to(GolangPrepareService); container.bind(PREPARE_TOOL_TOKEN).to(JavaPrepareService); container.bind(PREPARE_TOOL_TOKEN).to(JavaJrePrepareService); diff --git a/src/cli/services/apt.service.spec.ts b/src/cli/services/apt.service.spec.ts index 5a71774c9e..35bc07ab69 100644 --- a/src/cli/services/apt.service.spec.ts +++ b/src/cli/services/apt.service.spec.ts @@ -41,6 +41,16 @@ describe('cli/services/apt.service', () => { expect(mocks.rm).not.toHaveBeenCalled(); }); + test('removes packages', async () => { + await svc.remove('some-pkg'); + expect(mocks.execa).toHaveBeenCalledExactlyOnceWith('apt-get', [ + '-qq', + 'remove', + '-y', + 'some-pkg', + ]); + }); + test('uses proxy', async () => { vi.stubEnv('APT_HTTP_PROXY', 'http://proxy'); mocks.execa.mockRejectedValueOnce(new Error('not installed')); diff --git a/src/cli/services/apt.service.ts b/src/cli/services/apt.service.ts index 85e18e7184..5035b3ccac 100644 --- a/src/cli/services/apt.service.ts +++ b/src/cli/services/apt.service.ts @@ -56,6 +56,12 @@ export class AptService { } } + async remove(...packages: string[]): Promise { + logger.debug({ packages }, 'removing packages'); + + await execa('apt-get', ['-qq', 'remove', '-y', ...packages]); + } + private async isInstalled(pkg: string): Promise { try { const res = await execa('dpkg', ['-s', pkg]); diff --git a/src/cli/tools/git/index.spec.ts b/src/cli/tools/git/index.spec.ts new file mode 100644 index 0000000000..713b81d2c7 --- /dev/null +++ b/src/cli/tools/git/index.spec.ts @@ -0,0 +1,129 @@ +import { readFile } from 'node:fs/promises'; +import { beforeAll, beforeEach, describe, expect, test, vi } from 'vitest'; +import { getDistro, logger } from '../../utils/index.ts'; +import { GitInstallService, GitPrepareService } from './index.ts'; +import { scope } from '~test/http-mock.ts'; +import { ensurePaths, rootPath } from '~test/path.ts'; +import { toolContext } from '~test/tool.ts'; + +const { execaMock } = vi.hoisted(() => ({ execaMock: vi.fn() })); +vi.mock('execa', () => ({ execa: execaMock })); +vi.mock('../../utils/index.ts', async (importActual) => ({ + ...(await importActual()), + getDistro: vi.fn(), +})); + +describe('cli/tools/git/index', () => { + beforeAll(async () => { + await ensurePaths(['tmp', 'opt/containerbase/bin']); + }); + + beforeEach(() => { + vi.mocked(getDistro).mockResolvedValue({ + name: 'Ubuntu', + versionCode: 'noble', + versionId: '24.04', + }); + // CI configures an apt proxy, which `AptService` would write to `/etc` + vi.stubEnv('APT_HTTP_PROXY', undefined); + execaMock.mockResolvedValue({ + failed: false, + stdout: 'git version 2.55.0', + }); + }); + + describe('GitPrepareService', () => { + test('adds the ppa', async () => { + const { svc } = await toolContext(GitPrepareService); + scope('http://keyserver.ubuntu.com') + .get('/pks/lookup') + .query(true) + .reply(200, 'public key'); + + await expect(svc.prepare()).resolves.toBeUndefined(); + + expect(await readFile(rootPath('etc/apt/keyrings/git.asc'), 'utf8')).toBe( + 'public key', + ); + expect( + await readFile(rootPath('etc/apt/sources.list.d/git.list'), 'utf8'), + ).toBe( + 'deb [arch=amd64 signed-by=/etc/apt/keyrings/git.asc] https://ppa.launchpadcontent.net/git-core/ppa/ubuntu noble main\n', + ); + }); + }); + + describe('GitInstallService', () => { + test('install', async () => { + const { svc } = await toolContext(GitInstallService); + + await expect(svc.install('2.55.0')).resolves.toBeUndefined(); + + expect(execaMock).toHaveBeenCalledWith('apt-get', [ + '-qq', + 'install', + '-y', + 'git', + ]); + expect(logger.debug).toHaveBeenCalledWith( + { version: '2.55.0' }, + 'installed git version', + ); + }); + + test('install: rejects a version below the minimum', async () => { + const { svc } = await toolContext(GitInstallService); + execaMock.mockResolvedValue({ + failed: false, + stdout: 'git version 2.32.0', + }); + + await expect(svc.install('2.32.0')).rejects.toThrow( + 'Git version mismatch! Expected: 2.33.0, got: 2.32.0', + ); + }); + + test('link does nothing', async () => { + const { svc } = await toolContext(GitInstallService); + + await expect(svc.link('2.55.0')).resolves.toBeUndefined(); + }); + + test('allows all safe directories', async () => { + const { svc } = await toolContext(GitInstallService); + + await expect(svc.postInstall('2.55.0')).resolves.toBeUndefined(); + + expect(execaMock).toHaveBeenCalledWith( + 'git', + ['config', '--system', 'safe.directory', '*'], + expect.objectContaining({ stdio: ['inherit', 'inherit', 1] }), + ); + }); + + test('prints the version', async () => { + const { svc } = await toolContext(GitInstallService); + + await expect(svc.test('2.55.0')).resolves.toBeUndefined(); + + expect(execaMock).toHaveBeenCalledWith( + 'git', + ['--version'], + expect.objectContaining({ stdio: ['inherit', 'inherit', 1] }), + ); + }); + + test('uninstall', async () => { + const { svc } = await toolContext(GitInstallService); + + await expect(svc.uninstall('2.55.0')).resolves.toBeUndefined(); + + expect(execaMock).toHaveBeenCalledWith('apt-get', [ + '-qq', + 'remove', + '-y', + 'git', + ]); + }); + }); +}); diff --git a/src/cli/tools/git/index.ts b/src/cli/tools/git/index.ts new file mode 100644 index 0000000000..70060cfa2f --- /dev/null +++ b/src/cli/tools/git/index.ts @@ -0,0 +1,102 @@ +import { mkdir, writeFile } from 'node:fs/promises'; +import { join } from 'node:path'; +import { execa } from 'execa'; +import { inject, injectFromHierarchy, injectable } from 'inversify'; +import { BaseInstallService } from '../../install-tool/base-install.service.ts'; +import { BasePrepareService } from '../../prepare-tool/base-prepare.service.ts'; +import { AptService, HttpService } from '../../services/index.ts'; +import { getDistro, logger, parse, semverGte } from '../../utils/index.ts'; + +/** + * Keep in sync with the minimum git version renovate needs. + * https://github.com/renovatebot/renovate/blob/main/lib/util/git/index.ts#L180 + */ +const minVersion = '2.33.0'; + +const keyUrl = + 'http://keyserver.ubuntu.com/pks/lookup?op=get&search=0xF911AB184317630C59970973E363C90F8F1B6217'; +const keyPath = 'etc/apt/keyrings/git.asc'; + +@injectable() +@injectFromHierarchy() +export class GitPrepareService extends BasePrepareService { + @inject(HttpService) + private readonly http!: HttpService; + + override readonly name = 'git'; + + /** + * Adds the `git-core` ppa, ubuntu ships a git version which is too old. + */ + override async prepare(): Promise { + const distro = await getDistro(); + const key = await this.http.get(keyUrl); + + await mkdir(join(this.envSvc.rootDir, 'etc/apt/keyrings'), { + recursive: true, + mode: 0o755, + }); + await mkdir(join(this.envSvc.rootDir, 'etc/apt/sources.list.d'), { + recursive: true, + mode: 0o755, + }); + + await writeFile(join(this.envSvc.rootDir, keyPath), key, { mode: 0o644 }); + await writeFile( + join(this.envSvc.rootDir, 'etc/apt/sources.list.d/git.list'), + `deb [arch=${this.envSvc.arch} signed-by=/${keyPath}] https://ppa.launchpadcontent.net/git-core/ppa/ubuntu ${distro.versionCode} main\n`, + ); + } +} + +@injectable() +@injectFromHierarchy() +export class GitInstallService extends BaseInstallService { + @inject(AptService) + private readonly aptSvc!: AptService; + + override readonly name = 'git'; + + override readonly needsRoot = true; + + override async install(_version: string): Promise { + // TODO: the ppa only serves the latest version, so the requested version is ignored + await this.aptSvc.install(this.name); + + const version = await this.installedVersion(); + if (!semverGte(version, minVersion)) { + throw new Error( + `Git version mismatch! Expected: ${minVersion}, got: ${version}`, + ); + } + } + + /** + * git is installed system wide by apt, so there is nothing to link. + */ + override link(_version: string): Promise { + return Promise.resolve(); + } + + override async postInstall(_version: string): Promise { + // flutter workaround + // allow all, so it works in older git versions when the ppa is not working + await this._spawn(this.name, ['config', '--system', 'safe.directory', '*']); + } + + override async test(_version: string): Promise { + await this._spawn(this.name, ['--version']); + } + + override async uninstall(_version: string): Promise { + await this.aptSvc.remove(this.name); + } + + private async installedVersion(): Promise { + // `git --version` prints eg. `git version 2.55.0` + const res = await execa(this.name, ['--version']); + const { version } = parse(res.stdout.split(' ').pop()); + logger.debug({ version }, 'installed git version'); + return version; + } +} diff --git a/src/cli/utils/codes.ts b/src/cli/utils/codes.ts index 78c935d2df..cd50a5358b 100644 --- a/src/cli/utils/codes.ts +++ b/src/cli/utils/codes.ts @@ -22,3 +22,8 @@ export const CurrentVersion = 17; * A child dependency blocks removal of parent. */ export const BlockingChild = 18; + +/** + * The tool can only be installed or uninstalled as root. + */ +export const NotRoot = 19; diff --git a/src/usr/local/containerbase/tools/git.sh b/src/usr/local/containerbase/tools/git.sh deleted file mode 100644 index acb2da7822..0000000000 --- a/src/usr/local/containerbase/tools/git.sh +++ /dev/null @@ -1,39 +0,0 @@ -#!/bin/bash - -require_root - -version_codename=$(get_distro) - -install -m 0755 -d /etc/apt/keyrings -curl --retry 3 -fsSL -o /etc/apt/keyrings/git.asc \ - 'http://keyserver.ubuntu.com/pks/lookup?op=get&search=0xF911AB184317630C59970973E363C90F8F1B6217' -chmod a+r /etc/apt/keyrings/git.asc - -echo "deb [arch=$(dpkg --print-architecture) signed-by=/etc/apt/keyrings/git.asc] https://ppa.launchpadcontent.net/git-core/ppa/ubuntu ${version_codename} main" | tee /etc/apt/sources.list.d/git.list - - -# TODO: Only latest version available on launchpad :-/ -#apt_install git=1:${TOOL_VERSION}* - -apt_install git - -# Keep in sync renovate minimum git version -# https://github.com/renovatebot/renovate/blob/main/lib/util/git/index.ts#L180 -function validate_git_version() { - local required_version="2.33.0" - local current_version - current_version=$(git --version | awk '{print $3}') - - if dpkg --compare-versions "${current_version}" lt "${required_version}"; then - echo "Git version mismatch! Expected: ${required_version}, got: ${current_version}" - exit 1 - fi -} - -validate_git_version - -# flutter workaround -# allow all, so it works in older git versions when ppa is not working -git config --system safe.directory "*" - -[[ -n $SKIP_VERSION ]] || git --version From b270d0e2e09eef97a998f3a37ecb42cb9cb5d446 Mon Sep 17 00:00:00 2001 From: Michael Kriese Date: Wed, 23 Sep 2026 08:59:53 +0200 Subject: [PATCH 2/9] fix: don't create the apt sources directory `etc/apt/sources.list.d` ships with the image, so the prepare step can just write the source list. The unit test creates the directory with `ensurePaths`, mirroring the image layout. Co-Authored-By: Claude Opus 5 --- src/cli/tools/git/index.spec.ts | 7 ++++++- src/cli/tools/git/index.ts | 4 ---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/cli/tools/git/index.spec.ts b/src/cli/tools/git/index.spec.ts index 713b81d2c7..4e7fbc7a5d 100644 --- a/src/cli/tools/git/index.spec.ts +++ b/src/cli/tools/git/index.spec.ts @@ -15,7 +15,12 @@ vi.mock('../../utils/index.ts', async (importActual) => ({ describe('cli/tools/git/index', () => { beforeAll(async () => { - await ensurePaths(['tmp', 'opt/containerbase/bin']); + // `etc/apt/sources.list.d` ships with the image + await ensurePaths([ + 'tmp', + 'etc/apt/sources.list.d', + 'opt/containerbase/bin', + ]); }); beforeEach(() => { diff --git a/src/cli/tools/git/index.ts b/src/cli/tools/git/index.ts index 70060cfa2f..166c3e9bf1 100644 --- a/src/cli/tools/git/index.ts +++ b/src/cli/tools/git/index.ts @@ -36,10 +36,6 @@ export class GitPrepareService extends BasePrepareService { recursive: true, mode: 0o755, }); - await mkdir(join(this.envSvc.rootDir, 'etc/apt/sources.list.d'), { - recursive: true, - mode: 0o755, - }); await writeFile(join(this.envSvc.rootDir, keyPath), key, { mode: 0o644 }); await writeFile( From c3b98e74b51d11981abfb02e024846d9b61f1738 Mon Sep 17 00:00:00 2001 From: Michael Kriese Date: Wed, 23 Sep 2026 09:04:44 +0200 Subject: [PATCH 3/9] test: drop the extra container from the install tool service spec Binds the root only tool into the existing container and mocks the bun service through its prototype, like the surrounding tests already do, so no second container is needed to keep the tool lookups unambiguous. Co-Authored-By: Claude Opus 5 --- .../install-tool/install-tool.service.spec.ts | 42 ++++++++----------- 1 file changed, 18 insertions(+), 24 deletions(-) diff --git a/src/cli/install-tool/install-tool.service.spec.ts b/src/cli/install-tool/install-tool.service.spec.ts index 2fd911e8f6..bc1fdea450 100644 --- a/src/cli/install-tool/install-tool.service.spec.ts +++ b/src/cli/install-tool/install-tool.service.spec.ts @@ -52,16 +52,7 @@ describe('cli/install-tool/install-tool.service', () => { parent.bind(InstallToolService).toSelf(); parent.bind(V1ToolInstallService).toSelf(); parent.bind(INSTALL_TOOL_TOKEN).to(BunInstallService); - - // a second container, so the tool lookups above stay unambiguous - const rootOnlyParent = createContainer(); - rootOnlyParent.bind(InstallToolService).toSelf(); - rootOnlyParent.bind(V1ToolInstallService).toSelf(); - rootOnlyParent.bind(INSTALL_TOOL_TOKEN).to(RootOnlyInstallService); - - function rootOnlyService(): Promise { - return createContainer(rootOnlyParent).getAsync(InstallToolService); - } + parent.bind(INSTALL_TOOL_TOKEN).to(RootOnlyInstallService); let child: Container; let install: InstallToolService; @@ -85,9 +76,12 @@ describe('cli/install-tool/install-tool.service', () => { describe('install', () => { test('writes version if tool is not installed', async () => { const ver = await child.getAsync(VersionService); - const bun = await child.getAsync(INSTALL_TOOL_TOKEN); - vi.mocked(bun).needsInitialize.mockReturnValueOnce(true); - vi.mocked(bun).needsPrepare.mockReturnValueOnce(true); + vi.mocked( + BunInstallService.prototype, + ).needsInitialize.mockReturnValueOnce(true); + vi.mocked(BunInstallService.prototype).needsPrepare.mockReturnValueOnce( + true, + ); expect(await install.install('bun', '1.0.0')).toBeUndefined(); expect(await ver.getCurrent('bun')).toMatchObject({ name: 'bun', @@ -96,9 +90,7 @@ describe('cli/install-tool/install-tool.service', () => { }); test('fails if the tool needs root', async () => { - const svc = await rootOnlyService(); - - expect(await svc.install('root-only', '1.0.0')).toBe(NotRoot); + expect(await install.install('root-only', '1.0.0')).toBe(NotRoot); expect(logger.fatal).toHaveBeenCalledExactlyOnceWith( { tool: 'root-only' }, 'tool must be installed as root', @@ -107,8 +99,9 @@ describe('cli/install-tool/install-tool.service', () => { test('writes version even if tool is installed', async () => { const ver = await child.getAsync(VersionService); - const bun = await child.getAsync(INSTALL_TOOL_TOKEN); - vi.mocked(bun).isInstalled.mockResolvedValueOnce(true); + vi.mocked(BunInstallService.prototype).isInstalled.mockResolvedValueOnce( + true, + ); expect(await install.install('bun', '1.0.1')).toBeUndefined(); expect(await ver.getCurrent('bun')).toMatchObject({ name: 'bun', @@ -198,16 +191,18 @@ describe('cli/install-tool/install-tool.service', () => { }); test('aborts when the tool cannot be prepared', async () => { - const bun = await child.getAsync(INSTALL_TOOL_TOKEN); - vi.mocked(bun).needsPrepare.mockReturnValueOnce(true); + vi.mocked(BunInstallService.prototype).needsPrepare.mockReturnValueOnce( + true, + ); vi.mocked(prepareTools).mockResolvedValueOnce(1); expect(await install.install('bun', '1.1.0')).toBe(1); }); test('aborts when the tool cannot be initialized', async () => { - const bun = await child.getAsync(INSTALL_TOOL_TOKEN); - vi.mocked(bun).needsInitialize.mockReturnValueOnce(true); + vi.mocked( + BunInstallService.prototype, + ).needsInitialize.mockReturnValueOnce(true); vi.mocked(initializeTools).mockResolvedValueOnce(1); expect(await install.install('bun', '1.1.1')).toBe(1); @@ -255,11 +250,10 @@ describe('cli/install-tool/install-tool.service', () => { }); test('fails if the tool needs root', async () => { - const svc = await rootOnlyService(); const ver = await child.getAsync(VersionService); await ver.addInstalled({ name: 'root-only', version: '1.0.0' }); - expect(await svc.uninstall('root-only', '1.0.0')).toBe(NotRoot); + expect(await install.uninstall('root-only', '1.0.0')).toBe(NotRoot); expect(logger.fatal).toHaveBeenCalledExactlyOnceWith( { tool: 'root-only' }, 'tool must be uninstalled as root', From 343be1ac677d7410e4df4fb1cd4f871767233ec7 Mon Sep 17 00:00:00 2001 From: Michael Kriese Date: Wed, 23 Sep 2026 09:07:42 +0200 Subject: [PATCH 4/9] feat: write a deb822 apt sources file Writes `etc/apt/sources.list.d/git.sources` in the deb822 format instead of the one line `git.list`, matching how apt sources are written nowadays. Co-Authored-By: Claude Opus 5 --- src/cli/tools/git/index.spec.ts | 10 ++++++++-- src/cli/tools/git/index.ts | 12 ++++++++++-- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/src/cli/tools/git/index.spec.ts b/src/cli/tools/git/index.spec.ts index 4e7fbc7a5d..ad54bc1044 100644 --- a/src/cli/tools/git/index.spec.ts +++ b/src/cli/tools/git/index.spec.ts @@ -51,9 +51,15 @@ describe('cli/tools/git/index', () => { 'public key', ); expect( - await readFile(rootPath('etc/apt/sources.list.d/git.list'), 'utf8'), + await readFile(rootPath('etc/apt/sources.list.d/git.sources'), 'utf8'), ).toBe( - 'deb [arch=amd64 signed-by=/etc/apt/keyrings/git.asc] https://ppa.launchpadcontent.net/git-core/ppa/ubuntu noble main\n', + `Types: deb +URIs: https://ppa.launchpadcontent.net/git-core/ppa/ubuntu +Suites: noble +Components: main +Architectures: amd64 +Signed-By: /etc/apt/keyrings/git.asc +`, ); }); }); diff --git a/src/cli/tools/git/index.ts b/src/cli/tools/git/index.ts index 166c3e9bf1..7b993894bf 100644 --- a/src/cli/tools/git/index.ts +++ b/src/cli/tools/git/index.ts @@ -39,8 +39,16 @@ export class GitPrepareService extends BasePrepareService { await writeFile(join(this.envSvc.rootDir, keyPath), key, { mode: 0o644 }); await writeFile( - join(this.envSvc.rootDir, 'etc/apt/sources.list.d/git.list'), - `deb [arch=${this.envSvc.arch} signed-by=/${keyPath}] https://ppa.launchpadcontent.net/git-core/ppa/ubuntu ${distro.versionCode} main\n`, + join(this.envSvc.rootDir, 'etc/apt/sources.list.d/git.sources'), + [ + 'Types: deb', + 'URIs: https://ppa.launchpadcontent.net/git-core/ppa/ubuntu', + `Suites: ${distro.versionCode}`, + 'Components: main', + `Architectures: ${this.envSvc.arch}`, + `Signed-By: /${keyPath}`, + '', + ].join('\n'), ); } } From d83cd2ca316ce4c447bd7af0e85bc930a4a5263c Mon Sep 17 00:00:00 2001 From: Michael Kriese Date: Wed, 23 Sep 2026 09:09:14 +0200 Subject: [PATCH 5/9] refactor: build the apt sources file with codeBlock Uses the `codeBlock` tag from common-tags, like the other generated config files in this repo, instead of joining an array of lines. The dedented output has no trailing newline, which matches how the conan profile and the maven settings are written. Co-Authored-By: Claude Opus 5 --- src/cli/tools/git/index.spec.ts | 3 +-- src/cli/tools/git/index.ts | 18 +++++++++--------- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/src/cli/tools/git/index.spec.ts b/src/cli/tools/git/index.spec.ts index ad54bc1044..ad63b258db 100644 --- a/src/cli/tools/git/index.spec.ts +++ b/src/cli/tools/git/index.spec.ts @@ -58,8 +58,7 @@ URIs: https://ppa.launchpadcontent.net/git-core/ppa/ubuntu Suites: noble Components: main Architectures: amd64 -Signed-By: /etc/apt/keyrings/git.asc -`, +Signed-By: /etc/apt/keyrings/git.asc`, ); }); }); diff --git a/src/cli/tools/git/index.ts b/src/cli/tools/git/index.ts index 7b993894bf..af194187ed 100644 --- a/src/cli/tools/git/index.ts +++ b/src/cli/tools/git/index.ts @@ -1,5 +1,6 @@ import { mkdir, writeFile } from 'node:fs/promises'; import { join } from 'node:path'; +import { codeBlock } from 'common-tags'; import { execa } from 'execa'; import { inject, injectFromHierarchy, injectable } from 'inversify'; import { BaseInstallService } from '../../install-tool/base-install.service.ts'; @@ -40,15 +41,14 @@ export class GitPrepareService extends BasePrepareService { await writeFile(join(this.envSvc.rootDir, keyPath), key, { mode: 0o644 }); await writeFile( join(this.envSvc.rootDir, 'etc/apt/sources.list.d/git.sources'), - [ - 'Types: deb', - 'URIs: https://ppa.launchpadcontent.net/git-core/ppa/ubuntu', - `Suites: ${distro.versionCode}`, - 'Components: main', - `Architectures: ${this.envSvc.arch}`, - `Signed-By: /${keyPath}`, - '', - ].join('\n'), + codeBlock` + Types: deb + URIs: https://ppa.launchpadcontent.net/git-core/ppa/ubuntu + Suites: ${distro.versionCode} + Components: main + Architectures: ${this.envSvc.arch} + Signed-By: /${keyPath} + `, ); } } From 0531ff64172365fdb243df7351e0f2919f375fcf Mon Sep 17 00:00:00 2001 From: Michael Kriese Date: Wed, 23 Sep 2026 09:17:01 +0200 Subject: [PATCH 6/9] test: use codeBlock for the apt sources expectation Keeps the expected file contents indented with the surrounding test code instead of flush to column zero. Co-Authored-By: Claude Opus 5 --- src/cli/tools/git/index.spec.ts | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/cli/tools/git/index.spec.ts b/src/cli/tools/git/index.spec.ts index ad63b258db..ddd4fd63d1 100644 --- a/src/cli/tools/git/index.spec.ts +++ b/src/cli/tools/git/index.spec.ts @@ -1,4 +1,5 @@ import { readFile } from 'node:fs/promises'; +import { codeBlock } from 'common-tags'; import { beforeAll, beforeEach, describe, expect, test, vi } from 'vitest'; import { getDistro, logger } from '../../utils/index.ts'; import { GitInstallService, GitPrepareService } from './index.ts'; @@ -52,14 +53,14 @@ describe('cli/tools/git/index', () => { ); expect( await readFile(rootPath('etc/apt/sources.list.d/git.sources'), 'utf8'), - ).toBe( - `Types: deb -URIs: https://ppa.launchpadcontent.net/git-core/ppa/ubuntu -Suites: noble -Components: main -Architectures: amd64 -Signed-By: /etc/apt/keyrings/git.asc`, - ); + ).toBe(codeBlock` + Types: deb + URIs: https://ppa.launchpadcontent.net/git-core/ppa/ubuntu + Suites: noble + Components: main + Architectures: amd64 + Signed-By: /etc/apt/keyrings/git.asc + `); }); }); From 22d8743e85733a112b4c65d56780f7d4920b85d7 Mon Sep 17 00:00:00 2001 From: Michael Kriese Date: Wed, 23 Sep 2026 09:24:47 +0200 Subject: [PATCH 7/9] fix: coerce the installed git version Distributions may append a suffix like `2.55.0-1ubuntu1` to the version git prints, which `dpkg --compare-versions` handled but semver cannot parse. Coerces the output before comparing it with the minimum version, and fails with a clear message when no version can be found at all. Co-Authored-By: Claude Opus 5 --- src/cli/tools/git/index.spec.ts | 24 ++++++++++++++++++++++++ src/cli/tools/git/index.ts | 16 +++++++++++++--- 2 files changed, 37 insertions(+), 3 deletions(-) diff --git a/src/cli/tools/git/index.spec.ts b/src/cli/tools/git/index.spec.ts index ddd4fd63d1..df53c33b19 100644 --- a/src/cli/tools/git/index.spec.ts +++ b/src/cli/tools/git/index.spec.ts @@ -82,6 +82,30 @@ describe('cli/tools/git/index', () => { ); }); + test('install: coerces a vendor version suffix', async () => { + const { svc } = await toolContext(GitInstallService); + execaMock.mockResolvedValue({ + failed: false, + stdout: 'git version 2.55.0-1ubuntu1', + }); + + await expect(svc.install('2.55.0')).resolves.toBeUndefined(); + + expect(logger.debug).toHaveBeenCalledWith( + { version: '2.55.0' }, + 'installed git version', + ); + }); + + test('install: throws on an unparsable version', async () => { + const { svc } = await toolContext(GitInstallService); + execaMock.mockResolvedValue({ failed: false, stdout: 'git version foo' }); + + await expect(svc.install('2.55.0')).rejects.toThrow( + 'Could not parse the git version: git version foo', + ); + }); + test('install: rejects a version below the minimum', async () => { const { svc } = await toolContext(GitInstallService); execaMock.mockResolvedValue({ diff --git a/src/cli/tools/git/index.ts b/src/cli/tools/git/index.ts index af194187ed..93868090d2 100644 --- a/src/cli/tools/git/index.ts +++ b/src/cli/tools/git/index.ts @@ -6,7 +6,12 @@ import { inject, injectFromHierarchy, injectable } from 'inversify'; import { BaseInstallService } from '../../install-tool/base-install.service.ts'; import { BasePrepareService } from '../../prepare-tool/base-prepare.service.ts'; import { AptService, HttpService } from '../../services/index.ts'; -import { getDistro, logger, parse, semverGte } from '../../utils/index.ts'; +import { + getDistro, + logger, + semverCoerce, + semverGte, +} from '../../utils/index.ts'; /** * Keep in sync with the minimum git version renovate needs. @@ -97,9 +102,14 @@ export class GitInstallService extends BaseInstallService { } private async installedVersion(): Promise { - // `git --version` prints eg. `git version 2.55.0` const res = await execa(this.name, ['--version']); - const { version } = parse(res.stdout.split(' ').pop()); + // `git --version` prints eg. `git version 2.55.0`, but the vendor may add a + // suffix like `2.55.0-1ubuntu1`, which semver can't parse + const coerced = semverCoerce(res.stdout); + if (!coerced) { + throw new Error(`Could not parse the git version: ${res.stdout}`); + } + const version = coerced.version; logger.debug({ version }, 'installed git version'); return version; } From 4aaea1b15e11b42668433a400c94d249799d3c81 Mon Sep 17 00:00:00 2001 From: Michael Kriese Date: Wed, 23 Sep 2026 09:59:47 +0200 Subject: [PATCH 8/9] refactor: drop the version debug log The installed version is already printed by the test step, which runs `git --version`. Co-Authored-By: Claude Opus 5 --- src/cli/tools/git/index.spec.ts | 11 +---------- src/cli/tools/git/index.ts | 11 ++--------- 2 files changed, 3 insertions(+), 19 deletions(-) diff --git a/src/cli/tools/git/index.spec.ts b/src/cli/tools/git/index.spec.ts index df53c33b19..a6ba5159c5 100644 --- a/src/cli/tools/git/index.spec.ts +++ b/src/cli/tools/git/index.spec.ts @@ -1,7 +1,7 @@ import { readFile } from 'node:fs/promises'; import { codeBlock } from 'common-tags'; import { beforeAll, beforeEach, describe, expect, test, vi } from 'vitest'; -import { getDistro, logger } from '../../utils/index.ts'; +import { getDistro } from '../../utils/index.ts'; import { GitInstallService, GitPrepareService } from './index.ts'; import { scope } from '~test/http-mock.ts'; import { ensurePaths, rootPath } from '~test/path.ts'; @@ -76,10 +76,6 @@ describe('cli/tools/git/index', () => { '-y', 'git', ]); - expect(logger.debug).toHaveBeenCalledWith( - { version: '2.55.0' }, - 'installed git version', - ); }); test('install: coerces a vendor version suffix', async () => { @@ -90,11 +86,6 @@ describe('cli/tools/git/index', () => { }); await expect(svc.install('2.55.0')).resolves.toBeUndefined(); - - expect(logger.debug).toHaveBeenCalledWith( - { version: '2.55.0' }, - 'installed git version', - ); }); test('install: throws on an unparsable version', async () => { diff --git a/src/cli/tools/git/index.ts b/src/cli/tools/git/index.ts index 93868090d2..fcee2721bd 100644 --- a/src/cli/tools/git/index.ts +++ b/src/cli/tools/git/index.ts @@ -6,12 +6,7 @@ import { inject, injectFromHierarchy, injectable } from 'inversify'; import { BaseInstallService } from '../../install-tool/base-install.service.ts'; import { BasePrepareService } from '../../prepare-tool/base-prepare.service.ts'; import { AptService, HttpService } from '../../services/index.ts'; -import { - getDistro, - logger, - semverCoerce, - semverGte, -} from '../../utils/index.ts'; +import { getDistro, semverCoerce, semverGte } from '../../utils/index.ts'; /** * Keep in sync with the minimum git version renovate needs. @@ -109,8 +104,6 @@ export class GitInstallService extends BaseInstallService { if (!coerced) { throw new Error(`Could not parse the git version: ${res.stdout}`); } - const version = coerced.version; - logger.debug({ version }, 'installed git version'); - return version; + return coerced.version; } } From 9732ac67c0a2c46370dd24a6d0f3dbf27ccd58ce Mon Sep 17 00:00:00 2001 From: Michael Kriese Date: Wed, 23 Sep 2026 10:15:12 +0200 Subject: [PATCH 9/9] fix: don't support uninstalling git The apt package is shared by every recorded git version, so uninstalling one of them would remove the system git while another version is still current and git-lfs still depends on it. Adds a `canUninstall` flag to the install services, which git sets to false, so `uninstall-tool git` reports `NotSupported` like it did before the conversion. Drops the now unused `AptService.remove` again. Co-Authored-By: Claude Opus 5 --- src/cli/install-tool/base-install.service.ts | 6 +++ .../install-tool/install-tool.service.spec.ts | 39 ++++++++++++++++--- src/cli/install-tool/install-tool.service.ts | 5 +++ src/cli/services/apt.service.spec.ts | 10 ----- src/cli/services/apt.service.ts | 6 --- src/cli/tools/git/index.spec.ts | 11 +----- src/cli/tools/git/index.ts | 7 ++-- 7 files changed, 49 insertions(+), 35 deletions(-) diff --git a/src/cli/install-tool/base-install.service.ts b/src/cli/install-tool/base-install.service.ts index fae7b218b8..42bbe982be 100644 --- a/src/cli/install-tool/base-install.service.ts +++ b/src/cli/install-tool/base-install.service.ts @@ -54,6 +54,12 @@ export abstract class BaseInstallService { */ readonly needsRoot: boolean = false; + /** + * Tools which are not installed into a versioned tool path can't be + * uninstalled, eg. git is installed system wide via apt. + */ + readonly canUninstall: boolean = true; + /** * Optional tool type for dynamic uninstallation support. * Currently `npm`, `gem` or `pip`. diff --git a/src/cli/install-tool/install-tool.service.spec.ts b/src/cli/install-tool/install-tool.service.spec.ts index bc1fdea450..87849d97d1 100644 --- a/src/cli/install-tool/install-tool.service.spec.ts +++ b/src/cli/install-tool/install-tool.service.spec.ts @@ -30,14 +30,9 @@ vi.mock('../utils/index.ts', async (importActual) => ({ isDockerBuild: vi.fn(), })); -/** a tool which can only be installed at image build time, like `git` */ @injectable() @injectFromHierarchy() -class RootOnlyInstallService extends BaseInstallService { - override readonly name = 'root-only'; - - override readonly needsRoot = true; - +abstract class TestInstallService extends BaseInstallService { override install(_version: string): Promise { return Promise.resolve(); } @@ -47,12 +42,31 @@ class RootOnlyInstallService extends BaseInstallService { } } +/** a tool which can only be installed at image build time, like `git` */ +@injectable() +@injectFromHierarchy() +class RootOnlyInstallService extends TestInstallService { + override readonly name = 'root-only'; + + override readonly needsRoot = true; +} + +/** a tool which is installed system wide, like `git` */ +@injectable() +@injectFromHierarchy() +class NoUninstallInstallService extends TestInstallService { + override readonly name = 'no-uninstall'; + + override readonly canUninstall = false; +} + describe('cli/install-tool/install-tool.service', () => { const parent = createContainer(); parent.bind(InstallToolService).toSelf(); parent.bind(V1ToolInstallService).toSelf(); parent.bind(INSTALL_TOOL_TOKEN).to(BunInstallService); parent.bind(INSTALL_TOOL_TOKEN).to(RootOnlyInstallService); + parent.bind(INSTALL_TOOL_TOKEN).to(NoUninstallInstallService); let child: Container; let install: InstallToolService; @@ -249,6 +263,19 @@ describe('cli/install-tool/install-tool.service', () => { ); }); + test('fails if the tool cannot be uninstalled', async () => { + const ver = await child.getAsync(VersionService); + await ver.addInstalled({ name: 'no-uninstall', version: '1.0.0' }); + + expect(await install.uninstall('no-uninstall', '1.0.0')).toBe( + NotSupported, + ); + expect(logger.fatal).toHaveBeenCalledExactlyOnceWith( + { tool: 'no-uninstall' }, + 'tool cannot be uninstalled', + ); + }); + test('fails if the tool needs root', async () => { const ver = await child.getAsync(VersionService); await ver.addInstalled({ name: 'root-only', version: '1.0.0' }); diff --git a/src/cli/install-tool/install-tool.service.ts b/src/cli/install-tool/install-tool.service.ts index d8efaa18e2..78fb08370d 100644 --- a/src/cli/install-tool/install-tool.service.ts +++ b/src/cli/install-tool/install-tool.service.ts @@ -247,6 +247,11 @@ export class InstallToolService { const toolSvc = this.toolSvcs.find((t) => t.name === tool); if (toolSvc) { + if (!toolSvc.canUninstall) { + logger.fatal({ tool }, 'tool cannot be uninstalled'); + return NotSupported; + } + if (toolSvc.needsRoot && !this.envSvc.isRoot) { logger.fatal({ tool }, 'tool must be uninstalled as root'); return NotRoot; diff --git a/src/cli/services/apt.service.spec.ts b/src/cli/services/apt.service.spec.ts index 35bc07ab69..5a71774c9e 100644 --- a/src/cli/services/apt.service.spec.ts +++ b/src/cli/services/apt.service.spec.ts @@ -41,16 +41,6 @@ describe('cli/services/apt.service', () => { expect(mocks.rm).not.toHaveBeenCalled(); }); - test('removes packages', async () => { - await svc.remove('some-pkg'); - expect(mocks.execa).toHaveBeenCalledExactlyOnceWith('apt-get', [ - '-qq', - 'remove', - '-y', - 'some-pkg', - ]); - }); - test('uses proxy', async () => { vi.stubEnv('APT_HTTP_PROXY', 'http://proxy'); mocks.execa.mockRejectedValueOnce(new Error('not installed')); diff --git a/src/cli/services/apt.service.ts b/src/cli/services/apt.service.ts index 5035b3ccac..85e18e7184 100644 --- a/src/cli/services/apt.service.ts +++ b/src/cli/services/apt.service.ts @@ -56,12 +56,6 @@ export class AptService { } } - async remove(...packages: string[]): Promise { - logger.debug({ packages }, 'removing packages'); - - await execa('apt-get', ['-qq', 'remove', '-y', ...packages]); - } - private async isInstalled(pkg: string): Promise { try { const res = await execa('dpkg', ['-s', pkg]); diff --git a/src/cli/tools/git/index.spec.ts b/src/cli/tools/git/index.spec.ts index a6ba5159c5..2070b60d28 100644 --- a/src/cli/tools/git/index.spec.ts +++ b/src/cli/tools/git/index.spec.ts @@ -139,17 +139,10 @@ describe('cli/tools/git/index', () => { ); }); - test('uninstall', async () => { + test('cannot be uninstalled', async () => { const { svc } = await toolContext(GitInstallService); - await expect(svc.uninstall('2.55.0')).resolves.toBeUndefined(); - - expect(execaMock).toHaveBeenCalledWith('apt-get', [ - '-qq', - 'remove', - '-y', - 'git', - ]); + expect(svc.canUninstall).toBe(false); }); }); }); diff --git a/src/cli/tools/git/index.ts b/src/cli/tools/git/index.ts index fcee2721bd..1df15693dc 100644 --- a/src/cli/tools/git/index.ts +++ b/src/cli/tools/git/index.ts @@ -63,6 +63,9 @@ export class GitInstallService extends BaseInstallService { override readonly needsRoot = true; + /** git is installed system wide by apt, other tools depend on it */ + override readonly canUninstall = false; + override async install(_version: string): Promise { // TODO: the ppa only serves the latest version, so the requested version is ignored await this.aptSvc.install(this.name); @@ -92,10 +95,6 @@ export class GitInstallService extends BaseInstallService { await this._spawn(this.name, ['--version']); } - override async uninstall(_version: string): Promise { - await this.aptSvc.remove(this.name); - } - private async installedVersion(): Promise { const res = await execa(this.name, ['--version']); // `git --version` prints eg. `git version 2.55.0`, but the vendor may add a