diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 32d6177c148..9d0bc384102 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -198,6 +198,42 @@ jobs: test-32bit: runs-on: ubuntu-latest + container: i386/debian:stable-slim + + steps: + - name: Prerequisites + run: | + prerequisites=( + build-essential + ca-certificates + cmake + curl + git + jq + libssl-dev + libstdc++6:amd64 # To support external 64-bit Node.js for actions. + pkgconf + ) + dpkg --add-architecture amd64 + apt-get update + apt-get install --no-install-recommends -y -- "${prerequisites[@]}" + shell: bash + - uses: actions/checkout@v4 + - uses: dtolnay/rust-toolchain@stable + with: + toolchain: stable-i686-unknown-linux-gnu # Otherwise it may misdetect based on the amd64 kernel. + - uses: Swatinem/rust-cache@v2 + - uses: taiki-e/install-action@v2 + with: + tool: nextest + - name: Make `system` scope nonempty for "GitInstallation" tests + run: git config --system gitoxide.imaginary.arbitraryVariable arbitraryValue + - name: Test (nextest) + run: cargo nextest run --workspace --no-fail-fast + + test-32bit-cross: + runs-on: ubuntu-latest + strategy: matrix: target: [ armv7-linux-androideabi ] @@ -211,15 +247,17 @@ jobs: with: toolchain: stable targets: ${{ matrix.target }} - - uses: taiki-e/install-action@v2 + - name: Install cross + uses: taiki-e/install-action@v2 with: tool: cross - name: check run: cross check -p gix --target ${{ matrix.target }} - name: Test (unit) - # run high-level unit tests that exercise a lot of code while being pure Rust to ease building test binaries. - # TODO: figure out why `git` doesn't pick up environment configuration so build scripts fail when using `-p gix`. - run: cross test -p gix-hashtable --target ${{ matrix.target }} + run: | + # Run some high-level unit tests that exercise various pure Rust code to ease building test binaries. + # We would prefer `-p gix`. But with `cross`, fixture scripts try to run amd64 `git` as an armv7 binary. + cross test -p gix-hashtable --target ${{ matrix.target }} lint: runs-on: ubuntu-latest @@ -396,6 +434,7 @@ jobs: - test-journey - test-fast - test-32bit + - test-32bit-cross - lint - cargo-deny - check-packetline diff --git a/Cargo.lock b/Cargo.lock index a28e647dd7e..dcfc0729ce5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2027,6 +2027,7 @@ dependencies = [ "gix-hash 0.15.0", "gix-lock 15.0.0", "gix-object 0.45.0", + "gix-testtools", "gix-traverse 0.42.0", "gix-utils 0.1.13", "gix-validate 0.9.1", @@ -2535,6 +2536,7 @@ dependencies = [ "gix-hash 0.15.0", "gix-hashtable 0.6.0", "gix-object 0.45.0", + "gix-testtools", "smallvec", "thiserror 2.0.3", ] diff --git a/gix-archive/tests/archive.rs b/gix-archive/tests/archive.rs index 9ffc9bf60ad..b2310075588 100644 --- a/gix-archive/tests/archive.rs +++ b/gix-archive/tests/archive.rs @@ -14,10 +14,15 @@ mod from_tree { use crate::hex_to_id; + #[cfg(target_pointer_width = "64")] + const EXPECTED_BUFFER_LENGTH: usize = 551; + #[cfg(target_pointer_width = "32")] + const EXPECTED_BUFFER_LENGTH: usize = 479; + #[test] fn basic_usage_internal() -> gix_testtools::Result { basic_usage(gix_archive::Format::InternalTransientNonPersistable, |buf| { - assert_eq!(buf.len(), 551); + assert_eq!(buf.len(), EXPECTED_BUFFER_LENGTH); let mut stream = gix_worktree_stream::Stream::from_read(std::io::Cursor::new(buf)); let mut paths_and_modes = Vec::new(); diff --git a/gix-attributes/tests/search/mod.rs b/gix-attributes/tests/search/mod.rs index c9b33faae10..59690fa8ec5 100644 --- a/gix-attributes/tests/search/mod.rs +++ b/gix-attributes/tests/search/mod.rs @@ -6,6 +6,7 @@ use gix_attributes::{ AssignmentRef, NameRef, StateRef, }; use gix_glob::pattern::Case; +use gix_testtools::size_ok; mod specials { use std::path::Path; @@ -268,10 +269,11 @@ fn given_attributes_are_made_available_in_given_order() -> crate::Result { #[test] fn size_of_outcome() { - assert_eq!( - std::mem::size_of::(), - 840, - "it's quite big, shouldn't change without us noticing" + let actual = std::mem::size_of::(); + let expected = 840; + assert!( + size_ok(actual, expected), + "it's quite big, shouldn't change without us noticing: {actual} <~ {expected}" ); } diff --git a/gix-index/Cargo.toml b/gix-index/Cargo.toml index 77cf8f8460a..29f72f7a61f 100644 --- a/gix-index/Cargo.toml +++ b/gix-index/Cargo.toml @@ -58,5 +58,8 @@ rustix = { version = "0.38.20", default-features = false, features = [ ] } libc = { version = "0.2.149" } +[dev-dependencies] +gix-testtools = { path = "../tests/tools" } + [package.metadata.docs.rs] features = ["document-features", "serde"] diff --git a/gix-index/src/extension/tree/mod.rs b/gix-index/src/extension/tree/mod.rs index f2075939978..5e68ff7197f 100644 --- a/gix-index/src/extension/tree/mod.rs +++ b/gix-index/src/extension/tree/mod.rs @@ -13,9 +13,15 @@ mod write; #[cfg(test)] mod tests { + use gix_testtools::size_ok; #[test] fn size_of_tree() { - assert_eq!(std::mem::size_of::(), 88); + let actual = std::mem::size_of::(); + let expected = 88; + assert!( + size_ok(actual, expected), + "the size of this structure should not change unexpectedly: {actual} <~ {expected}" + ); } } diff --git a/gix-index/src/lib.rs b/gix-index/src/lib.rs index de0b17eadd3..a9d853f6960 100644 --- a/gix-index/src/lib.rs +++ b/gix-index/src/lib.rs @@ -237,12 +237,3 @@ pub(crate) mod util { data.split_at(pos).into() } } - -#[test] -fn size_of_entry() { - assert_eq!(std::mem::size_of::(), 80); - - // the reason we have our own time is half the size. - assert_eq!(std::mem::size_of::(), 8); - assert_eq!(std::mem::size_of::(), 16); -} diff --git a/gix-index/tests/index/mod.rs b/gix-index/tests/index/mod.rs index e6606f16346..1878edcaa1f 100644 --- a/gix-index/tests/index/mod.rs +++ b/gix-index/tests/index/mod.rs @@ -1,13 +1,14 @@ -use std::path::{Path, PathBuf}; - -use gix_hash::ObjectId; - mod access; mod entry; mod file; mod fs; mod init; +use std::path::{Path, PathBuf}; + +use gix_hash::ObjectId; +use gix_testtools::size_ok; + pub fn hex_to_id(hex: &str) -> ObjectId { ObjectId::from_hex(hex.as_bytes()).expect("40 bytes hex") } @@ -25,11 +26,29 @@ pub fn loose_file_path(name: &str) -> PathBuf { #[test] fn size_of_entry() { - assert_eq!(std::mem::size_of::(), 80); + let actual = std::mem::size_of::(); + let expected = 80; + assert!( + size_ok(actual, expected), + "the size of this structure should not change unexpectedly: {actual} <~ {expected}" + ); +} - // the reason we have our own time is half the size. - assert_eq!(std::mem::size_of::(), 8); - assert_eq!(std::mem::size_of::(), 16); +#[test] +fn size_of_entry_time() { + // The reason we have our own time is that it is half the size. + let ent_actual = std::mem::size_of::(); + let ent_expected = 8; + assert!( + size_ok(ent_actual, ent_expected), + "the size of this structure should not change unexpectedly: {ent_actual} <~ {ent_expected}" + ); + let ft_actual = std::mem::size_of::(); + let ft_expected = 16; + assert!( + size_ok(ft_actual, ft_expected), + "we will want to know if the size of this structure changes: {ft_actual} <~ {ft_expected}" + ); } enum Fixture { diff --git a/gix-negotiate/tests/negotiate.rs b/gix-negotiate/tests/negotiate.rs index bb56d06bf06..9410308e39f 100644 --- a/gix-negotiate/tests/negotiate.rs +++ b/gix-negotiate/tests/negotiate.rs @@ -1,4 +1,4 @@ -use gix_testtools::Result; +use gix_testtools::{size_ok, Result}; mod window_size { use gix_negotiate::window_size; @@ -38,9 +38,10 @@ mod baseline; #[test] fn size_of_entry() { - assert_eq!( - std::mem::size_of::>(), - 56, - "we may keep a lot of these, so let's not let them grow unnoticed" + let actual = std::mem::size_of::>(); + let expected = 56; + assert!( + size_ok(actual, expected), + "we may keep a lot of these, so let's not let them grow unnoticed: {actual} <~ {expected}" ); } diff --git a/gix-pack/src/cache/delta/mod.rs b/gix-pack/src/cache/delta/mod.rs index c923b5d1078..09dc1c3fcd1 100644 --- a/gix-pack/src/cache/delta/mod.rs +++ b/gix-pack/src/cache/delta/mod.rs @@ -22,29 +22,3 @@ pub mod from_offsets; mod tree; pub use tree::{Item, Tree}; - -#[cfg(test)] -mod tests { - - #[test] - fn size_of_pack_tree_item() { - use super::Item; - assert_eq!(std::mem::size_of::<[Item<()>; 7_500_000]>(), 300_000_000); - } - - #[test] - fn size_of_pack_verify_data_structure() { - use super::Item; - pub struct EntryWithDefault { - _index_entry: crate::index::Entry, - _kind: gix_object::Kind, - _object_size: u64, - _decompressed_size: u64, - _compressed_size: u64, - _header_size: u16, - _level: u16, - } - - assert_eq!(std::mem::size_of::<[Item; 7_500_000]>(), 840_000_000); - } -} diff --git a/gix-pack/src/cache/delta/tree.rs b/gix-pack/src/cache/delta/tree.rs index f26cf5a0564..cf26f5440ea 100644 --- a/gix-pack/src/cache/delta/tree.rs +++ b/gix-pack/src/cache/delta/tree.rs @@ -226,25 +226,38 @@ mod tests { } } - #[test] - fn size_of_pack_tree_item() { - use super::Item; - assert_eq!(std::mem::size_of::<[Item<()>; 7_500_000]>(), 300_000_000); - } + mod size { + use super::super::Item; + use gix_testtools::size_ok; - #[test] - fn size_of_pack_verify_data_structure() { - use super::Item; - pub struct EntryWithDefault { - _index_entry: crate::index::Entry, - _kind: gix_object::Kind, - _object_size: u64, - _decompressed_size: u64, - _compressed_size: u64, - _header_size: u16, - _level: u16, + #[test] + fn size_of_pack_tree_item() { + let actual = std::mem::size_of::<[Item<()>; 7_500_000]>(); + let expected = 300_000_000; + assert!( + size_ok(actual, expected), + "we don't want these to grow unnoticed: {actual} <~ {expected}" + ); } - assert_eq!(std::mem::size_of::<[Item; 7_500_000]>(), 840_000_000); + #[test] + fn size_of_pack_verify_data_structure() { + pub struct EntryWithDefault { + _index_entry: crate::index::Entry, + _kind: gix_object::Kind, + _object_size: u64, + _decompressed_size: u64, + _compressed_size: u64, + _header_size: u16, + _level: u16, + } + + let actual = std::mem::size_of::<[Item; 7_500_000]>(); + let expected = 840_000_000; + assert!( + size_ok(actual, expected), + "we don't want these to grow unnoticed: {actual} <~ {expected}" + ); + } } } diff --git a/gix-pack/src/data/file/decode/entry.rs b/gix-pack/src/data/file/decode/entry.rs index 5613d70ee74..a3248b30ab5 100644 --- a/gix-pack/src/data/file/decode/entry.rs +++ b/gix-pack/src/data/file/decode/entry.rs @@ -420,13 +420,15 @@ impl File { #[cfg(test)] mod tests { use super::*; + use gix_testtools::size_ok; #[test] fn size_of_decode_entry_outcome() { - assert_eq!( - std::mem::size_of::(), - 32, - "this shouldn't change without use noticing as it's returned a lot" + let actual = std::mem::size_of::(); + let expected = 32; + assert!( + size_ok(actual, expected), + "this shouldn't change without use noticing as it's returned a lot: {actual} <~ {expected}" ); } } diff --git a/gix-pack/tests/pack/data/output/mod.rs b/gix-pack/tests/pack/data/output/mod.rs index ea173b4317c..59e80d2b303 100644 --- a/gix-pack/tests/pack/data/output/mod.rs +++ b/gix-pack/tests/pack/data/output/mod.rs @@ -1,22 +1,25 @@ use std::{path::PathBuf, sync::Arc}; use gix_pack::data::output; +use gix_testtools::size_ok; #[test] fn size_of_entry() { - assert_eq!( - std::mem::size_of::(), - 80, - "The size of the structure shouldn't change unexpectedly" + let actual = std::mem::size_of::(); + let expected = 80; + assert!( + size_ok(actual, expected), + "The size of the structure shouldn't change unexpectedly: {actual} <~ {expected}" ); } #[test] fn size_of_count() { - assert_eq!( - std::mem::size_of::(), - 56, - "The size of the structure shouldn't change unexpectedly" + let actual = std::mem::size_of::(); + let expected = 56; + assert!( + size_ok(actual, expected), + "The size of the structure shouldn't change unexpectedly: {actual} <~ {expected}" ); } diff --git a/gix-pack/tests/pack/iter.rs b/gix-pack/tests/pack/iter.rs index c9dd7d445c6..4b7b0cb32f3 100644 --- a/gix-pack/tests/pack/iter.rs +++ b/gix-pack/tests/pack/iter.rs @@ -1,11 +1,13 @@ use gix_odb::pack; +use gix_testtools::size_ok; #[test] fn size_of_entry() { - assert_eq!( - std::mem::size_of::(), - 104, - "let's keep the size in check as we have many of them" + let actual = std::mem::size_of::(); + let expected = 104; + assert!( + size_ok(actual, expected), + "let's keep the size in check as we have many of them: {actual} <~ {expected}" ); } diff --git a/gix-ref/src/raw.rs b/gix-ref/src/raw.rs index 34eb6e814dc..e6da3201bc9 100644 --- a/gix-ref/src/raw.rs +++ b/gix-ref/src/raw.rs @@ -94,13 +94,15 @@ mod access { #[cfg(test)] mod tests { use super::*; + use gix_testtools::size_ok; #[test] fn size_of_reference() { - assert_eq!( - std::mem::size_of::(), - 80, - "let's not let it change size undetected" + let actual = std::mem::size_of::(); + let expected = 80; + assert!( + size_ok(actual, expected), + "let's not let it change size undetected: {actual} <~ {expected}" ); } } diff --git a/gix-revwalk/Cargo.toml b/gix-revwalk/Cargo.toml index 76d5fc9443d..0a89f82ed84 100644 --- a/gix-revwalk/Cargo.toml +++ b/gix-revwalk/Cargo.toml @@ -23,3 +23,6 @@ gix-commitgraph = { version = "^0.25.0", path = "../gix-commitgraph" } thiserror = "2.0.0" smallvec = "1.10.0" + +[dev-dependencies] +gix-testtools = { path = "../tests/tools" } diff --git a/gix-revwalk/tests/revwalk.rs b/gix-revwalk/tests/revwalk.rs index 061329f9348..371eee9a781 100644 --- a/gix-revwalk/tests/revwalk.rs +++ b/gix-revwalk/tests/revwalk.rs @@ -1,11 +1,14 @@ mod graph { mod commit { + use gix_testtools::size_ok; + #[test] fn size_of_commit() { - assert_eq!( - std::mem::size_of::>(), - 48, - "We might see quite a lot of these, so they shouldn't grow unexpectedly" + let actual = std::mem::size_of::>(); + let expected = 48; + assert!( + size_ok(actual, expected), + "We might see quite a lot of these, so they shouldn't grow unexpectedly: {actual} <~ {expected}" ); } } diff --git a/gix-url/tests/fixtures/make_baseline.sh b/gix-url/tests/fixtures/make_baseline.sh index 8419578150a..5c7d4ff00ec 100755 --- a/gix-url/tests/fixtures/make_baseline.sh +++ b/gix-url/tests/fixtures/make_baseline.sh @@ -55,14 +55,21 @@ tests_windows+=("c:repo") tests_unix+=("${tests[@]}") tests_windows+=("${tests[@]}") +# We will run `git fetch-pack` in this repo instead of the outer gitoxide repo, +# for full isolation. This avoids assuming there *is* a gitoxide repo, and also +# avoids `safe.directory` errors if the gitoxide repo has unusual ownership. +git init -q temp-repo + for url in "${tests_unix[@]}" do echo ";" # there are no `;` in the tested urls - git fetch-pack --diag-url "$url" + git -C temp-repo fetch-pack --diag-url "$url" done >git-baseline.unix for url in "${tests_windows[@]}" do echo ";" # there are no `;` in the tested urls - git fetch-pack --diag-url "$url" + git -C temp-repo fetch-pack --diag-url "$url" done >git-baseline.windows + +rm -rf temp-repo diff --git a/gix-worktree-stream/tests/stream.rs b/gix-worktree-stream/tests/stream.rs index f66e24698f2..46551193ff7 100644 --- a/gix-worktree-stream/tests/stream.rs +++ b/gix-worktree-stream/tests/stream.rs @@ -58,6 +58,11 @@ mod from_tree { Ok(()) } + #[cfg(target_pointer_width = "64")] + const EXPECTED_BUFFER_LENGTH: usize = 320302; + #[cfg(target_pointer_width = "32")] + const EXPECTED_BUFFER_LENGTH: usize = 320198; + #[test] fn will_provide_all_information_and_respect_export_ignore() -> gix_testtools::Result { let (dir, head_tree, odb, mut cache) = basic()?; @@ -186,7 +191,7 @@ mod from_tree { ); assert_eq!( copy.lock().len(), - 320302, + EXPECTED_BUFFER_LENGTH, "keep track of file size changes of the streaming format" ); diff --git a/gix/tests/gix/object/mod.rs b/gix/tests/gix/object/mod.rs index ebb076cb618..8dd1aedaae5 100644 --- a/gix/tests/gix/object/mod.rs +++ b/gix/tests/gix/object/mod.rs @@ -2,20 +2,24 @@ mod blob; mod commit; mod tree; +use gix_testtools::size_ok; + #[test] fn object_ref_size_in_memory() { - assert_eq!( - std::mem::size_of::>(), - 56, - "the size of this structure should not changed unexpectedly" + let actual = std::mem::size_of::>(); + let expected = 56; + assert!( + size_ok(actual, expected), + "the size of this structure should not change unexpectedly: {actual} <~ {expected}" ); } #[test] fn oid_size_in_memory() { - assert_eq!( - std::mem::size_of::>(), - 32, - "the size of this structure should not changed unexpectedly" + let actual = std::mem::size_of::>(); + let expected = 32; + assert!( + size_ok(actual, expected), + "the size of this structure should not change unexpectedly: {actual} <~ {expected}" ); } diff --git a/gix/tests/gix/repository/worktree.rs b/gix/tests/gix/repository/worktree.rs index 4c66b452b97..b39db66b9f7 100644 --- a/gix/tests/gix/repository/worktree.rs +++ b/gix/tests/gix/repository/worktree.rs @@ -1,5 +1,10 @@ use gix_ref::bstr; +#[cfg(target_pointer_width = "64")] +const EXPECTED_BUFFER_LENGTH: usize = 102; +#[cfg(target_pointer_width = "32")] +const EXPECTED_BUFFER_LENGTH: usize = 86; + #[test] #[cfg(feature = "worktree-stream")] fn stream() -> crate::Result { @@ -7,7 +12,7 @@ fn stream() -> crate::Result { let mut stream = repo.worktree_stream(repo.head_commit()?.tree_id()?)?.0.into_read(); assert_eq!( std::io::copy(&mut stream, &mut std::io::sink())?, - 102, + EXPECTED_BUFFER_LENGTH as u64, "there is some content in the stream, it works" ); Ok(()) @@ -27,7 +32,7 @@ fn archive() -> crate::Result { &std::sync::atomic::AtomicBool::default(), Default::default(), )?; - assert_eq!(buf.len(), 102, "default format is internal"); + assert_eq!(buf.len(), EXPECTED_BUFFER_LENGTH, "default format is internal"); Ok(()) } diff --git a/gix/tests/gix/status.rs b/gix/tests/gix/status.rs index 32d4f5e903f..dad2eb188e7 100644 --- a/gix/tests/gix/status.rs +++ b/gix/tests/gix/status.rs @@ -20,14 +20,16 @@ mod index_worktree { mod iter { use crate::status::{repo, submodule_repo}; use gix::status::index_worktree::iter::Item; + use gix_testtools::size_ok; use pretty_assertions::assert_eq; #[test] fn item_size() { - assert_eq!( - std::mem::size_of::(), - 264, - "The size is pretty huge and goes down ideally" + let actual = std::mem::size_of::(); + let expected = 264; + assert!( + size_ok(actual, expected), + "The size is pretty huge and goes down ideally: {actual} <~ {expected}" ); } diff --git a/justfile b/justfile index 8a80f5370e8..6323a0670da 100755 --- a/justfile +++ b/justfile @@ -84,7 +84,7 @@ check: cargo check -p gix-pack --features object-cache-dynamic cargo check -p gix-packetline --features blocking-io cargo check -p gix-packetline --features async-io - cargo check -p gix-index --features serde + cd gix-index && cargo check --features serde cargo check -p gix-credentials --features serde cargo check -p gix-sec --features serde cargo check -p gix-revision --features serde diff --git a/tests/tools/src/lib.rs b/tests/tools/src/lib.rs index a17c1aab8e8..27fb06d5006 100644 --- a/tests/tools/src/lib.rs +++ b/tests/tools/src/lib.rs @@ -882,6 +882,30 @@ impl Drop for Env<'_> { } } +/// Check data structure size, comparing strictly on 64-bit targets. +/// +/// - On 32-bit targets, checks if `actual_size` is at most `expected_64_bit_size`. +/// - On 64-bit targets, checks if `actual_size` is exactly `expected_64_bit_size`. +/// +/// This is for assertions about the size of data structures, when the goal is to keep them from +/// growing too large even across breaking changes. Such assertions must always fail when data +/// structures grow larger than they have ever been, for which `<=` is enough. But it also helps to +/// know when they have shrunk unexpectedly. They may shrink, other changes may rely on the smaller +/// size for acceptable performance, and then they may grow again to their earlier size. +/// +/// The problem with `==` is that data structures are often smaller on 32-bit targets. This could +/// be addressed by asserting separate exact 64-bit and 32-bit sizes. But sizes may also differ +/// across 32-bit targets, due to ABI and layout/packing details. That can happen across 64-bit +/// targets too, but it seems less common. +/// +/// For those reasons, this function does a `==` on 64-bit targets, but a `<=` on 32-bit targets. +pub fn size_ok(actual_size: usize, expected_64_bit_size: usize) -> bool { + #[cfg(target_pointer_width = "64")] + return actual_size == expected_64_bit_size; + #[cfg(target_pointer_width = "32")] + return actual_size <= expected_64_bit_size; +} + #[cfg(test)] mod tests { use super::*;