Duy Nguyen <pclouds@xxxxxxxxx> writes:
> On Sun, Jul 7, 2013 at 3:11 PM, Thomas Gummerer <t.gummerer@xxxxxxxxx> wrote:
>> +/*
>> + * 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;
>> };
>
> ...
>
>> -/* 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");
>>
>> 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).
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.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html