diff --git a/action.yml b/action.yml index 0d120eb..2caa965 100644 --- a/action.yml +++ b/action.yml @@ -29,7 +29,7 @@ inputs: required: false default: "error" debug: - description: "Enable debugging when there are issues with action. Minor performance penalty." + description: "Enable debugging when there are issues with action. Also uploads the action logs and manifests as a workflow artifact, which is otherwise done only when the action fails. Minor performance penalty." required: false default: "false" @@ -37,9 +37,15 @@ outputs: cache-hit: description: "A boolean value to indicate a cache was found for the packages requested." package-version-list: - description: "The main requested packages and versions that are installed. Represented as a sorted comma delimited list with equals delimit on the package version (i.e. =,=,...). Packages installed for a non-native architecture include it (i.e. :=)." + description: > + The main requested packages and versions that are installed. + Represented as a sorted comma delimited list with equals delimit on the package version (i.e. =,=,...). + Packages installed for a non-native architecture include it (i.e. :=). all-package-version-list: - description: "All the pulled in packages and versions, including dependencies, that are installed. Represented as a sorted comma delimited list with equals delimit on the package version (i.e. =,=,...). Packages installed for a non-native architecture include it (i.e. :=)." + description: > + All the pulled in packages and versions, including dependencies, that are installed. + Represented as a sorted comma delimited list with equals delimit on the package version (i.e. =,=,...). + Packages installed for a non-native architecture include it (i.e. :=). runs: using: "node24" diff --git a/src/action.ts b/src/action.ts index 3f08c8c..13636a0 100644 --- a/src/action.ts +++ b/src/action.ts @@ -95,8 +95,9 @@ export class ActionRunner { const cacheHit = manifest !== undefined; // Needed on a cache hit too, to resolve the native arch for outputs. + // apt-fast only speeds up installs, so skip probing for it on a hit. const installManager = await createPackageManager( - true, + !cacheHit, this.logger, this.logger, ); diff --git a/src/cache.ts b/src/cache.ts index b36285a..91859c4 100644 --- a/src/cache.ts +++ b/src/cache.ts @@ -31,6 +31,15 @@ export function resolveCacheDir( */ const NO_TIMEOUT_MS = -1; +/** + * Shell script that extracts each archive given as an argument onto `/`. + * + * Paths are passed as positional arguments, not interpolated, so they are + * never parsed by the shell. + */ +const EXTRACT_ARCHIVES_SCRIPT = + 'for archive do tar --extract --file="$archive" --directory=/ --keep-directory-symlink || exit 1; done'; + /** Maintainer scripts run on restore when install scripts are enabled. */ type InstallScriptType = "preinst" | "postinst"; @@ -258,12 +267,15 @@ export class Cache { } } - for (const { packageName, archivePath } of archivePaths) { - await this.extractArchive(archivePath); - - if (executeInstallScripts) { + if (executeInstallScripts) { + // Each package's scripts must run before the next package is extracted. + for (const { packageName, archivePath } of archivePaths) { + await this.extractArchives([archivePath]); await this.runInstallScripts(packageName); } + } else { + // One process for all archives avoids a sudo and tar spawn per package. + await this.extractArchives(archivePaths.map((item) => item.archivePath)); } // Return every installed package so outputs can report dependencies too. @@ -271,23 +283,23 @@ export class Cache { } /** - * Extracts a package archive onto the root filesystem. + * Extracts package archives, in order, onto the root filesystem. * * Runs system tar under sudo because package files live in root-owned - * directories. Fails on any entry error instead of skipping it. + * directories. All archives share one sudo shell so the process cost is paid + * once. Stops at the first archive with any entry error. * - * @param archivePath Absolute path to the package archive. + * @param archivePaths Absolute paths to the package archives. */ - private async extractArchive(archivePath: string): Promise { + private async extractArchives( + archivePaths: readonly string[], + ): Promise { + if (archivePaths.length === 0) { + return; + } await this.commandRunner.run( "sudo", - [ - "tar", - "--extract", - `--file=${archivePath}`, - "--directory=/", - "--keep-directory-symlink", - ], + ["sh", "-c", EXTRACT_ARCHIVES_SCRIPT, "sh", ...archivePaths], { timeoutMs: NO_TIMEOUT_MS }, ); } diff --git a/src/index.ts b/src/index.ts index 8a63177..fd4f6fe 100644 --- a/src/index.ts +++ b/src/index.ts @@ -75,7 +75,9 @@ function getInputs(): ActionInputs { async function main(): Promise { const cacheDir = CACHE_DEFAULT_DIRNAME; const inputs = getInputs(); - const instruments = new Instruments(cacheDir, inputs.debug || core.isDebug()); + const debug = inputs.debug || core.isDebug(); + const instruments = new Instruments(cacheDir, debug); + let failed = false; try { ensureBundledScriptsExecutable(); const commandRunner = new DefaultCommandRunner( @@ -96,12 +98,16 @@ async function main(): Promise { } catch (error) { const message = error instanceof Error ? error.message : String(error); core.setFailed(message); + failed = true; } finally { - // Logs are diagnostics, so a failed upload shouldn't fail the action. - await instruments.artifacts.upload().catch((error: unknown) => { - const message = error instanceof Error ? error.message : String(error); - core.warning(`Failed to upload diagnostic artifacts: ${message}`); - }); + // The upload takes over a second, so only pay for it when the logs are + // wanted. Logs are diagnostics, so a failed upload shouldn't fail the action. + if (debug || failed) { + await instruments.artifacts.upload().catch((error: unknown) => { + const message = error instanceof Error ? error.message : String(error); + core.warning(`Failed to upload diagnostic artifacts: ${message}`); + }); + } } } diff --git a/src/manifest.ts b/src/manifest.ts index a14e859..fe4a509 100644 --- a/src/manifest.ts +++ b/src/manifest.ts @@ -2,15 +2,32 @@ import fs from "node:fs"; import { CacheKey } from "./cache.js"; import { packageNameFromJSON, PackageName } from "ts-apt"; +/** One installed package and the files archived for it in the cache. */ export class ManifestEntry { + /** Installed package, with the version and arch apt resolved. */ readonly packageName: PackageName; + /** + * Files installed by the package, relative to `/` (no leading slash) so they + * can be passed straight to tar. Sorted for stable output. Empty for + * packages without files (e.g. metapackages), which get no archive. + */ readonly filepaths: string[]; + /** + * @param packageName Installed package. + * @param filepaths Root-relative file paths; copied and sorted. + */ constructor(packageName: PackageName, filepaths: string[] = []) { this.packageName = packageName; this.filepaths = [...filepaths].sort((a, b) => a.localeCompare(b)); } + /** + * Creates an entry from its parsed JSON form. + * + * @param json Parsed JSON object as written by {@link Manifest.writeToFile}. + * @returns The deserialized entry. + */ static fromJSON(json: any): ManifestEntry { return new ManifestEntry( packageNameFromJSON(json.packageName), @@ -19,14 +36,35 @@ export class ManifestEntry { } } +/** + * Record of the packages stored in a cache, written alongside the archives. + * + * Restore reads it to find each package's archive and to report outputs. + */ export class Manifest { + /** + * @param created When the manifest was created. + * @param entries Packages in the order apt installed them. Restore relies on + * this order because install scripts can depend on earlier packages + * (see issue #196). + * @param cacheKey Key the cache was saved under. + */ constructor( + /** When the manifest was created. */ public readonly created: Date, + /** Packages in the order apt installed them. */ public readonly entries: ManifestEntry[], + /** Key the cache was saved under. */ public readonly cacheKey: CacheKey, ) {} - // Static factory method + /** + * Creates a manifest from its parsed JSON form. + * + * @param json Parsed JSON object as written by {@link Manifest.writeToFile}. + * Its `cacheKey` is the serialized string from {@link CacheKey.toJSON}. + * @returns The deserialized manifest. + */ static fromJSON(json: any): Manifest { return new Manifest( new Date(json.created), @@ -41,11 +79,23 @@ export class Manifest { ); } + /** + * Reads and parses a manifest file. + * + * @param filePath Path to a manifest JSON file. + * @returns The parsed manifest. + * @throws Error when the file can't be read or isn't a valid manifest. + */ static async readFromFile(filePath: string): Promise { const content = await fs.promises.readFile(filePath, "utf-8"); return Manifest.fromJSON(JSON.parse(content)); } + /** + * Writes the manifest as pretty-printed JSON, replacing any existing file. + * + * @param filePath Destination path. + */ async writeToFile(filePath: string): Promise { await fs.promises.writeFile( filePath, diff --git a/src/packages.ts b/src/packages.ts index d197d2f..ff52e5e 100644 --- a/src/packages.ts +++ b/src/packages.ts @@ -66,10 +66,15 @@ export async function findInstallScript( return undefined; } - const pattern = new RegExp(`^${packageName.name}(:.*)?\\.${extension}$`); + // Match on plain strings since package names can hold regex metacharacters + // (e.g. g++, libstdc++6). Scripts are named [:].. + const suffix = `.${extension}`; + const isMatch = (entry: string) => + entry === `${packageName.name}${suffix}` || + (entry.startsWith(`${packageName.name}:`) && entry.endsWith(suffix)); const matches = fs .readdirSync(scriptsDir) - .filter((entry) => pattern.test(entry)) + .filter(isMatch) .sort((a, b) => a.localeCompare(b)); const candidate = matches[0]; if (!candidate) { diff --git a/test/action-runner.test.ts b/test/action-runner.test.ts index ee1d0ef..9df6a17 100644 --- a/test/action-runner.test.ts +++ b/test/action-runner.test.ts @@ -63,6 +63,28 @@ describe("ActionRunner", () => { expect(cache.archiveAndSave).not.toHaveBeenCalled(); }); + it("runAction cache hit creates package manager without apt-fast", async () => { + const { runner, cache } = createRunner(); + const packageNames = ActionPackageNames.fromInput("curl"); + const cacheKey = new CacheKey("v1", "0", process.arch, packageNames); + cache.loadAndRestore.mockResolvedValue( + new Manifest(new Date(), [], cacheKey), + ); + vi.mocked(createPackageManager).mockResolvedValue({ + getNativeArchitecture: vi.fn().mockResolvedValue(NATIVE_ARCH), + } as never); + + await runner.runAction({ + packages: "curl", + version: "v1", + executeInstallScripts: false, + emptyPackagesBehavior: "error", + debug: false, + }); + + expect(vi.mocked(createPackageManager).mock.calls[0]?.[0]).toBe(false); + }); + it("runAction cache miss archives installed packages with their files", async () => { // Unlikely to have dpkg install scripts on the test host. const installedPackage = createPackageName("test-pkg-not-real", "1.0.0"); diff --git a/test/cache-restore.test.ts b/test/cache-restore.test.ts index 991f524..1d988ef 100644 --- a/test/cache-restore.test.ts +++ b/test/cache-restore.test.ts @@ -87,6 +87,8 @@ async function writeCache(archivedPackages = INSTALL_ORDER): Promise { interface TarCalls { created: [archive: string, files: string[]][]; extracted: string[]; + /** Number of commands run that extracted archives. */ + extractCommands?: number; } const argValue = (args: string[], flag: string) => @@ -100,8 +102,10 @@ function recordTar(calls: TarCalls) { if (args.includes("--create")) { calls.created.push([archive, String(options.input ?? "").split("\0")]); } - if (args.includes("--extract")) { - calls.extracted.push(archive); + // Extraction runs as `sh -c