Implement Package Caching #60
Conversation
There was a problem hiding this comment.
This is fantastic! Love the big picture here.
I have left a number of small comments but am marking this "approve" because they are all nits or possible style things to consider. The only one that I would maybe feel a smidge stronger about is using the standard library Once, if there isn't something I'm missing about that, but even that is not the end of the world, and it's possible I am missing something, as I know that the standard library version isn't exactly equivalent.
| // TODO: Consider using async compression, async tar. | ||
| // It's not the *worst* thing in the world for a packaging tool to block | ||
| // here, but it would help the other async threads remain responsive if | ||
| // we avoided blocking. |
There was a problem hiding this comment.
👍 full agreement that this isn't needed but could be cool (I know this comment was moved from old code, but imho it's still true and so worth remaking on)
There was a problem hiding this comment.
Totally! With the block_on calls this is slightly mitigated, but it still has a small cost.
|
|
||
| // Writes a manifest file to a particular location. | ||
| async fn write_to(&self, path: &Utf8PathBuf) -> anyhow::Result<()> { | ||
| let Some(extension) = path.extension() else { |
There was a problem hiding this comment.
this is my first time running into let else in the wild! Neat!
| .map_err(|e| anyhow!(e))?; | ||
|
|
||
| // In the case that we cannot read the manifest, treat it as "missing". | ||
| // This will force a rebuild anyway. |
There was a problem hiding this comment.
Thanks! I had a preference for: "In case of error, treat it like a cache miss, and rebuild anyway", because I figured most users don't care why their cache is invalid, they just want a package that works.
Thanks for the quick feedback, very appreciated! I think I've addressed everything |
This PR pulls in the changes introduced in oxidecomputer/omicron-package#60, which has been published as a breaking version of omicron-package. These changes include caching, a non-TUI logfile, and camino paths. Caching can optionally be disabled with: ```bash omicron-package package --disable-cache ``` Though the cache is now enabled by default
This PR is a bit of an overhaul of omicron-package, doing the following:
Overview
archive.rsfor building archives,digest.rsfor collecting file digests,input.rsfor collecting inputs.timer.rsfor ease-of-use, measuring build phases.Progresstrait so we can get better debug information without fighting the TUI-based interface. In Omicron, this will default toout/LOG.std::path::Pathtocamino.So How Does Caching Work
out/.out/manifest-cache, which stores a JSON file for each output we expect to build. This JSON file contains the set of all inputs used to construct the output, with their digests. Although this PR adds support for both SHA-2 and Blake-3, we use Blake-3 by default.TO-DO list before merging