Bump opendal to 0.58.1 and fix fallout (fixes local GCS cache usage)

[flip1995]

Motivation

• feat: opendal should support gcs authorized_user credential apache/opendal#6287

The gcs cache can currently not be used with authorized_user application-default credentials, as this was not supported in reqsign version 0.16.5, which opendal version 0.55.0 depended on. This was fixed in a later version of reqsign and since has been updated in opendal

• Tracking issues for reqsign-core v2 apache/opendal#6553

With this, the gcs cache should Just Work ™️ after a gcloud auth application-default login.

Changes

I split this PR into the following commits for easier commit-by-commit review:

Bump MSRV to 1.91.0

opendal 0.58.1 has an MSRV of 1.91.0, so sccache needs to match this in order to bump the opendal dependency.

Bump opendal to 0.58.1

• Required bump of reqwest to 0.13.0
  • Rename and remove features deprecated in that release
  • https://github.com/seanmonstar/reqwest/releases/tag/v0.13.0
  • feat: consolidate TLS options with rustls-platform-verifier seanmonstar/reqwest#2891
• Remove direct reqsign dependency that was not used in the code
• Add new (optional) opendal-* dependencies that were split out of opendal

Run cargo update

I first did just a partial Cargo.lock update for only the dependencies that were bumped in this PR. But the churn was quite big when just doing that. So I figured why not do a full, clean cargo update of the Cargo.lock file.

Fix fallout in code

• HttpClientLayer was removed and now has to be built from the OperationContext
• LoggingLayer was moved to its own crate
• allow_anonymous was deprecated in favor of skip_signature
• Use sha256 instead of sha1, as that is quasi-deprecated. I'm not sure if this is actually necessary. I think it fixed some issue in the tests? In any case, I think this is a good change
• Check if native_certs are available when building a reqwest client, and if not set the TLS certs to an empty vector. With reqwest 0.13, the client would fail otherwise.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 48.27586% with 15 lines in your changes missing coverage. Please review.
✅ Project coverage is 72.85%. Comparing base (46e96ab) to head (160d965).

Files with missing lines	Patch %	Lines
src/util.rs	0.00%	5 Missing ⚠️
src/cache/oss.rs	0.00%	3 Missing ⚠️
src/cache/cos.rs	0.00%	2 Missing ⚠️
src/cache/gcs.rs	0.00%	2 Missing ⚠️
src/cache/gha.rs	0.00%	2 Missing ⚠️
src/cache/redis.rs	50.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2715      +/-   ##
==========================================
- Coverage   72.90%   72.85%   -0.05%     
==========================================
  Files          72       72              
  Lines       37275    36390     -885     
==========================================
- Hits        27176    26513     -663     
+ Misses      10099     9877     -222     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[flip1995]

Not sure why CI is failing. I'd appreciate any pointers how I can try to reproduce those locally and debug.

EDIT: Managed to reproduce the dist tests locally.

[flip1995]

Hm, the dist tests are running into this issue: seanmonstar/reqwest#2988

[flip1995]

@sylvestre could you approve the CI run please. I think my latest commit has fixed the test_dist_ part. But locally I'm getting weird timeouts for the test_dist_cargo_ tests, where I'm not sure if some local config is causing this or it's actually still an issue.

[flip1995]

Blocked by apache/opendal#7701 and the following opendal release.

[flip1995]

@sylvestre With de7fa43 I'm patching the opendal dependency to my branch of apache/opendal#7701. This should make the CI of this PR pass. But to merge this PR, we'll have to wait until my opendal fix is released. This commit is just for me to check if there's anything else I need to fix after this.

Could you approve the CI run one more time, please? Totally understandable if you don't want to approve this with such external changes included in the PR. Happy to wait until the opendal fix is through in that case.

[sylvestre]

sure, done

[flip1995]

Converting to draft until there's a new opendal version released.

[irh]

@flip1995 apache/opendal#7701 was included in 0.58.

[flip1995]

Yes, but version 0.58.0 was yanked and I'm waiting for release 0.58.1: apache/opendal#7920

Don't worry, I'm subscribed to the opendal GH project for releases. So as soon as that lands, I will bump to 0.58.1 here (latest commit already contains all of the fallout work necessary for that) and finish up this PR 👍

[flip1995]

@sylvestre This is now ready for another CI run and a subsequent review. I rebased on latest master, re-organized the commits and updated the PR description to list out all the changes that were required by the dump.

[codspeed-hq]

Merging this PR will regress 4 benchmarks

⚠️ Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

⚡ 8 improved benchmarks
❌ 4 regressed benchmarks
✅ 46 untouched benchmarks
⏩ 6 skipped benchmarks¹

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

	Mode	Benchmark	BASE	HEAD	Efficiency
❌	Simulation	cache_entry_create_large	4.9 ms	5.5 ms	-11.17%
❌	Simulation	build_workflow_incremental	6.7 ms	7.4 ms	-10.72%
❌	Simulation	cache_entry_roundtrip_large	5.5 ms	5.8 ms	-5.02%
❌	Simulation	compression_high_compressibility	2.4 ms	2.5 ms	-3.41%
⚡	Memory	hash_large_data	324 B	64 B	×5.1
⚡	Memory	hash_header_file	324 B	64 B	×5.1
⚡	Memory	cache_key_generation	324 B	64 B	×5.1
⚡	Memory	time_macro_finder_with_macros	324 B	64 B	×5.1
⚡	Memory	time_macro_finder_no_macros	324 B	64 B	×5.1
⚡	Memory	hash_multiple_files	260 B	64 B	×4.1
⚡	Simulation	time_macro_finder_chunked	1,026.7 µs	862.9 µs	+18.98%
⚡	Simulation	time_macro_finder_no_macros	3 ms	2.8 ms	+5.74%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.

