From d1487bd04467c29791612c19755253c5ae07c713 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Sat, 5 Apr 2025 13:08:09 -0400 Subject: [PATCH 1/4] Test 32-bit Windows build Since it is still running in a 64-bit environment, and running the whole test suite is slow on Windows, this will likely not be worth keeping in full. It may subsequently be modified to run only some restricted subset of the tests, so that it completes faster. --- .github/workflows/ci.yml | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c1df7e2d27f..2a713c36057 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -309,6 +309,26 @@ jobs: GIX_TEST_IGNORE_ARCHIVES: '1' run: cargo nextest run --workspace --no-fail-fast + test-32bit-windows: + runs-on: windows-latest + + env: + TARGET: i686-pc-windows-msvc + + steps: + - uses: actions/checkout@v4 + - uses: dtolnay/rust-toolchain@stable + with: + targets: ${{ env.TARGET }} + - uses: Swatinem/rust-cache@v2 + - name: cargo check default features + run: cargo check --target $env:TARGET --workspace --bins --examples + - uses: taiki-e/install-action@v2 + with: + tool: nextest + - name: Test (nextest) + run: cargo nextest run --target $env:TARGET --workspace --no-fail-fast + lint: runs-on: ubuntu-latest @@ -504,6 +524,7 @@ jobs: - test-fast - test-fixtures-windows - test-32bit + - test-32bit-windows - lint - cargo-deny - check-packetline From cb45d896222d1f1581dba3e58684116f3055c7b7 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Sat, 5 Apr 2025 13:54:30 -0400 Subject: [PATCH 2/4] Try building with the 32-bit toolchain To see if it produces build failures or new test failures on CI. --- .github/workflows/ci.yml | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2a713c36057..3d9e28e96dd 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -312,22 +312,19 @@ jobs: test-32bit-windows: runs-on: windows-latest - env: - TARGET: i686-pc-windows-msvc - steps: - uses: actions/checkout@v4 - - uses: dtolnay/rust-toolchain@stable + - uses: dtolnay/rust-toolchain@master with: - targets: ${{ env.TARGET }} + toolchain: stable-i686-pc-windows-msvc - uses: Swatinem/rust-cache@v2 - name: cargo check default features - run: cargo check --target $env:TARGET --workspace --bins --examples + run: cargo check --workspace --bins --examples - uses: taiki-e/install-action@v2 with: tool: nextest - name: Test (nextest) - run: cargo nextest run --target $env:TARGET --workspace --no-fail-fast + run: cargo nextest run --workspace --no-fail-fast lint: runs-on: ubuntu-latest From cfb9bea4d9d42efa70bc0d3e4139f04e354a0d08 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Sat, 5 Apr 2025 14:24:00 -0400 Subject: [PATCH 3/4] Go back to cross compiling and only run size tests - Use platform-default Windows toolchain with 32-bit target again. - Remove the "cargo check default features" step. - Filter the tests so only those with "size" in their name are run. - Rename job from to `test-32bit-windows-size`. --- .github/workflows/ci.yml | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3d9e28e96dd..f80297f737c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -309,22 +309,23 @@ jobs: GIX_TEST_IGNORE_ARCHIVES: '1' run: cargo nextest run --workspace --no-fail-fast - test-32bit-windows: + test-32bit-windows-size: runs-on: windows-latest + env: + TARGET: i686-pc-windows-msvc + steps: - uses: actions/checkout@v4 - - uses: dtolnay/rust-toolchain@master + - uses: dtolnay/rust-toolchain@stable with: - toolchain: stable-i686-pc-windows-msvc + targets: ${{ env.TARGET }} - uses: Swatinem/rust-cache@v2 - - name: cargo check default features - run: cargo check --workspace --bins --examples - uses: taiki-e/install-action@v2 with: tool: nextest - name: Test (nextest) - run: cargo nextest run --workspace --no-fail-fast + run: cargo nextest run --target $env:TARGET --workspace --no-fail-fast size lint: runs-on: ubuntu-latest @@ -521,7 +522,7 @@ jobs: - test-fast - test-fixtures-windows - test-32bit - - test-32bit-windows + - test-32bit-windows-size - lint - cargo-deny - check-packetline From 4cea38bb093717b63cacb910c49e57840d07a1de Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Sat, 5 Apr 2025 15:13:42 -0400 Subject: [PATCH 4/4] Fix `size_of_hasher` test for 32-bit Windows Due to ABI differences between different 32-bit targets, the `size_of_hasher` test wrongly failed on `i686-pc-windows-msvc`. Although the test case with that name was introduced in #1915, the failure is actually long-standing, in that an analogous faiure occurred in the old `size_of_sha1` test that preceded it and on which it is based. That failure only happened when the old `fast-sha1` feature was enabled, and not with the old `rustsha1` feature. It was not detected earlier as that target is irregularly tested, and built with `--no-default-features --features max-pure` more often than other targets due to difficulties building some other non-Rust dependencies on it (when not cross-compiling). Since #1915, the failure is easier to detect, since we now use only one SHA-1 implementation, `sha1-checked`, so the test always fails on `i686-pc-windows-msvc`. This is only a bug in the test itself, not in any of the code under test. This commit changes the test to use `gix_testtools::size_ok`, which makes a `==` comparison on 64-bit targets but a `<=` comparison on 32-bit targets where there tends to be more variation in data structures' sizes. This is similar to some of the size assertion fixes in #1687 (77c3c59, fc13fc3). --- gix-hash/tests/hash/hasher.rs | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/gix-hash/tests/hash/hasher.rs b/gix-hash/tests/hash/hasher.rs index 282b7c8666f..ee73631d69c 100644 --- a/gix-hash/tests/hash/hasher.rs +++ b/gix-hash/tests/hash/hasher.rs @@ -1,12 +1,15 @@ use gix_hash::{Hasher, ObjectId}; +use gix_testtools::size_ok; #[test] fn size_of_hasher() { - assert_eq!( - std::mem::size_of::(), - if cfg!(target_arch = "x86") { 820 } else { 824 }, - "The size of this type may be relevant when hashing millions of objects,\ - and shouldn't change unnoticed. The DetectionState alone clocks in at 724 bytes." + let actual = std::mem::size_of::(); + let expected = 824; + assert!( + size_ok(actual, expected), + "The size of this type may be relevant when hashing millions of objects, and shouldn't\ + change unnoticed: {actual} <~ {expected}\ + (The DetectionState alone clocked in at 724 bytes when last examined.)" ); }