gix-transport: add a download-progress byte counter to http::Options#2706
gix-transport: add a download-progress byte counter to http::Options#2706Nik B (nikicat) wants to merge 1 commit into
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 47b4e81782
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "Codex (@codex) review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "Codex (@codex) address that feedback".
| /// (e.g. the v2 `ls-refs` ref advertisement). Cumulative across requests on | ||
| /// the same transport; the caller owns resetting it if it wants per-request | ||
| /// figures. Only the curl backend honors it. | ||
| pub download_progress: Option<Arc<std::sync::atomic::AtomicU64>>, |
There was a problem hiding this comment.
When this is released, any dependent crate constructing http::Options with an exhaustive struct literal now has to add download_progress; STABILITY.md defines that as a breaking change, but the commit subject is not a breaking conventional message as required by the repo instructions, so smart-release/changelog handling can treat this as a normal add instead of a breaking API change.
Useful? React with 👍 / 👎.
| io::{Read, Write}, | ||
| sync::{ | ||
| Arc, | ||
| atomic::{AtomicU64, Ordering}, |
There was a problem hiding this comment.
On targets without native 64-bit atomics, std::sync::atomic::AtomicU64 is unavailable; for example rustc --print cfg --target armv5te-unknown-linux-gnueabi lacks target_has_atomic="64", and this repo already handles that case in gix-status with a portable_atomic fallback. As written, enabling the curl HTTP transport on such targets fails to compile before users can opt out of the new progress counter.
Useful? React with 👍 / 👎.
Nik B (nikicat)
force-pushed
the
pr-download-progress
branch
from
47b4e81 to
ea6256e
Compare
|
Both review findings are addressed in the force-push (
Curl integration tests (including the new counter test) and the |
The curl backend now adds the length of each response-body chunk it receives to an optional `Arc<AtomicU64>` carried on `http::Options`. This gives callers a live byte signal for phases the protocol layer reports no progress for — notably the v2 `ls-refs` advertisement, which on a large-ref mirror is many MiB and can take minutes with no feedback. Headers flow through the separate `header()` callback, so the counter only ever accumulates body bytes. Covered by a curl-backend test that drives a handshake against the mock server and asserts the count is non-zero. This is a breaking change: `http::Options` is constructible via a struct literal (it is not `#[non_exhaustive]`), so dependents that spell out every field must add the new field. The counter type is re-exported as `http::AtomicU64`: `std`'s atomic where the target has native 64-bit atomics, with the same `portable-atomic` fallback `gix-status` uses everywhere else, so the curl transport keeps compiling on targets like `armv5te` that lack 64-bit atomics. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Nik B (nikicat)
force-pushed
the
pr-download-progress
branch
from
ea6256e to
c7d0347
Compare
1 participant
The curl backend adds the length of each response-body chunk it receives to an optional
Arc<AtomicU64>carried onhttp::Options. This gives callers a live byte signal for phases the protocol layer reports no progress for — notably the v2ls-refsadvertisement, which on a large-ref mirror is many MiB and can take a long time with no feedback at all (transfer_progressonly starts moving once the pack begins).Headers flow through the separate
header()callback, so the counter only ever accumulates body bytes.Covered by
download_progress_counts_response_body_bytes, which drives a handshake against the mock server and asserts the count is non-zero and consistent.🤖 Generated with Claude Code