Skip to content

Use GIX_TEST_FIXTURE_HASH for gix-index tests#2598

Open
Christoph Rüßler (cruessler) wants to merge 4 commits into
GitoxideLabs:mainfrom
cruessler:run-gix-index-tests-with-sha-256
Open

Use GIX_TEST_FIXTURE_HASH for gix-index tests#2598
Christoph Rüßler (cruessler) wants to merge 4 commits into
GitoxideLabs:mainfrom
cruessler:run-gix-index-tests-with-sha-256

Conversation

@cruessler
Copy link
Copy Markdown
Contributor

@cruessler Christoph Rüßler (cruessler) commented May 15, 2026

This PR makes gix-index tests run with GIX_TEST_FIXTURE_HASH=sha256.

From what I can tell, the error on CI is not related to changes in this PR, so I’m going to mark it as ready for review.

From what I understand, there’s a couple of fixtures in gix-index that contain hard-coded SHA-1 hashes and can’t be generated. In particular, there’s enum Fixture that has two variants, Generated and Loose. My changes assume that only Generated fixtures can be created for SHA-256, so SHA-256 tests are skipped for Loose fixtures.

This is part of #281.

@cruessler Christoph Rüßler (cruessler) marked this pull request as ready for review May 16, 2026 17:10
Copilot AI review requested due to automatic review settings May 16, 2026 17:10
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR extends gix-index's test suite to run under GIX_TEST_FIXTURE_HASH=sha256 in addition to sha1, as part of the broader SHA-256 transition tracked in issue #281. Generated fixtures are re-built through gix_testtools (which honors the env var via GIT_DEFAULT_HASH), while Loose fixtures (which embed hard-coded SHA-1 bytes) are kept on SHA-1; tests that depend on hash-specific assertions are gated to SHA-1 only.

Changes:

  • Plumb gix_testtools::object_hash() (instead of unconditional gix_hash::Kind::Sha1) through the gix-index test helpers (Fixture::open, odb_at, hex_to_id, direct gix_index::File::at/State::from_bytes callers), with a small SHA-1→SHA-256 hex translation map for fixture-specific hashes.
  • Skip tests that rely on un-regeneratable hard-coded SHA-1 hashes when running under SHA-256 (early return or continue on Loose fixtures and _needs_archive paths).
  • Update the v2_empty.sh fixture to pick the empty-tree hash by GIX_TEST_FIXTURE_HASH, and add the SHA-1+SHA-256 gix-index invocations in justfile. Also a drive-by switch in gix-traverse/tests/traverse/util.rs::odb_at from object_hash_from_env().unwrap_or_default() to object_hash().

Reviewed changes

Copilot reviewed 8 out of 26 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
justfile Run gix-index tests under both sha1 and sha256 fixture hashes.
gix-traverse/tests/traverse/util.rs Use the equivalent object_hash() helper.
gix-index/tests/index/main.rs Hash-aware hex_to_id, new SHA-1↔SHA-256 hash map, new odb_at, and per-variant hash selection in Fixture::open.
gix-index/tests/index/init.rs Use the hash-aware odb_at/object_hash() instead of hard-coded SHA-1.
gix-index/tests/index/file/write.rs Pass object_hash() to State::from_bytes; skip Loose fixtures under SHA-256.
gix-index/tests/index/file/read.rs Pass object_hash() to gix_index::File::at; early-return for SHA-1-only _needs_archive tests.
gix-index/tests/index/file/init.rs Use object_hash(); skip Loose fixtures under SHA-256.
gix-index/tests/fixtures/make_index/v2_empty.sh Pick the empty-tree id based on GIX_TEST_FIXTURE_HASH.
Comments suppressed due to low confidence (3)

gix-index/tests/index/main.rs:50

  • Matching on gix_hash::Kind with a wildcard arm that calls unimplemented!() defeats Rust's exhaustiveness checking: if a new Kind variant is added in the future, this helper will silently compile and only fail at runtime. Consider matching the variants exhaustively (without a wildcard) so the compiler flags missing cases.
    match gix_testtools::object_hash_from_env().unwrap_or_default() {
        gix_hash::Kind::Sha1 => ObjectId::from_hex(hex.as_bytes()).expect("40 bytes hex"),
        gix_hash::Kind::Sha256 => ObjectId::from_hex(
            SHA1_TO_SHA256_HASHES
                .get(hex)
                .unwrap_or_else(|| panic!("SHA-1 {hex} wasn't mapped to SHA-256 yet"))
                .as_bytes(),
        )
        .expect("64 bytes hex"),
        _ => unimplemented!(),
    }

gix-index/tests/index/main.rs:63

  • There is significant duplication between this odb_at helper and the one updated in gix-traverse/tests/traverse/util.rs in this same PR (and there is a similar pattern in other test crates). Consider extracting a shared helper into gix-testtools so that all test crates can use one canonical implementation that respects GIX_TEST_FIXTURE_HASH. This is optional, but it would reduce drift as more crates gain SHA-256 test coverage.
pub fn odb_at(objects_dir: impl Into<PathBuf>) -> Result<gix_odb::Handle> {
    Ok(gix_odb::at_opts(
        objects_dir,
        Vec::new(),
        gix_odb::store::init::Options {
            object_hash: gix_testtools::object_hash(),
            ..Default::default()
        },
    )?)
}

gix-index/tests/index/main.rs:141

  • In Fixture::open(), the Loose arm always hard-codes gix_hash::Kind::Sha1 regardless of GIX_TEST_FIXTURE_HASH. Several test sites (e.g. in gix-index/tests/index/access.rs like dirwalk_api_and_icase_support, entry_by_path_with_conflicting_file, remove_entries, etc.) call Fixture::Loose(...).open() without the SHA-1-only guard that was added in file/write.rs and file/init.rs. Under GIX_TEST_FIXTURE_HASH=sha256, these tests will still run and exercise a SHA-1 File — which is what makes them pass, but it also means the test suite's SHA-256 mode silently degrades coverage of these code paths. It's worth at least leaving a comment near Fixture::open() documenting this intentional behavior so future readers don't try to "fix" it.
    pub fn open(&self) -> gix_index::File {
        let object_hash = match self {
            // Generated fixtures are generated by `gix_testtools`, respecting
            // `GIX_TEST_FIXTURE_HASH`.
            Fixture::Generated(_) => gix_testtools::object_hash(),
            // Loose fixtures are not generated by `gix_testtools`. They contain hard-coded SHA-1
            // hashes.
            Fixture::Loose(_) => gix_hash::Kind::Sha1,
        };
        gix_index::File::at(self.to_path(), object_hash, false, Default::default())
            .expect("fixtures are always readable")
    }

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

/// Convert a hexadecimal hash into its corresponding `ObjectId` or _panic_.
pub fn hex_to_id(hex: &str) -> ObjectId {
ObjectId::from_hex(hex.as_bytes()).expect("40 bytes hex")
match gix_testtools::object_hash_from_env().unwrap_or_default() {
Comment on lines +18 to +33
(
"496d6428b9cf92981dc9495211e6e1120fb6f2ba",
"5f6f307bcc469c02acba4f7da42d8d4defdda8209777fe732956f1e2fa0db3ff",
),
(
"4b825dc642cb6eb9a060e54bf8d69288fbee4904",
"6ef19b41225c5369f1c104d45d8d85efa9b057b53b14b4b9b939dd74decc5321",
),
(
"72d53f787d86a932a25a8537cee236d81846a8f1",
"645f454faab2cb984a331efafb76345242aa1d2029dff13a63935a31752f05ee",
),
(
"e69de29bb2d1d6434b8b29ae775ad8c2e48c5391",
"473a0f4c3be8a93681a267e3b1e9a7dcda1185436fe141f7749120a303721813",
),
Comment on lines +136 to +138
if gix_testtools::object_hash() != gix_hash::Kind::Sha1 {
return;
}
git read-tree 4b825dc642cb6eb9a060e54bf8d69288fbee4904 ;;
sha256)
git read-tree 6ef19b41225c5369f1c104d45d8d85efa9b057b53b14b4b9b939dd74decc5321 ;;
*)
Copy link
Copy Markdown
Contributor

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2d56ceb732

ℹ️ 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".

Comment on lines +30 to +33
let object_hash = if needs_archive {
// Fixtures that need an archive contain hard-coded SHA-1 hashes.
gix_hash::Kind::Sha1
} else {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Respect fixture hash when loading archived index files

When needs_archive is true, try_file() always decodes the index as SHA-1, even though scripted_fixture_read_only_needs_archive() selects hash-specific archives (for example v2_more_files_sha256.tar under GIX_TEST_FIXTURE_HASH=sha256). This makes SHA-256 archived fixtures unreadable or misinterpreted and is why SHA-256 coverage had to be skipped instead of exercised. The helper should pass gix_testtools::object_hash() (or derive from fixture type), not hardcode SHA-1 for all archived fixtures.

Useful? React with 👍 / 👎.

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