Thanks for the PR 👀

I made a few minor changes - I wanted to make sure that we supported FIPS mode on SHA1 as well, to ensure that we had more test coverage. Doing so actually unearthed a problem in the MIDX and commit graph code where we were finalizing the hash context and then trying to update it. For most of the hashes that's actually okay, but not for the new FIPS compliant hash.

If you wouldn't mind taking a look at your leisure to make sure that everything makes sense. 🙏

LGTM!

Great; thanks again!

I have only one question about the change a7ca589

In particular, the way you set NULL

	hash_cb_data.ctx = NULL;

	error = write_cb((char *)checksum, checksum_size, cb_data);
	if (error < 0)
		goto cleanup;
cleanup:
	git_array_clear(object_entries_array);
	git_vector_dispose(&object_entries);
	git_str_dispose(&packfile_names);
	git_str_dispose(&oid_lookup);
	git_str_dispose(&object_offsets);
	git_str_dispose(&object_large_offsets);
	git_hash_ctx_cleanup(&ctx);
	return error;

is there any reason why we don't do hash_cb_data.ctx = NULL inside the cleanup label? If "it doesn't matter" when we do it, maybe for consistency we should consider to move it there?

It does matter; we need to clear it before the write callback. If it's null, the write callback will not try to calculate the hash, it will just do the write.

This is important in the final step, which is writing the calculated hash. We need to not try to add the hash itself to the calculate of the hash.

(In fact we've already finalized the hash context. In all the other hash implementations, it's okay to continue asking it to hash despite finalization, that's no longer true in the FIPS style routines.)

I see, thanks a lot for your explanation!

Happily, it was a very good question that should have been better explained in the code, commit message, or PR.