Index v5

Add an api for access to the index file. Currently there is only a very basic api for accessing the index file, which only allows a full read of the index, and lets the users of the data filter it. The new index api gives the users the possibility to use only part of the index and provides functions for iterating over and accessing cache entries.

This simplifies future improvements to the in-memory format, as changes will be concentrated on one file, instead of the whole git source code.

Signed-off-by Thomas Gummerer <[email protected]>

---
 cache.h         |  57 +++++++++++++++++++++++++++++-
 read-cache-v2.c |  96 +++++++++++++++++++++++++++++++++++++++++++++++--
 read-cache.c    | 108 ++++++++++++++++++++++++++++++++++++++++++++++++++++----
 read-cache.h    |  12 ++++++-
 4 files changed, 263 insertions(+), 10 deletions(-)

cache.h +0 -0

index 5082b34..d38dfbd 100644

Quoted 17 lines · Thomas Gummerer

Add an api for access to the index file. Currently there is only a very basic api for accessing the index file, which only allows a full read of the index, and lets the users of the data filter it. The new index api gives the users the possibility to use only part of the index and provides functions for iterating over and accessing cache entries.

This simplifies future improvements to the in-memory format, as changes will be concentrated on one file, instead of the whole git source code.

Signed-off-by Thomas Gummerer <[email protected]>

---
 cache.h         |  57 +++++++++++++++++++++++++++++-
 read-cache-v2.c |  96 +++++++++++++++++++++++++++++++++++++++++++++++--
 read-cache.c    | 108 ++++++++++++++++++++++++++++++++++++++++++++++++++++----
 read-cache.h    |  12 ++++++-
 4 files changed, 263 insertions(+), 10 deletions(-)

