On 2020-05-17 at 18:16:37, Martin Ågren wrote:
> On Sat, 16 May 2020 at 22:47, brian m. carlson
> <sandals@crustytoothpaste.net> wrote:
> >  That is valid without --stdin, if uncommon, and it needs to be
> > supported.  I can prevent it from being used with --stdin, though.
> 
> Hmm, that might make sense. I suppose it could quickly get out of
> control with bug reports coming in along the lines of "if I do this
> really crazy git index-pack invocation, I manage to mess things up". The
> easiest way to address this might be through documentation, i.e., "don't
> use this option", "for internal use" or even "to be used by the test
> suite only" for which there is even precedence in git-index-pack(1).
> 
> On the other hand, if we need to detect such hash mismatch even once the
> SHA-256 work is 100% complete, then I suppose we really should try a
> bit to catch bad invocations.

I can add documentation and a warning there.

If we actually verified the checksum at the end of the pack first, then
we'd be able to distinguish the two cases, because we'd try to compute a
clearly invalid hash over the body, and the likelihood of it matching
would be very small.  We don't at the moment, for reasons I'm unclear
about, but it's probably performance.

> As a tangent, I see that v2.27.0 will come with `git init
> --object-format=<format>` and `GIT_DEFAULT_HASH_ALGORITHM`. The docs for
> the former mentions "(if enabled)". Should we add something more scary
> to those to make it clear that they shouldn't be used and that you
> basically shouldn't even try to figure out how to enable them? I can
> already see the tweets and blog posts a few weeks from now about how you
> can build Git from source setting a single switch, run
> 
>   git init --object-format=sha256
> 
> and you're in the future! Which will just lead to pain some days or
> weeks later.... "I've done lots of work. How do I convert my repo to
> SHA-1 so I can share it?"...
> 
> We've added "experimental" things before and tried to document the
> experimental nature. Maybe here we're not even "experimental" -- more
> like "if you use this in production, you *will* suffer"?

Well, the option is there, but it produces the following:

  % git init --object-format=sha256
  fatal: The hash algorithm sha256 is not supported in this build.

which can be distinguished from this:

  % git init --object-format=blake2b
  fatal: unknown hash algorithm 'blake2b'

Right now it's pretty broken without this series, so you can't use it.
I mean, you have the source and can remove the check, but it doesn't
work as it stands, so I'm not too worried about people trying to do that
at the moment.  I'll sneak in some documentation for the end product,
though.
-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204