`cargo` and `rustup` get deleted when saving cache

[Procrat]

Since we switched to running on self-hosted runners, we noticed that some jobs failed because they couldn't find the cargo executable anymore. After some digging, we noticed that after a successful run with this action on the same machine, .cargo/bin was empty, which is normally where cargo and rustup are installed (when installing the rustup.rs way at least).

Looking through the code, it seems that .cargo/bin gets cleaned apart for the binaries it finds a matching crate for, which would end up removing cargo and rustup. Was this intentional? If so, should I be doing something differently?

[Swatinem]

Yes that was intentional. Since .cargo/bin is saved in the cache as well, I cleared out all the not directly installed binaries from there.

Under the assumption that runners get a fresh snapshot every time they run, it was a safe thing to do.

[ErichDonGubler]

Under the assumption that runners get a fresh snapshot every time they run, it was a safe thing to do.

This is not a safe assumption in the case of self-hosted runners, I think? We at @NZXTCorp are interested in giving this action a try, but currently our self-hosted runners just call rustup show to force installation of overridden toolchain in our repos, This breaks on self-hosted runners because...well, rustup.exe is assumed to exist in the first place!

[Swatinem]

I think a reasonable workaround in that case is to move the files that are not supposed to be cached away, and back after saving the cache.
Or alternatively to use a dedicated folder just for the bin files that are supposed to be persisted in the cache, and move them to .cargo/bin on "startup".

[benjyw]

@Swatinem thanks for this useful action! Would you be open to a PR that adds an option to not touch .cargo/bin? E.g., for the self-hosted runner case?

[fpoli]

Instead of an option to not touch .cargo/bin, I would consider modifying this action so that it temporarly moves the unwanted files away instead of removing them. The code is around here:

rust-cache/src/cleanup.ts

Lines 72 to 86 in b8e72aa

	export async function cleanBin() {
	const bins = await getCargoBins();
	const oldBins = JSON.parse(core.getState(STATE_BINS));
	for (const bin of oldBins) {
	bins.delete(bin);
	}
	const dir = await fs.promises.opendir(path.join(CARGO_HOME, "bin"));
	for await (const dirent of dir) {
	if (dirent.isFile() && !bins.has(dirent.name)) {
	await rm(dir.path, dirent);
	}
	}
	}

[Swatinem]

I had the exact same idea:

• temporarily move files out of the directory
• persist the cache so it uploads only the currently existing files
• move files back afterwards

I just never put in the effort to make that happen ;-) But feel free to contribute here. :-)

[stefnotch]

Would it be possible to change the cache paths (

rust-cache/src/config.ts

Line 237 in d30f114

self.cachePaths = [CARGO_HOME];

) to be more granular, and to simply ignore the not relevant files inside the .cargo/bin folder?
With more granular paths, we should be able to get rid of all of those deletion commands

rust-cache/src/save.ts

Lines 49 to 69 in d30f114

	try {
	const crates = core.getInput("cache-all-crates").toLowerCase() || "false";
	core.info(`... Cleaning cargo registry (cache-all-crates: ${crates}) ...`);
	await cleanRegistry(allPackages, crates !== "true");
	} catch (e) {
	core.debug(`${(e as any).stack}`);
	}
	try {
	core.info(`... Cleaning cargo/bin ...`);
	await cleanBin(config.cargoBins);
	} catch (e) {
	core.debug(`${(e as any).stack}`);
	}
	try {
	core.info(`... Cleaning cargo git cache ...`);
	await cleanGit(allPackages);
	} catch (e) {
	core.debug(`${(e as any).stack}`);
	}

Or is there anything I'm overlooking?

Okay, so looking at it a bit more, which this approach one would have to

• Start by reading the files in .cargo/bin and note down their timestamps
• Run the build
• Read the files in .cargo/bin again, and only cache the new ones

And probably also deal with the registry and git folders.