From 8ea7501400ba97ab49d5df00ceb5cc18f7d2a5d6 Mon Sep 17 00:00:00 2001
From: Eliah Kagan <degeneracypressure@gmail.com>
Date: Tue, 12 Nov 2024 15:36:22 -0500
Subject: [PATCH 01/21] Revert "Remove 32bit platform that seemingly fails due
 to libc updates"

This reverts commit c8c25c12912d8e2f301a9e56ff410f45b944c57c.
---
 .github/workflows/ci.yml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index 32d6177c148..889181bc854 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -200,7 +200,7 @@ jobs:
 
     strategy:
       matrix:
-        target: [ armv7-linux-androideabi ]
+        target: [ i686-unknown-linux-gnu, armv7-linux-androideabi ]
 
     steps:
       - uses: actions/checkout@v4

From a78e8f2932196d82efaa2b36fc8e00c9d0b329bd Mon Sep 17 00:00:00 2001
From: Eliah Kagan <degeneracypressure@gmail.com>
Date: Tue, 12 Nov 2024 15:42:52 -0500
Subject: [PATCH 02/21] Try i686-unknown-linux-musl

Instead of i686-unknown-linux-gnu.
---
 .github/workflows/ci.yml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index 889181bc854..51449eb9804 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -200,7 +200,7 @@ jobs:
 
     strategy:
       matrix:
-        target: [ i686-unknown-linux-gnu, armv7-linux-androideabi ]
+        target: [ i686-unknown-linux-musl, armv7-linux-androideabi ]
 
     steps:
       - uses: actions/checkout@v4

From 811dc6dece392b894d8ba48faad6d696fc5b83d7 Mon Sep 17 00:00:00 2001
From: Eliah Kagan <degeneracypressure@gmail.com>
Date: Tue, 12 Nov 2024 15:56:09 -0500
Subject: [PATCH 03/21] Don't use `cross` for the i686 target

---
 .github/workflows/ci.yml | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index 51449eb9804..e3a7f8d481b 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -200,7 +200,14 @@ jobs:
 
     strategy:
       matrix:
-        target: [ i686-unknown-linux-musl, armv7-linux-androideabi ]
+        target:
+          - i686-unknown-linux-musl
+          - armv7-linux-androideabi
+        include:
+          - target: i686-unknown-linux-musl
+            cargo: cargo
+          - target: armv7-linux-androideabi
+            cargo: cross
 
     steps:
       - uses: actions/checkout@v4
@@ -211,15 +218,17 @@ jobs:
         with:
           toolchain: stable
           targets: ${{ matrix.target }}
-      - uses: taiki-e/install-action@v2
+      - name: Install cross
+        if: matrix.cargo == 'cross'
+        uses: taiki-e/install-action@v2
         with:
           tool: cross
       - name: check
-        run: cross check -p gix --target ${{ matrix.target }}
+        run: ${{ matrix.cargo }} 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: ${{ matrix.cargo }} test -p gix-hashtable --target ${{ matrix.target }}
 
   lint:
     runs-on: ubuntu-latest

From bb06629cd0f840842493957fdc17d6e19c8aea46 Mon Sep 17 00:00:00 2001
From: Eliah Kagan <degeneracypressure@gmail.com>
Date: Tue, 12 Nov 2024 17:02:46 -0500
Subject: [PATCH 04/21] See what happens with `-p gix` for the test command

---
 .github/workflows/ci.yml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index e3a7f8d481b..86d7ea3e890 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -228,7 +228,7 @@ jobs:
       - 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: ${{ matrix.cargo }} test -p gix-hashtable --target ${{ matrix.target }}
+        run: ${{ matrix.cargo }} test -p gix --target ${{ matrix.target }}
 
   lint:
     runs-on: ubuntu-latest

From 6771838806830a4726aeade21c621dcf4449c004 Mon Sep 17 00:00:00 2001
From: Eliah Kagan <degeneracypressure@gmail.com>
Date: Tue, 12 Nov 2024 17:13:23 -0500
Subject: [PATCH 05/21] Temporarily keep i686 job failure from canceling armv7
 job

`test-32bit` fails in the job for the `i686-unknown-linux-musl`
target since changing `-p gix-hashtable` to `-p gix` in the test
step because various data structure size assertions fail, which is
because some data structures are smaller on 32-bit archictectures
than on 64-bit architectures.

That is unrelated to the commented problem of environment not being
passed into the test environment, which does not happen on
`i686-unknown-linux-musl`. But maybe it would happen on
`armv7-linux-androideabi`, due to its use of cross.

The i686 job, which doesn't require emulation, is faster, and when
it fails due to those assertions, it keeps the armv7 job from
getting far enough to reveal if it has its own environment-related
(or other) failures. This temporarily sets `fail-fast: false` so
that the armv7 job can get far enough that we find out.
---
 .github/workflows/ci.yml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index 86d7ea3e890..c8fc14749f2 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -208,6 +208,7 @@ jobs:
             cargo: cargo
           - target: armv7-linux-androideabi
             cargo: cross
+      fail-fast: false  # FIXME: Remove this after testing.
 
     steps:
       - uses: actions/checkout@v4

From e94526e1b66ef2c7b3531d13ece2e1b458b24290 Mon Sep 17 00:00:00 2001
From: Eliah Kagan <degeneracypressure@gmail.com>
Date: Tue, 12 Nov 2024 19:56:56 -0500
Subject: [PATCH 06/21] Say why we test gix-hashtable instead of gix on armv7

