Re: [PATCH v2 1/1] setup: recognise extensions.objectFormat
On 30/01/2018 02:38, Jeff King wrote:
Show 18 quoted lines
> 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 = <value>
>>
>> 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?).
Show 5 quoted lines
> 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.
Show 6 quoted lines
> 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.
Show 6 quoted lines
> 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
Patryk Obara
