Re: git clone of empty repositories doesn't preserve hash

Jeff King <peff@peff.net> writes:

Thanks for coming up with an excellent guess that helped come up with a reproduction.

With Brian's patch a bit tweaked (attached below), the test does fail with the current 'master' and passes before the merge in question. And the trace clearly shows that without being told anything about the object format via the capability, the client chooses to honor GIT_DEFAULT_HASH to initialize the new repository with sha256.

There is this entry in the release notes of 2.29.0:

 * "git clone" that clones from SHA-1 repository, while
   GIT_DEFAULT_HASH set to use SHA-256 already, resulted in an
   unusable repository that half-claims to be SHA-256 repository
   with SHA-1 objects and refs.  This has been corrected.

This refers to 47ac9703 (builtin/clone: avoid failure with GIT_DEFAULT_HASH, 2020-09-20). It seems that the "fix" described there was incomplete and did not address "clone a void" case, and the patch under discussion finished the incomplete fix without knowing by accident. The test that was added by 47ac9703 to t5601 does use non-empty repositories (one with SHA-1, the other with SHA-256) and tries to check the interaction with "clone" with the environment variable. With the patch under discussion, this test did not break and "clone" is still ignoring the GIT_DEFAULT_HASH variable.

Also the description in the document of GIT_DEFAULT_HASH makes readers ambivalent:

`GIT_DEFAULT_HASH`::
	If this variable is set, the default hash algorithm for new
	repositories will be set to this value. This value is currently
	ignored when cloning; the setting of the remote repository
	is used instead. The default is "sha1". THIS VARIABLE IS
	EXPERIMENTAL! See `--object-format` in linkgit:git-init[1].

To me, "is currently ignored" hints that the author, or somebody close to the author, of this entry feels that it is a bug that "clone" does not honor it. The log message of 47ac9703 also phrases "failed to honor the GIT_DEFAULT_HASH" as if it were a bad thing [*].

On the other hand, it is clearly documented as experimental so anything that has been relying on the behaviour of any command with a particular value set to this variable are waiting to be broken.

I am torn about this. Even though we may have been very clear that GIT_DEFAULT_HASH should not kick in in this case, "clone" was buggy (in the sense that it did not behave as documented in an empty repository) and whatever thing that changed behaviour that Brian noticed was ignoring the documentation and taking advantage of that "bug", and Hyrum's law ensues.

In the longer term, after/when we allow incremental/over-the-wire migration of object-format, i.e. clone from SHA-1 repository to create SHA-256 repository (or vice versa) and fetching and pushing between them would bidirectionally convert the object format on the fly, it is likely we would teach a new option "--object-format" to "git clone" to say "you would use whatever object format the origin uses by default, but this time, I am telling you to use this format on our side, doing on-the-fly object format conversion as needed".

So it would be OK to make sure that GIT_DEFAULT_HASH is ignored by "clone" as documented now, and even after on-the-fly object-format migration is implemented, I would think.

[Footnote]

* ... but I think it was misguided.  What it says there is this:

    And we also don't want to initialize the repository as SHA-1
    initially, since that means if we're cloning an empty
    repository, we'll have failed to honor the GIT_DEFAULT_HASH
    variable and will end up with a SHA-1 repository, not a SHA-256
    repository.

If this were "When we're cloning an empty repository, we'd have failed to honor the object format the other side has chosen and will end up with a SHA-1 repository, not a SHA-256 repository.", then it is very much in line with the reality before the patch under discussion and also in line with the official stance that "clone" should not honor GIT_DEFAULT_HASH.

Where the original description breaks down is when the other side is SHA-1 and this side has GIT_DEFAULT_HASH set to SHA-256. If we honored the variable, we'd create a SHA-256 repository that will talk to SHA-1 repository before the rest of the system is ready.

 t/t5700-protocol-v1.sh | 13 +++++++++++++
 1 file changed, 13 insertions(+)