Skip to content

Commit 9b4b3d7

Browse files
committed
fix: cache key stability
Ensure consistency of main and post configuration by storing and restoring it from state, which in turn ensures cache key stability. Also: * Fixed some typos. * Use core.error for logging errors. Reverts: Swatinem#138 Fixes Swatinem#140
1 parent 060bda3 commit 9b4b3d7

9 files changed

Lines changed: 222 additions & 172 deletions

File tree

CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
# Changelog
22

3+
## 2.3.1
4+
5+
- Fix cache key stability.
6+
37
## 2.3.0
48

59
- Add `cache-all-crates` option, which enables caching of crates installed by workflows.

README.md

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,6 @@ This cache is automatically keyed by:
101101
- the value of some compiler-specific environment variables (eg. RUSTFLAGS, etc), and
102102
- a hash of all `Cargo.lock` / `Cargo.toml` files found anywhere in the repository (if present).
103103
- a hash of all `rust-toolchain` / `rust-toolchain.toml` files in the root of the repository (if present).
104-
- a hash of installed packages as generated by `cargo install --list`.
105104

106105
An additional input `key` can be provided if the builtin keys are not sufficient.
107106

@@ -137,7 +136,7 @@ otherwise corrupt the cache on macOS builds.
137136
This specialized cache action is built on top of the upstream cache action
138137
maintained by GitHub. The same restrictions and limits apply, which are
139138
documented here:
140-
https://docs.github.com/en/actions/using-workflows/caching-dependencies-to-speed-up-workflows
139+
[Caching dependencies to speed up workflows](https://docs.github.com/en/actions/using-workflows/caching-dependencies-to-speed-up-workflows)
141140

142141
In particular, caches are currently limited to 10 GB in total and exceeding that
143142
limit will cause eviction of older caches.

dist/restore/index.js

Lines changed: 63 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -59977,8 +59977,8 @@ async function getCmdOutput(cmd, args = [], options = {}) {
5997759977
});
5997859978
}
5997959979
catch (e) {
59980-
lib_core.info(`[warning] Command failed: ${cmd} ${args.join(" ")}`);
59981-
lib_core.info(`[warning] ${stderr}`);
59980+
lib_core.error(`Command failed: ${cmd} ${args.join(" ")}`);
59981+
lib_core.error(stderr);
5998259982
throw e;
5998359983
}
5998459984
return stdout;
@@ -60024,12 +60024,10 @@ class Workspace {
6002460024

6002560025

6002660026

60027+
6002760028
const HOME = external_os_default().homedir();
6002860029
const config_CARGO_HOME = process.env.CARGO_HOME || external_path_default().join(HOME, ".cargo");
60029-
const STATE_LOCKFILE_HASH = "RUST_CACHE_LOCKFILE_HASH";
60030-
const STATE_LOCKFILES = "RUST_CACHE_LOCKFILES";
60031-
const config_STATE_BINS = "RUST_CACHE_BINS";
60032-
const STATE_KEY = "RUST_CACHE_KEY";
60030+
const STATE_CONFIG = "RUST_CACHE_CONFIG";
6003360031
class CacheConfig {
6003460032
constructor() {
6003560033
/** All the paths we want to cache */
@@ -60040,6 +60038,8 @@ class CacheConfig {
6004060038
this.restoreKey = "";
6004160039
/** The workspace configurations */
6004260040
this.workspaces = [];
60041+
/** The cargo binaries present during main step */
60042+
this.cargoBins = [];
6004360043
/** The prefix portion of the cache key */
6004460044
this.keyPrefix = "";
6004560045
/** The rust version considered for the cache key */
@@ -60056,6 +60056,14 @@ class CacheConfig {
6005660056
*/
6005760057
static async new() {
6005860058
const self = new CacheConfig();
60059+
const source = lib_core.getState(STATE_CONFIG);
60060+
if (source !== "") {
60061+
// Post action, use what we calculated in the main action ensuring consistency.
60062+
Object.assign(self, JSON.parse(source));
60063+
self.workspaces = self.workspaces
60064+
.map((w) => new Workspace(w.root, w.target));
60065+
return self;
60066+
}
6005960067
// Construct key prefix:
6006060068
// This uses either the `shared-key` input,
6006160069
// or the `key` input combined with the `job` key.
@@ -60103,20 +60111,11 @@ class CacheConfig {
6010360111
}
6010460112
}
6010560113
self.keyEnvs = keyEnvs;
60106-
// Installed packages and their versions are also considered for the key.
60107-
const packages = await getPackages();
60108-
hasher.update(packages);
6010960114
key += `-${hasher.digest("hex")}`;
6011060115
self.restoreKey = key;
6011160116
// Construct the lockfiles portion of the key:
6011260117
// This considers all the files found via globbing for various manifests
6011360118
// and lockfiles.
60114-
// This part is computed in the "pre"/"restore" part of the job and persisted
60115-
// into the `state`. That state is loaded in the "post"/"save" part of the
60116-
// job so we have consistent values even though the "main" actions run
60117-
// might create/overwrite lockfiles.
60118-
let lockHash = lib_core.getState(STATE_LOCKFILE_HASH);
60119-
let keyFiles = JSON.parse(lib_core.getState(STATE_LOCKFILES) || "[]");
6012060119
// Constructs the workspace config and paths to restore:
6012160120
// The workspaces are given using a `$workspace -> $target` syntax.
6012260121
const workspaces = [];
@@ -60128,24 +60127,20 @@ class CacheConfig {
6012860127
workspaces.push(new Workspace(root, target));
6012960128
}
6013060129
self.workspaces = workspaces;
60131-
if (!lockHash) {
60132-
keyFiles = keyFiles.concat(await globFiles("rust-toolchain\nrust-toolchain.toml"));
60133-
for (const workspace of workspaces) {
60134-
const root = workspace.root;
60135-
keyFiles.push(...(await globFiles(`${root}/**/Cargo.toml\n${root}/**/Cargo.lock\n${root}/**/rust-toolchain\n${root}/**/rust-toolchain.toml`)));
60136-
}
60137-
keyFiles = keyFiles.filter(file => !external_fs_default().statSync(file).isDirectory());
60138-
keyFiles.sort((a, b) => a.localeCompare(b));
60139-
hasher = external_crypto_default().createHash("sha1");
60140-
for (const file of keyFiles) {
60141-
for await (const chunk of external_fs_default().createReadStream(file)) {
60142-
hasher.update(chunk);
60143-
}
60130+
let keyFiles = await globFiles("rust-toolchain\nrust-toolchain.toml");
60131+
for (const workspace of workspaces) {
60132+
const root = workspace.root;
60133+
keyFiles.push(...(await globFiles(`${root}/**/Cargo.toml\n${root}/**/Cargo.lock\n${root}/**/rust-toolchain\n${root}/**/rust-toolchain.toml`)));
60134+
}
60135+
keyFiles = keyFiles.filter(file => !external_fs_default().statSync(file).isDirectory());
60136+
keyFiles.sort((a, b) => a.localeCompare(b));
60137+
hasher = external_crypto_default().createHash("sha1");
60138+
for (const file of keyFiles) {
60139+
for await (const chunk of external_fs_default().createReadStream(file)) {
60140+
hasher.update(chunk);
6014460141
}
60145-
lockHash = hasher.digest("hex");
60146-
lib_core.saveState(STATE_LOCKFILE_HASH, lockHash);
60147-
lib_core.saveState(STATE_LOCKFILES, JSON.stringify(keyFiles));
6014860142
}
60143+
let lockHash = hasher.digest("hex");
6014960144
self.keyFiles = keyFiles;
6015060145
key += `-${lockHash}`;
6015160146
self.cacheKey = key;
@@ -60158,8 +60153,13 @@ class CacheConfig {
6015860153
for (const dir of cacheDirectories.trim().split(/\s+/).filter(Boolean)) {
6015960154
self.cachePaths.push(dir);
6016060155
}
60156+
const bins = await getCargoBins();
60157+
self.cargoBins = Array.from(bins.values());
6016160158
return self;
6016260159
}
60160+
/**
60161+
* Prints the configuration to the action log.
60162+
*/
6016360163
printInfo() {
6016460164
lib_core.startGroup("Cache Configuration");
6016560165
lib_core.info(`Workspaces:`);
@@ -60187,6 +60187,21 @@ class CacheConfig {
6018760187
}
6018860188
lib_core.endGroup();
6018960189
}
60190+
/**
60191+
* Saves the configuration to the state store.
60192+
* This is used to restore the configuration in the post action.
60193+
*/
60194+
saveState() {
60195+
lib_core.saveState(STATE_CONFIG, this);
60196+
}
60197+
}
60198+
/**
60199+
* Checks if the cache is up to date.
60200+
*
60201+
* @returns `true` if the cache is up to date, `false` otherwise.
60202+
*/
60203+
function isCacheUpToDate() {
60204+
return core.getState(STATE_CONFIG) === "";
6019060205
}
6019160206
async function getRustVersion() {
6019260207
const stdout = await getCmdOutput("rustc", ["-vV"]);
@@ -60197,11 +60212,6 @@ async function getRustVersion() {
6019760212
.filter((s) => s.length === 2);
6019860213
return Object.fromEntries(splits);
6019960214
}
60200-
async function getPackages() {
60201-
let stdout = await getCmdOutput("cargo", ["install", "--list"]);
60202-
// Make OS independent.
60203-
return stdout.split(/[\n\r]+/).join("\n");
60204-
}
6020560215
async function globFiles(pattern) {
6020660216
const globber = await glob.create(pattern, {
6020760217
followSymbolicLinks: false,
@@ -60269,9 +60279,14 @@ async function getCargoBins() {
6026960279
catch { }
6027060280
return bins;
6027160281
}
60272-
async function cleanBin() {
60282+
/**
60283+
* Clean the cargo bin directory, removing the binaries that existed
60284+
* when the action started, as they were not created by the build.
60285+
*
60286+
* @param oldBins The binaries that existed when the action started.
60287+
*/
60288+
async function cleanBin(oldBins) {
6027360289
const bins = await getCargoBins();
60274-
const oldBins = JSON.parse(core.getState(STATE_BINS));
6027560290
for (const bin of oldBins) {
6027660291
bins.delete(bin);
6027760292
}
@@ -60439,9 +60454,9 @@ async function exists(path) {
6043960454

6044060455

6044160456
process.on("uncaughtException", (e) => {
60442-
lib_core.info(`[warning] ${e.message}`);
60457+
lib_core.error(e.message);
6044360458
if (e.stack) {
60444-
lib_core.info(e.stack);
60459+
lib_core.error(e.stack);
6044560460
}
6044660461
});
6044760462
async function run() {
@@ -60459,36 +60474,37 @@ async function run() {
6045960474
const config = await CacheConfig["new"]();
6046060475
config.printInfo();
6046160476
lib_core.info("");
60462-
const bins = await getCargoBins();
60463-
lib_core.saveState(config_STATE_BINS, JSON.stringify([...bins]));
6046460477
lib_core.info(`... Restoring cache ...`);
6046560478
const key = config.cacheKey;
6046660479
// Pass a copy of cachePaths to avoid mutating the original array as reported by:
6046760480
// https://github.com/actions/toolkit/pull/1378
6046860481
// TODO: remove this once the underlying bug is fixed.
6046960482
const restoreKey = await cache.restoreCache(config.cachePaths.slice(), key, [config.restoreKey]);
6047060483
if (restoreKey) {
60471-
lib_core.info(`Restored from cache key "${restoreKey}".`);
60472-
lib_core.saveState(STATE_KEY, restoreKey);
60473-
if (restoreKey !== key) {
60484+
const match = restoreKey === key;
60485+
lib_core.info(`Restored from cache key "${restoreKey}" full match: ${match}.`);
60486+
if (!match) {
6047460487
// pre-clean the target directory on cache mismatch
6047560488
for (const workspace of config.workspaces) {
6047660489
try {
6047760490
await cleanTargetDir(workspace.target, [], true);
6047860491
}
6047960492
catch { }
6048060493
}
60494+
// We restored the cache but it is not a full match.
60495+
config.saveState();
6048160496
}
60482-
setCacheHitOutput(restoreKey === key);
60497+
setCacheHitOutput(match);
6048360498
}
6048460499
else {
6048560500
lib_core.info("No cache found.");
60501+
config.saveState();
6048660502
setCacheHitOutput(false);
6048760503
}
6048860504
}
6048960505
catch (e) {
6049060506
setCacheHitOutput(false);
60491-
lib_core.info(`[warning] ${e.stack}`);
60507+
lib_core.error(`${e.stack}`);
6049260508
}
6049360509
}
6049460510
function setCacheHitOutput(cacheHit) {

0 commit comments

Comments
 (0)