cache.h struct cache_entry { · line 131

Quoted 11 lines

cache.h +2 -1

index 5082b34..d38dfbd 100644
@@ -127,7 +127,8 @@ struct cache_entry {
 	unsigned int ce_flags;
 	unsigned int ce_namelen;
 	unsigned char sha1[20];
-	struct cache_entry *next;
+	struct cache_entry *next; /* used by name_hash */
+	struct cache_entry *next_ce; /* used to keep a list of cache entries */

The reader often needs to rewind the read-pointer partially while walking the index (e.g. next_cache_entry() in unpack-trees.c and how the o->cache_bottom position is used throughout the subsystem). I am not sure if this singly-linked list is a good way to go.

cache.h static inline unsigned int canon_mode(unsigned int mode) · line 269

Quoted 8 lines

cache.h +8 -0

+/*
+ * Options by which the index should be filtered when read partially.
+ *
+ * pathspec: The pathspec which the index entries have to match
+ * seen: Used to return the seen parameter from match_pathspec()
+ * max_prefix, max_prefix_len: These variables are set to the longest
+ *     common prefix, the length of the longest common prefix of the
+ *     given pathspec

These probably should use "struct pathspec" abstration, not just the "array of raw strings", no?

Quoted 4 lines

The reader often needs to rewind the read-pointer partially while walking the index (e.g. next_cache_entry() in unpack-trees.c and how the o->cache_bottom position is used throughout the subsystem). I am not sure if this singly-linked list is a good way to go.

I'm not very familiar with the unpack-trees code, but from a quick look the pointer (or position in the cache) is always only moved forward.

Quoted 7 lines · Thomas Gummerer

Quoted 4 lines

The reader often needs to rewind the read-pointer partially while walking the index (e.g. next_cache_entry() in unpack-trees.c and how the o->cache_bottom position is used throughout the subsystem). I am not sure if this singly-linked list is a good way to go.

I'm not very familiar with the unpack-trees code, but from a quick look the pointer (or position in the cache) is always only moved forward.

I am more worried about o->cache_bottom processing, where it currently is an index into an array.

With your ce->next_in_list_of_read_entries change, a natural rewrite would be to point at the ce with o->cache_bottom, but then that would mean you cannot in-place replace the entries like we used to be able to in an array based implementation.

But your series does not seem to touch unpack-trees yet, so I may be worried too much before it becomes necessary.

Add an api for access to the index file. Currently there is only a very basic api for accessing the index file, which only allows a full read of the index, and lets the users of the data filter it. The new index api gives the users the possibility to use only part of the index and provides functions for iterating over and accessing cache entries.

This simplifies future improvements to the in-memory format, as changes will be concentrated on one file, instead of the whole git source code.

Signed-off-by Thomas Gummerer <[email protected]>

---
 cache.h         |  57 +++++++++++++++++++++++++++++-
 read-cache-v2.c |  96 +++++++++++++++++++++++++++++++++++++++++++++++--
 read-cache.c    | 108 ++++++++++++++++++++++++++++++++++++++++++++++++++++----
 read-cache.h    |  12 ++++++-
 4 files changed, 263 insertions(+), 10 deletions(-)

cache.h +2 -1

index 5082b34..d38dfbd 100644
@@ -127,7 +127,8 @@ struct cache_entry {
        unsigned int ce_flags;
        unsigned int ce_namelen;
        unsigned char sha1[20];
-       struct cache_entry *next;
+       struct cache_entry *next; /* used by name_hash */
+       struct cache_entry *next_ce; /* used to keep a list of cache entries */
        char name[FLEX_ARRAY]; /* more */
 };
 

From what I read, doing

Quoted 22 lines · Thomas Gummerer

Add an api for access to the index file. Currently there is only a very basic api for accessing the index file, which only allows a full read of the index, and lets the users of the data filter it. The new index api gives the users the possibility to use only part of the index and provides functions for iterating over and accessing cache entries.

This simplifies future improvements to the in-memory format, as changes will be concentrated on one file, instead of the whole git source code.

Signed-off-by Thomas Gummerer <[email protected]>

---
 cache.h         |  57 +++++++++++++++++++++++++++++-
 read-cache-v2.c |  96 +++++++++++++++++++++++++++++++++++++++++++++++--
 read-cache.c    | 108 ++++++++++++++++++++++++++++++++++++++++++++++++++++----
 read-cache.h    |  12 ++++++-
 4 files changed, 263 insertions(+), 10 deletions(-)

cache.h +0 -0

index 5082b34..d38dfbd 100644

cache.h struct cache_entry { · line 133

Quoted 8 lines

cache.h +2 -1

@@ -127,7 +127,8 @@ struct cache_entry {
        unsigned int ce_flags;
        unsigned int ce_namelen;
        unsigned char sha1[20];
-       struct cache_entry *next;
+       struct cache_entry *next; /* used by name_hash */
+       struct cache_entry *next_ce; /* used to keep a list of cache entries */
        char name[FLEX_ARRAY]; /* more */
 };

From what I read, doing

    ce = start;
    while (ce) { do(something); ce = next_cache_entry(ce); }

is the same as

    i = start_index;
    while (i < active_nr) { ce = active_cache[i]; do(something); i++; }

What's the advantage of using the former over the latter? Do you plan to eliminate the latter loop (by hiding "struct cache_entry **cache;" from public index_state structure?

cache.h +28 -0

+/*
+ * Options by which the index should be filtered when read partially.
+ *
+ * pathspec: The pathspec which the index entries have to match
+ * seen: Used to return the seen parameter from match_pathspec()
+ * max_prefix, max_prefix_len: These variables are set to the longest
+ *     common prefix, the length of the longest common prefix of the
+ *     given pathspec
+ *
+ * read_staged: used to indicate if the conflicted entries (entries
+ *     with a stage) should be included
+ * read_cache_tree: used to indicate if the cache-tree should be read
+ * read_resolve_undo: used to indicate if the resolve undo data should
+ *     be read
+ */
+struct filter_opts {
+       const char **pathspec;
+       char *seen;
+       char *max_prefix;
+       int max_prefix_len;
+
+       int read_staged;
+       int read_cache_tree;
+       int read_resolve_undo;
+};
+
 struct index_state {
        struct cache_entry **cache;
        unsigned int version;

@@ -270,6 +297,8 @@ struct index_state {
        struct hash_table name_hash;
        struct hash_table dir_hash;
        struct index_ops *ops;
+       struct internal_ops *internal_ops;
+       struct filter_opts *filter_opts;
 };

read-cache.c +4 -3

-/* remember to discard_cache() before reading a different cache! */
-int read_index_from(struct index_state *istate, const char *path)
+
+int read_index_filtered_from(struct index_state *istate, const char *path,
+                            struct filter_opts *opts)
 {
        int fd, err, i;
        struct stat st_old, st_new;

@@ -1337,7 +1425,7 @@ int read_index_from(struct index_state *istate, const char *path)
                if (istate->ops->verify_hdr(mmap, mmap_size) < 0)
                        err = 1;
 
-               if (istate->ops->read_index(istate, mmap, mmap_size) < 0)
+               if (istate->ops->read_index(istate, mmap, mmap_size, opts) < 0)
                        err = 1;
                istate->timestamp.sec = st_old.st_mtime;
                istate->timestamp.nsec = ST_MTIME_NSEC(st_old);

Quoted 36 lines · Thomas Gummerer

cache.h +28 -0

+/*
+ * Options by which the index should be filtered when read partially.
+ *
+ * pathspec: The pathspec which the index entries have to match
+ * seen: Used to return the seen parameter from match_pathspec()
+ * max_prefix, max_prefix_len: These variables are set to the longest
+ *     common prefix, the length of the longest common prefix of the
+ *     given pathspec
+ *
+ * read_staged: used to indicate if the conflicted entries (entries
+ *     with a stage) should be included
+ * read_cache_tree: used to indicate if the cache-tree should be read
+ * read_resolve_undo: used to indicate if the resolve undo data should
+ *     be read
+ */
+struct filter_opts {
+       const char **pathspec;
+       char *seen;
+       char *max_prefix;
+       int max_prefix_len;
+
+       int read_staged;
+       int read_cache_tree;
+       int read_resolve_undo;
+};
+
 struct index_state {
        struct cache_entry **cache;
        unsigned int version;

@@ -270,6 +297,8 @@ struct index_state {
        struct hash_table name_hash;
        struct hash_table dir_hash;
        struct index_ops *ops;
+       struct internal_ops *internal_ops;
+       struct filter_opts *filter_opts;
 };

...

Quoted 20 lines

read-cache.c +4 -3

-/* remember to discard_cache() before reading a different cache! */
-int read_index_from(struct index_state *istate, const char *path)
+
+int read_index_filtered_from(struct index_state *istate, const char *path,
+                            struct filter_opts *opts)
 {
        int fd, err, i;
        struct stat st_old, st_new;

@@ -1337,7 +1425,7 @@ int read_index_from(struct index_state *istate, const char *path)
                if (istate->ops->verify_hdr(mmap, mmap_size) < 0)
                        err = 1;
 
-               if (istate->ops->read_index(istate, mmap, mmap_size) < 0)
+               if (istate->ops->read_index(istate, mmap, mmap_size, opts) < 0)
                        err = 1;
                istate->timestamp.sec = st_old.st_mtime;
                istate->timestamp.nsec = ST_MTIME_NSEC(st_old);

@@ -1345,6 +1433,7 @@ int read_index_from(struct index_state *istate, const char *path)
                        die_errno("cannot stat the open index");
 

read-cache.c int read_index_from(struct index_state *istate, const char *path) · line 1439

Quoted 5 lines

read-cache.c +1 -0

                munmap(mmap, mmap_size);
+               istate->filter_opts = opts;
                if (!index_changed(&st_old, &st_new) && !err)
                        return istate->cache_nr;
        }

Putting filter_opts in index_state feels like a bad design. Iterator information should be separated from the iterated object, so that two callers can walk through the same index without stepping on each other (I'm not talking about multithreading, a caller may walk a bit, then the other caller starts walking, then the former caller resumes walking again in a call chain).

Quoted 6 lines · Duy Nguyen

Putting filter_opts in index_state feels like a bad design. Iterator information should be separated from the iterated object, so that two callers can walk through the same index without stepping on each other (I'm not talking about multithreading, a caller may walk a bit, then the other caller starts walking, then the former caller resumes walking again in a call chain).

Yes, you're right. We need the filter_opts to see what part of the index has been loaded [1] and which part has been skipped, but it shouldn't be used for filtering in the for_each_index_entry function.

I think there should be two versions of the for_each_index_entry
function then, where the for_each_index_entry function would behave the
same way as the for_each_index_entry_filtered function with the
filter_opts parameter set to NULL:
for_each_index_entry_filtered(struct index_state *, each_cache_entry_fn, void *cb_data, struct filter_opts *) for_each_index_entry(struct index_state *, each_cache_entry_fn, void *cb_data)

Both of them then should call index_change_filter_opts to make sure all the entries that are needed are loaded in the in-memory format.

Does that make sense?

[1] That is only important for the new index-v5 file format, which can be loaded partially. The filter_opts could always be set to NULL, as the whole index is always loaded anyway.

Hmm.. I was confused actually (documentation on the api would help greatly).

As promised, a draft for a documentation for the index api as it is in this series.

+read_index_filtered(opts)::

Documentation/technical/api-in-core-index.txt +10 -0

+       This method behaves differently for index-v2 and index-v5.
+
+       For index-v2 it simply reads the whole index as read_index()
+       does, so we are sure we don't have to reload anything if the
+       user wants a different filter.  It also sets the filter_opts
+       in the index_state, which is used to limit the results when
+       iterating over the index with for_each_index_entry().
+
+       The whole index is read to avoid the need to eventually
+       re-read the index later, because the performance is no

+next_index_entry(ce)::
+ Returns the cache_entry that follows after ce

+index_change_filter_opts(opts)::

Documentation/technical/api-in-core-index.txt +5 -0

+       This function again has a slightly different functionality for
+       index-v2 and index-v5.
+
+       For index-v2 it simply changes the filter_opts, so
+       for_each_index_entry uses the changed index_opts, to iterate

Quoted 2 lines · Thomas Gummerer

As promised, a draft for a documentation for the index api as it is in this series.

First of all, it may be a good idea to acknowledge index_state->cache[] as part of the API for now. Not hiding it simplifies a few things (no need for new next_ce field, no worries about rewinding in unpack-trees..). Supporting partial loading (and maybe partial update in some cases) with this API and index_state->cache[] part of the API are good enough for me. We can do another tree-based API or something update later when it's formed (I looked at your index-v5api branch but I don't think a tree-based api was there, my concern is how much extra head pre-v5 has to pay to use tree-based api).

Quoted 11 lines

+read_index_filtered(opts)::

Documentation/technical/api-in-core-index.txt +10 -0

+       This method behaves differently for index-v2 and index-v5.
+
+       For index-v2 it simply reads the whole index as read_index()
+       does, so we are sure we don't have to reload anything if the
+       user wants a different filter.  It also sets the filter_opts
+       in the index_state, which is used to limit the results when
+       iterating over the index with for_each_index_entry().
+
+       The whole index is read to avoid the need to eventually
+       re-read the index later, because the performance is no

Documentation/technical/api-in-core-index.txt line 44

Quoted 10 lines

Documentation/technical/api-in-core-index.txt +10 -0

+       different when reading it partially.
+
+       For index-v5 it creates an adjusted_pathspec to filter the
+       reading.  First all the directory entries are read and then
+       the cache_entries in the directories that match the adjusted
+       pathspec are read.  The filter_opts in the index_state are set
+       to filter out the rest of the cache_entries that are matched
+       by the adjusted pathspec but not by the pathspec given.  The
+       rest of the index entries are filtered out when iterating over
+       the cache with for_each_index_entries.

You can state in the API that the input pathspec is used as a hint to load only a portion of the index. read_index_filtered may load _more_ than necessary. It's the caller's responsibility to verify again which is matched and which is not. That's how read_directory is done. I think it gives you more liberty in loading strategy. It's already true for v2 because full index is loaded regardless of the given pathspec. In the end, we have a linear list (from public view) of cache entries, accessible via index_state->cache[].

If you happen to know that certain entries match the given pathspec, you could help the caller avoid match_pathspec'ing again by set a bit in ce_flags.

To know which entry exists in the index and which is
new, use another flag. Most reader code won't change if we do it this way, all match_pathspec() remain where they are.

Documentation/technical/api-in-core-index.txt line 57

Quoted 5 lines

+for_each_index_entry(fn, cb_data)::

Documentation/technical/api-in-core-index.txt +4 -0

+       Iterates over all cache_entries in the index filtered by
+       filter_opts in the index_stat.  For each cache entry fn is
+       executed with cb_data as callback data.  From within the loop
+       do `return 0` to continue, or `return 1` to break the loop.

Because we don't attempt to hide index_state->cache[], this one may be for convenience, the user is not required to convert to it. Actually I think this may be slower because of the cost of calling function pointer.

Quoted 2 lines

+next_index_entry(ce)::
+ Returns the cache_entry that follows after ce

next_ce field and this method may be gone too, just access index_state->cache[]

Quoted 6 lines

+index_change_filter_opts(opts)::

Documentation/technical/api-in-core-index.txt +5 -0

+       This function again has a slightly different functionality for
+       index-v2 and index-v5.
+
+       For index-v2 it simply changes the filter_opts, so
+       for_each_index_entry uses the changed index_opts, to iterate

Documentation/technical/api-in-core-index.txt line 77

Quoted 10 lines

Documentation/technical/api-in-core-index.txt +10 -0

+       over a different set of cache entries.
+
+       For index-v5 it refreshes the index if the filter_opts have
+       changed and sets the new filter_opts in the index state, again
+       to iterate over a different set of cache entries as with
+       index-v2.
+
+       This has some optimization potential, in the case that the
+       opts get stricter (less of the index should be read) it
+       doesn't have to reload anything, but currently does.

The only use case I see so far is converting a partial index_state back to a full one. Apart from doing so in order to write the new index, I think some operation (like rename tracking in diff or unpack-trees) may expect full index. I think we should support that. I doubt we need to change pathspec to something different than the one we used to load the index. When a user passes a pathspec to a command, the user expects the command to operate on that set only, not outside.

If you take the input pathspec at loading just as a hint, you could load all related directory blocks and all files in those blocks, so that expanding to full index is simply adding more files from missing directory blocks (and their files). An advantage of not strictly follow the input pathspec.

Some thoughts about the writing api.

In think we should avoid automatically converting partial index into a full one before writing. Push that back to the caller and die() when asked to update partial index. They know at what point the index may be updated and even what part of it may be updated. I think all commands fall into two categories, tree-wide updates (merge, checkout...) and limited by the user-given pathspec. "what part to be updated" is not so hard to determine.

If the caller promises not to update or read outside certain pathspec (part of API), we don't really need to load full index until write_index is called. At that point I think we also know what directory blocks are completely untouched by the caller (by promise) and could copy them over from the old index byte-by-byte (or something close, some offsets may be recalculated). That may keep tree compiling cost proportional to the number of changed entries, not the number of entries in index.

There is another partial write case that's not covered this round (or was it discussed and discarded?): refreshing the index. This operation could be treated as a standalone, special one: refresh and update the index file directly without waiting until write_index is called. There are some commands that follow this scheme by doing update_index_if_able() after refresh_index(). Those will be cheaper with v5 because we don't need write a full new index.

Quoted 3 lines

If you happen to know that certain entries match the given pathspec, you could help the caller avoid match_pathspec'ing again by set a bit in ce_flags.

I currently don't know which entries do match the pathspec from just reading the index file, additional calls would be needed. I don't think that would be worth the overhead.

Quoted 3 lines

To know which entry exists in the index and which is
new, use another flag. Most reader code won't change if we do it this way, all match_pathspec() remain where they are.

Hrm you mean to know which cache entries are added (or changed) in the in-memory index and will have to be written later? I'm not sure I understand correctly what you mean here.

Quoted 17 lines

Quoted 6 lines

+index_change_filter_opts(opts)::

Documentation/technical/api-in-core-index.txt +5 -0

+       This function again has a slightly different functionality for
+       index-v2 and index-v5.
+
+       For index-v2 it simply changes the filter_opts, so
+       for_each_index_entry uses the changed index_opts, to iterate

Documentation/technical/api-in-core-index.txt line 77

Quoted 10 lines

Documentation/technical/api-in-core-index.txt +10 -0

+       over a different set of cache entries.
+
+       For index-v5 it refreshes the index if the filter_opts have
+       changed and sets the new filter_opts in the index state, again
+       to iterate over a different set of cache entries as with
+       index-v2.
+
+       This has some optimization potential, in the case that the
+       opts get stricter (less of the index should be read) it
+       doesn't have to reload anything, but currently does.

Quoted 7 lines

The only use case I see so far is converting a partial index_state back to a full one. Apart from doing so in order to write the new index, I think some operation (like rename tracking in diff or unpack-trees) may expect full index. I think we should support that. I doubt we need to change pathspec to something different than the one we used to load the index. When a user passes a pathspec to a command, the user expects the command to operate on that set only, not outside.

One application was in ls-files, where we strip the trailing slash from the pathspecs for submodules. But when we let the caller filter the rest out it's not needed anymore. We load all entries without the trailing slash anyway.

Quoted 9 lines

Some thoughts about the writing api.

In think we should avoid automatically converting partial index into a full one before writing. Push that back to the caller and die() when asked to update partial index. They know at what point the index may be updated and even what part of it may be updated. I think all commands fall into two categories, tree-wide updates (merge, checkout...) and limited by the user-given pathspec. "what part to be updated" is not so hard to determine.

Hrm this is only true if index entries are added or removed, not if they are only changed. If they are only changed we can write a partially read index once we have partial writing.

For now it would make sense to just die() though, until we have that in place.

Quoted 7 lines · Thomas Gummerer

Quoted 3 lines

If you happen to know that certain entries match the given pathspec, you could help the caller avoid match_pathspec'ing again by set a bit in ce_flags.

I currently don't know which entries do match the pathspec from just reading the index file, additional calls would be needed. I don't think that would be worth the overhead.

Yeah I now see that you select what to load in v5 with the adjusted pathspec, not the input pathspec. Originally I thought you match the input pathspec against every file entry in the index :P Your adjusted pathspec looks like what common_prefix is for. It's cheaper than creating adjusted_pathspec from match_pathspec and reduces loading in major cases, where glob is not used.

Still, creating an adjusted pathspec this way looks iffy. You need to understand pathspec in order to strip the filename part out to match the directory match only. An alternative is use tree_entry_interesting. It goes along well with tree traversal and can be used to match directories with original pathspec. Once you see it matches an entry in a directory, you could skip matching the rest of the files and load the whole directory. read_index_filtered_v5 and read_entries may need some tweaking though. I'll try it and post a patch later if I succeed.

Quoted 7 lines

Quoted 3 lines

To know which entry exists in the index and which is
new, use another flag. Most reader code won't change if we do it this way, all match_pathspec() remain where they are.

Hrm you mean to know which cache entries are added (or changed) in the in-memory index and will have to be written later? I'm not sure I understand correctly what you mean here.

Oh.. The "to know.." sentence was nonsense. We probably don't need to know. We may track changed entries for partial writing, but let's leave that out for now.

Quoted 25 lines

Quoted 17 lines

Quoted 6 lines

+index_change_filter_opts(opts)::

Documentation/technical/api-in-core-index.txt +5 -0

+       This function again has a slightly different functionality for
+       index-v2 and index-v5.
+
+       For index-v2 it simply changes the filter_opts, so
+       for_each_index_entry uses the changed index_opts, to iterate

Documentation/technical/api-in-core-index.txt line 77

Quoted 10 lines

Documentation/technical/api-in-core-index.txt +10 -0

+       over a different set of cache entries.
+
+       For index-v5 it refreshes the index if the filter_opts have
+       changed and sets the new filter_opts in the index state, again
+       to iterate over a different set of cache entries as with
+       index-v2.
+
+       This has some optimization potential, in the case that the
+       opts get stricter (less of the index should be read) it
+       doesn't have to reload anything, but currently does.

Quoted 7 lines

The only use case I see so far is converting a partial index_state back to a full one. Apart from doing so in order to write the new index, I think some operation (like rename tracking in diff or unpack-trees) may expect full index. I think we should support that. I doubt we need to change pathspec to something different than the one we used to load the index. When a user passes a pathspec to a command, the user expects the command to operate on that set only, not outside.

Quoted 4 lines

One application was in ls-files, where we strip the trailing slash from the pathspecs for submodules. But when we let the caller filter the rest out it's not needed anymore. We load all entries without the trailing slash anyway.

That submodule trailing slash stripping code will be moved away soon (I've been working on it for some time now). There's similar code in pathspec.c. I hope by the time this series becomes a candidate for 'next', those pathspec manipulation is already gone. For strip_trailing_slash_from_submodules, peeking in index file for a few entries is probably ok. For check_path_for_gitlink, full index is loaded until we figure out a clever way.

Quoted 10 lines

Quoted 9 lines

Some thoughts about the writing api.

In think we should avoid automatically converting partial index into a full one before writing. Push that back to the caller and die() when asked to update partial index. They know at what point the index may be updated and even what part of it may be updated. I think all commands fall into two categories, tree-wide updates (merge, checkout...) and limited by the user-given pathspec. "what part to be updated" is not so hard to determine.

Quoted 3 lines

Hrm this is only true if index entries are added or removed, not if they are only changed. If they are only changed we can write a partially read index once we have partial writing.

Yep. We can detect if changes are updates only, no additions nor removals. If so do partial write, else full write. These little details are hidden from the user, as long as they keep their promise about read/write regions.

Quoted 1 line

For now it would make sense to just die() though, until we have that in place.

Agreed.
--
Duy

Quoted 16 lines · Duy Nguyen

Quoted 7 lines · Thomas Gummerer

Quoted 3 lines

If you happen to know that certain entries match the given pathspec, you could help the caller avoid match_pathspec'ing again by set a bit in ce_flags.

I currently don't know which entries do match the pathspec from just reading the index file, additional calls would be needed. I don't think that would be worth the overhead.

Yeah I now see that you select what to load in v5 with the adjusted pathspec, not the input pathspec. Originally I thought you match the input pathspec against every file entry in the index :P Your adjusted pathspec looks like what common_prefix is for. It's cheaper than creating adjusted_pathspec from match_pathspec and reduces loading in major cases, where glob is not used.

Quoted 9 lines

Still, creating an adjusted pathspec this way looks iffy. You need to understand pathspec in order to strip the filename part out to match the directory match only. An alternative is use tree_entry_interesting. It goes along well with tree traversal and can be used to match directories with original pathspec. Once you see it matches an entry in a directory, you could skip matching the rest of the files and load the whole directory. read_index_filtered_v5 and read_entries may need some tweaking though. I'll try it and post a patch later if I succeed.

Hrm, I played around a bit with this idea, but I couldn't figure out how to make it work. For it to work we would still have to load some entries in a directory at least? Or is there a way to match the directories, which I just haven't figured out yet?

Quoted 5 lines · Thomas Gummerer

Duy Nguyen <[email protected]> writes:
Hrm, I played around a bit with this idea, but I couldn't figure out how to make it work. For it to work we would still have to load some entries in a directory at least? Or is there a way to match the directories, which I just haven't figured out yet?

Yes you have to load some entries first. Even if a directory does not match, we only know until at least the first file in the directory. OK there might be problems because tree_entry_interesting expects all entries in a directory to be memcmp sorted, without trailing slash for subdirectories. I need to check again if v5 sort order is compatible..

Quoted 19 lines · Thomas Gummerer

Quoted 6 lines · Duy Nguyen

Putting filter_opts in index_state feels like a bad design. Iterator information should be separated from the iterated object, so that two callers can walk through the same index without stepping on each other (I'm not talking about multithreading, a caller may walk a bit, then the other caller starts walking, then the former caller resumes walking again in a call chain).

Yes, you're right. We need the filter_opts to see what part of the index has been loaded [1] and which part has been skipped, but it shouldn't be used for filtering in the for_each_index_entry function.

I think there should be two versions of the for_each_index_entry
function then, where the for_each_index_entry function would behave the
same way as the for_each_index_entry_filtered function with the
filter_opts parameter set to NULL:
for_each_index_entry_filtered(struct index_state *, each_cache_entry_fn, void *cb_data, struct filter_opts *) for_each_index_entry(struct index_state *, each_cache_entry_fn, void *cb_data)

Quoted 4 lines

Both of them then should call index_change_filter_opts to make sure all the entries that are needed are loaded in the in-memory format.

Does that make sense?

Hmm.. I was confused actually (documentation on the api would help greatly). If you already filter at load time, I don't think you need to match again. The caller asked for filter and it should know what's in there so for_each_index_entry just goes through all entries without extra match_pathspec. Or is that what next_index_entry for? match_pathspec function could be expensive when glob is involved. If the caller wants extra matching, it could do inside the callback function.

It seems you could change the filter with index_change_filter_opts. In v5 the index will be reloaded. What happens when some index entries area already modified? Do we start to have read-only index "views" and one read-write view? If partial views are always read-only, perhaps we just allow the user to create a new index_state (or view) with new filter and destroy the old one. We don't have to care about changing or separating filter in that case because the view is the iterator.

I wanted to have a tree-based iterator api, but that seems incompatible with pre-v5 (or at least adds some overhead on pre-v5 to rebuild the tree structure). It looks like using pathspec to build a list of entries, as you did, is a good way to take advantage of tree-based v5 while maintaining code compatibility with pre-v5. By the way with tree structure, you could use tree_entry_interesting in read_index_filtered_v5. I think it's more efficient than match_pathspec.

I'm still studying the code. Some of what I wrote here may be totally wrong due to my lack of understanding. I'll get back to you later if I find something else.

Quoted 3 lines

[1] That is only important for the new index-v5 file format, which can be loaded partially. The filter_opts could always be set to NULL, as the whole index is always loaded anyway.

--
Duy

A partially read index file currently cannot be written to disk. Make sure that never happens, by re-reading the index file if the index file wasn't read completely before changing the in-memory index.

Signed-off-by Thomas Gummerer <[email protected]>

---
 builtin/update-index.c | 4 ++++
 cache.h                | 4 +++-
 read-cache-v2.c        | 3 +++
 read-cache.c           | 8 ++++++++
 4 files changed, 18 insertions(+), 1 deletion(-)

builtin/update-index.c +4 -0

index 5c7762e..03f6426 100644
@@ -49,6 +49,8 @@ static int mark_ce_flags(const char *path, int flag, int mark)
 	int namelen = strlen(path);
 	int pos = cache_name_pos(path, namelen);
 	if (0 <= pos) {
+		if (active_cache_partially_read)
+			cache_change_filter_opts(NULL);
 		if (mark)
 			active_cache[pos]->ce_flags |= flag;
 		else

@@ -253,6 +255,8 @@ static void chmod_path(int flip, const char *path)
 	pos = cache_name_pos(path, strlen(path));
 	if (pos < 0)
 		goto fail;
+	if (active_cache_partially_read)
+		cache_change_filter_opts(NULL);
 	ce = active_cache[pos];
 	mode = ce->ce_mode;
 	if (!S_ISREG(mode))

cache.h +3 -1

index d38dfbd..f6c3407 100644
@@ -293,7 +293,8 @@ struct index_state {
 	struct cache_tree *cache_tree;
 	struct cache_time timestamp;
 	unsigned name_hash_initialized : 1,
-		 initialized : 1;
+		 initialized : 1,
+		 partially_read : 1;
 	struct hash_table name_hash;
 	struct hash_table dir_hash;
 	struct index_ops *ops;

@@ -315,6 +316,7 @@ extern void free_name_hash(struct index_state *istate);
 #define active_alloc (the_index.cache_alloc)
 #define active_cache_changed (the_index.cache_changed)
 #define active_cache_tree (the_index.cache_tree)
+#define active_cache_partially_read (the_index.partially_read)
 
 #define read_cache() read_index(&the_index)
 #define read_cache_from(path) read_index_from(&the_index, (path))

read-cache-v2.c +3 -0

index 1ed640d..2cc792d 100644
@@ -273,6 +273,7 @@ static int read_index_v2(struct index_state *istate, void *mmap,
 		src_offset += 8;
 		src_offset += extsize;
 	}
+	istate->partially_read = 0;
 	return 0;
 unmap:
 	munmap(mmap, mmap_size);

@@ -495,6 +496,8 @@ static int write_index_v2(struct index_state *istate, int newfd)
 	struct stat st;
 	struct strbuf previous_name_buf = STRBUF_INIT, *previous_name;
 
+	if (istate->partially_read)
+		die("BUG: index: cannot write a partially read index");
 	for (i = removed = extended = 0; i < entries; i++) {
 		if (cache[i]->ce_flags & CE_REMOVE)
 			removed++;

read-cache.c +8 -0

index b30ee75..4529fab 100644
@@ -30,6 +30,8 @@ static void replace_index_entry(struct index_state *istate, int nr, struct cache
 {
 	struct cache_entry *old = istate->cache[nr];
 
+	if (istate->partially_read)
+		index_change_filter_opts(istate, NULL);
 	remove_name_hash(istate, old);
 	set_index_entry(istate, nr, ce);
 	istate->cache_changed = 1;

@@ -467,6 +469,8 @@ int remove_index_entry_at(struct index_state *istate, int pos)
 {
 	struct cache_entry *ce = istate->cache[pos];
 
+	if (istate->partially_read)
+		index_change_filter_opts(istate, NULL);
 	record_resolve_undo(istate, ce);
 	remove_name_hash(istate, ce);
 	istate->cache_changed = 1;

@@ -978,6 +982,8 @@ int add_index_entry(struct index_state *istate, struct cache_entry *ce, int opti
 {
 	int pos;
 
+	if (istate->partially_read)
+		index_change_filter_opts(istate, NULL);
 	if (option & ADD_CACHE_JUST_APPEND)
 		pos = istate->cache_nr;
 	else {

@@ -1184,6 +1190,8 @@ int refresh_index(struct index_state *istate, unsigned int flags, const char **p
 				/* If we are doing --really-refresh that
 				 * means the index is not valid anymore.
 				 */
+				if (istate->partially_read)
+					index_change_filter_opts(istate, NULL);
 				ce->ce_flags &= ~CE_VALID;
 				istate->cache_changed = 1;
 			}

Quoted 3 lines · Thomas Gummerer

A partially read index file currently cannot be written to disk. Make sure that never happens, by re-reading the index file if the index file wasn't read completely before changing the in-memory index.

I am not quite sure what you are trying to do.

In operations that modify the index (replace_index_entry(), remove_index_entry_at(), etc.) you lift the filter_ops and keep partially_read flag still on. In the write-out codepath, you have an assert to make sure the caller has cleared the partially_read flag. A natural way to clear the flag is to re-read the index from the file, but then you can easily lose the modifications.

Also shouldn't the flag be cleared upon discard_index()? If it is done there, you probably would not need to clear it in read_index().

Should
there be another safety that says "calling read_index() with the partially_read flag on is a bug" or something?

Quoted 114 lines

Signed-off-by Thomas Gummerer <[email protected]>

---
 builtin/update-index.c | 4 ++++
 cache.h                | 4 +++-
 read-cache-v2.c        | 3 +++
 read-cache.c           | 8 ++++++++
 4 files changed, 18 insertions(+), 1 deletion(-)

builtin/update-index.c +4 -0

index 5c7762e..03f6426 100644
@@ -49,6 +49,8 @@ static int mark_ce_flags(const char *path, int flag, int mark)
 	int namelen = strlen(path);
 	int pos = cache_name_pos(path, namelen);
 	if (0 <= pos) {
+		if (active_cache_partially_read)
+			cache_change_filter_opts(NULL);
 		if (mark)
 			active_cache[pos]->ce_flags |= flag;
 		else

@@ -253,6 +255,8 @@ static void chmod_path(int flip, const char *path)
 	pos = cache_name_pos(path, strlen(path));
 	if (pos < 0)
 		goto fail;
+	if (active_cache_partially_read)
+		cache_change_filter_opts(NULL);
 	ce = active_cache[pos];
 	mode = ce->ce_mode;
 	if (!S_ISREG(mode))

cache.h +3 -1

index d38dfbd..f6c3407 100644
@@ -293,7 +293,8 @@ struct index_state {
 	struct cache_tree *cache_tree;
 	struct cache_time timestamp;
 	unsigned name_hash_initialized : 1,
-		 initialized : 1;
+		 initialized : 1,
+		 partially_read : 1;
 	struct hash_table name_hash;
 	struct hash_table dir_hash;
 	struct index_ops *ops;

@@ -315,6 +316,7 @@ extern void free_name_hash(struct index_state *istate);
 #define active_alloc (the_index.cache_alloc)
 #define active_cache_changed (the_index.cache_changed)
 #define active_cache_tree (the_index.cache_tree)
+#define active_cache_partially_read (the_index.partially_read)
 
 #define read_cache() read_index(&the_index)
 #define read_cache_from(path) read_index_from(&the_index, (path))

read-cache-v2.c +3 -0

index 1ed640d..2cc792d 100644
@@ -273,6 +273,7 @@ static int read_index_v2(struct index_state *istate, void *mmap,
 		src_offset += 8;
 		src_offset += extsize;
 	}
+	istate->partially_read = 0;
 	return 0;
 unmap:
 	munmap(mmap, mmap_size);

@@ -495,6 +496,8 @@ static int write_index_v2(struct index_state *istate, int newfd)
 	struct stat st;
 	struct strbuf previous_name_buf = STRBUF_INIT, *previous_name;
 
+	if (istate->partially_read)
+		die("BUG: index: cannot write a partially read index");
 	for (i = removed = extended = 0; i < entries; i++) {
 		if (cache[i]->ce_flags & CE_REMOVE)
 			removed++;

read-cache.c +8 -0

index b30ee75..4529fab 100644
@@ -30,6 +30,8 @@ static void replace_index_entry(struct index_state *istate, int nr, struct cache
 {
 	struct cache_entry *old = istate->cache[nr];
 
+	if (istate->partially_read)
+		index_change_filter_opts(istate, NULL);
 	remove_name_hash(istate, old);
 	set_index_entry(istate, nr, ce);
 	istate->cache_changed = 1;

@@ -467,6 +469,8 @@ int remove_index_entry_at(struct index_state *istate, int pos)
 {
 	struct cache_entry *ce = istate->cache[pos];
 
+	if (istate->partially_read)
+		index_change_filter_opts(istate, NULL);
 	record_resolve_undo(istate, ce);
 	remove_name_hash(istate, ce);
 	istate->cache_changed = 1;

@@ -978,6 +982,8 @@ int add_index_entry(struct index_state *istate, struct cache_entry *ce, int opti
 {
 	int pos;
 
+	if (istate->partially_read)
+		index_change_filter_opts(istate, NULL);
 	if (option & ADD_CACHE_JUST_APPEND)
 		pos = istate->cache_nr;
 	else {

@@ -1184,6 +1190,8 @@ int refresh_index(struct index_state *istate, unsigned int flags, const char **p
 				/* If we are doing --really-refresh that
 				 * means the index is not valid anymore.
 				 */
+				if (istate->partially_read)
+					index_change_filter_opts(istate, NULL);
 				ce->ce_flags &= ~CE_VALID;
 				istate->cache_changed = 1;
 			}

+== File entry (fileentries)

Documentation/technical/index-file-format-v5.txt new file +46 -0

+
+  File entries are sorted in ascending order on the name field, after the
+  respective offset given by the directory entries. All file names are
+  prefix compressed, meaning the file name is relative to the directory.
+
+  filename (variable length, nul terminated). The exact encoding is
+    undefined, but the filename cannot contain a NUL byte (iow, the same
+    encoding as a UNIX pathname).
+
+  flags (16-bits): 'flags' field split into (high to low bits)
+
+    assumevalid (1-bit): assume-valid flag
+
+    intenttoadd (1-bit): intent-to-add flag, used by "git add -N".
+      Extended flag in index v3.
+
+    stage (2-bit): stage of the file during merge
+
+    skipworktree (1-bit): skip-worktree flag, used by sparse checkout.
+      Extended flag in index v3.
+
+    smudged (1-bit): indicates if the file is racily smudged.
+
+    10-bit unused, must be zero [6]
+
+  mode (16-bits): file mode, split into (high to low bits)
+
+    objtype (4-bits): object type
+      valid values in binary are 1000 (regular file), 1010 (symbolic
+      link) and 1110 (gitlink)
+
+    3-bit unused
+
+    permission (9-bits): unix permission. Only 0755 and 0644 are valid
+      for regular files. Symbolic links and gitlinks have value 0 in
+      this field.
+
+  mtimes (32-bits): mtime seconds, the last time a file's data changed
+    this is stat(2) data
+
+  mtimens (32-bits): mtime nanosecond fractions
+    this is stat(2) data
+
+  file size (32-bits): The on-disk size, trucated to 32-bit.
+    this is stat(2) data
+

Quoted 47 lines · Thomas Gummerer

+== File entry (fileentries)

Documentation/technical/index-file-format-v5.txt new file +46 -0

+
+  File entries are sorted in ascending order on the name field, after the
+  respective offset given by the directory entries. All file names are
+  prefix compressed, meaning the file name is relative to the directory.
+
+  filename (variable length, nul terminated). The exact encoding is
+    undefined, but the filename cannot contain a NUL byte (iow, the same
+    encoding as a UNIX pathname).
+
+  flags (16-bits): 'flags' field split into (high to low bits)
+
+    assumevalid (1-bit): assume-valid flag
+
+    intenttoadd (1-bit): intent-to-add flag, used by "git add -N".
+      Extended flag in index v3.
+
+    stage (2-bit): stage of the file during merge
+
+    skipworktree (1-bit): skip-worktree flag, used by sparse checkout.
+      Extended flag in index v3.
+
+    smudged (1-bit): indicates if the file is racily smudged.
+
+    10-bit unused, must be zero [6]
+
+  mode (16-bits): file mode, split into (high to low bits)
+
+    objtype (4-bits): object type
+      valid values in binary are 1000 (regular file), 1010 (symbolic
+      link) and 1110 (gitlink)
+
+    3-bit unused
+
+    permission (9-bits): unix permission. Only 0755 and 0644 are valid
+      for regular files. Symbolic links and gitlinks have value 0 in
+      this field.
+
+  mtimes (32-bits): mtime seconds, the last time a file's data changed
+    this is stat(2) data
+
+  mtimens (32-bits): mtime nanosecond fractions
+    this is stat(2) data
+
+  file size (32-bits): The on-disk size, trucated to 32-bit.
+    this is stat(2) data
+

Documentation/technical/index-file-format-v5.txt line 217

Quoted 10 lines

Documentation/technical/index-file-format-v5.txt new file +10 -0

+  statcrc (32-bits): crc32 checksum over ctime seconds, ctime
+    nanoseconds, ino, dev, uid, gid (All stat(2) data
+    except mtime and file size). If the statcrc is 0 it will
+    be ignored. [7]
+
+  objhash (160-bits): SHA-1 for the represented object
+
+  entrycrc (32-bits): crc32 checksum for the file entry. The crc code
+    includes the offset to the offset to the file, relative to the
+    beginning of the file.

Question about the possibility of updating index file directly. If git updates a few fields of an entry (but not entrycrc yet) and crashes, the entry would become corrupt because its entrycrc does not match the content. What do we do? Do we need to save a copy of the entry somewhere in the index file (maybe in the conflict data section), so that the reader can recover the index? Losing the index because of bugs is big deal in my opinion. pre-v5 never faces this because we keep the original copy til the end.

Maybe entrycrc should not cover stat fields and statcrc. It would make refreshing safer. If the above happens during refresh, only statcrc is corrupt and we can just refresh the entry. entrycrc still says the other fields are good (and they are).

Quoted 9 lines

Question about the possibility of updating index file directly. If git updates a few fields of an entry (but not entrycrc yet) and crashes, the entry would become corrupt because its entrycrc does not match the content. What do we do? Do we need to save a copy of the entry somewhere in the index file (maybe in the conflict data section), so that the reader can recover the index? Losing the index because of bugs is big deal in my opinion. pre-v5 never faces this because we keep the original copy til the end.

Quoted 4 lines

Maybe entrycrc should not cover stat fields and statcrc. It would make refreshing safer. If the above happens during refresh, only statcrc is corrupt and we can just refresh the entry. entrycrc still says the other fields are good (and they are).

The original idea was to change the lock-file for partial writing to make it work for this case. The exact structure of the file still has to be defined, but generally it would be done in the following steps:

1. Write the changed entry to the lock-file
2. Change the entry in the index
3. If we succeed delete the lock-file (commit the transaction)

If git crashes, and leaves the index corrupted, we can recover the information from the lock-file and write the new information to the index file and then delete the lock-file.

Quoted 22 lines · Thomas Gummerer

Quoted 9 lines

Question about the possibility of updating index file directly. If git updates a few fields of an entry (but not entrycrc yet) and crashes, the entry would become corrupt because its entrycrc does not match the content. What do we do? Do we need to save a copy of the entry somewhere in the index file (maybe in the conflict data section), so that the reader can recover the index? Losing the index because of bugs is big deal in my opinion. pre-v5 never faces this because we keep the original copy til the end.

Quoted 4 lines

Maybe entrycrc should not cover stat fields and statcrc. It would make refreshing safer. If the above happens during refresh, only statcrc is corrupt and we can just refresh the entry. entrycrc still says the other fields are good (and they are).

The original idea was to change the lock-file for partial writing to make it work for this case. The exact structure of the file still has to be defined, but generally it would be done in the following steps:

1. Write the changed entry to the lock-file
2. Change the entry in the index
3. If we succeed delete the lock-file (commit the transaction)

Quoted 3 lines

If git crashes, and leaves the index corrupted, we can recover the information from the lock-file and write the new information to the index file and then delete the lock-file.

Ah makes sense. Still concerned about refreshing though. Updated files are usually few while refreshed files could be a lot more, increasing the cost at #1.

Quoted 27 lines · Duy Nguyen

Quoted 25 lines · Thomas Gummerer

Quoted 9 lines

Question about the possibility of updating index file directly. If git updates a few fields of an entry (but not entrycrc yet) and crashes, the entry would become corrupt because its entrycrc does not match the content. What do we do? Do we need to save a copy of the entry somewhere in the index file (maybe in the conflict data section), so that the reader can recover the index? Losing the index because of bugs is big deal in my opinion. pre-v5 never faces this because we keep the original copy til the end.

Quoted 4 lines

Maybe entrycrc should not cover stat fields and statcrc. It would make refreshing safer. If the above happens during refresh, only statcrc is corrupt and we can just refresh the entry. entrycrc still says the other fields are good (and they are).

The original idea was to change the lock-file for partial writing to make it work for this case. The exact structure of the file still has to be defined, but generally it would be done in the following steps:

1. Write the changed entry to the lock-file
2. Change the entry in the index
3. If we succeed delete the lock-file (commit the transaction)

If git crashes, and leaves the index corrupted, we can recover the information from the lock-file and write the new information to the index file and then delete the lock-file.

Quoted 3 lines

Ah makes sense. Still concerned about refreshing though. Updated files are usually few while refreshed files could be a lot more, increasing the cost at #1.

Any idea how common refreshing a big part of the cache is?

If it's not to common, I'd prefer to leave the stat data and stat crc in the entrycrc, as we can inform the user if something is wrong with the index, be it from git failing, or from disk corruption.

On the other hand if refresh_cache is relatively common and usually changes a big part of the index we should leave them out, as git can still run correctly with incorrect stat data, but takes a little longer, because it may have to check the file contents. That will be trade-off to make here.

Make git read the index file version 5 without complaining.

This version of the reader doesn't read neither the cache-tree nor the resolve undo data, but doesn't choke on an index that includes such data.
---

read-cache-v5.c new file +6 -0

new file mode 100644
index 0000000..e319f30
@@ -0,0 +1,658 @@
+static struct directory_entry *read_directories(unsigned int *dir_offset,
+                               unsigned int *dir_table_offset,
+                               void *mmap,
+                               int mmap_size)
+{
+       int i, ondisk_directory_size;

Quoted 18 lines · Thomas Gummerer

Make git read the index file version 5 without complaining.

This version of the reader doesn't read neither the cache-tree nor the resolve undo data, but doesn't choke on an index that includes such data.
---

read-cache-v5.c new file +6 -0

new file mode 100644
index 0000000..e319f30
@@ -0,0 +1,658 @@
+static struct directory_entry *read_directories(unsigned int *dir_offset,
+                               unsigned int *dir_table_offset,
+                               void *mmap,
+                               int mmap_size)
+{
+       int i, ondisk_directory_size;

read-cache-v5.c line 16

Quoted 10 lines

read-cache-v5.c new file +10 -0

+       uint32_t *filecrc, *beginning, *end;
+       struct directory_entry *current = NULL;
+       struct ondisk_directory_entry *disk_de;
+       struct directory_entry *de;
+       unsigned int data_len, len;
+       char *name;
+
+       /* Length of pathname + nul byte for termination + size of
+        * members of ondisk_directory_entry. (Just using the size
+        * of the stuct doesn't work, because there may be padding

s/stuct/struct/

read-cache-v5.c line 234

Quoted 2 lines

read-cache-v5.c new file +2 -0

+        * bytes for the struct)
+        */

Also:

/*
* Format multi-line comment
* like this.
*/

Remaining multi-line comments appear to be formatted correctly.

Since the cache-tree data is saved as part of the directory data, we already read it at the beginning of the index. The cache-tree is only converted from this directory data.

The cache-tree data is arranged in a tree, with the children sorted by pathlen at each node, while the ondisk format is sorted lexically. So we have to rebuild this format from the on-disk directory list.

Signed-off-by Thomas Gummerer <[email protected]>

---

read-cache-v5.c +0 -0

index 193970a..f1ad132 100644

read-cache-v5.c +16 -0

+ * Don't use it if the directory entries are still needed after.
+ */
+static struct cache_tree *cache_tree_convert_v5(struct directory_entry *de)
+{
+       struct directory_queue *queue;
+
+       if (!de->de_nentries)
+               return NULL;
+       queue = xcalloc(1, sizeof(struct directory_queue));
+       queue[0].de = de;
+       queue[0].down = xcalloc(de->de_nsubtrees, sizeof(struct directory_queue));
+
+       sort_directories(de, queue[0].down);
+       return convert_one(queue, 0);
+}
+
 static void resolve_undo_convert_v5(struct index_state *istate,
                                    struct conflict_entry *conflict)
 {

Write the index version 5 file format to disk. This version doesn't write the cache-tree data and resolve-undo data to the file.

The main work is done when filtering out the directories from the current in-memory format, where in the same turn also the conflicts and the file data is calculated.

Helped-by Nguyen Thai Ngoc Duy <[email protected]>

Helped-by Thomas Rast <[email protected]>

Signed-off-by Thomas Gummerer <[email protected]>

---

read-cache-v5.c +24 -0

index f1ad132..f056f6b 100644
+static int write_index_v5(struct index_state *istate, int newfd)
+{
+       struct cache_version_header hdr;
+       struct cache_header hdr_v5;
+       struct cache_entry **cache = istate->cache;
+       struct directory_entry *de;
+       struct ondisk_directory_entry *ondisk;
+       int entries = istate->cache_nr;
+       int i, removed, non_conflicted, total_dir_len, ondisk_directory_size;
+       int total_file_len, conflict_offset, offset_to_offset;
+       unsigned int ndir;
+       uint32_t crc;
+
+       if (istate->partially_read)
+               die("BUG: index: cannot write a partially read index");
+
+       for (i = removed = 0; i < entries; i++) {
+               if (cache[i]->ce_flags & CE_REMOVE)
+                       removed++;
+       }
+       hdr.hdr_signature = htonl(CACHE_SIGNATURE);
+       hdr.hdr_version = htonl(istate->version);
+       hdr_v5.hdr_nfile = htonl(entries - removed);
+       hdr_v5.hdr_nextension = htonl(0); /* Currently no extensions are supported */

read-cache-v5.c +45 -0

+        * of 4
+        */
+       ondisk_directory_size = sizeof(ondisk->flags)
+               + sizeof(ondisk->foffset)
+               + sizeof(ondisk->cr)
+               + sizeof(ondisk->ncr)
+               + sizeof(ondisk->nsubtrees)
+               + sizeof(ondisk->nfiles)
+               + sizeof(ondisk->nentries)
+               + sizeof(ondisk->sha1);
+       hdr_v5.hdr_fblockoffset = htonl(sizeof(hdr) + sizeof(hdr_v5) + 4
+               + (ndir + 1) * 4
+               + total_dir_len
+               + ndir * (ondisk_directory_size + 4)
+               + (non_conflicted + 1) * 4);
+
+       crc = 0;
+       if (ce_write(&crc, newfd, &hdr, sizeof(hdr)) < 0)
+               return -1;
+       if (ce_write(&crc, newfd, &hdr_v5, sizeof(hdr_v5)) < 0)
+               return -1;
+       crc = htonl(crc);
+       if (ce_write(NULL, newfd, &crc, 4) < 0)
+               return -1;
+
+       conflict_offset = sizeof(hdr) + sizeof(hdr_v5) + 4
+               + (ndir + 1) * 4
+               + total_dir_len
+               + ndir * (ondisk_directory_size + 4)
+               + (non_conflicted + 1) * 4
+               + total_file_len
+               + non_conflicted * (sizeof(struct ondisk_cache_entry) + 4);
+       if (write_directories(de, newfd, conflict_offset) < 0)
+               return -1;
+       offset_to_offset = sizeof(hdr) + sizeof(hdr_v5) + 4
+               + (ndir + 1) * 4
+               + total_dir_len
+               + ndir * (ondisk_directory_size + 4);
+       if (write_entries(istate, de, entries, newfd, offset_to_offset) < 0)
+               return -1;
+       if (write_conflicts(istate, de, newfd) < 0)
+               return -1;
+       return ce_flush(newfd);
+}
+