Re: [PATCH 0/7] clone: fix init of refdb with wrong object format
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Dec 11, 2023, 14:57 UTC
- Message-ID
- <xmqqmsugvlbe.fsf@gitster.g>
- In-Reply-To
- <ZXbzzlyWC3HTUyDA@tanuki>
Patrick Steinhardt <ps@pks.im> writes:
Show 12 quoted lines
>> 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.