Subject: Re: [PATCH 0/7] clone: fix init of refdb with wrong object format

From: Junio C Hamano <[email-masked]>
Date: Mon, 11 Dec 2023 06:57:25 -0800

Patrick Steinhardt <[email-masked]> writes:

>> An existing test
>> 
>>     $ make && cd t && GIT_TEST_DEFAULT_HASH=sha256 sh t5550-http-fetch-dumb.sh
>> 
>> passes with vanilla Git 2.43, but with these patches applied, it
>> fails the "7 - empty dumb HTTP" step.
> ...
> Before my refactorings we used to fall back to the local default hash
> format with which we've already initialized the repository, which is
> wrong. Now we use the hash format we detected via the remote, which we
> cannot detect because the remote is empty and does not advertise the
> hash function, so we fall back to SHA1 and thus also do the wrong thing.

Yeah, that is why I did *not* say "the series *breaks* existing
tests".  It triggers a failure, and in this case, a test failure
does not mean the behaviour is broken because there is no correct
answer.  ... oh, wait.  There isn't?

I wonder if the right thing to do is to advertise the hash function
even from an absolutely empty repository.  There are no objects in
such a repository, but it should already know what hash function to
use when it creates its first object (determined at the repository
creation time), so that _could_ be advertised.  

> The only correct thing here would be to use the actual hash function
> that the remote repository uses, but we have no to do so.

We have "no way to do so"?  We have "not done so"?

It is possible for the client side to download the $GIT_DIR/config
file from the remote to learn what value extensions.objectFormat is
in use over there instead, I think, but at the same time, I highly
suspect that dumb HTTP outlived its usefulness to warrant such an
additional investment of engineering resource.

The simplest "fix" might be to leave what happens in this narrow
case (i.e. cloning over dumb HTTP from an empty repository)
undefined by not testing (or not insisting on one particular
outcome), but ...

> +Cc brian, as he's the author of [2].

... of course I trust Brian more than I trust myself in this area ;-)

> Patrick
>
> [1]: https://gitlab.com/gitlab-org/git/-/jobs/5723052108
> [2]: ac093d0790 (remote-curl: detect algorithm for dumb HTTP by size, 2020-06-19)

Thanks.