feat(ci): cache registry baseline rustdoc JSON

[thomaseizinger]

Description

The latest release (v0.18) of cargo semver-checks includes a feature where the rustdoc JSON files for registry baselines is cached in target/semver-checks/cache. The rustdoc JSON files for released crates never changes as the source code on crates.io cannot be changed once a version is published. Thus, it is unnecessary to generate that JSON on every CI run.

We extract a separate action that installs cargo semver-checks for us and also sets up a cache. The cache is scoped to released version of the crate, meaning it automatically invalidates once we publish a new version.

This speeds up the cargo semver-checks step by 50% which is ~ 1 minute per job.

Notes

In total, this allows our entire CI workflow to finish in as little as 11 minutes!

See https://github.com/libp2p/rust-libp2p/actions/runs/4258774854 for example.

Draft because I am still testing this on my fork.

Links to any relevant issues

Open Questions

Change checklist

• I have performed a self-review of my own code
• I have made corresponding changes to the documentation
• I have added tests that prove my fix is effective or that my feature works
• A changelog entry has been made in the appropriate crates

[thomaseizinger]

@obi1kenobi Upgrading to v0.18 results in a failing semver check despite this crate not having received any significant changes. Could you look into this?

[thomaseizinger]

@obi1kenobi Upgrading to v0.18 results in a failing semver check despite this crate not having received any significant changes. Could you look into this?

This is the only diff since the last release, produced with git diff v0.50.0 -- transports/uds:

diff --git a/transports/uds/Cargo.toml b/transports/uds/Cargo.toml
index 9dd1321a..476888c8 100644
--- a/transports/uds/Cargo.toml
+++ b/transports/uds/Cargo.toml
@@ -12,15 +12,15 @@ categories = ["network-programming", "asynchronous"]
 
 [target.'cfg(all(unix, not(target_os = "emscripten")))'.dependencies]
 async-std = { version = "1.6.2", optional = true }
-libp2p-core = { version = "0.38.0", path = "../../core" }
+libp2p-core = { version = "0.39.0", path = "../../core" }
 log = "0.4.1"
-futures = "0.3.1"
+futures = "0.3.26"
 tokio = { version = "1.15", default-features = false, features = ["net"], optional = true }
 
 [target.'cfg(all(unix, not(target_os = "emscripten")))'.dev-dependencies]
 tempfile = "3.0"
 
-# Passing arguments to the docsrs builder in order to properly document cfg's. 
+# Passing arguments to the docsrs builder in order to properly document cfg's.
 # More information: https://docs.rs/about/builds#cross-compiling
 [package.metadata.docs.rs]
 all-features = true

[obi1kenobi]

Confirmed bug, sorry about that. The query itself is fine, but the input rustdoc JSON is legitimately missing those structs.

We seem to have run into yet another edge case in rustdoc generation.

[obi1kenobi]

On a lighter note, I believe this moves the rust-libp2p repo into first place on our integration tests scoreboard for "bugs found in cargo-semver-checks" 😅

[thomaseizinger]

On a lighter note, I believe this moves the rust-libp2p repo into first place on our integration tests scoreboard for "bugs found in cargo-semver-checks" 😅

I guess somebody has to lead that board 😁

[obi1kenobi]

We're getting ready to roll out a mini-crater for semver-checking so we can spot issues like these early (before releasing), so if it all goes well, you'll be in first place for a long time because it'll get a lot more difficult to find issues like this :)

[obi1kenobi]

Heads-up that I'm about to release a new patch version with a bunch of CLI bugfixes sometime in the next ~hour. It might be good to adopt it ASAP since it's possible some of the bugs were causing false-negatives.

[obi1kenobi]

As far as I can tell, this doesn't help at all right now — it ends up rebuilding everything for the "current" rustdoc, and you can confirm this by setting CARGO_TERM_VERBOSE=true and observing that compilations are happening instead of being shown as skipped due to Fresh build data.

I know that cargo uses mtimes to decide whether to rebuild or not. I tried setting a synthetic mtime in the future on the restored build artifacts, but even that didn't seem to help: obi1kenobi/cargo-semver-checks#399

I don't think I have the cycles to continue digging into the mtimes issue right now, but I thought I'd give you a heads-up in case you're able to dig further.

[thomaseizinger]

Hmm, thanks for that! Yeah I did wonder why this isn't working as expected. I may resort to only caching the registry JSONs files for now.

It is a bit annoying because this job by itself takes almost an hour for us and would run on every PR merged into master with most of the compute being wasted if I can't cache the debug symbols for all the crates.

[obi1kenobi]

Yeah, I feel that :( I opened obi1kenobi/cargo-semver-checks#401 for this.

I'm not sure how to pull this off while cargo-semver-checks is generating the "current" rustdoc itself, but if generating the current rustdoc separately is on the table then I wrote up an idea in that issue for how that might work. If you're open to exploring it together, I'd be happy to help.

[thomaseizinger]

Ah damn, now I understand why my workflow wasn't working as expected. I was still working with 0.16 locally where there was still a target directory ...

[thomaseizinger]

@obi1kenobi I am reasonably happy with the solution now. It uses almost no storage space in the GitHub cache, invalidates correctly on new releases and saves us 50% compute while running cargo semver-checks. Perhaps it is useful for v2 of your action!

[thomaseizinger]

@obi1kenobi I am reasonably happy with the solution now. It uses almost no storage space in the GitHub cache, invalidates correctly on new releases and saves us 50% compute while running cargo semver-checks. Perhaps it is useful for v2 of your action!

I just had a dirty idea. I am trying to copy in the workspace's target directory and see if I can at least reuse some of the build artifacts from the dependencies.

[obi1kenobi]

Setting CARGO_TERM_VERBOSE=true as an env var when running cargo-semver-checks will show whether cargo is rebuilding (Running rustdoc ...) or using existing artifacts (Fresh <artifact-name>).

Hoping it works! 🤞

[thomaseizinger]

I tried to set that: https://github.com/libp2p/rust-libp2p/actions/runs/4259376112/jobs/7411549615#step:13:73

[thomaseizinger]

Okay, this is ready for review again. For now, we are only caching the registry baseline JSON which already gives us a good speedup. Further improvements are tracked in cargo semver-checks: obi1kenobi/cargo-semver-checks#401

[obi1kenobi]

I tried to set that: https://github.com/libp2p/rust-libp2p/actions/runs/4259376112/jobs/7411549615#step:13:73

Ah, sorry — you'll also need to pass --verbose to cargo-semver-checks, so it allows cargo build output to pass through. Otherwise it just swallows the verbose cargo output.

[thomaseizinger]

Merging because it is likely to be uncontroversial and speeds up our CI.