gix-transport: add SHA-256 object-format negotiation test coverage#2647
Closed
Adrian Ratiu (10ne1) wants to merge 4 commits into
Closed
gix-transport: add SHA-256 object-format negotiation test coverage#2647Adrian Ratiu (10ne1) wants to merge 4 commits into
Adrian Ratiu (10ne1) wants to merge 4 commits into
Conversation
Add a sha256 analog of the existing `from_bytes` capability test,
asserting that gix-transport's capability parser reads a server
`object-format=sha256` advertisement as sha256:
supports("sha256") is true, and supports("sha1") is false for this
capability, the negative control mirroring the existing sha1 test.
No production change is needed: the parser is generic over the
advertised value, so this just verifies the transport layer passes
sha256 through.
Co-Authored-By: Claude Opus 4.8 <[email protected]>
Signed-off-by: Adrian Ratiu <[email protected]>
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds coverage and fixtures to ensure Git protocol v2 handshakes correctly surface object-format=sha256 across transport variants.
Changes:
- Added new handshake fixtures advertising
object-format=sha256(plain and HTTP). - Added client tests verifying v2 handshake capabilities include
object-format=sha256. - Added capability parsing test ensuring
object-format=sha256is recognized and excludessha1.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| gix-transport/tests/fixtures/v2/http-handshake-sha256.response | Adds an HTTP v2 handshake fixture advertising SHA-256 object format. |
| gix-transport/tests/fixtures/v2/handshake-sha256.response | Adds a non-HTTP v2 handshake fixture advertising SHA-256 object format. |
| gix-transport/tests/client/git.rs | Adds handshake test asserting SHA-256 object-format is surfaced from v2 handshake. |
| gix-transport/tests/client/capabilities.rs | Adds parsing test for object-format=sha256 capability semantics. |
| gix-transport/tests/client/blocking_io/http/mod.rs | Adds blocking HTTP test asserting SHA-256 object-format is surfaced from v2 handshake. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Add a daemon-transport handshake test backed by a minimal v2 fixture that advertises `object-format=sha256`. It asserts the handshake reports protocol v2, defers refs to a later ls-refs trip and surfaces the sha256 object-format capability. Co-Authored-By: Claude Opus 4.8 <[email protected]> Signed-off-by: Adrian Ratiu <[email protected]>
Add an http handshake test backed by `v2/http-handshake-sha256.response` (modeled on the existing sha1 `v2/http-handshake.response`, with an `object-format=sha256` capability and Content-Length adjusted to match). It asserts the handshake reports protocol v2, returns no refs (deferred to ls-refs) and surfaces the sha256 object-format. Verified on both the reqwest and curl http backends. No production change is required: the http transport relays the server sha256 negotiation untouched. Co-Authored-By: Claude Opus 4.8 <[email protected]> Signed-off-by: Adrian Ratiu <[email protected]>
The .response and .request fixtures are byte-exact protocol captures: CRLF HTTP headers, pkt-line length prefixes and a Content-Length that must match the body. Git line-ending conversion (e.g. core.autocrlf) could rewrite them on checkout, making the byte-exact tests platform dependent or fail unexpectedly, so mark the whole fixtures directory "-text" so the on-disk bytes stay stable across environments, matching the existing gix-protocol/tests/fixtures policy. Co-Authored-By: Claude Opus 4.8 <[email protected]> Signed-off-by: Adrian Ratiu <[email protected]>
Adrian Ratiu (10ne1)
force-pushed
the
dev/aratiu/sha256-transport-fixtures
branch
2 times, most recently
from
June 18, 2026 17:38
1f695de to
00a11c7
Compare
Contributor
Author
|
I reworded the commit message subjects to conform to the gitoxide rules, no other code changes. |
Member
|
Thanks a lot for driving this forward! While for a moment I was thinking that this is probably "over"-testing it, I also thought that having those tests probably won't hurt. Please note that this PR once again had to be superseded by #2656 to capture my own commit. If it's no hassle for you, allowing me to push back would be appreciated. Closing as superseded. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
This PR is a companion to #2642 (which does the protocol-level SHA-256 negotiation in
gix-protocolandgix) and covers the layer below it: provinggix-transportrelays the serverobject-format=sha256capability through the V2 handshake.This just adds three tests and doesn't require any "production" changes, because
gix-transportis hash-agnostic by design and treats capabilities as simple key=value pairs, so the sha256 advertisments flow through unmodified.Part of #281 (Batch 3, "Protocol and Transport"), transport half.
Testing
cargo nextest run -p gix-transport --no-default-features --features blocking-client,http-client-reqwest,http-client-insecure-credentials,maybe-async/is_sync --test blocking-transportcargo nextest run -p gix-transport --features http-client-curl,http-client-insecure-credentials,maybe-async/is_sync --test blocking-transport-http-onlycargo nextest run -p gix-transport --features async-clientThese are canned byte fixtures, so they run under the existing default-feature test runs, no
GIX_TEST_FIXTURE_HASHor generated archives involved.AI disclosure
This PR was written with the help of Claude Code, but was not vibe-coded. I'm not a fan of vibe-coding. I used AI to better understand the codebase myself (asked it a lot of questions), review both my code and the code generated by AI, used it especially to detect bugs and corner-cases and suggest fixes, however I very carefully reviewed & edited each code/comment lines, the commit messages and so on, until I was satisfied with the result.