This completes the TODO about investigating what was thought to be
an environment error, possibly because it references `LD_PRELOAD`.
We can't run `cross test -p gix --target armv7-linux-androideabi`
because the armv7 `bash` that runs the test fixture scripts (whose
archives are `.gitignore`d) finds the `git` command on the runner,
which is an amd64 build, and tries to run it as though it is a
dynamically linked armv7 executable. This relates to the use of
`cross`; no analogous problem occurs with the i686 builds, which
are able to run the amd64 `git` avaialble on the system.

There are various ways to try to fix this, but for now we do not
have any such fix and this is putting it back to testing with
`-p gix-hashtable` on armv7 (while still using `-p gix` on i686),
and also replacing the old TODO comment with comments on the newly
introduced matrix variable about why `gix` is tested on one target
and `gix-hashtable` is tested on another.
---
 .github/workflows/ci.yml | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index c8fc14749f2..536234a8c1b 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -206,8 +206,10 @@ jobs:
         include:
           - target: i686-unknown-linux-musl
             cargo: cargo
+            package-to-test: gix  # Without cross, can test gix. Fixture scripts use amd64 git.
           - target: armv7-linux-androideabi
             cargo: cross
+            package-to-test: gix-hashtable  # With cross, can't test gix. Fixture scripts need armv7 git.
       fail-fast: false  # FIXME: Remove this after testing.
 
     steps:
@@ -227,9 +229,8 @@ jobs:
       - name: check
         run: ${{ matrix.cargo }} 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: ${{ matrix.cargo }} test -p gix --target ${{ matrix.target }}
+        # Run high-level unit tests that exercise a lot of code while being pure Rust to ease building test binaries.
+        run: ${{ matrix.cargo }} test -p ${{ matrix.package-to-test }} --target ${{ matrix.target }}
 
   lint:
     runs-on: ubuntu-latest

From 094023ac6a4d2c242e9b3be9f13821daaa51635d Mon Sep 17 00:00:00 2001
From: Eliah Kagan <degeneracypressure@gmail.com>
Date: Wed, 13 Nov 2024 01:06:41 -0500
Subject: [PATCH 07/21] Split i686 and armv7 jobs; run all tests on i686

The test failures that appeared with `i686-unknown-linux-musl` are
assertions about the size of data structures, where the actual
sizes on 32-bit targets are smaller. But these are not the only
such failing assertions in the test suite: local testing with
`i686-unknown-linux-gnu` on a 32-bit x86 Debian 12 system, and with
`i686-pc-windows-msvc` on a 32-bit x86 Windows 10 system, reveals
that there are significantly more such failures. About 20 tests
usually fail on these platforms, with most failures being of data
structure size assertions.

To catch such failures when they arise as regressions, it would be
useful to have CI coverage of most of the test suite on some 32-bit
target. Since no emulation is needed to run i686 binaries on amd64
CI runners, let try `i686-unknown-linux-musl` for this.

This doesn't include running tests with `GIX_TEST_IGNORE_ARCHIVES`,
because a test that specifically attempts to exercise fixture
scripts should run in an environment where the tools being called,
including `git`, and `bash` itself, are 32-bit builds. (That could
probably be achieved with a `container` job.)
---
 .github/workflows/ci.yml | 41 +++++++++++++++++++++++++---------------
 1 file changed, 26 insertions(+), 15 deletions(-)

diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index 536234a8c1b..8c52985f915 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -198,19 +198,28 @@ jobs:
   test-32bit:
     runs-on: ubuntu-latest
 
+    env:
+      TARGET: i686-unknown-linux-musl
+
+    steps:
+      - uses: actions/checkout@v4
+      - uses: dtolnay/rust-toolchain@master
+        with:
+          toolchain: stable
+          targets: ${{ env.TARGET }}
+      - uses: Swatinem/rust-cache@v2
+      - uses: taiki-e/install-action@v2
+        with:
+          tool: nextest
+      - name: Test (nextest)
+        run: cargo nextest run --target "$TARGET" --workspace --no-fail-fast
+
+  test-32bit-cross:
+    runs-on: ubuntu-latest
+
     strategy:
       matrix:
-        target:
-          - i686-unknown-linux-musl
-          - armv7-linux-androideabi
-        include:
-          - target: i686-unknown-linux-musl
-            cargo: cargo
-            package-to-test: gix  # Without cross, can test gix. Fixture scripts use amd64 git.
-          - target: armv7-linux-androideabi
-            cargo: cross
-            package-to-test: gix-hashtable  # With cross, can't test gix. Fixture scripts need armv7 git.
-      fail-fast: false  # FIXME: Remove this after testing.
+        target: [ armv7-linux-androideabi ]
 
     steps:
       - uses: actions/checkout@v4
@@ -222,15 +231,16 @@ jobs:
           toolchain: stable
           targets: ${{ matrix.target }}
       - name: Install cross
-        if: matrix.cargo == 'cross'
         uses: taiki-e/install-action@v2
         with:
           tool: cross
       - name: check
-        run: ${{ matrix.cargo }} check -p gix --target ${{ matrix.target }}
+        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.
-        run: ${{ matrix.cargo }} test -p ${{ matrix.package-to-test }} --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
@@ -407,6 +417,7 @@ jobs:
       - test-journey
       - test-fast
       - test-32bit
+      - test-32bit-cross
       - lint
       - cargo-deny
       - check-packetline

From 0d54b5f60ff5301770a799b47c3306b28c1a706f Mon Sep 17 00:00:00 2001
From: Eliah Kagan <degeneracypressure@gmail.com>
Date: Wed, 13 Nov 2024 01:42:05 -0500
Subject: [PATCH 08/21] Change i686 job to i686-unknown-linux-gnu, run in
 container

