Feat/multi hash support #33

jackieismpc · 2025-11-27T13:59:19Z

This PR implements protocol-level multi-hash support for low-level objects as part of [r2cn] multi-hash protocol support (Issue #31).

What this PR does

Introduces a proper hash abstraction in hash.rs:
- Defines HashKind and ObjectHash to represent object IDs independently of the concrete hash algorithm.
- Centralizes hash-related operations so that adding new algorithms in the future requires minimal changes.
Updates object implementations (e.g. blobs, trees, commits) to work with ObjectHash and support multiple hash algorithms instead of being hard-wired to SHA-1.
Adjusts pack encoding/decoding logic to be hash-agnostic, so pack handling now works correctly with both SHA-1 and SHA-256.
Wires the protocol core (core.rs / GitProtocol) to the negotiation logic in smart.rs, so protocol operations (info_refs, upload_pack, receive_pack) are aware of the negotiated hash and are ready for multi-hash compatibility via object-format.

Notes

Each repository still uses a single hash algorithm at a time; the protocol and storage layers are now structured so that supporting SHA-256 and additional algorithms is consistent with Git’s hash-transition design.
Existing SHA-1–only users remain compatible via the new abstraction.

Fixes #31.

…the tests in hash.rs 3. Import a pack file for SHA-256 testing Signed-off-by: jackieismpc <[email protected]>

…ure hash.rs passes all tests 3.Fix issues in the object module after the refactor and add new test cases for sha256 Signed-off-by: jackieismpc <[email protected]>

…mocked pack files for testing purposes（no big pack for sha256 test） Signed-off-by: jackieismpc <[email protected]>

…due to existing parsing issues in Index::from_file 2.Added SHA-256 index test files 3.Plan to perform a small-scale refactor of the utility functions next Signed-off-by: jackieismpc <[email protected]>

…outside the protocol module Signed-off-by: jackieismpc <[email protected]>

…eady supports negotiating different hash algorithms. Signed-off-by: jackieismpc <[email protected]>

Copilot

Pull request overview

This PR implements protocol-level multi-hash support to enable SHA-256 alongside SHA-1 for Git object storage and transport. The implementation replaces the concrete SHA1 type with an abstract ObjectHash enum that can represent either hash algorithm.

Key Changes:

Introduces HashKind and ObjectHash abstractions in src/hash.rs with thread-local hash kind management
Updates all object types (blob, tree, commit, tag, note) to use ObjectHash instead of SHA1
Modifies pack encoding/decoding and protocol operations to be hash-agnostic
Adds object-format capability negotiation in the Git protocol layer

Reviewed changes

Copilot reviewed 28 out of 31 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
src/hash.rs	Core hash abstraction with `HashKind` enum and `ObjectHash` type, thread-local state management
src/utils.rs	Adds `Hashalgorithm` wrapper for computing hashes; renamed `read_sha1` → `read_sha`
src/protocol/smart.rs	Adds wire_hash_kind, local_hash_kind, and zero_id fields; implements object-format capability
src/protocol/core.rs	Updates RepositoryAccess trait methods to use ObjectHash
src/protocol/pack.rs	Updates pack generation/unpacking to use ObjectHash
src/internal/pack/*.rs	Updates wrapper, encode, decode, cache, waitlist for hash-agnostic operations
src/internal/object/*.rs	Updates all object types (blob, tree, commit, tag, note) to use ObjectHash
src/internal/index.rs	Updates index entry handling for variable hash lengths
src/diff.rs	Updates diff operations to work with ObjectHash
tests/data/packs/*.pack	Adds SHA-256 test pack files via Git LFS
Cargo.toml	Adds sha2 dependency for SHA-256 support

Copilot · 2025-11-27T14:41:50Z

src/internal/pack/cache_object.rs

+            } else {
+                panic!("Expected WouldEjectLru error");
+            }
+            // 使用不同的键插入b，这样a会被驱逐


The Chinese comment "匹配到指定错误，不需要额外操作" (line 535) and similar comments should be in English for consistency with the rest of the codebase and to ensure all contributors can understand the code.

Suggested change

// 使用不同的键插入b，这样a会被驱逐

// Insert b with a different key, so that a will be evicted

src/internal/object/tree.rs

src/utils.rs

src/internal/index.rs

Copilot · 2025-11-27T14:41:51Z

src/internal/pack/encode.rs

+
+        let encoder = PackEncoder::new(entries_number, 10, tx);
+
+        let start = Instant::now(); // 开始时间


Chinese comment "开始时间" on line 1230 should be in English. Suggested: "// start time"

Suggested change

let start = Instant::now(); // 开始时间

let start = Instant::now(); // start time

src/internal/object/commit.rs

src/utils.rs

src/protocol/smart.rs

Copilot · 2025-11-27T14:41:53Z

src/internal/object/tree.rs

+                let next = i + index + get_hash_kind().size() + 1; // +1 for the null byte
+                if next > data.len() {
+                    return Err(GitError::InvalidTreeObject);
+                } //check bounds TreeItem::from_bytes will panic if out of bounds


The bounds check was added but the error message says "check bounds TreeItem::from_bytes will panic if out of bounds". This comment is unclear - it should explain what happens if the check fails (returns an error) rather than talking about a panic that no longer occurs. Consider: "// Ensure we have enough bytes remaining for the hash"

Suggested change

} //check bounds TreeItem::from_bytes will panic if out of bounds

} // Ensure we have enough bytes remaining for the hash

jackieismpc · 2025-11-28T02:19:13Z

本地测试通过，线上测试拉取 LFS 失败 @genedna
Fetching LFS objects
/usr/bin/git lfs fetch origin refs/remotes/pull/33/merge
fetch: Fetching reference refs/remotes/pull/33/merge
batch response: This repository exceeded its LFS budget. The account responsible for the budget should increase it to restore access.
Error: error: failed to fetch some objects from 'https://github.com/web3infra-foundation/git-internal.git/info/lfs'
The process '/usr/bin/git' failed with exit code 2
Waiting 20 seconds before trying again

Copilot

Pull request overview

Copilot reviewed 29 out of 32 changed files in this pull request and generated 5 comments.

src/internal/pack/decode.rs

src/internal/pack/encode.rs

src/internal/index.rs

Copilot · 2025-11-28T02:28:04Z

src/internal/index.rs

-            let padding = 8 - ((22 + name_len) % 8); // 22 = sha1 + flags, others are 40 % 8 == 0
+            let hash_len = get_hash_kind().size();
+            let entry_len = hash_len + 2 + name_len;
+            let padding = 1 + ((8 - ((entry_len + 1) % 8)) % 8); // at least 1 byte nul


The comment "// 22 = sha1 + flags..." is obsolete. The code now correctly computes padding based on hash_len, but the comment references the old hardcoded 22-byte assumption (20 for SHA1 + 2 for flags). Remove this outdated comment.

Suggested change

let padding = 1 + ((8 - ((entry_len + 1) % 8)) % 8); // at least 1 byte nul

let padding = 1 + ((8 - ((entry_len + 1) % 8)) % 8);

src/internal/pack/cache.rs

genedna · 2025-11-28T02:28:53Z

本地测试通过，线上测试拉取 LFS 失败 @genedna Fetching LFS objects /usr/bin/git lfs fetch origin refs/remotes/pull/33/merge fetch: Fetching reference refs/remotes/pull/33/merge batch response: This repository exceeded its LFS budget. The account responsible for the budget should increase it to restore access. Error: error: failed to fetch some objects from 'https://github.com/web3infra-foundation/git-internal.git/info/lfs' The process '/usr/bin/git' failed with exit code 2 Waiting 20 seconds before trying again

我调整了 Budget

Signed-off-by: jackieismpc <[email protected]>

jackieismpc added 6 commits November 25, 2025 15:50

1.Introduce SHA-256 support using two abstraction layers 2.Refactor …

d612fe6

…the tests in hash.rs 3. Import a pack file for SHA-256 testing Signed-off-by: jackieismpc <[email protected]>

1.Temporarily fix all issues introduced by the hash abstraction 2.Ens…

775f1ec

…ure hash.rs passes all tests 3.Fix issues in the object module after the refactor and add new test cases for sha256 Signed-off-by: jackieismpc <[email protected]>

1.Completed pack module updates and added SHA-256 tests 2.Introduced …

69638ba

…mocked pack files for testing purposes（no big pack for sha256 test） Signed-off-by: jackieismpc <[email protected]>

1.Completed updates to the index module, but tests are still failing …

7f2807a

…due to existing parsing issues in Index::from_file 2.Added SHA-256 index test files 3.Plan to perform a small-scale refactor of the utility functions next Signed-off-by: jackieismpc <[email protected]>

1.Completed refactor of the utility functions 2.Finished all changes …

b9bd890

…outside the protocol module Signed-off-by: jackieismpc <[email protected]>

1.Complete the remaining changes. 2.The underlying smart.rs layer alr…

ecf0156

…eady supports negotiating different hash algorithms. Signed-off-by: jackieismpc <[email protected]>

genedna requested a review from Copilot November 27, 2025 14:38

Copilot started reviewing on behalf of genedna November 27, 2025 14:38 View session

Copilot finished reviewing on behalf of genedna November 27, 2025 14:40

Copilot AI reviewed Nov 27, 2025

View reviewed changes

genedna requested a review from Copilot November 28, 2025 02:25

Copilot started reviewing on behalf of genedna November 28, 2025 02:25 View session

Copilot finished reviewing on behalf of genedna November 28, 2025 02:27

Copilot AI reviewed Nov 28, 2025

View reviewed changes

jackieismpc force-pushed the feat/multi-hash-support branch 3 times, most recently from 08ac180 to 21e5d78 Compare November 28, 2025 06:09

[ci] Fix CI issues for multi-hash support

b480c4f

Signed-off-by: jackieismpc <[email protected]>

jackieismpc force-pushed the feat/multi-hash-support branch from 21e5d78 to b480c4f Compare November 28, 2025 06:49

genedna approved these changes Nov 28, 2025

View reviewed changes

genedna merged commit 6e3be96 into web3infra-foundation:main Nov 28, 2025
18 of 25 checks passed

	// 使用不同的键插入b，这样a会被驱逐
	// Insert b with a different key, so that a will be evicted


		let encoder = PackEncoder::new(entries_number, 10, tx);

		let start = Instant::now(); // 开始时间

	let start = Instant::now(); // 开始时间
	let start = Instant::now(); // start time

	} //check bounds TreeItem::from_bytes will panic if out of bounds
	} // Ensure we have enough bytes remaining for the hash

	let padding = 1 + ((8 - ((entry_len + 1) % 8)) % 8); // at least 1 byte nul
	let padding = 1 + ((8 - ((entry_len + 1) % 8)) % 8);

Feat/multi hash support #33

Feat/multi hash support #33

Conversation

jackieismpc commented Nov 27, 2025

What this PR does

Notes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Nov 27, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Nov 27, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Nov 27, 2025

Choose a reason for hiding this comment

Uh oh!

jackieismpc commented Nov 28, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

genedna commented Nov 28, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants