Skip to content

feat: add sha256 feature to gix-object #2359

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
cruessler wants to merge 4 commits into
base: main
Choose a base branch
from
+87 −24
Draft

feat: add sha256 feature to gix-object #2359

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
32d7e57
feat: add `sha256` feature to `gix-object`
cruessler Jan 9, 2026
6d950f0
feat: add `Kind::all()`
cruessler Jan 13, 2026
63d82b0
Simplify tests using `Kind::all()`
cruessler Jan 13, 2026
31f45d2
Merge remote-tracking branch 'upstream/main' into add-sha-256-to-gix-…
cruessler Jan 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions gix-object/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ path = "./benches/edit_tree.rs"


[features]
Copy link
Member

@Byron Byron Jan 14, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

I also realise that for docs.rs one can only set features that are defined in the crate itself, which means that we'd always have to repeat the hash features here (and enable one of them for docs.rs) so docs don't break.
On the bright side, this automatically sets everything up to for a non-default SHA1 feature which should follow soon. Maybe it can be implemented so that we also enable at least one hash with a RUSTFLAG, that way CI can easily be tuned to not fail, and to no need adjustments for each and every invocation… heck, this flag could even be set in .cargo/config.toml in this repository.
It feels a bit like I should set that up, but what matters most as that you can keep going exactly like this.

I have also update the tasks here: #281 (comment) - sorry the fuzzyness, the groundworks I should just do in this case.

Previously written

When seeing this, I wonder if we really should go down that route. Albeit having written about it, i.e. having each consumer of gix-hash provide a sha1/sha256 feature, I also remember now that for this very case we have gix-features.
The idea is that at most, consumers pull that in and use it to configure certain basics. gix-hash technically is such a basic now.
Thus we could claim that without hash configuration, which really can only happen in plumbing, there will be a helpful compiler error guiding users to pulling in gix-hash to set up its hash support.

If you don't feel strongly about that, then let's try this? If it's unclear what to do, I could also make the change here, please let me know.

Copy link
Contributor Author

@cruessler cruessler Jan 14, 2026

Choose a reason for hiding this comment

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

I’m not sure I fully understand the relationship between gix-features and feature flags just yet, but regardless, if you want to do some or most parts of this yourself, that’s completely fine with me! You know better what the strategic goals are while I’ve been approaching these tasks more from a tactical point of view so far. The only thing I’d ask for is advance notice on relevant PRs, so I can avoid doing unnecessary work. 😄

Copy link
Member

@Byron Byron Jan 14, 2026

Choose a reason for hiding this comment

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

Yes, I'd also want to avoid waste. To remove some of the insecurity I stirred up here, I think gix-features isn't involved as each downstream crate must have their own feature toggles for hashes. The only reason for this is docs.rs which now must also set the hash (once there is no default hash), and can only do so for the crate it generates docs for.

## Support for SHA256 hashes and digests.
sha256 = ["gix-hash/sha256"]
Copy link
Member

@Byron Byron Jan 14, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

I also realise that the test has to enable all hashes itself, it can do so via gix-hash or via its own crate (we need these on the crate so docs.rs can work).
Also, I feel like I will go hands-on with this soon as this PR may very well set up how we will do it for a ton of other crates, so we should be sure it's the 'right' path 😅.

## Data structures implement `serde::Serialize` and `serde::Deserialize`.
serde = [
"dep:serde",
Expand Down
52 changes: 28 additions & 24 deletions gix-object/tests/object/encode.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,34 +108,38 @@ mod tree {

#[test]
fn write_to_does_not_validate() {
let mut tree = gix_object::Tree::empty();
tree.entries.push(tree::Entry {
mode: EntryKind::Blob.into(),
filename: "".into(),
oid: gix_hash::Kind::Sha1.null(),
});
tree.entries.push(tree::Entry {
mode: EntryKind::Tree.into(),
filename: "something\nwith\newlines\n".into(),
oid: gix_hash::ObjectId::empty_tree(gix_hash::Kind::Sha1),
});
tree.write_to(&mut std::io::sink())
.expect("write succeeds, no validation is performed");
for hash_kind in gix_hash::Kind::all() {
let mut tree = gix_object::Tree::empty();
tree.entries.push(tree::Entry {
mode: EntryKind::Blob.into(),
filename: "".into(),
oid: hash_kind.null(),
});
tree.entries.push(tree::Entry {
mode: EntryKind::Tree.into(),
filename: "something\nwith\newlines\n".into(),
oid: gix_hash::ObjectId::empty_tree(*hash_kind),
});
tree.write_to(&mut std::io::sink())
.expect("write succeeds, no validation is performed");
}
}

#[test]
fn write_to_does_not_allow_separator() {
let mut tree = gix_object::Tree::empty();
tree.entries.push(tree::Entry {
mode: EntryKind::Blob.into(),
filename: "hi\0ho".into(),
oid: gix_hash::Kind::Sha1.null(),
});
let err = tree.write_to(&mut std::io::sink()).unwrap_err();
assert_eq!(
err.to_string(),
r#"Nullbytes are invalid in file paths as they are separators: "hi\0ho""#
);
for hash_kind in gix_hash::Kind::all() {
let mut tree = gix_object::Tree::empty();
tree.entries.push(tree::Entry {
mode: EntryKind::Blob.into(),
filename: "hi\0ho".into(),
oid: hash_kind.null(),
});
let err = tree.write_to(&mut std::io::sink()).unwrap_err();
assert_eq!(
err.to_string(),
r#"Nullbytes are invalid in file paths as they are separators: "hi\0ho""#
);
}
}

round_trip!(gix_object::Tree, gix_object::TreeRef, "tree/everything.tree");
Expand Down
55 changes: 55 additions & 0 deletions gix-object/tests/object/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,20 @@ fn compute_hash() {
);
}

#[test]
#[cfg(feature = "sha256")]
fn compute_hash_sha256() {
let hk = gix_hash::Kind::Sha256;
assert_eq!(
gix_object::compute_hash(hk, gix_object::Kind::Blob, &[]).expect("empty hash doesn’t collide"),
gix_hash::ObjectId::empty_blob(hk)
);
assert_eq!(
gix_object::compute_hash(hk, gix_object::Kind::Tree, &[]).expect("empty hash doesn’t collide"),
gix_hash::ObjectId::empty_tree(hk)
);
}

#[test]
fn compute_stream_hash() {
let hk = gix_hash::Kind::Sha1;
Expand Down Expand Up @@ -50,6 +64,36 @@ fn compute_stream_hash() {
);
}

#[test]
#[cfg(feature = "sha256")]
fn compute_stream_hash_sha256() {
let hk = gix_hash::Kind::Sha256;
assert_eq!(
gix_object::compute_stream_hash(
hk,
gix_object::Kind::Blob,
&mut &[][..],
0,
&mut gix_features::progress::Discard,
&AtomicBool::default()
)
.expect("in-memory works"),
gix_hash::ObjectId::empty_blob(hk)
);
assert_eq!(
gix_object::compute_stream_hash(
hk,
gix_object::Kind::Tree,
&mut &[][..],
0,
&mut gix_features::progress::Discard,
&AtomicBool::default()
)
.expect("in-memory works"),
gix_hash::ObjectId::empty_tree(hk)
);
}

use gix_testtools::Result;

#[cfg(not(windows))]
Expand Down Expand Up @@ -78,6 +122,7 @@ fn fixture_name(kind: &str, path: &str) -> Vec<u8> {
}

#[test]
#[cfg(not(feature = "sha256"))]
fn size_in_memory() {
let actual = std::mem::size_of::<gix_object::Object>();
assert!(
Expand All @@ -86,6 +131,16 @@ fn size_in_memory() {
);
}

#[test]
#[cfg(feature = "sha256")]
fn size_in_memory() {
let actual = std::mem::size_of::<gix_object::Object>();
assert!(
actual <= 288,
"{actual} <= 288: Prevent unexpected growth of what should be lightweight objects"
);
}

fn hex_to_id(hex: &str) -> ObjectId {
ObjectId::from_hex(hex.as_bytes()).expect("40 bytes hex")
}
Expand Down
2 changes: 2 additions & 0 deletions justfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ check:
cargo check -p gix-hash --no-default-features --features sha256
cargo check -p gix-object --all-features
cargo check -p gix-object --features verbose-object-parsing-errors
cargo check -p gix-object --features sha256
cargo check -p gix-attributes --features serde
cargo check -p gix-glob --features serde
cargo check -p gix-worktree --features serde
Expand Down Expand Up @@ -162,6 +163,7 @@ unit-tests:
cargo nextest run -p gix-hash --no-default-features --features sha256 --no-fail-fast # TODO: make this actually work by removing 'sha1' from default features.
cargo nextest run -p gix-object --no-fail-fast
cargo nextest run -p gix-object --features verbose-object-parsing-errors --no-fail-fast
cargo nextest run -p gix-object --features sha256 --no-fail-fast
cargo nextest run -p gix-tempfile --features signals --no-fail-fast
cargo nextest run -p gix-features --all-features --no-fail-fast
cargo nextest run -p gix-ref-tests --all-features --no-fail-fast
Expand Down
Loading