Re: [PATCH v2 13/13] commit-graph: specify OID version for SHA-256

From: Duy Nguyen <pclouds@gmail.com>
To: "brian m. carlson" <sandals@crustytoothpaste.net>,
	"Derrick Stolee" <stolee@gmail.com>,
	"Git Mailing List" <git@vger.kernel.org>,
	"Jeff King" <peff@peff.net>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Subject: Re: [PATCH v2 13/13] commit-graph: specify OID version for SHA-256
Date: Wed, 17 Oct 2018 16:31:19 +0200	[thread overview]
Message-ID: <CACsJy8C9nySzG0Uqs6_5V2rXHkAKr-ShXU7N9sPp4N5z=D0zQA@mail.gmail.com> (raw)
In-Reply-To: <20181016224404.GD432229@genre.crustytoothpaste.net>

On Wed, Oct 17, 2018 at 12:44 AM brian m. carlson
<sandals@crustytoothpaste.net> wrote:
> > > >>   static uint8_t oid_version(void)
> > > >>   {
> > > >> -       return 1;
> > > >> +       switch (hash_algo_by_ptr(the_hash_algo)) {
> > > >> +               case GIT_HASH_SHA1:
> > > >> +                       return 1;
> > > >> +               case GIT_HASH_SHA256:
> > > >> +                       return 2;
> > > > Should we just increase this field to uint32_t and store format_id
> > > > instead? That will keep oid version unique in all data formats.
> > > Both the commit-graph and multi-pack-index store a single byte for the
> > > hash version, so that ship has sailed (without incrementing the full
> > > file version number in each format).
> >
> > And it's probably premature to add the oid version field when multiple
> > hash support has not been fully realized. Now we have different ways
> > of storing hash id and need separate mappings.
>
> Honestly, anything in the .git directory that is not the v3 pack indexes
> or the loose object file should be in exactly one hash algorithm.  We
> could simply just leave this value at 1 all the time and ignore the
> field, since we already know what algorithm it will use.

In this particular case, I agree, but not as a general principle. It's
nice to have independence for fsck-like tools. I don't know if we have
a tool that simply validates commit-graph file format (and not trying
to access any real object). But for such a tool, I guess we can just
pass the hash algorithm from command line. The user would have to
guess a bit.
-- 
Duy