git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Patrick Steinhardt <ps@pks.im>
Cc: git@vger.kernel.org,  "brian m. carlson" <sandals@crustytoothpaste.net>
Subject: Re: [PATCH 0/7] clone: fix init of refdb with wrong object format
Date: Mon, 11 Dec 2023 06:57:25 -0800	[thread overview]
Message-ID: <xmqqmsugvlbe.fsf@gitster.g> (raw)
In-Reply-To: <ZXbzzlyWC3HTUyDA@tanuki> (Patrick Steinhardt's message of "Mon, 11 Dec 2023 12:34:38 +0100")

Patrick Steinhardt <ps@pks.im> 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.

  reply	other threads:[~2023-12-11 14:57 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-12-06 12:39 [PATCH 0/7] clone: fix init of refdb with wrong object format Patrick Steinhardt
2023-12-06 12:39 ` [PATCH 1/7] setup: extract function to create the refdb Patrick Steinhardt
2023-12-06 21:10   ` Karthik Nayak
2023-12-07  7:22     ` Patrick Steinhardt
2023-12-08 22:54       ` Junio C Hamano
2023-12-11 11:34         ` Patrick Steinhardt
2023-12-11 15:50       ` Karthik Nayak
2023-12-06 12:39 ` [PATCH 2/7] setup: allow skipping creation of " Patrick Steinhardt
2023-12-06 12:39 ` [PATCH 3/7] remote-curl: rediscover repository when fetching refs Patrick Steinhardt
2023-12-08 23:09   ` Junio C Hamano
2023-12-11 11:35     ` Patrick Steinhardt
2023-12-06 12:40 ` [PATCH 4/7] builtin/clone: fix bundle URIs with mismatching object formats Patrick Steinhardt
2023-12-06 21:13   ` Karthik Nayak
2023-12-07  7:22     ` Patrick Steinhardt
2023-12-08 23:11   ` Junio C Hamano
2023-12-06 12:40 ` [PATCH 5/7] builtin/clone: set up sparse checkout later Patrick Steinhardt
2023-12-06 12:40 ` [PATCH 6/7] builtin/clone: skip reading HEAD when retrieving remote Patrick Steinhardt
2023-12-06 12:40 ` [PATCH 7/7] builtin/clone: create the refdb with the correct object format Patrick Steinhardt
2023-12-06 13:09 ` [PATCH 0/7] clone: fix init of refdb with wrong " Patrick Steinhardt
2023-12-10  3:16 ` Junio C Hamano
2023-12-11 11:34   ` Patrick Steinhardt
2023-12-11 14:57     ` Junio C Hamano [this message]
2023-12-11 15:32       ` Patrick Steinhardt
2023-12-11 22:17         ` brian m. carlson
2023-12-12  7:00 ` [PATCH v2 " Patrick Steinhardt
2023-12-12  7:00   ` [PATCH v2 1/7] setup: extract function to create the refdb Patrick Steinhardt
2023-12-12  7:00   ` [PATCH v2 2/7] setup: allow skipping creation of " Patrick Steinhardt
2023-12-12  7:00   ` [PATCH v2 3/7] remote-curl: rediscover repository when fetching refs Patrick Steinhardt
2023-12-12  7:00   ` [PATCH v2 4/7] builtin/clone: fix bundle URIs with mismatching object formats Patrick Steinhardt
2023-12-12  7:00   ` [PATCH v2 5/7] builtin/clone: set up sparse checkout later Patrick Steinhardt
2023-12-12  7:01   ` [PATCH v2 6/7] builtin/clone: skip reading HEAD when retrieving remote Patrick Steinhardt
2023-12-12  7:01   ` [PATCH v2 7/7] builtin/clone: create the refdb with the correct object format Patrick Steinhardt
2023-12-12 19:20   ` [PATCH v2 0/7] clone: fix init of refdb with wrong " Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqmsugvlbe.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=ps@pks.im \
    --cc=sandals@crustytoothpaste.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).