-
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
Open
mario-campos
wants to merge
10
commits into
main
Choose a base branch
from
mario-campos/cache-cli-resolve-langs
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
311292c
Cache the output of `codeql resolve languages`
mario-campos 6010f85
Reimplement `resolveExtractor()` as wrapper over `resolveLanguages()`
mario-campos 445107e
Validate numbers, objects, and undefinables in the `json` module
mario-campos 587fcb3
Refactor `isVersionInfo()`` to use `json` module
mario-campos 889ae42
Refactor CLI executions into helper functions
mario-campos dc8e1e9
Refactor CLI JSON handling into a dedicated `runCliJson` function
mario-campos a602287
Refactor CLI caching with in-memory and file storage
mario-campos b18df17
Rebased onto main; fixups were needed
mario-campos 553eef0
Add error handling for undefined extractors in language resolution
mario-campos c8e32e4
Refactor `resolveLanguages()` to cache output according to CLI featur…
mario-campos File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| 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 }), | ||
| ); | ||
| }); | ||
| }, | ||
| ); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| 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). */ | ||
| 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). | ||
| */ | ||
| 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; | ||
| } | ||
|
|
||
| /** | ||
| * 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>(); | ||
|
|
||
| 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 {}; | ||
| } | ||
| try { | ||
| const parsed = json.parseString(contents); | ||
| if (json.isObject(parsed)) { | ||
| return parsed; | ||
| } | ||
| } catch { | ||
| // Fall through and treat a malformed file as empty. | ||
| } | ||
| 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. | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * 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, | ||
| 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}'.`, | ||
| ); | ||
| } | ||
| const entry: CacheEntry = { cmd, output }; | ||
| inMemoryCache.set(key, entry); | ||
|
|
||
| const data = readCommandCacheFile(); | ||
| data[key] = entry; | ||
| writeCommandCacheFile(data); | ||
| } | ||
|
|
||
| /** | ||
| * 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; | ||
| } | ||
|
|
||
| // Tier 2: the temporary file persisted by an earlier step, if any. | ||
| const entry = readCommandCacheFile()[key] as unknown; | ||
| if ( | ||
| !json.isObject<CacheEntry>(entry) || | ||
| !json.isString(entry.cmd) || | ||
| (cmd !== undefined && entry.cmd !== cmd) || | ||
| (validate !== undefined && !validate(entry.output)) | ||
| ) { | ||
| return undefined; | ||
| } | ||
|
|
||
| // Memoize so subsequent lookups in this process hit tier 1. | ||
| inMemoryCache.set(key, { cmd: entry.cmd, output: entry.output }); | ||
| 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(); | ||
| } | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.