From: Jeff King <peff@peff.net>
To: "brian m. carlson" <sandals@crustytoothpaste.net>
Cc: Junio C Hamano <gitster@pobox.com>,
Adam Majer <adamm@zombino.com>,
git@vger.kernel.org
Subject: Re: git clone of empty repositories doesn't preserve hash
Date: Wed, 26 Apr 2023 07:25:08 -0400 [thread overview]
Message-ID: <20230426112508.GB130148@coredump.intra.peff.net> (raw)
In-Reply-To: <ZEhuMML6n8F+cNLg@tapette.crustytoothpaste.net>
On Wed, Apr 26, 2023 at 12:20:00AM +0000, brian m. carlson wrote:
> In my case, the clone is over HTTP, so this may not be the ideal way to
> reproduce it and it may need a better testcase, but it does bisect to
> the patch above and it is new in master (and doesn't reproduce in
> 2.40.0). Note that in our case in the Git LFS testsuite, we're using
> GIT_DEFAULT_HASH=sha256.
>
> I believe what is happening is that for some reason, the object-format
> data in v0 and v1 is not being read properly, and so we're now setting
> it to sha1 whereas before we were reading the value from the default
> setting of the repository (sha256).
I'm having trouble finding any breakage at all. E.g., this test passes:
diff --git a/t/t5551-http-fetch-smart.sh b/t/t5551-http-fetch-smart.sh
index 0908534f25..95b10288e7 100755
--- a/t/t5551-http-fetch-smart.sh
+++ b/t/t5551-http-fetch-smart.sh
@@ -704,4 +704,24 @@ test_expect_success 'no empty path components' '
! grep "//" log
'
+test_expect_success 'v0 clone over http recognizes object-format' '
+ git init --bare --object-format=sha256 \
+ "$HTTPD_DOCUMENT_ROOT_PATH/sha256.git" &&
+
+ # do not test an empty repo. In v0, we have no way for an
+ # empty server to report its object format, so we would
+ # always default to sha1. We could in theory test that
+ # a client who wants to default to sha256 will realize
+ # the other side is sha1, but we have no way to set that local
+ # default. Unlike git-init, git-clone does not support
+ # --object-format, nor GIT_DEFAULT_HASH.
+ git -C "$HTTPD_DOCUMENT_ROOT_PATH/sha256.git" --work-tree=. \
+ commit --allow-empty -m foo &&
+
+ git -c protocol.version=0 clone $HTTPD_URL/smart/sha256.git sha256 &&
+ git -C sha256 rev-parse --show-object-format >actual &&
+ echo sha256 >expect &&
+ test_cmp expect actual
+'
+
test_done
I'd expect it to break in the empty-repo case, for the reasons given in
the comment (v0 cannot communicate object-format in an empty repo). But
that is nothing new.
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.
But I think that is a wrong expectation, at least from the
client's perspective. An empty repository cannot communicate its
object-format over v0, so the client should assume its v0, and should
then itself become v0. And that last "should itself become" is what
Junio's patch fixed.
The first part, "empty repository cannot communicate its object-format
over v0" is the part is "it's always been broken". We could fix it, but
I'm not sure if it is worth the trouble (see my other message).
> It very well may be that it's always been broken and this has just made
> it obvious that it's broken, but I'll look tomorrow and probably send a
> patch. I don't think we should revert this change, but I do think we
> need to fix it before 2.41, since I think it means right now that all
> clones over protocol v0 and v1 end up with a SHA-1 repository.
Hopefully my guess at what your test is doing is correct, and I didn't
just leave us off on a tangent. ;)
But if it is, then I think that everything in Git is OK. Non-empty repos
over v0 work correctly both before and after Junio's patch. Empty ones
before his patch were erroneously using sha256 if they preferred it
locally, even when the other side really was sha1. They _also_ were
using sha256 erroneously when the other side was sha256 but wasn't able
to report it (because of v0 limitations). Which is counter-intuitive,
perhaps, but was still the wrong thing for a client to do.
-Peff
next prev parent reply other threads:[~2023-04-26 11:25 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 [this message]
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 ` git clone of empty repositories doesn't preserve hash Jeff King
2023-04-26 10:51 ` 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=20230426112508.GB130148@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).