git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Patrick Steinhardt <ps@pks.im>
To: "brian m. carlson" <sandals@crustytoothpaste.net>,
	Junio C Hamano <gitster@pobox.com>, Ilya K <me@0upti.me>,
	git@vger.kernel.org
Subject: Re: git 2.46.0 crashes when trying to verify-pack outside of a repo
Date: Mon, 2 Sep 2024 15:47:55 +0200	[thread overview]
Message-ID: <ZtXCBcn6WZIHr65b@tanuki> (raw)
In-Reply-To: <ZtW7LtQEobPpVB99@tapette.crustytoothpaste.net>

On Mon, Sep 02, 2024 at 01:18:38PM +0000, brian m. carlson wrote:
> On 2024-09-01 at 23:45:43, Patrick Steinhardt wrote:
> > So we basically have three different options:
> > 
> >   - Accept that we just don't handle this case correctly and let the
> >     code error out. This pessimizes all hashes but SHA256.
> > 
> >   - Bail out when outside of a repository when `--object-format=` wasn't
> >     given. This pessimizes all hashes, but gives a clear indicator to
> >     the user why things don't work.
> 
> This is what I would recommend.

I'm also leaning into this direction. I want us to move to a world where
SHA1 and SHA256 are equal citizens. We won't ever get folks to move on
to SHA256 repositories when it continues to be an afterthought that
behaves worse than SHA1 repositories.

That's basically where this whole exercise to stop setting the default
hash came from. GitLab nowadays has support for SHA256 repositories, so
I want to ensure that it can be used as a drop-in replacement for SHA1
in basically all ways.

> >   - Introduce packfiles v3 and encode the object format into the header.
> >     Then do either (1) or (2) on top.
> 
> I think we have pack v3 already (which is the same as v2), and v4 was
> for an experimental format that never landed fully.  Maybe v5?

Ah, fair enough.

> If you wanted to do this, you could add support for arbitrary chunks,
> like with multi-pack indexes, that would allow for extensibility in the
> future.  However, you'd also need some protocol capabilities if you
> want to send pack v5 or certain chunks over the protocol.
> 
> > The last option is of course the cleanest, but also the most involved.
> 
> I'd personally recommend just requiring the `--object-format=` option,
> but of course if you want to write pack v5, don't let me stop you.

Well, in the context of this issue I'd definitely aim for the easier fix
first. Regardless of whether or not we introduce v5, we'd still have to
address the underlying issue for repositories that do not (yet) have v5
packfiles.

Patrick

  reply	other threads:[~2024-09-02 13:47 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-08-31  6:46 git 2.46.0 crashes when trying to verify-pack outside of a repo Ilya K
2024-09-01 15:26 ` Junio C Hamano
2024-09-01 23:45   ` Patrick Steinhardt
2024-09-02 13:18     ` brian m. carlson
2024-09-02 13:47       ` Patrick Steinhardt [this message]
2024-09-03 15:52         ` Junio C Hamano
2024-09-04  6:26 ` [PATCH] builtin/index-pack: fix segfaults when running " Patrick Steinhardt

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=ZtXCBcn6WZIHr65b@tanuki \
    --to=ps@pks.im \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=me@0upti.me \
    --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).