-
Notifications
You must be signed in to change notification settings - Fork 469
Cache CLI extractor paths across Actions steps #3950
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
311292c
6010f85
445107e
587fcb3
889ae42
dc8e1e9
a602287
b18df17
553eef0
c8e32e4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,226 @@ | ||
| import * as fs from "fs"; | ||
| import * as os from "os"; | ||
| import path from "path"; | ||
|
|
||
| import test from "ava"; | ||
|
|
||
| import { | ||
| cacheCommandOutput, | ||
| getCachedCommandOutput, | ||
| resetCachedCommandOutputs, | ||
| CommandCacheKey, | ||
| } from "./cache"; | ||
| import { isVersionInfo } from "./codeql"; | ||
| import { setupTests } from "./testing-utils"; | ||
|
|
||
| setupTests(test); | ||
|
|
||
| const COMMAND_CACHE_FILENAME = "codeql-action-command-cache.json"; | ||
|
|
||
| /** | ||
| * Runs `body` with a temporary directory configured as the cache's backing | ||
| * store (`RUNNER_TEMP`). `CODEQL_ACTION_TEMP` is cleared so that | ||
| * `getTemporaryDirectory()` falls back to `RUNNER_TEMP`. | ||
| * | ||
| * `setupTests` snapshots and restores `process.env` around every test, so we | ||
| * don't restore the environment variables we set here ourselves. | ||
| */ | ||
| async function withCacheDir( | ||
| body: (cacheFilePath: string) => Promise<void> | void, | ||
| ): Promise<void> { | ||
| const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "cache-test-")); | ||
| process.env["RUNNER_TEMP"] = tmpDir; | ||
| delete process.env["CODEQL_ACTION_TEMP"]; | ||
| resetCachedCommandOutputs(); | ||
| try { | ||
| await body(path.join(tmpDir, COMMAND_CACHE_FILENAME)); | ||
| } finally { | ||
| await fs.promises.rm(tmpDir, { force: true, recursive: true }); | ||
| } | ||
| } | ||
|
|
||
| function writeCacheFile( | ||
| cacheFilePath: string, | ||
| contents: Record<string, unknown>, | ||
| ): void { | ||
| fs.writeFileSync(cacheFilePath, JSON.stringify(contents)); | ||
| } | ||
|
|
||
| test.serial( | ||
| "getCachedCommandOutput reuses an output persisted by an earlier step", | ||
| async (t) => { | ||
| await withCacheDir((cacheFilePath) => { | ||
| writeCacheFile(cacheFilePath, { | ||
| [CommandCacheKey.Version]: { | ||
| cmd: "/path/to/codeql", | ||
| output: { version: "2.20.0" }, | ||
| }, | ||
| }); | ||
| t.deepEqual( | ||
| getCachedCommandOutput( | ||
| CommandCacheKey.Version, | ||
| "/path/to/codeql", | ||
| isVersionInfo, | ||
| ), | ||
| { version: "2.20.0" }, | ||
| ); | ||
| }); | ||
| }, | ||
| ); | ||
|
|
||
| test.serial( | ||
| "getCachedCommandOutput ignores an output persisted from a different CLI", | ||
| async (t) => { | ||
| await withCacheDir((cacheFilePath) => { | ||
| writeCacheFile(cacheFilePath, { | ||
| [CommandCacheKey.Version]: { | ||
| cmd: "/path/to/other-codeql", | ||
| output: { version: "2.20.0" }, | ||
| }, | ||
| }); | ||
| t.is( | ||
| getCachedCommandOutput( | ||
| CommandCacheKey.Version, | ||
| "/path/to/codeql", | ||
| isVersionInfo, | ||
| ), | ||
| undefined, | ||
| ); | ||
| }); | ||
| }, | ||
| ); | ||
|
|
||
| test.serial( | ||
| "getCachedCommandOutput ignores a malformed cache file", | ||
| async (t) => { | ||
| await withCacheDir((cacheFilePath) => { | ||
| fs.writeFileSync(cacheFilePath, "not valid json"); | ||
| t.is( | ||
| getCachedCommandOutput( | ||
| CommandCacheKey.Version, | ||
| "/path/to/codeql", | ||
| isVersionInfo, | ||
| ), | ||
| undefined, | ||
| ); | ||
| }); | ||
| }, | ||
| ); | ||
|
|
||
| test.serial( | ||
| "getCachedCommandOutput returns undefined when there is no cache file", | ||
| async (t) => { | ||
| await withCacheDir(() => { | ||
| t.is( | ||
| getCachedCommandOutput( | ||
| CommandCacheKey.Version, | ||
| "/path/to/codeql", | ||
| isVersionInfo, | ||
| ), | ||
| undefined, | ||
| ); | ||
| }); | ||
| }, | ||
| ); | ||
|
|
||
| test.serial( | ||
| "getCachedCommandOutput ignores an output that fails validation", | ||
| async (t) => { | ||
| await withCacheDir((cacheFilePath) => { | ||
| for (const output of [ | ||
| {}, | ||
| { version: 2 }, | ||
| { version: "2.20.0", overlayVersion: "1" }, | ||
| { version: "2.20.0", features: "nope" }, | ||
| ]) { | ||
| resetCachedCommandOutputs(); | ||
| writeCacheFile(cacheFilePath, { | ||
| [CommandCacheKey.Version]: { cmd: "/path/to/codeql", output }, | ||
| }); | ||
| t.is( | ||
| getCachedCommandOutput( | ||
| CommandCacheKey.Version, | ||
| "/path/to/codeql", | ||
| isVersionInfo, | ||
| ), | ||
| undefined, | ||
| JSON.stringify(output), | ||
| ); | ||
| } | ||
| }); | ||
| }, | ||
| ); | ||
|
|
||
| test.serial( | ||
| "getCachedCommandOutput ignores an entry missing the cmd field", | ||
| async (t) => { | ||
| await withCacheDir((cacheFilePath) => { | ||
| writeCacheFile(cacheFilePath, { | ||
| [CommandCacheKey.Version]: { output: { version: "2.20.0" } }, | ||
| }); | ||
| t.is( | ||
| getCachedCommandOutput( | ||
| CommandCacheKey.Version, | ||
| "/path/to/codeql", | ||
| isVersionInfo, | ||
| ), | ||
| undefined, | ||
| ); | ||
| }); | ||
| }, | ||
| ); | ||
|
|
||
| test.serial( | ||
| "cacheCommandOutput persists the output to both the memo and the file", | ||
| async (t) => { | ||
| await withCacheDir((cacheFilePath) => { | ||
| cacheCommandOutput("some-command", "/path/to/codeql", { | ||
| hello: "world", | ||
| }); | ||
|
|
||
| // Tier 2: the temporary file contains the entry. | ||
| const onDisk = JSON.parse( | ||
| fs.readFileSync(cacheFilePath, "utf8"), | ||
| ) as Record<string, unknown>; | ||
| t.deepEqual(onDisk["some-command"], { | ||
| cmd: "/path/to/codeql", | ||
| output: { hello: "world" }, | ||
| }); | ||
|
|
||
| // Tier 1: the value is served from the memo even after the file is gone. | ||
| fs.rmSync(cacheFilePath); | ||
| t.deepEqual(getCachedCommandOutput("some-command", "/path/to/codeql"), { | ||
| hello: "world", | ||
| }); | ||
| }); | ||
| }, | ||
| ); | ||
|
|
||
| test.serial( | ||
| "getCachedCommandOutput prefers the in-memory memo over the file", | ||
| async (t) => { | ||
| await withCacheDir((cacheFilePath) => { | ||
| cacheCommandOutput("some-command", "/path/to/codeql", { value: 1 }); | ||
|
|
||
| // Overwrite the file with a different value; the memo (tier 1) should win. | ||
| writeCacheFile(cacheFilePath, { | ||
| "some-command": { cmd: "/path/to/codeql", output: { value: 2 } }, | ||
| }); | ||
| t.deepEqual(getCachedCommandOutput("some-command", "/path/to/codeql"), { | ||
| value: 1, | ||
| }); | ||
| }); | ||
| }, | ||
| ); | ||
|
|
||
| test.serial( | ||
| "cacheCommandOutput throws if called twice for the same key", | ||
| async (t) => { | ||
| await withCacheDir(() => { | ||
| cacheCommandOutput("some-command", "/path/to/codeql", { value: 1 }); | ||
| t.throws(() => | ||
| cacheCommandOutput("some-command", "/path/to/codeql", { value: 2 }), | ||
| ); | ||
| }); | ||
| }, | ||
| ); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,147 @@ | ||
| import * as fs from "fs"; | ||
| import * as path from "path"; | ||
|
|
||
| import { getTemporaryDirectory } from "./actions-util"; | ||
| import * as json from "./json"; | ||
|
|
||
| /** The name of the temporary file backing the cache (tier 2). */ | ||
|
mario-campos marked this conversation as resolved.
|
||
| const COMMAND_CACHE_FILENAME = "codeql-action-command-cache.json"; | ||
|
|
||
| /** | ||
| * The keys under which the output of cached `codeql` commands is stored. Each | ||
| * key is shared by the producer (the corresponding method in `codeql.ts`) and | ||
| * any consumers (e.g. `status-report.ts`, which peeks the cached version | ||
| * without invoking the CLI). | ||
| */ | ||
|
mario-campos marked this conversation as resolved.
|
||
| export enum CommandCacheKey { | ||
| Version = "version", | ||
| ResolveLanguages = "resolveLanguages", | ||
| } | ||
|
|
||
| /** A single cached command output together with the CLI path it came from. */ | ||
| interface CacheEntry { | ||
| /** | ||
| * The path to the CodeQL CLI that produced `output`. Persisted so that a | ||
| * different step using a different CodeQL bundle doesn't pick up a stale | ||
| * value. | ||
| */ | ||
| cmd: string; | ||
| output: unknown; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could we type this by having a type parameter on |
||
| } | ||
|
|
||
| /** | ||
| * Tier 1: the in-process memo. Consulted first on every lookup and populated | ||
| * whenever a value is read from the file (tier 2) or computed via the CLI | ||
| * (tier 3). | ||
| */ | ||
| const inMemoryCache = new Map<string, CacheEntry>(); | ||
|
mario-campos marked this conversation as resolved.
|
||
|
|
||
| function getCommandCacheFilePath(): string { | ||
| return path.join(getTemporaryDirectory(), COMMAND_CACHE_FILENAME); | ||
| } | ||
|
|
||
| /** | ||
| * Reads and parses the temporary cache file. Best-effort: a missing, malformed, | ||
| * or otherwise unreadable file is treated as an empty cache. | ||
| */ | ||
| function readCommandCacheFile(): Record<string, CacheEntry> { | ||
| let contents: string; | ||
| try { | ||
| contents = fs.readFileSync(getCommandCacheFilePath(), "utf8"); | ||
| } catch { | ||
| return {}; | ||
| } | ||
|
mario-campos marked this conversation as resolved.
|
||
| try { | ||
| const parsed = json.parseString(contents); | ||
| if (json.isObject(parsed)) { | ||
| return parsed; | ||
| } | ||
| } catch { | ||
| // Fall through and treat a malformed file as empty. | ||
|
mario-campos marked this conversation as resolved.
|
||
| } | ||
| return {}; | ||
| } | ||
|
|
||
| /** | ||
| * Persists the cache to the temporary file. Best-effort: a failure to write | ||
| * just means a later step re-runs the CLI. | ||
| */ | ||
| function writeCommandCacheFile(data: Record<string, CacheEntry>): void { | ||
| try { | ||
| fs.writeFileSync(getCommandCacheFilePath(), JSON.stringify(data)); | ||
| } catch { | ||
| // Best-effort; ignore write failures. | ||
|
mario-campos marked this conversation as resolved.
|
||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Stores the output of a command under `key`, writing it to both the in-memory | ||
| * memo (tier 1) and the temporary file (tier 2). | ||
| * | ||
| * Should only be called once per key within a single process; doing otherwise | ||
| * indicates a logic error, since a value that has already been cached should be | ||
| * served from the memo rather than recomputed. | ||
| */ | ||
| export function cacheCommandOutput( | ||
| key: string, | ||
|
mario-campos marked this conversation as resolved.
|
||
| cmd: string, | ||
| output: unknown, | ||
| ): void { | ||
| if (inMemoryCache.has(key)) { | ||
| throw new Error( | ||
| `cacheCommandOutput() should be called only once per key, but was called more than once for '${key}'.`, | ||
| ); | ||
|
mario-campos marked this conversation as resolved.
|
||
| } | ||
| const entry: CacheEntry = { cmd, output }; | ||
| inMemoryCache.set(key, entry); | ||
|
|
||
| const data = readCommandCacheFile(); | ||
| data[key] = entry; | ||
| writeCommandCacheFile(data); | ||
|
mario-campos marked this conversation as resolved.
|
||
| } | ||
|
|
||
| /** | ||
| * Returns the cached output for `key`, or `undefined` if it isn't cached. | ||
| * | ||
| * Resolves tier 1 (in-memory memo) first, then tier 2 (temporary file). A value | ||
| * loaded from the file is ignored unless its `cmd` matches the optional `cmd` | ||
| * argument, and it satisfies the optional `validate` type guard; valid values | ||
| * are memoized into tier 1 before being returned. | ||
| * | ||
| * A return value of `undefined` signals the caller to fall back to tier 3 (the | ||
| * CLI). | ||
| */ | ||
| export function getCachedCommandOutput<T>( | ||
| key: string, | ||
| cmd?: string, | ||
| validate?: (output: unknown) => output is T, | ||
| ): T | undefined { | ||
| // Tier 1: the in-memory variable. | ||
| const memoized = inMemoryCache.get(key); | ||
| if (memoized !== undefined) { | ||
| return memoized.output as T; | ||
| } | ||
|
Comment on lines
+119
to
+123
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This seems correct? At least with respect to the |
||
|
|
||
| // Tier 2: the temporary file persisted by an earlier step, if any. | ||
| const entry = readCommandCacheFile()[key] as unknown; | ||
|
mario-campos marked this conversation as resolved.
|
||
| if ( | ||
| !json.isObject<CacheEntry>(entry) || | ||
| !json.isString(entry.cmd) || | ||
| (cmd !== undefined && entry.cmd !== cmd) || | ||
| (validate !== undefined && !validate(entry.output)) | ||
| ) { | ||
| return undefined; | ||
|
mario-campos marked this conversation as resolved.
|
||
| } | ||
|
|
||
| // Memoize so subsequent lookups in this process hit tier 1. | ||
| inMemoryCache.set(key, { cmd: entry.cmd, output: entry.output }); | ||
|
mario-campos marked this conversation as resolved.
|
||
| return entry.output as T; | ||
| } | ||
|
|
||
| /** | ||
| * Clears the in-process memo (tier 1). Only for use in tests, which exercise | ||
| * multiple "steps" within a single process. | ||
| */ | ||
| export function resetCachedCommandOutputs(): void { | ||
| inMemoryCache.clear(); | ||
| } | ||
|
Comment on lines
+141
to
+147
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In some other files, we export these test-only functions in a nested object to make it clearer that they are for test-use only. That said, my general preference for global state in the CodeQL Action is to try and make it explicit in the way I have done recently in e.g. #3963 |
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have several different types of caches that are handled by the CodeQL Action, so the name
cache.tsis a bit too generic for this since it suggests to me thatcache.tscontains implementations that are shared between all the different types of caches / caching implementations.How about
cli-cache.tsorcli-config-cache.ts? Or, alternatively, we could add a newclifolder and then movecache.tsinto it (or call itconfig-cache.ts).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair enough. If it's all the same, I think I'll go with either
cli-cache.tsorcli/cache.ts.