The previous approach did not work because musl dependencies were
not installed. (32-bit `-gnu` dependencies would also not be
installed, in that non-container environment.)

Since this is switching to a container, it may as well target the
OS of the container.

The previous reasoning for not (also?) having this 32-bit test use
`GIX_TEST_IGNORE_ARCHIVES=1` no longer applies, but that is not
done at this time.
---
 .github/workflows/ci.yml | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index 8c52985f915..9804a296831 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -198,21 +198,23 @@ jobs:
   test-32bit:
     runs-on: ubuntu-latest
 
-    env:
-      TARGET: i686-unknown-linux-musl
+    container: i386/debian:stable-slim
 
     steps:
+      - name: Prerequisites
+        run: |
+          apt-get update
+          apt-get install --no-install-recommends -y build-essential ca-certificates cmake curl git jq libssl-dev pkgconf
       - uses: actions/checkout@v4
-      - uses: dtolnay/rust-toolchain@master
+      - uses: dtolnay/rust-toolchain@stable
         with:
-          toolchain: stable
-          targets: ${{ env.TARGET }}
+          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: Test (nextest)
-        run: cargo nextest run --target "$TARGET" --workspace --no-fail-fast
+        run: cargo nextest run --workspace --no-fail-fast
 
   test-32bit-cross:
     runs-on: ubuntu-latest

From 967970b732d9603da2ab4b833317a4ec8c870c3d Mon Sep 17 00:00:00 2001
From: Eliah Kagan <degeneracypressure@gmail.com>
Date: Wed, 13 Nov 2024 17:25:34 -0500
Subject: [PATCH 09/21] Install `node` in container; try to help actions find
 it

One possible problem is that this is actually the downstream Debian
build of Node 18, which may have security patches, but which does
not have any features or breaking changes new in Node 20, and also
which will not report itself as being Node 20.

(GitHub Actions switched from Node 16 to Node 20, so I don't think
using older actions versions would necessarily help.)
---
 .github/workflows/ci.yml | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index 9804a296831..1c30def2fbc 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -204,7 +204,9 @@ jobs:
       - name: Prerequisites
         run: |
           apt-get update
-          apt-get install --no-install-recommends -y build-essential ca-certificates cmake curl git jq libssl-dev pkgconf
+          apt-get install --no-install-recommends -y build-essential ca-certificates cmake curl git jq libssl-dev nodejs pkgconf
+          mkdir -p /__e/node20/bin
+          ln -s /usr/bin/node /__e/node20/bin/node
       - uses: actions/checkout@v4
       - uses: dtolnay/rust-toolchain@stable
         with:

From bb430d7b9053a2c72b5e2d004958947f9123daaf Mon Sep 17 00:00:00 2001
From: Eliah Kagan <degeneracypressure@gmail.com>
Date: Wed, 13 Nov 2024 17:29:27 -0500
Subject: [PATCH 10/21] Try to figure out what is going on

---
 .github/workflows/ci.yml | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index 1c30def2fbc..7e8e48f4802 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -205,8 +205,9 @@ jobs:
         run: |
           apt-get update
           apt-get install --no-install-recommends -y build-essential ca-certificates cmake curl git jq libssl-dev nodejs pkgconf
-          mkdir -p /__e/node20/bin
-          ln -s /usr/bin/node /__e/node20/bin/node
+          #mkdir -p /__e/node20/bin
+          #ln -s /usr/bin/node /__e/node20/bin/node
+          file /__e/node20/bin/node
       - uses: actions/checkout@v4
       - uses: dtolnay/rust-toolchain@stable
         with:

From d221572dea824d3eaa906994c4c02653c1d57105 Mon Sep 17 00:00:00 2001
From: Eliah Kagan <degeneracypressure@gmail.com>
Date: Wed, 13 Nov 2024 17:39:17 -0500
Subject: [PATCH 11/21] Install 64-bit Node in container, for its libraries

This is a workaround for a problem that is the same or similar to
https://github.com/actions/checkout/issues/334:

    Run actions/checkout@v4
    /usr/bin/docker exec  a7f94cb30b99bd1ee851270fad4768421eddecd7e8fa674f56c4f2d2d64210b6 sh -c "cat /etc/*release | grep ^ID"
    exec /__e/node20/bin/node: no such file or directory

I had not originally noticed the `cat /etc/*release` part of that
output, and had surmised that this was a subtly different problem,
but it looks like it's probably exactly the same. The workaround in
https://github.com/actions/checkout/issues/334#issuecomment-2435101135
is applied in this commit and seems to be working.

As noted there, the absence of some 64-bit libraries in 32-bit
containers prevents the (existing and already mapped) 64-bit
Node.js outside the container from being run, and installing 64-bit
Node.js in the container via `apt-get` (even though, in this case,
it is a different version) installs the needed dependencies,
letting the (existing) Node.js run.

(Other problems, related to `safe.directory` protections as well as
tests that are known to fail due to assertions about the size of
data structures that implicitly assume 64-bit targets, do still
prevent the job from succeeding. But `actions/checkout@v4`, which
was failing before, seems to be working, as do the other actions.)
---
 .github/workflows/ci.yml | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index 7e8e48f4802..05ce36503a9 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -203,11 +203,9 @@ jobs:
     steps:
       - name: Prerequisites
         run: |
+          dpkg --add-architecture amd64  # For 64-bit Node.js, for actions.
           apt-get update
