On 2020-05-16 at 11:18:12, Martin Ågren wrote:
> On Wed, 13 May 2020 at 02:56, brian m. carlson
> <sandals@crustytoothpaste.net> wrote:
> >
> > git index-pack is usually run in a repository, but need not be. Since
> > packs don't contains information on the algorithm in use, instead
> > relying on context, add an option to index-pack to tell it which one
> > we're using in case someone runs it outside of a repository.
> >
> > Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> > ---
> >  builtin/index-pack.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/builtin/index-pack.c b/builtin/index-pack.c
> > index 7bea1fba52..89f4962a00 100644
> > --- a/builtin/index-pack.c
> > +++ b/builtin/index-pack.c
> > @@ -1760,6 +1760,11 @@ int cmd_index_pack(int argc, const char **argv, const char *prefix)
> >                                         die(_("bad %s"), arg);
> >                         } else if (skip_prefix(arg, "--max-input-size=", &arg)) {
> >                                 max_input_size = strtoumax(arg, NULL, 10);
> > +                       } else if (skip_prefix(arg, "--object-format=", &arg)) {
> > +                               int hash_algo = hash_algo_by_name(arg);
> > +                               if (hash_algo == GIT_HASH_UNKNOWN)
> > +                                       die(_("unknown hash algorithm '%s'"), arg);
> > +                               repo_set_hash_algo(the_repository, hash_algo);
> >                         } else
> 
> Patch 27 added `--hash` to `git show-index` and I almost commented on
> "hash" vs "object-format". In the end I figured the object format was a
> more technical (protocol) term. But now I wonder. Should we try to align
> such options from the start? Or is there perhaps a reason for those
> different approaches?

I'll bring them into sync.

> Similar to an earlier patch where we modify `the_hash_algo` like this, I
> feel a bit nervous. What happens if you pass in a "wrong" algo here,
> i.e., SHA-1 in a SHA-256 repo? Or, given the motivation in the commit
> message, should this only be allowed if we really *are* outside a repo?

Unfortunately, we can't prevent the user from being inside repository A,
which is SHA-1, while invoking git index-pack on repository B, which is
SHA-256.  That is valid without --stdin, if uncommon, and it needs to be
supported.  I can prevent it from being used with --stdin, though.

If you pass in a wrong algorithm, we usually blow up with an inflate
error because we consume more bytes than expected with our ref deltas.
I'm not aware of any cases where we segfault or access invalid memory;
we just blow up in a nonobvious way.  That's true, too, if you manually
tamper with the algorithm in extensions.objectformat; usually we blow up
(but not segfault) because the index is "corrupt".
-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204