---

_{Comparing flip1995:gcs-reqsign-fix (160d965) with main (46e96ab)}

Footnotes

1. 6 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

[flip1995]

The Clippy commit is best reviewed with hiding whitespace changes. The Clippy fixes are probably necessary due to the MSRV bump.

[Xuanwo]

Thank you for helping bump this!

[sylvestre]

please split this PR
the update of rust min version should be done in a diffferent pr, thanks

[flip1995]

I've split out the MSRV bump to #2793. I still kept the commit in this PR, but will remove it and rebase once the other PR is merged.

I rebased on master to resolve the nvcc conflict and the last Windows Clippy finding should be fixed now as well. I also moved most of the Clippy fixes to #2793, as they are necessary due to the MSRV bump.

[Xuanwo]

#2793 is now merged. Thank you @flip1995 for working on this!

[flip1995]

PR is rebased, the 2 commits merged in #2793 removed 👍

Do you want me to look into the 4 perf regressions? I might need some guidance on where to start with that though.

[sylvestre]

@codspeedbot fix this regression

[codspeed-hq]

@codspeedbot fix this regression

Looked into it — here's what I found and what I changed.

Root cause

The regressions are not caused by the opendal/reqwest work. They come from the cargo update commit, which upgraded the bundled zstd C library:

zstd-sys 2.0.10+zstd.1.5.6  →  2.0.16+zstd.1.5.7

I confirmed this by isolating zstd in a standalone binary under callgrind: compressing identical input at level 3 costs ~6.8% more instructions with 1.5.7 (21.9M → 23.4M instructions for 2 MiB). The head flamegraphs also show two new 1.5.7 functions, ZSTD_splitBlock and ZSTD_recordFingerprint_43, taking ~11% of cache_entry_create_large.

Then I rebuilt the benchmarks on this branch with only the zstd stack downgraded to the main versions, and got results that match the CI report almost exactly:

Benchmark (Simulation)	zstd 1.5.6	zstd 1.5.7	CI BASE → HEAD
cache_entry_create_large	4.88 ms	5.50 ms	4.9 → 5.5 ms
cache_entry_roundtrip_large	5.14 ms	5.43 ms	5.5 → 5.8 ms
compression_high_compressibility	2.41 ms	2.49 ms	2.4 → 2.5 ms

So 3 of the 4 are fully explained by the upstream library bump. build_workflow_incremental measured identically (6.83 ms) with both zstd versions on my machine, so its 6.7 → 7.4 ms in CI looks related to the runtime-environment difference CodSpeed flagged on that report rather than to a code change.

The fix

Pinning zstd back didn't seem right, so I instead went after what the profile actually pointed at. In build_workflow_incremental, CacheWrite::put_bytes compressing b"success\n" — 8 bytes — cost 18.65% of the whole benchmark, almost entirely ZSTD_resetCCtx_internal → memset.

The reason: put_bytes went through zstd::stream::copy_encode, which takes a Read and cannot know the input size, so zstd sized its compression context for the default 2 MiB window and zeroed that workspace on every call — for a few bytes of stdout/stderr. put_bytes always knows the length, so the new commit compresses those blobs with an explicit encoder and pledges the source size. Compression ratio is unchanged (verified: identical output sizes), and put_object is untouched (pledging the size there measured slightly worse for multi-megabyte objects, so I left it alone).

Measured locally, same machine and same lockfile, only this change:

Benchmark (Simulation)	Before	After	Change
cache_entry_batch_create	47.99 ms	25.27 ms	−47%
build_workflow_initial	29.19 ms	15.56 ms	−47%
cache_entry_batch_roundtrip	22.33 ms	13.25 ms	−41%
cache_entry_create_small	1.39 ms	934 µs	−33%
compression_high_compressibility	2.49 ms	1.70 ms	−32%
cache_entry_roundtrip_small	1.79 ms	1.34 ms	−25%
compression_low_compressibility	2.11 ms	1.66 ms	−21%
build_workflow_incremental	6.78 ms	5.42 ms	−20%
cache_entry_roundtrip_large	4.83 ms	3.92 ms	−19%
cache_entry_create_large	5.50 ms	4.59 ms	−17%
build_workflow_rebuild, decompression_*	—	—	unchanged

Nothing regressed, and all four originally-flagged benchmarks now land below the main baseline, so the upstream zstd cost is more than offset. cargo clippy --all-targets --all-features is clean and the 472 lib tests pass.

Pushed as 88fce3a and added a section to the PR description. The remaining ~7% zstd-per-byte cost is inherent to the upstream 1.5.7 release and isn't something to fix here.

Summary

• Warming up the performance wizard...
• Investigating the reported regressions — 3 of 4 regressions reproduce exactly and come from the bundled zstd 1.5.6 -> 1.5.7 upgrade pulled in by cargo update
• Validating the fix — All 4 regressed benchmarks are now 16-32% faster than the PR head and below the main baseline, with no benchmark regressing
• Committing and pushing — Pushed the cache_io optimization to gcs-reqsign-fix
• Handling comment — Comment handling finished successfully

_{Was this response helpful? React with 👍 or 👎 to help us improve CodSpeedBot.}

[flip1995]

• Confirmed with an isolated callgrind experiment that bundled zstd 1.5.6 -> 1.5.7 costs ~7% more instructions; now rebuilding sccache benchmarks with the old zstd to confirm end-to-end

Hm, if this is caused by a dep bump, I can revert the cargo update full-deps update and just update the necessary dependencies (opendal, reqwest and transitive deps)

[sylvestre]

yeah, it would be great! thanks codspeed!