dev/aratiu/sha256 pack#2657
Merged
Merged
Conversation
The checksum-verification child progress was always labelled "Sha1 of index", even when verifying a SHA-256 index. The checksum itself already uses the index's own object hash; only the label was misleading. Co-Authored-By: Claude Opus 4.8 <[email protected]> Signed-off-by: Adrian Ratiu <[email protected]>
The existing multi-index write test feeds the static SHA-1 packs and checks pinned SHA-1 values, so writing a multi-index was never exercised with SHA-256 object ids. Add a companion test that builds a multi-index from the hash-parameterized fixture (via GIX_TEST_FIXTURE_HASH), so the writer now runs under both hashes. Co-Authored-By: Claude Opus 4.8 <[email protected]> Signed-off-by: Adrian Ratiu <[email protected]>
- refactored both tests for readability, given most of them was duplicated. Co-authored-by: Sebastian Thiel <[email protected]>
There was a problem hiding this comment.
Pull request overview
Extends gix-pack multi-index writer test coverage to run against the hash-parameterized fixture (so it gets exercised under both SHA-1 and SHA-256), and updates an index verification progress label to no longer hardcode SHA-1.
Changes:
- Refactors the existing multi-index write test into shared helpers and adds a new test that uses the hash-parameterized pack fixture (dual-hash coverage).
- Copies referenced pack/index files into a temp dir for full integrity verification of the written multi-index.
- Renames a progress label in index verification from SHA-1-specific wording to generic wording.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| gix-pack/tests/pack/multi_index/write.rs | Refactors and extends tests to write/verify multi-indexes under both SHA-1 and SHA-256 fixtures. |
| gix-pack/src/index/verify.rs | Makes the checksum progress label hash-agnostic. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Sebastian Thiel (Byron)
enabled auto-merge
June 20, 2026 06:05
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.
This PR is #2655 with a review commit added on top.
No rewrites.