indexer: return a valid pack checksum OID by penielgold-hub · Pull Request #7427 · libgit2/libgit2 · GitHub
Skip to content

indexer: return a valid pack checksum OID - #7427

Open
penielgold-hub wants to merge 1 commit into
libgit2:mainfrom
penielgold-hub:fix/indexer-checksum-oid
Open

penielgold-hub wants to merge 1 commit into
libgit2:mainfrom
penielgold-hub:fix/indexer-checksum-oid

Conversation

@penielgold-hub

Copy link
Copy Markdown

Summary

Fix git_indexer_hash() returning an incorrectly represented object ID.

The existing implementation casts raw pack checksum bytes directly to git_oid *. With the current git_oid layout, the first checksum byte is incorrectly interpreted as the OID type.

This change:

  • Stores the checksum in a properly initialized git_oid, using the indexer's object format.
  • Returns the correctly typed OID from git_indexer_hash().
  • Adds a focused regression assertion checking both the SHA-1 OID type and the expected checksum.

Regression evidence

Before the fix:

  • pack::indexer::out_of_order fails.
  • Expected OID type: GIT_OID_SHA1 (1).
  • Actual OID type: 222 (0xde), the first checksum byte.
  • Exit code: 1.

After the fix:

  • The focused regression test passes.
  • Clar reports ok 1 - pack::indexer::out_of_order.
  • Exit code: 0.

Validation

  • Built libgit2_tests using Visual Studio 2022 MSVC, x64, Debug configuration.
  • Build succeeded with parallelism limited to two jobs.
  • Focused regression test passed on the updated upstream main base.
  • git diff --check passed.

The regression test uses the existing SHA-1 pack fixture. SHA-256 compatibility is handled by the format-aware conversion but is not directly covered by this native regression test.

This correction was identified while testing libgit2 integration for rust-lang/git2-rs#1259.

@ethomson

Copy link
Copy Markdown
Member

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants