Re: [PATCH 2/3] ieot: default to not writing IEOT section

On 11/13/2018 1:18 PM, Jonathan Nieder wrote:

Hi,

Ben Peart wrote:

quoted

On 11/12/2018 7:39 PM, Jonathan Nieder wrote:

quoted

quoted

As with EOIE, popular versions of Git do not support the new IEOT
extension yet.  When accessing a Git repository written by a more
modern version of Git, they correctly ignore the unrecognized section,
but in the process they loudly warn

	ignoring IEOT extension

resulting in confusion for users.  Introduce the index extension more
gently by not writing it yet in this first version with support for
it.

[...]

quoted

quoted

Introduce a '[index] recordOffsetTable' configuration variable to
control whether the new index extension is written.

Why introduce a new setting to disable writing the IEOT extension instead of
just using the existing index.threads setting?  If index.threads=1 then the
IEOT extension isn't written which (I believe) will accomplish the same
goal.

Do you mean defaulting to index.threads=1?  I don't think that would
be a good default, but if you have a different change in mind then I'd
be happy to hear it.

Or do you mean that if the user has explicitly specified index.threads=true,
then that should imply index.recordOffsetTable=true so users only have
to set one setting to turn it on?  I can imagine that working well.

Reading the index with multiple threads requires the EOIE and IEOT extensions to exist in the index.  If either extension doesn't exist, then the code falls back to the single threaded path.  That means you can't have both 1) no warning for old versions of git and 2) multi-threaded reading for new versions of git.

If you set index.threads=1, that will prevent the IEOT extension from being written and there will be no "ignoring IEOT extension" warning in older versions of git.

With this patch 'as is' you would have to set both index.threads=true and index.recordOffsetTable=true to get multi-threaded index reads.  If either is set to false, it will silently drop back to single threaded reads.

Thanks,
Jonathan