-          apt-get install --no-install-recommends -y build-essential ca-certificates cmake curl git jq libssl-dev nodejs pkgconf
-          #mkdir -p /__e/node20/bin
-          #ln -s /usr/bin/node /__e/node20/bin/node
-          file /__e/node20/bin/node
+          apt-get install --no-install-recommends -y build-essential ca-certificates cmake curl git jq libssl-dev nodejs:amd64 pkgconf
       - uses: actions/checkout@v4
       - uses: dtolnay/rust-toolchain@stable
         with:

From 64f219827c33325ef3194709209f62b19137e504 Mon Sep 17 00:00:00 2001
From: Eliah Kagan <degeneracypressure@gmail.com>
Date: Wed, 13 Nov 2024 20:27:40 -0500
Subject: [PATCH 12/21] Install libstdc++6:amd64 (and not Node) in the
 container

Installing `nodejs:amd64` in a 32-bit container to work around
https://github.com/actions/checkout/issues/334 works by causing
64-bit libraries the external mapped `node20` needs (even though
`nodejs:amd64` in Debian is Node 18), as described in
https://github.com/actions/checkout/issues/334#issuecomment-2435101135.
But this installed more packages than are needed.

It turns out that, at least with this image, the only libraries the
container is missing are the 32-bit libstdc++ and its dependencies.
It is therefore sufficient to install the `libstdc++6:amd64`
package. This commit makes that change, squashing a few
investigatory steps that led to it (EliahKagan#5).

* See if libssl3:amd64 is all we need

* Temporarily omit even libssl3:amd64

With only `libssl3:amd64` and its dependencies, `actions/checkout`
did fail, but with a different error than before, showing a clear
error about a missing library, `libstdc++`.

    /usr/bin/docker exec  c1e55a75713e2c4fd08241fae9f4fecbe3cbb179be45174b3138390a1238090e sh -c "cat /etc/*release | grep ^ID"
    /__e/node20/bin/node: error while loading shared libraries: libstdc++.so.6: cannot open shared object file: No such file or directory

Before trying adding that, I want to check that enabling `amd64` in
`dpkg` has the expected *no effect* on the error messages, compared
to not enabling it.

* Install libstdc++6:amd64 in the container

This does not add back libssl3:amd64 yet, in case it is not needed,
though I expect that it will be needed.
---
 .github/workflows/ci.yml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index 05ce36503a9..3f823fec5aa 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -205,7 +205,7 @@ jobs:
         run: |
           dpkg --add-architecture amd64  # For 64-bit Node.js, for actions.
           apt-get update
-          apt-get install --no-install-recommends -y build-essential ca-certificates cmake curl git jq libssl-dev nodejs:amd64 pkgconf
+          apt-get install --no-install-recommends -y build-essential ca-certificates cmake curl git jq libssl-dev libstdc++6:amd64 pkgconf
       - uses: actions/checkout@v4
       - uses: dtolnay/rust-toolchain@stable
         with:

From eaf22432ce8a537e5b2200ad75395df177f649f0 Mon Sep 17 00:00:00 2001
From: Eliah Kagan <degeneracypressure@gmail.com>
Date: Wed, 13 Nov 2024 20:39:44 -0500
Subject: [PATCH 13/21] Reorganize `test-32bit` prerequisites to explain
 `libstdc++6:amd64`

This uses the same array+comment style as in 8c71bd1 (#1672).
---
 .github/workflows/ci.yml | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index 3f823fec5aa..cd83942b96f 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -203,9 +203,21 @@ jobs:
     steps:
       - name: Prerequisites
         run: |
-          dpkg --add-architecture amd64  # For 64-bit Node.js, for actions.
+          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 build-essential ca-certificates cmake curl git jq libssl-dev libstdc++6:amd64 pkgconf
+          apt-get install --no-install-recommends -y -- "${prerequisites[@]}"
+        shell: bash
       - uses: actions/checkout@v4
       - uses: dtolnay/rust-toolchain@stable
         with:

From 47f3819d6d0f7a29f735ae4bd5844ba32a1030f3 Mon Sep 17 00:00:00 2001
From: Eliah Kagan <degeneracypressure@gmail.com>
Date: Thu, 14 Nov 2024 05:58:10 -0500
Subject: [PATCH 14/21] Take ownership of the repo in the container

And add a fixme about how the test suite could be improved to not
require this.
---
 .github/workflows/ci.yml | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index cd83942b96f..f61313e18b7 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -219,6 +219,12 @@ jobs:
           apt-get install --no-install-recommends -y -- "${prerequisites[@]}"
         shell: bash
       - uses: actions/checkout@v4
+      - name: Take ownership of repo
+        # FIXME: Eliminate the need for this by improving the test suite not to rely on ownership
+        # of preexisting files, nor on whether, where, or how we are contained in a repo (except
+        # where gix-testtools checks ignores to suppress archives). Note that `safe.directory`
+        # isn't a fix, due to config and environment sanitization when running fixture scripts.
+        run: chown -R "$(whoami)" -- "$GITHUB_WORKSPACE"
       - uses: dtolnay/rust-toolchain@stable
         with:
           toolchain: stable-i686-unknown-linux-gnu  # Otherwise it may misdetect based on the amd64 kernel.

From d07c32d2795e964b6296a4d6dba9b8b197438bb6 Mon Sep 17 00:00:00 2001
From: Eliah Kagan <degeneracypressure@gmail.com>
Date: Thu, 14 Nov 2024 16:47:19 -0500
Subject: [PATCH 15/21] Fix gix-url baseline script instead of taking ownership

Taking ownership of cloned files in the container only fixed one
failing test, `gix-url::baseline run`. This had been failing with a
git `safe.directory` error in the the gix-url `make_baseline.sh`
fixture script. That failure was also reproduced locally by
recursively `chown`ing the cloned files to another user (while
preserving write permissions for the first user via the group).

Because the tests shouldn't unnecessarily depend on starting out in
a repository (nor anything about the outer repository they start
in), this should probably be considered a test bug that is not
specific to the current container setup on CI that triggered it.

This commit undoes the `chown -R` step in the CI workflow that had
previously worked around this, and instead fixes it by having the
gix-url `make_baseline.sh` fixture script run its `git fetch-pack`
commands in a temporary repo created in a subdirectory of the
fixture directory (and deleted afterwards).
---
 .github/workflows/ci.yml                |  6 ------
 gix-url/tests/fixtures/make_baseline.sh | 11 +++++++++--
 2 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index f61313e18b7..cd83942b96f 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -219,12 +219,6 @@ jobs:
           apt-get install --no-install-recommends -y -- "${prerequisites[@]}"
         shell: bash
       - uses: actions/checkout@v4
-      - name: Take ownership of repo
-        # FIXME: Eliminate the need for this by improving the test suite not to rely on ownership
-        # of preexisting files, nor on whether, where, or how we are contained in a repo (except
-        # where gix-testtools checks ignores to suppress archives). Note that `safe.directory`
-        # isn't a fix, due to config and environment sanitization when running fixture scripts.
-        run: chown -R "$(whoami)" -- "$GITHUB_WORKSPACE"
       - uses: dtolnay/rust-toolchain@stable
         with:
           toolchain: stable-i686-unknown-linux-gnu  # Otherwise it may misdetect based on the amd64 kernel.
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

From 178cf256649b97a3e2dcd970725277a6ba3e728a Mon Sep 17 00:00:00 2001
From: Eliah Kagan <degeneracypressure@gmail.com>
Date: Fri, 15 Nov 2024 16:16:33 -0500
Subject: [PATCH 16/21] Make Git `system` scope nonempty in container

This creates `/etc/gitconfig` in the container and populates it
with a single dummy configuration variable, so that the `system`
scope will exist and tests of "GitInstallation"-related
functionality will be able to pass.

This is intended to let the following tests pass in the container:

- gix-config-tests::config source::git_config_no_system
- gix-path env::git::tests::exe_info::tolerates_broken_temp
- gix-path env::git::tests::exe_info::tolerates_git_config_env_var
- gix-path env::git::tests::exe_info::tolerates_oversanitized_env
- gix-path env::git::tests::exe_info::with_unmodified_environment
- gix-path::path env::installation_config

Those tests have been failing in the container on CI, and in local
container environments that are also Debian 12 and created through
a similar procedure. But they are not among the tests that fail in
a non-container 32-bit Debian 12 system, probably because that test
system has user config in the `global` scope. When that is the
highest scope, it is currently treated as "GitInstallation", even
though it is not truly associated with the Git installation.

The change in this commit is a workaround *specific* to one CI job:

- In at least the case of `git_config_no_system`, it seems like the
  test should automatically handle the case of there being no scope
  detected as "GitInstallation". For the others, it may be
  reasonable for them to fail in a CI environment that does not
  support robustly testing the scenarios they pertain to, but there
  should probably be a way to suppress or weaken them on systems
  that have no scopes that are intended to be detected as
  "GitInstallation", while still testing "GitInstallation"
  functionality as robustly as it can be tested under those
  conditions. This commit does not fix that.

- On macOS with Apple Git, the `unknown` scope higher than `system`
  is meant to be detected as "GitInstallation" in the usual case
  that it is nonempty. On other systems, `system` is the highest
  scope that is expected to exist. On some systems (besides macOS),
  the `system` scope is often empty, in which case the `global`
  scope, if nonempty, will be detected as "GitInstallation". But
  allowing the `global` scope, which in practice is rarely if ever
  associated with the installation itself (it is "global" to the
  user, and still separate from the `system` scope in a per-user Git
  installation), to be detected as a "GitInstallation" has never
  been guaranteed, and may be changed.

  Ever since the CVE-2024-45305 fix in #1523 and follow-up in
  #1567, `local` and `worktree` scopes are never detected as
  "GitInstallation", so it looks like there would be no harm, and
  some benefit, to excluding the `global` scope as well. This
  commit does not make that change.

- If/when the change of excluding `global` from consideration for
  "GitInstallation" is made, tests that require a variable to exist
  in an installation-associated scope will start to fail regularly
  on some developers' machines, similar to how they have been
  failing in the container. This commit does not address that; it
  only adds a configuration variable in the `system` scope in the
  container.
---
 .github/workflows/ci.yml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index cd83942b96f..9d0bc384102 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -226,6 +226,8 @@ jobs:
       - 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
 

From 77c3c59d1f3be76f228ada15304d5af1f3f03a14 Mon Sep 17 00:00:00 2001
From: Eliah Kagan <degeneracypressure@gmail.com>
Date: Sun, 17 Nov 2024 20:18:28 -0500
Subject: [PATCH 17/21] feat: Add `size_ok` for asserting size is not too big

This compares using `==` on 64-bit targets and `<=` on 32-bit
targets.

As noted in the documentation comment, when assertions about data
stuructures' sizes are being done to safeguard against them growing
too big, then it may be acceptable to use `<=` if the structure is
smaller on 32-bit targets, but it is still valuable to be able to
use `==` on 64-bit targets in the same assertions, since this
guards against a data structure becoming smaller, other changes
causing the smaller size to be important for memory usage or speed,
but then the data structure growing again, up to its original size.
An unconditional `<=` will not catch this, while `size_ok` usually
will.

A related reason to do a `==` on 64-bit systems is so that the
expected value being compared to remains tied to the code. It can
otherwise become unclear what the expected value's significance is
and whether it ought to be updated.
---
 tests/tools/src/lib.rs | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

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::*;

From fc13fc3eb950ebaff0c22c4d093a4d2300914f72 Mon Sep 17 00:00:00 2001
From: Eliah Kagan <degeneracypressure@gmail.com>
Date: Sun, 17 Nov 2024 20:02:13 -0500
Subject: [PATCH 18/21] Use `<=` on 32-bit for some size assertions

Some of the assertions comparing the sizes of data structures to
expected values have been using `==` (unconditionally). Of those,
most but not all are mainly to safeguard against data structures
growing larger. For those, this loosens the assertions on 32-bit
targets to use `<=`, while still having them use `==` on 64-bit
targets.

The new `gix_testtools::size_ok` function is used to help with
this. See the `size_ok` documentation comment for a rationale as to
why it is done this way (and why this approach is not taken with
assertions that seem intended to do more than keep the size from
growing too large or without being noticed).

This is to allow the following 18 tests to pass in the container
(and hopefully on 32-bit systems in general):

- gix-attributes::attributes search::size_of_outcome
- gix-index extension::tree::tests::size_of_tree
- gix-index size_of_entry
- gix-index-tests::integrate index::size_of_entry
- gix-negotiate::negotiate size_of_entry
- gix-pack cache::delta::tests::size_of_pack_tree_item
- gix-pack cache::delta::tests::size_of_pack_verify_data_structure
- gix-pack cache::delta::tree::tests::size_of_pack_tree_item
- gix-pack cache::delta::tree::tests::size_of_pack_verify_data_structure
- gix-pack data::file::decode::entry::tests::size_of_decode_entry_outcome
- gix-pack-tests::pack pack::data::output::size_of_count
- gix-pack-tests::pack pack::data::output::size_of_entry
- gix-pack-tests::pack pack::iter::size_of_entry
- gix-ref raw::tests::size_of_reference
- gix-revwalk::revwalk graph::commit::size_of_commit
- gix::gix object::object_ref_size_in_memory
- gix::gix object::oid_size_in_memory
- gix::gix status::index_worktree::iter::item_size

This splits a couple of test cases into two, where the added code
would otherwise make them less readable.

The duplicated tests described in #1685 are among those modified
here, so this exacerbates that duplication, in that there is more
duplicated code, but the duplicated test cases are still easy to
resolve, once it is clear which modules they should be in. This
adds "FIXME" comments to identify the duplication.
---
 Cargo.lock                             |  2 ++
 gix-attributes/tests/search/mod.rs     | 10 +++---
 gix-index/Cargo.toml                   |  3 ++
 gix-index/src/extension/tree/mod.rs    |  8 ++++-
 gix-index/src/lib.rs                   | 36 +++++++++++++++----
 gix-index/tests/index/mod.rs           | 35 ++++++++++++++-----
 gix-negotiate/tests/negotiate.rs       | 11 +++---
 gix-pack/src/cache/delta/mod.rs        | 19 +++++++---
 gix-pack/src/cache/delta/tree.rs       | 48 +++++++++++++++++---------
 gix-pack/src/data/file/decode/entry.rs | 10 +++---
 gix-pack/tests/pack/data/output/mod.rs | 19 +++++-----
 gix-pack/tests/pack/iter.rs            | 10 +++---
 gix-ref/src/raw.rs                     | 10 +++---
 gix-revwalk/Cargo.toml                 |  3 ++
 gix-revwalk/tests/revwalk.rs           | 11 +++---
 gix/tests/gix/object/mod.rs            | 20 ++++++-----
 gix/tests/gix/status.rs                | 10 +++---
 17 files changed, 184 insertions(+), 81 deletions(-)

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-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::<Outcome>(),
-        840,
-        "it's quite big, shouldn't change without us noticing"
+    let actual = std::mem::size_of::<Outcome>();
+    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::<crate::extension::Tree>(), 88);
+        let actual = std::mem::size_of::<crate::extension::Tree>();
+        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..eb50b1bb6a4 100644
--- a/gix-index/src/lib.rs
+++ b/gix-index/src/lib.rs
@@ -238,11 +238,35 @@ pub(crate) mod util {
     }
 }
 
