Re: [PATCH/RFC v2 0/16] Introduce index file format version 5

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 7, 2012, 23:26 UTC
Message-ID: <7vboimuvur.fsf@alter.siamese.dyndns.org>
In-Reply-To: <87393yz64x.fsf@thomas.inf.ethz.ch>

Thomas Rast <trast@student.ethz.ch> writes:

> I like the general idea, too, but I think there is a long way ahead, and
> we shouldn't hold up v5 on this.

We shouldn't rush, only to keep some deadline, and regret it later that we butchered the index format without thinking things through. When this was added to the GSoC idea page, I already said upfront that this was way too big a topic to be a GSoC project, didn't I?

> It would open some interesting possibilities.

It is unclear why the current format is less easier to get the same kind of enhancement compared to the proposed v5 for the same kind of "possibilities."

"This codepath currently does things this way because it is limited by the flat in-core index. That codepath does a similar thing, and that other one has the same issue. They all can benefit if we give them this API, and the implementation of the API could benefit if the underlying on-disk format is changed that way. And the other codepaths that use the current API won't be broken by the on-disk format change, as all the accesses are encapsulated with this patch without losing performance, readability nor modifiability" is very much acceptable [*1*], but "The new on-disk format is different from the current one, and as it is different from the current one, we can easily enhance it even more by hooking anything interesting to it!" does not sound like a valid argument.

Show 5 quoted lines

> For example, for v5 it
> would be far better if conflicted and resolve-undo entries were a
> property of the normal index entry, instead of something that so happens
> to be consecutive entries and in a completely different place,
> respectively.

I am not sure I am convinced. Conflicts are already expressed by an attribute on a normal index entry (it is called "stage"), and because we check for "is the index fully merged" fairly often, it makes sense to have it in each entry. Actually having an unmerged entry is a rare event (happens only during a mergy operation that gave control back to you), so we do not lose much by expressing them as consecutive entries. Resolve-undo is far less often used, and is not an essential feature, so it makes perfect sense to have it as an optional index extension to allow versions of Git that are unaware of it to still use an index file that has it.

I do not find your "For example" argument particularly convincing rationale to go to the proposed v5, even if I thought resolve-undo were one of the more important things in the index (which I don't).

[Footnote]

*1* Duy's "'ls-files $path' would benefit from a path-limited index file reader, and the function to do so would be an obvious new API that would benefit from tree-shaped on-disk format" suggestion is a design going in the right direction, as long as it is accompanied with "for the remaining users that need the whole index as a linear array, reading such a tree-shaped on-disk format can be supported without loss of performance with this patch".