Git Mailing List Archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: "brian m. carlson" <sandals@crustytoothpaste.net>,
	git@vger.kernel.org, Adam Majer <adamm@zombino.com>
Subject: Re: [PATCH 2/2] Honor GIT_DEFAULT_HASH for empty clones without remote algo
Date: Thu, 27 Apr 2023 01:43:43 -0400	[thread overview]
Message-ID: <20230427054343.GE982277@coredump.intra.peff.net> (raw)
In-Reply-To: <xmqqbkjaqqfp.fsf@gitster.g>

On Wed, Apr 26, 2023 at 02:33:30PM -0700, Junio C Hamano wrote:

> "brian m. carlson" <sandals@crustytoothpaste.net> writes:
> 
> >  `GIT_DEFAULT_HASH`::
> >  	If this variable is set, the default hash algorithm for new
> >  	repositories will be set to this value. This value is currently
> > +	ignored when cloning if the remote value can be definitively
> > +	determined; the setting of the remote repository is used
> > +	instead. The value is honored if the remote repository's
> > +	algorithm cannot be determined, such as some cases when
> > +	the remote repository is empty. The default is "sha1".
> > +	THIS VARIABLE IS EXPERIMENTAL! See `--object-format`
> > +	in linkgit:git-init[1].
> 
> We'd need to evantually cover all the transports (and non-transport
> like the "--local" optimization) so that the object-format and other
> choices are communicated from the origin to a new clone anyway, so
> this extra complexity "until X is fixed, it behaves this way, but
> otherwise the variable is read in the meantime" may be a disservice
> to the end users, even though it may make it easier in the shorter
> term for maintainers of programs that rely on the buggy "git clone"
> that partially honored this environment variable.
> 
> In short, I am still not convinced that the above is a good design
> choice in the longer term.

I also think it is working against the backwards-compatible design of
the hash function transition. If we do not see an object-format line
from the remote, then either:

  1. They sent us capabilities, but it did not include object-format. So
     if we are in GIT_DEFAULT_HASH=sha256 mode locally, but the other
     side is an older version of Git (or even a current version of other
     implementations, like Dulwich) that do not send object-format at
     all, then we will not correctly fall back to assuming they are
     sha1. In a non-empty repo, this means we'll fail to parse their ref
     advertisement (we'll expect sha256 hashes but get sha1), and
     cloning will be broken.

  2. They did not send us capabilities, because the repo is empty (and
     the server does not have brian's patch 1). The hash transition doc
     says we're supposed to assume they're sha1. It's _sort of_
     academic, in that they also are not telling us about any refs on
     their side. But we may end up mis-matched with the server (again,
     this is the 50/50 thing; we don't know what their format is).
     Presumably that bites us later when we try to push up new objects
     (but would eventually work when we support interop).

I think handling (2) is iffy as a goal, but the collateral damage of (1)
is a complete show-stopper for this patch. If we wanted to do (2) by
itself, we'd have to distinguish "did they even send us a capabilities
line" as a separate case (but I tend to agree with you that it is not
worth doing for now).

-Peff

  reply	other threads:[~2023-04-27  5:43 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                     ` 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 [this message]
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=20230427054343.GE982277@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).