-#[test]
-fn size_of_entry() {
-    assert_eq!(std::mem::size_of::<crate::Entry>(), 80);
+// FIXME: Probably remove these in favor of the equivalent tests in `gix-index/tests/index/mod.rs`.
+#[cfg(test)]
+mod tests {
+    use gix_testtools::size_ok;
 
-    // the reason we have our own time is half the size.
-    assert_eq!(std::mem::size_of::<crate::entry::stat::Time>(), 8);
-    assert_eq!(std::mem::size_of::<filetime::FileTime>(), 16);
+    #[test]
+    fn size_of_entry() {
+        let actual = std::mem::size_of::<crate::Entry>();
+        let expected = 80;
+        assert!(
+            size_ok(actual, expected),
+            "the size of this structure should not change unexpectedly: {actual} <~ {expected}"
+        );
+    }
+
+    #[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::<crate::entry::stat::Time>();
+        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::<filetime::FileTime>();
+        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}"
+        );
+    }
 }
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::<gix_index::Entry>(), 80);
+    let actual = std::mem::size_of::<gix_index::Entry>();
+    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::<gix_index::entry::stat::Time>(), 8);
-    assert_eq!(std::mem::size_of::<filetime::FileTime>(), 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::<gix_index::entry::stat::Time>();
+    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::<filetime::FileTime>();
+    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::<gix_revwalk::graph::Commit<gix_negotiate::Metadata>>(),
-        56,
-        "we may keep a lot of these, so let's not let them grow unnoticed"
+    let actual = std::mem::size_of::<gix_revwalk::graph::Commit<gix_negotiate::Metadata>>();
+    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..572450c18c7 100644
--- a/gix-pack/src/cache/delta/mod.rs
+++ b/gix-pack/src/cache/delta/mod.rs
@@ -23,18 +23,24 @@ mod tree;
 
 pub use tree::{Item, Tree};
 
+// FIXME: Probably remove this pair of tests or the equivalent pair in `gix-pack/src/cache/delta/tree.rs`.
 #[cfg(test)]
 mod tests {
+    use super::Item;
+    use gix_testtools::size_ok;
 
     #[test]
     fn size_of_pack_tree_item() {
-        use super::Item;
-        assert_eq!(std::mem::size_of::<[Item<()>; 7_500_000]>(), 300_000_000);
+        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}"
+        );
     }
 
     #[test]
     fn size_of_pack_verify_data_structure() {
-        use super::Item;
         pub struct EntryWithDefault {
             _index_entry: crate::index::Entry,
             _kind: gix_object::Kind,
@@ -45,6 +51,11 @@ mod tests {
             _level: u16,
         }
 
-        assert_eq!(std::mem::size_of::<[Item<EntryWithDefault>; 7_500_000]>(), 840_000_000);
+        let actual = std::mem::size_of::<[Item<EntryWithDefault>; 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/cache/delta/tree.rs b/gix-pack/src/cache/delta/tree.rs
index f26cf5a0564..dad425f6679 100644
--- a/gix-pack/src/cache/delta/tree.rs
+++ b/gix-pack/src/cache/delta/tree.rs
@@ -226,25 +226,39 @@ 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);
-    }
+    // FIXME: Probably remove this pair of tests or the equivalent pair in `gix-pack/src/cache/delta/mod.rs`.
+    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<EntryWithDefault>; 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<EntryWithDefault>; 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::<Outcome>(),
-            32,
-            "this shouldn't change without use noticing as it's returned a lot"
+        let actual = std::mem::size_of::<Outcome>();
+        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::<output::Entry>(),
-        80,
-        "The size of the structure shouldn't change unexpectedly"
+    let actual = std::mem::size_of::<output::Entry>();
+    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::<output::Count>(),
-        56,
-        "The size of the structure shouldn't change unexpectedly"
+    let actual = std::mem::size_of::<output::Count>();
+    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::<pack::data::input::Entry>(),
-        104,
-        "let's keep the size in check as we have many of them"
+    let actual = std::mem::size_of::<pack::data::input::Entry>();
+    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::<Reference>(),
-            80,
-            "let's not let it change size undetected"
+        let actual = std::mem::size_of::<Reference>();
+        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::<gix_revwalk::graph::Commit<()>>(),
-                48,
-                "We might see quite a lot of these, so they shouldn't grow unexpectedly"
+            let actual = std::mem::size_of::<gix_revwalk::graph::Commit<()>>();
+            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/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::<gix::Object<'_>>(),
-        56,
-        "the size of this structure should not changed unexpectedly"
+    let actual = std::mem::size_of::<gix::Object<'_>>();
+    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::<gix::Id<'_>>(),
-        32,
-        "the size of this structure should not changed unexpectedly"
+    let actual = std::mem::size_of::<gix::Id<'_>>();
+    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/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::<gix::status::index_worktree::iter::Item>(),
-                264,
-                "The size is pretty huge and goes down ideally"
+            let actual = std::mem::size_of::<gix::status::index_worktree::iter::Item>();
+            let expected = 264;
+            assert!(
+                size_ok(actual, expected),
+                "The size is pretty huge and goes down ideally: {actual} <~ {expected}"
             );
         }
 

