From: Patryk Obara Date: Tue, 30 Jan 2018 16:30:04 GMT Subject: Re: [PATCH v2 1/1] setup: recognise extensions.objectFormat Message-ID: In-Reply-To: <20180130013759.GA27694@sigill.intra.peff.net> On 30/01/2018 02:38, Jeff King wrote: > On Sun, Jan 28, 2018 at 01:36:17AM +0100, Patryk Obara wrote: > >> This extension selects which hashing algorithm from vtable should be >> used for reading and writing objects in the object store. At the moment >> supports only single value (sha-1). >> >> In case value of objectFormat is an unknown hashing algorithm, Git >> command will fail with following message: >> >> fatal: unknown repository extensions found: >> objectformat = >> >> To indicate, that this specific objectFormat value is not recognised. > > I don't have a strong opinion on this, but it does feel a little funny > to add this extension now, before we quite know what the code that uses > it is going to look like (or maybe we're farther along there than I > realize). Code using this is already in master - in the result of overwriting data->hash_algo, every piece of code, that was modernised starts using the selected hash algorithm (through the_hash_algo) instead of hardcoded sha-1. As far as I can tell status is following: Once following topics will land: - po/object-id (in pu) - brian's object-id-part-11 (in review now) - upcoming brian's object-id-part-12 (not sent to mailing list yet) - few more object-id conversions and uses of the_hash_algo we'll be in state, where just dropping new entry in hash_algos table will enable experimental switching of object format. With changes listed above "git hash-object -w -t blob" and "git cat-file" work with NewHash (whatever it may be - brian is using blake2 in his experiments, I am using openssl sha3-256). Right now I am looking at updating index structures and functions - after which git commit should work. In the transition plan it's described as "introducing index v3" (are there any new requirements, that constitute "v3" besides longer hash?). > What do we gain by doing this now as opposed to later? By the design of > the extension code, we should complain on older versions anyway. And by > doing it now we carry a small risk that it might not give us the > interface we want, and it will be slightly harder to paper over this > failed direction. Mostly convenience for developers, who want to work on transition. There's no need to re-compile only for changing default hashing algorithm (which is useful for testing and debugging). I could carry this patch around to every NewHash-related branch, that I work on but it's annoying me already ;) As long as hash_algos table contains only sha-1, users effectively see this extension as noop. > It wasn't intended that anyone would specify preciousObjects with repo > version 0. It's a dangerous misconfiguration (because versions which > predate the extensions mechanism won't actually respect it at all!). > > So we probably ought to complain loudly on having anything in > extensions.* when the repositoryformat is less than 1. > > I originally wrote it the other way out of an abundance of > backward-compatibility. After all "extension.*" doesn't mean anything in > format 0, and somebody _could_ have added such a config key for their > own purposes. But that's a pretty weak argument, and if we are going to > start marking some extensions as forbidden there, we might as well do > them all. What about users, who are using new version of Git, but have it misconfigured with preciousObjects and repo format 0? That's why I decided to make repo format check specific to objectFormat extension (initially I made it generic to all extensions). At the same time... there's extension.partialclone in pu and it does not have check on repo format. -- | ← Ceci n'est pas une pipe Patryk Obara