Add minimal-versions CI check#1940
Conversation
Signed-off-by: doxxx93 <[email protected]>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1940 +/- ##
=======================================
- Coverage 76.3% 76.2% -0.0%
=======================================
Files 89 89
Lines 8487 8487
=======================================
- Hits 6468 6466 -2
- Misses 2019 2021 +2 🚀 New features to boost your workflow:
|
|
@clux I put this as a separate job in One thing I'm wondering: should this run on PRs ( What do you think? |
yeah, makes sense.
mh, yeah, it would actually be good to have this pre-merge. maybe a |
|
I think If we want a dedicated What do you think about putting it in Another thought: if we eventually create a |
|
|
|
@clux The minimal-versions check is failing because Since this is an upstream issue (hyper-rustls → rustls → aws-lc-rs → aws-lc-sys chain), I'm thinking of using Does that sound reasonable? |
|
oh, convenient, it's actually failing now. |
hmm, we skip that in the feature powerset job purely because it's impractically large/long runtime otherwise. but i guess there's nothing that we can actually bump to fix this as it's not a direct dependency for us so skipping it still makes sense. (unless we took an explicit dep on aws-lc-rs and bumped two patch versions, but that's not something we should do without a lockfile) |
|
@clux After more local testing on the actual repo,
The skip list keeps growing, which reduces coverage. I see two options:
What's your preference? (Apologies for not testing on the actual repo earlier — I only ran it on a minimal test repo initially.) Update: Verified option 2 passes locally. This also means Update 2: Found a better approach — The difference: This means:
When I tested locally, it actually found 11 lower bounds in Updated recipe: Note: |
|
Ah, yes. Direct minimal sounds like a much better and more maintainable approach for a library without a lockfile! Let's do that, and bump the pins to the new required minimal versions while we are at it! |
Signed-off-by: doxxx <[email protected]>
Signed-off-by: doxxx <[email protected]>
|
This turned out to be a much deeper rabbit hole than expected, but happy with where it landed. With -Z direct-minimal-versions, we get accurate lower bound validation without fighting upstream transitive failures. Going forward, when k8s-openapi bumps its supported Kubernetes version, we just update K8S_OPENAPI_ENABLED_VERSION in the recipe — and the CI will catch any stale lower bounds before they hit users. |
ah, right. do you mind putting that in the justfile where we are doing |
|
no problem! |
Signed-off-by: doxxx <[email protected]>
|
Definitely the toughest PR I've worked on recently 😅 |
Motivation
After #1939, lower bounds declared in Cargo.toml can silently drift from what actually compiles. Since we don't use lockfiles, there's no safety net to catch this.
Solution
Add a
minimal-versionsjob tofeatures.ymlthat resolves all dependencies to their minimum declared versions and checks that the workspace still compiles.cargo hack --remove-dev-deps --workspaceto prevent dev-deps from affecting resolutioncargo +nightly update -Z minimal-versionsto lock to lowest allowed versionscargo check --workspace --all-featuresto verify compilationAlso adds a
just minimal-versionsrecipe for local use.