From 2f77c9bba07ab91b7e0e3e9aa4faaeba42a00a4e Mon Sep 17 00:00:00 2001
From: Eliah Kagan <degeneracypressure@gmail.com>
Date: Sun, 17 Nov 2024 23:46:09 -0500
Subject: [PATCH 19/21] Work around new `gix-index` ambiguity for `cargo check`

The previous commit added `gix-testtools` (by relative path) as a
dev dependency of `gix-index`, to use `gix_testtools::size_ok`.
Because `gix-testtools` itself depends on `gix-index` -- at an
earlier version to not break releasing with csr (see discussion
in #1510 for general info) -- this causes `cargo`, when running in
the top level workspace directory, to consider `-p gix-index`
without an explicit version to be ambiguous.

This made the full CI `test` job fail when the `check` recipe
attempts to run `cargo check` on `gix-index`, with the message

    error: There are multiple `gix-index` packages in your project, and the specification `gix-index` is ambiguous.
    Please re-run this command with one of the following specifications:
      gix-index@0.33.1
      gix-index@0.36.0
    error: Recipe `check` failed on line 87 with exit code 101

where the line number is from the `justfile`.

To fix this, this changes the command to change to the `gix-index`
directory instead of passing `-p gix-index`. (This technique is
used elsewhere in the same recipe already.)
---
 justfile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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

From daf999043779e7e8d9cc6a602a3a0f24024d38fa Mon Sep 17 00:00:00 2001
From: Eliah Kagan <degeneracypressure@gmail.com>
Date: Mon, 18 Nov 2024 01:30:27 -0500
Subject: [PATCH 20/21] Add 32-bit expectations for remaining `==` size
 assertions

Most `assert_eq!` data structure size assertions that had failed on
32-bit targets were to safeguard against the size increasing, so
using a `==` comparison on 64-bit targets and a `<=` comparison to
the same value on 32-bit targets was a suitable fix, which allowed
18 tests to go from failing to passing in the 32-bit container.

However, four remaining tests that assert sizes of buffers or
streamed data are not just for flagging increases, but are instead
cases where a change in either direction, on any platform, should
probably draw attention. The most important of these to keep `==`,
even on 32-bit platforms, may be two tests that assert the same
value for complementary operations:

- gix::gix repository::worktree::archive
- gix::gix repository::worktree::stream

But I think it's useful to do this in the two others, too:

- gix-archive::archive from_tree::basic_usage_internal
- gix-worktree-stream::stream from_tree::will_provide_all_information_and_respect_export_ignore

This commit adds constants with different 64-bit and 32-bit values
for the three buffer lengths asserted in those four tests (the
two `gix::gix repository::worktree::*` assert the same length).

This change should make the four remaining tests that had been
failing on the 32-bit container, and 32-bit GNU/Linux targets in
general, pass on them.

In specifying the expected 32-bit buffer lengths, I have treated
these as approval tests, rather than inferring that the exact
length and no other length must be correct. The asserted lengths:

- Appeared as the value when the tests were run in the 32-bit
  container.

- Are verified to appear repeatedly across different runs in the
  container as well as in runs on a non-containerized 32-bit Debian
  12 system.

- Are equal to the values in assertion messages for the same failed
  tests run on a 32-bit Windows 10 system. This is significant
  because some values differ between different 32-bit targets, even
  targets for the same 32-bit architecture, and two such targets
  where I've observed differences are `i686-unknown-linux-gnu` and
  `i686-pc-windows-msvc`.

- Are, of course, less than the 64-bit sizes, not greater.

- Make sense on inspection. (But please note that this is not the
  same as an analysis that proves they are the only possible
  correct values.)
---
 gix-archive/tests/archive.rs         | 7 ++++++-
 gix-worktree-stream/tests/stream.rs  | 7 ++++++-
 gix/tests/gix/repository/worktree.rs | 9 +++++++--
 3 files changed, 19 insertions(+), 4 deletions(-)

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-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/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(())
 }
 

