mirror of
https://github.com/awalsh128/cache-apt-pkgs-action.git
synced 2026-11-13 21:36:56 +00:00
fix: restore cached packages in apt install order
Restore extracted archives sorted by filename, so packages whose install scripts depend on files from other packages could break. Restore now follows the entry order in manifest_all.json, which comes from the apt install output, and fails explicitly when an archive is missing. Refs #196 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
parent
0bada9deed
commit
d47746ed94
45
src/cache.ts
45
src/cache.ts
|
|
@ -8,7 +8,6 @@ import * as path from "node:path";
|
|||
import { ActionPackageNames, findInstallScript } from "./packages.js";
|
||||
import { Manifest, ManifestEntry } from "./manifest.js";
|
||||
import {
|
||||
deserializePackageName,
|
||||
CommandRunner,
|
||||
PackageInfo,
|
||||
PackageName,
|
||||
|
|
@ -116,10 +115,7 @@ export class Cache {
|
|||
this.logger = logger;
|
||||
}
|
||||
|
||||
private async runInstallScripts(archiveFilename: string): Promise<void> {
|
||||
const serializePackageName = archiveFilename.replace(/\.tar$/, "");
|
||||
const packageName = deserializePackageName(serializePackageName)!;
|
||||
|
||||
private async runInstallScripts(packageName: PackageName): Promise<void> {
|
||||
const runScript = async (
|
||||
packageName: PackageName,
|
||||
scriptType: "preinst" | "postinst",
|
||||
|
|
@ -186,22 +182,37 @@ export class Cache {
|
|||
await logFileError(`Manifest file not found at ${manifestPath}`);
|
||||
return undefined;
|
||||
}
|
||||
const archives = await fs
|
||||
.readdir(this.path)
|
||||
.then((entries: string[]) =>
|
||||
entries.filter((entry) => entry.endsWith(".tar")),
|
||||
)
|
||||
.then((entries: string[]) => entries.sort());
|
||||
|
||||
if (archives.length === 0) {
|
||||
const manifestAllPath = path.join(this.path, MANIFEST_ALL_FILENAME);
|
||||
const manifestAll = await Manifest.readFromFile(manifestAllPath).catch(
|
||||
() => undefined,
|
||||
);
|
||||
if (!manifestAll || manifestAll.entries.length === 0) {
|
||||
await logFileError(
|
||||
`No archive files found in cache directory ${this.path}`,
|
||||
`Manifest of all packages not found or empty at ${manifestAllPath}`,
|
||||
);
|
||||
return undefined;
|
||||
}
|
||||
|
||||
for (const archiveFilename of archives) {
|
||||
const archivePath = path.join(this.path, archiveFilename);
|
||||
// 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"),
|
||||
}));
|
||||
for (const { archivePath } of archivePaths) {
|
||||
if (
|
||||
!(await fs
|
||||
.access(archivePath, fs.constants.R_OK)
|
||||
.then(() => true)
|
||||
.catch(() => false))
|
||||
) {
|
||||
await logFileError(`Archive file not found at ${archivePath}`);
|
||||
return undefined;
|
||||
}
|
||||
}
|
||||
|
||||
for (const { packageName, archivePath } of archivePaths) {
|
||||
await tar.extract({
|
||||
cwd: "/",
|
||||
file: archivePath,
|
||||
|
|
@ -209,7 +220,7 @@ export class Cache {
|
|||
});
|
||||
|
||||
if (executeInstallScripts) {
|
||||
await this.runInstallScripts(archiveFilename);
|
||||
await this.runInstallScripts(packageName);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
106
test/cache-restore.test.ts
Normal file
106
test/cache-restore.test.ts
Normal file
|
|
@ -0,0 +1,106 @@
|
|||
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 { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import winston from "winston";
|
||||
|
||||
import {
|
||||
Cache,
|
||||
CacheKey,
|
||||
CACHE_DEFAULT_DIRNAME,
|
||||
MANIFEST_ALL_FILENAME,
|
||||
MANIFEST_MAIN_FILENAME,
|
||||
} from "../src/cache.js";
|
||||
import { Manifest, ManifestEntry } from "../src/manifest.js";
|
||||
import { ActionPackageNames } from "../src/packages.js";
|
||||
|
||||
// Cache resolves its directory from os.homedir(), so point it at a temp dir.
|
||||
const { HOME_DIR } = await vi.hoisted(async () => {
|
||||
const nodeFs = await import("node:fs");
|
||||
const nodeOs = await import("node:os");
|
||||
const nodePath = await import("node:path");
|
||||
return {
|
||||
HOME_DIR: nodeFs.mkdtempSync(
|
||||
nodePath.join(nodeOs.tmpdir(), "cache-restore-test-"),
|
||||
),
|
||||
};
|
||||
});
|
||||
|
||||
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("tar", () => ({ extract: vi.fn(), create: vi.fn() }));
|
||||
|
||||
const CACHE_DIR = path.join(HOME_DIR, CACHE_DEFAULT_DIRNAME);
|
||||
const REQUESTED_PACKAGE = createPackageName("app", "1.0");
|
||||
// Install order from apt, deliberately not alphabetical.
|
||||
const INSTALL_ORDER = [
|
||||
createPackageName("zlib1g", "1.2"),
|
||||
createPackageName("libfoo", "2.0"),
|
||||
REQUESTED_PACKAGE,
|
||||
];
|
||||
const KEY = new CacheKey(
|
||||
"v1",
|
||||
"1",
|
||||
"x86_64",
|
||||
ActionPackageNames.fromInput(REQUESTED_PACKAGE.serialize()),
|
||||
);
|
||||
|
||||
const archivePath = (packageName: { serialize(): string }) =>
|
||||
path.join(CACHE_DIR, packageName.serialize() + ".tar");
|
||||
|
||||
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)],
|
||||
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));
|
||||
archivedPackages.forEach((pkg) => fs.writeFileSync(archivePath(pkg), ""));
|
||||
}
|
||||
|
||||
function createCache(): Cache {
|
||||
const logger = winston.createLogger({ silent: true });
|
||||
return new Cache({ run: vi.fn() } as never, logger);
|
||||
}
|
||||
|
||||
describe("Cache.loadAndRestore", () => {
|
||||
beforeEach(() => {
|
||||
vi.mocked(ghcache.restoreCache).mockResolvedValue(KEY.hash);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
fs.rmSync(CACHE_DIR, { recursive: true, force: true });
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
it("loadAndRestore cache hit extracts archives in install order", async () => {
|
||||
await writeCache();
|
||||
|
||||
const manifest = await createCache().loadAndRestore(KEY, false);
|
||||
|
||||
expect(manifest).toBeDefined();
|
||||
expect(
|
||||
vi.mocked(tar.extract).mock.calls.map(([options]) => options?.file),
|
||||
).toEqual(INSTALL_ORDER.map(archivePath));
|
||||
});
|
||||
|
||||
it("loadAndRestore archive missing returns undefined without extracting", async () => {
|
||||
await writeCache(INSTALL_ORDER.slice(1));
|
||||
|
||||
const manifest = await createCache().loadAndRestore(KEY, false);
|
||||
|
||||
expect(manifest).toBeUndefined();
|
||||
expect(tar.extract).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
Loading…
Reference in a new issue