Fix minor SHA-256 issues in CLI

[cruessler]

This PR fixes a few minor issues in the CLI related to SHA-256 support.

• Don't hardcode hash in gix free index. Instead, take it from --object-hash.
• Add SHA-256 to AsHashKind.
• Remove outdated note saying SHA-256 is not fully supported.
• Fix key name in comment.

Some of them were found by GPT 6.0 when I asked it to trim etc/plan/sha256-support.md to only those items that are still left open. This was the list it came up with:

• Honor gix free index --object-hash in from-list: forward the selection from src/plumbing/main.rs to gitoxide-core/src/repository/index/mod.rs, which still hardcodes SHA-1. Add a SHA-256 index write/read regression test.
• Support SHA-256 in query path tracing: replace the [u8; 20] SQL row decoding in gitoxide-core/src/query/engine/command.rs with hash-length-aware decoding. Add a regression test tracing a path in a SHA-256 repository.
• Complete the 32-bit SHA-256 stream-size expectations currently left as todo!(): basic_usage_internal in gix-archive/tests/archive.rs and will_provide_all_information_and_respect_export_ignore in gix-worktree-stream/tests/stream.rs. Run both tests on a 32-bit target with GIX_TEST_FIXTURE_HASH=sha256.
• Correct the stale extensions.objectFormat note in gix/src/config/tree/sections/extensions.rs, which still claims SHA-256 is rejected.

This PR addresses the first as well as the last item. Now I’m wondering: do we want to address the two other items as well? I don’t have access to a 32-bit machine, and I don’t know whether I want to spin up a 32-bit VM on my machine just to get the two missing values. 😀 Regarding the query engine, do you want SHA-256 support in it?

[EliahKagan]

I don’t have access to a 32-bit machine, and I don’t know whether I want to spin up a 32-bit VM on my machine just to get the two missing values.

For 32-bit testing, I suspect you may not need a 32-bit VM, nor a 32-bit operating system installed on a compatible 64-bit VM.

In principle you might be able to just install a 32-bit target and build for it and use it. This assumes you have a (physical or virtual) machine with a 64-bit CPU and operating system that is capable of running 32-bit binaries, either natively or by emulation. But even then there might be dependency problems on some 64-bit systems, which might not necessarily be easy to fix, especially if they come down to problems getting pkgconf/pkg-config to find libraries. If you do want to try that, the musl targets may be easier than glibc ones, though I think there's no guarantee that this will work well, especially for non-"pure" builds.

But containerization generally works fine! On a 64-bit system, you can use a 32-bit Docker image. This is how we test 32-bit ARM and x86 on CI, and those CI jobs should help point the way to doing it.

gitoxide/.github/workflows/ci.yml

Lines 387 to 409 in acf906f

	container: ${{ matrix.container-architecture }}/debian:stable-slim
	steps:
	- name: Prerequisites
	env:
	RUNNER_ARCHITECTURE: ${{ matrix.runner-architecture }}
	run: |
	prerequisites=(
	build-essential
	ca-certificates
	cmake
	curl
	git
	jq
	libssl-dev
	"libstdc++6:$RUNNER_ARCHITECTURE" # To support external 64-bit Node.js for actions.
	pkgconf
	python3-minimal
	)
	dpkg --add-architecture "$RUNNER_ARCHITECTURE"
	apt-get update
	apt-get install --no-install-recommends -y -- "${prerequisites[@]}"
	shell: bash # This step needs `bash`, and the default in container jobs is `sh`.

(You may also be able to test this by chrooting into a 32-bit userland environment without full containerization, but I don't think this would be any better or easier than using docker.)

If you're using a system where the CPU doesn't support running machine code for the target architecture, you can use QEMU binfmt_misc binary emulation with docker. This is much slower than if it's not needed, but it should be fine if you're only running a small selection of tests, especially for lightweight tests such as size tests. I find it to be much, much easier than setting up and testing in a VM with QEMU. This can be installed on Debian or Ubuntu (if not already installed) with:

sudo apt install binfmt-support qemu-user-static

If that isn't sufficient to cause docker to run the binaries with QEMU, you can use a privileged Docker container to automate configuring it (taken from this GitLab page about s390x testing, and this is what I used to do for some of my s390x testing, but this step is not s390x-specific):

docker run --rm --privileged multiarch/qemu-user-static --reset -p yes

(If you go the route of using binfmt_misc and have any trouble getting it working, or if you would go that route but need or prefer and alternative that avoids slowdown, please let me know.)

If using docker, I don't recommend preferring musl over glibc based targets; use whatever target best matches the operating system in the container. That would be musl if the container is Alpine Linux, but would be GNU if the container is Debian or Ubuntu or most systems. In particular, on Debian (or derivatives thereof) for 32-bit ARM, the best choice of target would be armv7-unknown-linux-gnueabihf unless you have reason to pick another one.

For size tests, there's no actual guarantee that data structures will be the same size across different platforms of the same word size, even if they have the same endianness. This can happen across different operating systems on the same architecture, as well as across different architectures on the same operating system, all with the same target word size. I recall that, in our use cases, this rarely happens for 64-bit architectures (even s390x, which is big-endian, didn't fail size tests, back when I was testing on that), but much more often happens on 32-bit architectures. The size_ok test helper is sometimes useful, in cases where you judge that a granular assertion is not a good tradeoff:

gitoxide/tests/tools/src/lib.rs

Lines 2584 to 2606 in acf906f

	/// 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;
	}

See #1687, where that was introduced, for further context. Note that I am not advocating that the best way forward is necessarily for the test suite to use size_ok more; I'm not sure. (Also, maybe you're already using this for some SHA256 tests--I haven't checked.) Whether or not you go that route, I think we should ideally have something set up so that it's easy to run tests that may be sensitive to word-size differences on 32-bit architectures. This is valuable both for supporting those architectures better and finding general bugs that are revealed by exercising assumptions related to the sizes of data types.

The real solution, I think--though I don't know if it's what you'd prefer right now--is to test SHA256 more heavily on CI. If I understand correctly what is happening, a lot of the important tests of SHA256 are currently tested on CI only in the full test job, which is "full" in the sense that it tests crates under the most different meaningful selections of features, but which is only run on x86-64 Ubuntu (via the ubuntu-latest label). I think that would be worth expanding. In the case of size tests for 32-bit systems, even if done with fully granular assertions, these are effectively approval tests and it might not be necessary to run the tests manually on 32-bit architectures at all if our 32-bit CI tests were to cover them.

More broadly, I think that any scenario where fully testing something would be inconvenient for architecture-related reasons is a sign that CI coverage of that architecture should be expanded if feasible. We're already testing substantially on two 32-bit architectures on CI, so expanding that should be feasible.

Sometimes adding new tests can slow down CI, which in principle could decrease the velocity at which PRs are integrated. But I think this is more of an issue for Windows--the Windows jobs tend to be the ones that take the longest. Even then, if it's just size tests, then it shouldn't be too bad. (And at least on x86 Windows, you generally can just install and use a 32-bit target on a 64-bit system and building and running work.)

[Byron]

Thanks a lot Christoph Rüßler (@cruessler), and Eliah Kagan (@EliahKagan) for chiming in!

I took the liberty of removing the SHA-256 plan to declare 'done enough', which also means that we could bring it back based on the status quo - the plan in question was fully generated anyway, but by an older model, and not necessarily kept up to date either.

The 32bit topic I am also not tackling here, but it's certainly something that could be looked at more particularly once there is issues coming up related to it (and maybe a greater amount of embedded usage).