darinmorrison opened PR #1314 from ci-cargo-audit
to master
:
This PR adds a
cargo-audit
job to the github actions workflow to scanCargo.lock
for crates with known security vulnerabilities.The first time the job is run, or whenever a new
cargo-audit
release is published, the job will take a few minutes since it has to fetch and build the binary. However, the binary is cached so subsequent runs will only take a few seconds.
alexcrichton submitted PR Review.
alexcrichton created PR Review Comment:
This seems like it's a lot of stuff to skip
cargo install
, is this all necessary? Most of our CI jobs aren't exactly speedy, how long doescargo install
take on CI without caching?Another alternative here is that Cargo now caches by default, so we could
cargo install
with a custom--root
and cache that directory. Cargo will then automatically only update if necessary.
darinmorrison submitted PR Review.
darinmorrison created PR Review Comment:
This seems like it's a lot of stuff to skip
cargo install
, is this all necessary? Most of our CI jobs aren't exactly speedy, how long doescargo install
take on CI without caching?When the job ran this time it took
5m 40s
. Most of the time I've seen the cached job run (on my other projects) it usually takes less than15s
.Another alternative here is that Cargo now caches by default, so we could
cargo install
with a custom--root
and cache that directory. Cargo will then automatically only update if necessary.Hmm, what would that look like?
The way the github actions caching works is you have to provide a key to retrieve the cache. In this case, the key is calculated by hashing
.github/caching/cargo-audit.lock
, which in turn is generated by fetching the current release version ofcargo-audit
. So the key will only change when the version changes, which results in rebuilding the binary (and saving a new cache).What would you use for the key in this alternative approach?
You could use a constant value that would result in the cache always being restored but then it wouldn't save any new changes to the cache from
cargo install
, as far as I understand.Instead you'd always get something like this:
Cache hit occurred on the primary key <some constant value>, not saving cache.
bjorn3 created PR Review Comment:
You could use the contents of
~/.cargo/.crates2.json
as cache key to cache~/.cargo/bin
. You do need to runsudo chown -R $(whoami):$(id -ng) ~/.cargo/
(including trailing/
!) to prevent permission problems when restoring the cache. See https://github.com/actions/cache/issues/133#issuecomment-599102035 and the rest of the issue for more info.
bjorn3 submitted PR Review.
alexcrichton submitted PR Review.
alexcrichton created PR Review Comment:
I think the key here doesn't need to matter so much because Cargo does its own internal caching now. This could
cargo install --root /path/to/custom
, and cache/path/to/custom
, and Cargo should take care of the rest. (and this should be simplified by quite a bit)
darinmorrison created PR Review Comment:
@alexcrichton the part about Cargo doing it's own caching makes sense to me but I don't understand how we could take advantage of that in GitHub actions. The GitHub actions caching (unlike Travis) doesn't automatically save changes to cached directories each run; it generates an entirely new cache but only when there is a miss from looking up a new key. (You can observe whether or not it saves the caches for a given run in the
Post Cache
steps for the job).So the way I understand it is that even if Cargo is making incremental changes to a cache, those won't be saved until the key changes. There may be a way to work around this by saving and restoring the cached directories as an artifact but I think that would be more complicated.
But even if we were able to use Cargo's internal caching, wouldn't we need to cache
~/.cargo
as well as the custom root?
darinmorrison submitted PR Review.
darinmorrison updated PR #1314 from ci-cargo-audit
to master
:
This PR adds a
cargo-audit
job to the github actions workflow to scanCargo.lock
for crates with known security vulnerabilities.The first time the job is run, or whenever a new
cargo-audit
release is published, the job will take a few minutes since it has to fetch and build the binary. However, the binary is cached so subsequent runs will only take a few seconds.
darinmorrison updated PR #1314 from ci-cargo-audit
to master
:
This PR adds a
cargo-audit
job to the github actions workflow to scanCargo.lock
for crates with known security vulnerabilities.The first time the job is run, or whenever a new
cargo-audit
release is published, the job will take a few minutes since it has to fetch and build the binary. However, the binary is cached so subsequent runs will only take a few seconds.
darinmorrison updated PR #1314 from ci-cargo-audit
to master
:
This PR adds a
cargo-audit
job to the github actions workflow to scanCargo.lock
for crates with known security vulnerabilities.The first time the job is run, or whenever a new
cargo-audit
release is published, the job will take a few minutes since it has to fetch and build the binary. However, the binary is cached so subsequent runs will only take a few seconds.
darinmorrison updated PR #1314 from ci-cargo-audit
to master
:
This PR adds a
cargo-audit
job to the github actions workflow to scanCargo.lock
for crates with known security vulnerabilities.The first time the job is run, or whenever a new
cargo-audit
release is published, the job will take a few minutes since it has to fetch and build the binary. However, the binary is cached so subsequent runs will only take a few seconds.
alexcrichton submitted PR Review.
alexcrichton created PR Review Comment:
I think this will want to use
--vers
to install the version inCARGO_AUDIT_VERSION
?
darinmorrison updated PR #1314 from ci-cargo-audit
to master
:
This PR adds a
cargo-audit
job to the github actions workflow to scanCargo.lock
for crates with known security vulnerabilities.The first time the job is run, or whenever a new
cargo-audit
release is published, the job will take a few minutes since it has to fetch and build the binary. However, the binary is cached so subsequent runs will only take a few seconds.
darinmorrison submitted PR Review.
darinmorrison created PR Review Comment:
Oops, forgot about that. Fixed in the recent push.
sunfishcode submitted PR Review.
alexcrichton merged PR #1314.
Last updated: Nov 22 2024 at 16:03 UTC