Add Rust CI improvements

[GnaneshGnani]

Rust CI

Summary

Enhances the Rust CI workflow with lockfile validation and improved caching to ensure consistent builds and catch dependency drift early.

Changes

.github/workflows/rust.yml

1. Lockfile Validation: Added validation step in both format-and-lint and test jobs

   • Generates a fresh lockfile and compares it with the committed version
   • Fails CI if lockfile is out of sync with Cargo.toml files
   • Provides clear error message with remediation steps

2. Cache Key Improvement: Updated cargo cache key to include **/Cargo.toml

   • Before: ${{ runner.os }}-cargo-${{ hashFiles('**/Cargo.lock') }}
   • After: ${{ runner.os }}-cargo-${{ hashFiles('**/Cargo.lock', '**/Cargo.toml') }}
   • Ensures cache is invalidated when dependencies change in Cargo.toml files

Benefits

• Consistency: Prevents "works on my machine" issues caused by mismatched lockfiles
• Early Detection: Catches lockfile drift before merge, not during deployment
• Better Caching: More accurate cache invalidation when dependencies change
• Developer Experience: Clear error messages guide developers to fix lockfile issues

Testing

• Verified both jobs pass with a valid lockfile
• Confirmed CI fails appropriately when lockfile is out of sync

[milindsrivastava1997]

@GnaneshGnani
Do we want to update Cargo.lock autonomously in CI or is this just to catch devs forgetting to commit Cargo.lock?

cargo generate-lockfileresolves all deps to their latest compatible versions from crates.io. This will cause spurious CI failures whenever any transitive dep publishes a new compatible release, even with no code changes.
(from Claude)

[GnaneshGnani]

@milindsrivastava1997, The goal is to catch Cargo.toml changed but Cargo.lock wasn’t committed, not to update the lockfile in CI.

cargo generate-lockfile regenerates the lockfile and resolves to the latest compatible versions from crates.io. That means CI can fail when a transitive dependency publishes a new compatible release, even if the project didn’t change. (from Calude)

[milindsrivastava1997]

@GnaneshGnani Seems like cargo check --lcoked is a better option then?

[GnaneshGnani]

@milindsrivastava1997, its a better option. We can add --locked to the existing cargo clippy and cargo test steps and remove the separate lockfile validation step.

[milindsrivastava1997]

Great let's do that please. Thanks

[milindsrivastava1997]

Please use --locked in existing cargo commands and remove cargo generate-lockfile

[GnaneshGnani]

@milindsrivastava1997 , seems like adding --locked will fail because of git dependencies

[milindsrivastava1997]

@GnaneshGnani Got it. Can you do a bit of research and suggest a few different options with tradeoffs?

[GnaneshGnani]

Solution 1: Generate Lockfile and Auto-Commit

Run cargo generate-lockfile in CI. If Cargo.lock changed, commit and push it back to the branch.

Pros:

• CI always passes. No lockfile-related failures.
• Cargo.lock stays up to date without manual steps.
• No need for --locked or pinning git deps.
• Contributors don't need to remember to run cargo update and commit.

Cons:

• Surprise lockfile-only commits. A transitive dependency can publish a new version and CI will commit that change. You can get commits that only touch Cargo.lock with no code changes.
• New transitive deps can slip in without explicit review.
• CI pushes to the branch, which can surprise authors and complicate workflows.
• Merge conflicts possible when multiple PRs each get their own lockfile updates.
• Requires CI to have write access (e.g. GITHUB_TOKEN with push permissions) and a step to commit and push.

---

Solution 2: Pin Git Dependencies

Add rev = "commit-hash" to git dependencies in Cargo.toml.

Pros:

• Catches forgotten Cargo.lock. If someone changes Cargo.toml and forgets to commit the lockfile, CI fails with a clear error.
• No spurious failures. CI only fails when the repo actually has a problem, not when upstream deps publish new versions.
• Reproducible builds. Same code always builds with same dependency versions.
• Standard practice for Rust projects with git dependencies.

Cons:

• Must manually update the rev when you want to pull in new changes from the git repos. Run cargo update -p dsrs (or sketchlib-rust), then copy the new commit hash from Cargo.lock into Cargo.toml.

---

Solution 3: Publish Internal Crates to Registry

Publish dsrs and sketchlib-rust to crates.io or a private registry (e.g. GitHub Packages). Use versioned dependencies instead of git.

Pros:

• No git pinning needed. Dependencies are versioned like normal crates.
• Standard, reproducible. Same as any other crate dependency.
• --locked works without any special handling.

Cons:

• Requires setting up a publishing workflow. Need to publish new versions when you update the crates.
• More setup and maintenance. Not a quick fix.

---

Solution 4: Git Submodules + Path Dependencies

Add datasketches-rs and sketchlib-rust as git submodules. Switch to path dependencies in Cargo.toml.

Pros:

• No git fetch during cargo build. Dependencies come from the checked-out submodules.
• --locked works.
• Lockfile validation works as intended.

Cons:

• Submodule management overhead. Need to remember to update submodule refs when you want new versions.
• Larger repo. Submodules add to clone size and complexity.
• Team needs to be comfortable with submodules (git submodule update, etc.).

---

Solution 5: cargo generate-lockfile + Diff

Copy Cargo.lock, run cargo generate-lockfile, diff against the copy, fail if different.

Pros:

• Catches forgotten Cargo.lock. If Cargo.toml changed and Cargo.lock wasn't updated, the diff will show it.

Cons:

• Spurious failures when any transitive dependency publishes a new compatible version. cargo generate-lockfile resolves to the latest compatible versions from crates.io, so the diff will change even if your code didn't.
• Spurious failures when git deps get new commits on their default branch.
• CI fails with no code changes. Someone can push a PR with no Rust changes and CI fails because a dependency released a new version overnight.

@milindsrivastava1997 pls check these solutions. Solution 2 seems good for me

[milindsrivastava1997]

Solution 3 might also work. ASAPQuery's CI already has the capability to push packages to ProjectASAP's ghcr, based on tags.
Any thoughts @zzylol ?

[zzylol]

Solution 3 might also work. ASAPQuery's CI already has the capability to push packages to ProjectASAP's ghcr, based on tags. Any thoughts @zzylol ?

So it seems Option 2: use --locked in CI, but only after pinning the git dependencies with rev = .... is better?

Best path here seems to be: keep the cache-key improvement, remove cargo generate-lockfile validation, pin git dependencies with rev = ..., and then enforce --locked on the existing cargo commands. cargo generate-lockfile can rebuild to newer compatible versions and cause unrelated CI failures, while --locked gives the behavior we actually want once git deps are pinned.
I think this one makes the most sense.

Regarding what if the rev = ... changes:

• Normal PRs that do not touch that dependency: no change to rev.
• A PR that updates the dependency repo to a newer commit: yes, update rev (in Cargo.toml, and then run Cargo build), and then commit the resulting Cargo.lock and updated Cargo.toml.
• A PR that only changes your app code: no change to rev.

[milindsrivastava1997]

My only concern is remembering to update the git revs, but maybe that's a problem for later.
Sounds good. Let's do Option 2 then @GnaneshGnani .

[GnaneshGnani]

@milindsrivastava1997, please review the changes. Pinned the git dependencies