Re: [PATCH v2 1/1] setup: recognise extensions.objectFormat

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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 = <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?).

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



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux