From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: "brian m. carlson" <sandals@crustytoothpaste.net>,
Adam Majer <adamm@zombino.com>,
git@vger.kernel.org
Subject: Re: git clone of empty repositories doesn't preserve hash
Date: Thu, 27 Apr 2023 00:46:33 -0400 [thread overview]
Message-ID: <20230427044633.GA982277@coredump.intra.peff.net> (raw)
In-Reply-To: <xmqqcz3qwuj7.fsf@gitster.g>
On Wed, Apr 26, 2023 at 08:08:28AM -0700, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > It sounds from your description that your test is running in a mode
> > where the client defaults to sha256 (though I'm not sure how, since we
> > explicitly document that GIT_DEFAULT_HASH should not affect clone), and
> > then you clone an empty sha256 repository via v0, expecting the result
> > to be sha256.
>
> 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.
Ah, OK. The fact that we do respect GIT_DEFAULT_HASH there solves my
remaining confusion. And in retrospect it makes sense, because clone is
going to use the equivalent of "git init" under the hood. So we'll start
with _something_, and then adjust it based on what we read from the
other side. And that "something" respects GIT_DEFAULT_HASH.
Sort of a side note:
This behavior is somewhat due to the fact that clone is implemented as
"init, then fetch into the new repository". It could also conceptually
be "talk to the remote, then init, then actually fetch". It never
mattered before, but with options like object-format, the "init" step
may be affected by things the remote said.
So in our world, clone has to "fix up" parts of the repository that
were initialized by tweaking the object-format config of the live
repo. In a world where the very first thing it did was talk to the
remote, then it would pass the option along to init in the first
place.
It's probably not worth trying to re-architect clone, though (not just
in terms of work, but who knows what other subtle assumptions are
baked into the current ordering). And it's not like it _solves_ this
issue. It just might have made finding it a little less confusing.
> [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.
Exactly. We are (and always have been) wrong 50% of the time in an empty
repo for v0. Your recent patch fixed v2 in an empty repo, but in doing
so flipped which halves of v0 were wrong.
-Peff
next prev parent reply other threads:[~2023-04-27 4:46 UTC|newest]
Thread overview: 58+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-04-05 10:28 git clone of empty repositories doesn't preserve hash Adam Majer
2023-04-05 19:04 ` Junio C Hamano
2023-04-05 19:47 ` Adam Majer
2023-04-05 20:01 ` Jeff King
2023-04-05 20:40 ` Junio C Hamano
2023-04-05 21:15 ` Junio C Hamano
2023-04-05 21:26 ` Jeff King
2023-04-05 22:48 ` brian m. carlson
2023-04-06 13:11 ` Adam Majer
2023-04-25 21:35 ` brian m. carlson
2023-04-25 22:24 ` Junio C Hamano
2023-04-25 23:12 ` Junio C Hamano
2023-04-26 0:20 ` brian m. carlson
2023-04-26 11:25 ` Jeff King
2023-04-26 15:08 ` Junio C Hamano
2023-04-26 15:13 ` [PATCH] doc: GIT_DEFAULT_HASH is and will be ignored during "clone" Junio C Hamano
2023-04-26 21:06 ` brian m. carlson
2023-04-27 4:46 ` Jeff King [this message]
2023-04-26 10:51 ` git clone of empty repositories doesn't preserve hash Jeff King
2023-04-26 15:42 ` Junio C Hamano
2023-04-26 20:40 ` brian m. carlson
2023-04-26 20:53 ` [PATCH 0/2] Fix empty SHA-256 clones with v0 and v1 brian m. carlson
2023-04-26 20:53 ` [PATCH 1/2] http: advertise capabilities when cloning empty repos brian m. carlson
2023-04-26 21:14 ` Junio C Hamano
2023-04-26 21:28 ` brian m. carlson
2023-04-27 5:00 ` Jeff King
2023-04-27 5:30 ` Jeff King
2023-04-27 20:40 ` Junio C Hamano
2023-04-26 20:53 ` [PATCH 2/2] Honor GIT_DEFAULT_HASH for empty clones without remote algo brian m. carlson
2023-04-26 21:18 ` Junio C Hamano
2023-04-26 21:33 ` Junio C Hamano
2023-04-27 5:43 ` Jeff King
2023-05-02 23:46 ` Is GIT_DEFAULT_HASH flawed? Felipe Contreras
2023-05-03 9:03 ` Adam Majer
2023-05-03 15:44 ` Felipe Contreras
2023-05-03 17:21 ` Adam Majer
2023-05-08 0:34 ` Felipe Contreras
2023-05-03 9:09 ` demerphq
2023-05-03 18:20 ` Felipe Contreras
2023-05-03 22:54 ` brian m. carlson
2023-05-08 2:00 ` Felipe Contreras
2023-05-08 21:38 ` brian m. carlson
2023-05-09 10:32 ` Oswald Buddenhagen
2023-05-09 16:47 ` Junio C Hamano
2023-04-26 21:12 ` [PATCH 0/2] Fix empty SHA-256 clones with v0 and v1 Junio C Hamano
2023-04-27 4:56 ` git clone of empty repositories doesn't preserve hash Jeff King
2023-05-01 17:00 ` [PATCH v2 0/1] Fix empty SHA-256 clones with v0 and v1 brian m. carlson
2023-05-01 17:00 ` [PATCH v2 1/1] upload-pack: advertise capabilities when cloning empty repos brian m. carlson
2023-05-01 22:40 ` Jeff King
2023-05-01 22:51 ` Junio C Hamano
2023-05-01 17:37 ` [PATCH v2 0/1] Fix empty SHA-256 clones with v0 and v1 Junio C Hamano
2023-05-17 19:24 ` [PATCH v3 " brian m. carlson
2023-05-17 19:24 ` [PATCH v3 1/1] upload-pack: advertise capabilities when cloning empty repos brian m. carlson
2023-05-17 21:48 ` [PATCH v3 0/1] Fix empty SHA-256 clones with v0 and v1 Junio C Hamano
2023-05-17 22:28 ` brian m. carlson
2023-05-18 18:28 ` Jeff King
2023-05-19 15:32 ` brian m. carlson
2023-04-05 21:23 ` git clone of empty repositories doesn't preserve hash Jeff King
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=20230427044633.GA982277@coredump.intra.peff.net \
--to=peff@peff.net \
--cc=adamm@zombino.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--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 read-only IMAP folder(s) and NNTP newsgroup(s).