From dc0a73ab0ff3e4e3731cf7cc01b418a56bb5953c Mon Sep 17 00:00:00 2001
From: Eliah Kagan <degeneracypressure@gmail.com>
Date: Mon, 18 Nov 2024 02:57:32 -0500
Subject: [PATCH 21/21] Deduplicate some repeated tests

This removes one of each pair of equivalent tests duplicated within
`gix-index` and (separately) within `gix-pack`. See #1685.
---
 gix-index/src/lib.rs             | 33 ----------------------------
 gix-pack/src/cache/delta/mod.rs  | 37 --------------------------------
 gix-pack/src/cache/delta/tree.rs |  1 -
 3 files changed, 71 deletions(-)

diff --git a/gix-index/src/lib.rs b/gix-index/src/lib.rs
index eb50b1bb6a4..a9d853f6960 100644
--- a/gix-index/src/lib.rs
+++ b/gix-index/src/lib.rs
@@ -237,36 +237,3 @@ pub(crate) mod util {
         data.split_at(pos).into()
     }
 }
-
-// FIXME: Probably remove these in favor of the equivalent tests in `gix-index/tests/index/mod.rs`.
-#[cfg(test)]
-mod tests {
-    use gix_testtools::size_ok;
-
-    #[test]
-    fn size_of_entry() {
-        let actual = std::mem::size_of::<crate::Entry>();
-        let expected = 80;
-        assert!(
-            size_ok(actual, expected),
-            "the size of this structure should not change unexpectedly: {actual} <~ {expected}"
-        );
-    }
-
-    #[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::<crate::entry::stat::Time>();
-        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::<filetime::FileTime>();
-        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}"
-        );
-    }
-}
diff --git a/gix-pack/src/cache/delta/mod.rs b/gix-pack/src/cache/delta/mod.rs
index 572450c18c7..09dc1c3fcd1 100644
--- a/gix-pack/src/cache/delta/mod.rs
+++ b/gix-pack/src/cache/delta/mod.rs
@@ -22,40 +22,3 @@ pub mod from_offsets;
 mod tree;
 
 pub use tree::{Item, Tree};
-
-// FIXME: Probably remove this pair of tests or the equivalent pair in `gix-pack/src/cache/delta/tree.rs`.
-#[cfg(test)]
-mod tests {
-    use super::Item;
-    use gix_testtools::size_ok;
-
-    #[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}"
-        );
-    }
-
-    #[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<EntryWithDefault>; 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/cache/delta/tree.rs b/gix-pack/src/cache/delta/tree.rs
index dad425f6679..cf26f5440ea 100644
--- a/gix-pack/src/cache/delta/tree.rs
+++ b/gix-pack/src/cache/delta/tree.rs
@@ -226,7 +226,6 @@ mod tests {
         }
     }
 
-    // FIXME: Probably remove this pair of tests or the equivalent pair in `gix-pack/src/cache/delta/mod.rs`.
     mod size {
         use super::super::Item;
         use gix_testtools::size_ok;