mirror of
https://github.com/awalsh128/cache-apt-pkgs-action.git
synced 2026-11-13 21:36:56 +00:00
fix: archive installed package files on cache miss
Manifest.from built every entry with an empty file list and buildFileList was never called, so tar.create was handed no paths. node-tar throws "no paths specified to add to archive" for that, so every cache miss failed before saving. The runner now builds each manifest entry from buildFileList and passes the manifest to archiveAndSave. Packages with no files (e.g. metapackages) are skipped on archive and on restore since tar can't create an empty archive. Manifest.from is removed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
parent
79721099c8
commit
c06fd86f51
|
|
@ -4,8 +4,8 @@ import {
|
|||
type PackageName,
|
||||
} from "ts-apt";
|
||||
import { Cache, CacheKey } from "./cache.js";
|
||||
import { Manifest } from "./manifest.js";
|
||||
import { ActionPackageNames } from "./packages.js";
|
||||
import { Manifest, ManifestEntry } from "./manifest.js";
|
||||
import { ActionPackageNames, buildFileList } from "./packages.js";
|
||||
import winston from "winston";
|
||||
|
||||
type EmptyPackageBehavior = "error" | "warn" | "ignore";
|
||||
|
|
@ -84,8 +84,18 @@ export class ActionRunner {
|
|||
this.logger,
|
||||
);
|
||||
const packageInfos = await installManager.install(packageNames.toArray());
|
||||
manifest = Manifest.from(new Date(), cacheKey, packageInfos);
|
||||
await this.cache.archiveAndSave(cacheKey, packageInfos);
|
||||
// Collect each package's installed files so the archives have content.
|
||||
const entries = await Promise.all(
|
||||
packageInfos.map(
|
||||
async (info) =>
|
||||
new ManifestEntry(
|
||||
info.name,
|
||||
await buildFileList(info.name, installManager),
|
||||
),
|
||||
),
|
||||
);
|
||||
manifest = new Manifest(new Date(), entries, cacheKey);
|
||||
await this.cache.archiveAndSave(manifest);
|
||||
}
|
||||
|
||||
return {
|
||||
|
|
|
|||
41
src/cache.ts
41
src/cache.ts
|
|
@ -7,11 +7,7 @@ import * as os from "node:os";
|
|||
import * as path from "node:path";
|
||||
import { ActionPackageNames, findInstallScript } from "./packages.js";
|
||||
import { Manifest, ManifestEntry } from "./manifest.js";
|
||||
import {
|
||||
CommandRunner,
|
||||
PackageInfo,
|
||||
PackageName,
|
||||
} from "ts-apt";
|
||||
import { CommandRunner, PackageName } from "ts-apt";
|
||||
|
||||
export const CACHE_DEFAULT_DIRNAME = "cache-apt-pkgs";
|
||||
export const CACHE_KEY_FILENAME = "cache_key.md5";
|
||||
|
|
@ -196,10 +192,16 @@ export class Cache {
|
|||
// Restore in the order apt installed the packages (i.e. the order of the
|
||||
// manifest entries) and not by archive filename. Install scripts can depend
|
||||
// on files from packages installed before them (see issue #196).
|
||||
const archivePaths = manifestAll.entries.map((entry) => ({
|
||||
packageName: entry.packageName,
|
||||
archivePath: path.join(this.path, entry.packageName.serialize() + ".tar"),
|
||||
}));
|
||||
// Packages without files (e.g. metapackages) have no archive to restore.
|
||||
const archivePaths = manifestAll.entries
|
||||
.filter((entry) => entry.filepaths.length > 0)
|
||||
.map((entry) => ({
|
||||
packageName: entry.packageName,
|
||||
archivePath: path.join(
|
||||
this.path,
|
||||
entry.packageName.serialize() + ".tar",
|
||||
),
|
||||
}));
|
||||
for (const { archivePath } of archivePaths) {
|
||||
if (
|
||||
!(await fs
|
||||
|
|
@ -243,6 +245,13 @@ export class Cache {
|
|||
);
|
||||
return;
|
||||
}
|
||||
if (manifestEntry.filepaths.length === 0) {
|
||||
// tar can't create an empty archive, and restore skips these entries.
|
||||
this.logger.info(
|
||||
`No files to archive for package ${manifestEntry.packageName.serialize()}, skipping creation.`,
|
||||
);
|
||||
return;
|
||||
}
|
||||
this.logger.info(
|
||||
`Archiving package ${manifestEntry.packageName.serialize()} with ${manifestEntry.filepaths.length} files...`,
|
||||
);
|
||||
|
|
@ -295,10 +304,14 @@ export class Cache {
|
|||
);
|
||||
}
|
||||
|
||||
async archiveAndSave(
|
||||
key: CacheKey,
|
||||
packageInfos: PackageInfo[],
|
||||
): Promise<number> {
|
||||
/**
|
||||
* Writes the manifests and per-package archives, then saves the cache.
|
||||
*
|
||||
* @param manifest Installed packages, in install order, with their files.
|
||||
* @returns GitHub Actions cache ID.
|
||||
*/
|
||||
async archiveAndSave(manifest: Manifest): Promise<number> {
|
||||
const key = manifest.cacheKey;
|
||||
if (!ghcache.isFeatureAvailable()) {
|
||||
throw new Error(
|
||||
"GitHub Actions cache service is not available in this environment",
|
||||
|
|
@ -316,8 +329,6 @@ export class Cache {
|
|||
);
|
||||
this.logger.info(`Wrote cache key`);
|
||||
|
||||
const manifest = Manifest.from(new Date(), key, packageInfos);
|
||||
|
||||
await this.writeManifests(manifest);
|
||||
for (const entry of manifest.entries) {
|
||||
await this.createArchive(entry);
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
import fs from "node:fs";
|
||||
import { CacheKey } from "./cache.js";
|
||||
import { packageNameFromJSON, PackageInfo, PackageName } from "ts-apt";
|
||||
import { packageNameFromJSON, PackageName } from "ts-apt";
|
||||
|
||||
export class ManifestEntry {
|
||||
readonly packageName: PackageName;
|
||||
|
|
@ -46,17 +46,6 @@ export class Manifest {
|
|||
return Manifest.fromJSON(JSON.parse(content));
|
||||
}
|
||||
|
||||
static from(
|
||||
date: Date,
|
||||
cacheKey: CacheKey,
|
||||
packageInfos: PackageInfo[],
|
||||
): Manifest {
|
||||
const entries = packageInfos.map(
|
||||
(info: PackageInfo) => new ManifestEntry(info.name, []),
|
||||
);
|
||||
return new Manifest(date, entries, cacheKey);
|
||||
}
|
||||
|
||||
async writeToFile(filePath: string): Promise<void> {
|
||||
await fs.promises.writeFile(
|
||||
filePath,
|
||||
|
|
|
|||
|
|
@ -1,9 +1,9 @@
|
|||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import winston from "winston";
|
||||
import { Manifest } from "../src/manifest.js";
|
||||
import { Manifest, ManifestEntry } from "../src/manifest.js";
|
||||
import { CacheKey } from "../src/cache.js";
|
||||
import { ActionPackageNames } from "../src/packages.js";
|
||||
import { createPackageManager } from "ts-apt";
|
||||
import { createPackageManager, createPackageName } from "ts-apt";
|
||||
|
||||
import { ActionRunner } from "../src/action.js";
|
||||
|
||||
|
|
@ -58,21 +58,20 @@ describe("ActionRunner", () => {
|
|||
expect(cache.archiveAndSave).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("archives installed packages on cache miss", async () => {
|
||||
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");
|
||||
// buildFileList keeps only files that exist, so use this test file.
|
||||
const installedFile = import.meta.filename;
|
||||
const { runner, cache } = createRunner();
|
||||
cache.loadAndRestore.mockResolvedValue(undefined);
|
||||
vi.mocked(createPackageManager).mockResolvedValue({
|
||||
install: vi.fn().mockResolvedValue([
|
||||
{
|
||||
name: {
|
||||
serialize: () => "curl=1.0.0",
|
||||
},
|
||||
},
|
||||
]),
|
||||
install: vi.fn().mockResolvedValue([{ name: installedPackage }]),
|
||||
listInstalledFiles: vi.fn().mockResolvedValue([installedFile]),
|
||||
} as never);
|
||||
|
||||
await runner.runAction({
|
||||
packages: "curl",
|
||||
packages: installedPackage.serialize(),
|
||||
version: "v1",
|
||||
executeInstallScripts: false,
|
||||
emptyPackagesBehavior: "error",
|
||||
|
|
@ -80,5 +79,9 @@ describe("ActionRunner", () => {
|
|||
});
|
||||
|
||||
expect(cache.archiveAndSave).toHaveBeenCalledTimes(1);
|
||||
const [manifest] = cache.archiveAndSave.mock.calls[0]!;
|
||||
expect(manifest.entries).toEqual([
|
||||
new ManifestEntry(installedPackage, [installedFile.slice(1)]),
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -2,7 +2,7 @@ import fs from "node:fs";
|
|||
import path from "node:path";
|
||||
import * as ghcache from "@actions/cache";
|
||||
import * as tar from "tar";
|
||||
import { createPackageName } from "ts-apt";
|
||||
import { createPackageName, type PackageName } from "ts-apt";
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import winston from "winston";
|
||||
|
||||
|
|
@ -32,10 +32,15 @@ vi.mock("node:os", async (importOriginal) => {
|
|||
const actual = await importOriginal<typeof import("node:os")>();
|
||||
return { ...actual, homedir: () => HOME_DIR };
|
||||
});
|
||||
vi.mock("@actions/cache", () => ({ restoreCache: vi.fn() }));
|
||||
vi.mock("@actions/cache", () => ({
|
||||
restoreCache: vi.fn(),
|
||||
saveCache: vi.fn(),
|
||||
isFeatureAvailable: vi.fn(),
|
||||
}));
|
||||
vi.mock("tar", () => ({ extract: vi.fn(), create: vi.fn() }));
|
||||
|
||||
const CACHE_DIR = path.join(HOME_DIR, CACHE_DEFAULT_DIRNAME);
|
||||
const CACHE_ID = 42;
|
||||
const REQUESTED_PACKAGE = createPackageName("app", "1.0");
|
||||
// Install order from apt, deliberately not alphabetical.
|
||||
const INSTALL_ORDER = [
|
||||
|
|
@ -43,6 +48,8 @@ const INSTALL_ORDER = [
|
|||
createPackageName("libfoo", "2.0"),
|
||||
REQUESTED_PACKAGE,
|
||||
];
|
||||
// Packages without files (e.g. metapackages) get no archive.
|
||||
const METAPACKAGE = createPackageName("meta", "1.0");
|
||||
const KEY = new CacheKey(
|
||||
"v1",
|
||||
"1",
|
||||
|
|
@ -50,22 +57,27 @@ const KEY = new CacheKey(
|
|||
ActionPackageNames.fromInput(REQUESTED_PACKAGE.serialize()),
|
||||
);
|
||||
|
||||
const archivePath = (packageName: { serialize(): string }) =>
|
||||
const archivePath = (packageName: PackageName) =>
|
||||
path.join(CACHE_DIR, packageName.serialize() + ".tar");
|
||||
const filesOf = (packageName: PackageName) => [
|
||||
`usr/lib/${packageName.name}.so`,
|
||||
];
|
||||
const entryOf = (packageName: PackageName) =>
|
||||
new ManifestEntry(packageName, filesOf(packageName));
|
||||
const MANIFEST = new Manifest(
|
||||
new Date(0),
|
||||
[...INSTALL_ORDER.map(entryOf), new ManifestEntry(METAPACKAGE)],
|
||||
KEY,
|
||||
);
|
||||
|
||||
async function writeCache(archivedPackages = INSTALL_ORDER): Promise<void> {
|
||||
fs.mkdirSync(CACHE_DIR, { recursive: true });
|
||||
const created = new Date(0);
|
||||
await new Manifest(
|
||||
created,
|
||||
[new ManifestEntry(REQUESTED_PACKAGE)],
|
||||
MANIFEST.created,
|
||||
[entryOf(REQUESTED_PACKAGE)],
|
||||
KEY,
|
||||
).writeToFile(path.join(CACHE_DIR, MANIFEST_MAIN_FILENAME));
|
||||
await new Manifest(
|
||||
created,
|
||||
INSTALL_ORDER.map((pkg) => new ManifestEntry(pkg)),
|
||||
KEY,
|
||||
).writeToFile(path.join(CACHE_DIR, MANIFEST_ALL_FILENAME));
|
||||
await MANIFEST.writeToFile(path.join(CACHE_DIR, MANIFEST_ALL_FILENAME));
|
||||
archivedPackages.forEach((pkg) => fs.writeFileSync(archivePath(pkg), ""));
|
||||
}
|
||||
|
||||
|
|
@ -74,9 +86,11 @@ function createCache(): Cache {
|
|||
return new Cache({ run: vi.fn() } as never, logger);
|
||||
}
|
||||
|
||||
describe("Cache.loadAndRestore", () => {
|
||||
describe("Cache", () => {
|
||||
beforeEach(() => {
|
||||
vi.mocked(ghcache.restoreCache).mockResolvedValue(KEY.hash);
|
||||
vi.mocked(ghcache.saveCache).mockResolvedValue(CACHE_ID);
|
||||
vi.mocked(ghcache.isFeatureAvailable).mockReturnValue(true);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
|
|
@ -103,4 +117,27 @@ describe("Cache.loadAndRestore", () => {
|
|||
expect(manifest).toBeUndefined();
|
||||
expect(tar.extract).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("archiveAndSave packages with files creates archives of their files", async () => {
|
||||
const cacheId = await createCache().archiveAndSave(MANIFEST);
|
||||
|
||||
expect(cacheId).toBe(CACHE_ID);
|
||||
// tar.create is overloaded, so type the (options, files) call shape used.
|
||||
const createCalls = vi.mocked(tar.create).mock.calls as unknown as [
|
||||
{ file: string },
|
||||
string[],
|
||||
][];
|
||||
expect(createCalls.map(([options, files]) => [options.file, files])).toEqual(INSTALL_ORDER.map((pkg) => [archivePath(pkg), filesOf(pkg)]));
|
||||
});
|
||||
|
||||
it("archiveAndSave manifest written records package files", async () => {
|
||||
await createCache().archiveAndSave(MANIFEST);
|
||||
|
||||
const written = await Manifest.readFromFile(
|
||||
path.join(CACHE_DIR, MANIFEST_ALL_FILENAME),
|
||||
);
|
||||
expect(written.entries.map((entry) => entry.filepaths)).toEqual(
|
||||
MANIFEST.entries.map((entry) => entry.filepaths),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Reference in a new issue