Re: [PATCH 6/8] sparse-index: complete partial expansion
From: Victoria Dye <hidden>
Date: 2022-05-16 21:01:56
Derrick Stolee via GitGitGadget wrote:
quoted hunk ↗ jump to hunk
From: Derrick Stolee <redacted> To complete the implementation of expand_to_pattern_list(), we need to detect when a sparse directory entry should remain sparse. This avoids a full expansion, so we now need to use the PARTIALLY_SPARSE mode to indicate this state. There still are no callers to this method, but we will add one in the next change. Signed-off-by: Derrick Stolee <redacted> --- sparse-index.c | 41 +++++++++++++++++++++++++++++++++++++---- 1 file changed, 37 insertions(+), 4 deletions(-)diff --git a/sparse-index.c b/sparse-index.c index 3d8eed585b5..0bad5503304 100644 --- a/sparse-index.c +++ b/sparse-index.c@@ -297,8 +297,24 @@ void expand_to_pattern_list(struct index_state *istate, * continue. A NULL pattern set indicates a full expansion to a * full index. */ - if (pl && !pl->use_cone_patterns) + if (pl && !pl->use_cone_patterns) { pl = NULL; + } else { + /* + * We might contract file entries into sparse-directory + * entries, and for that we will need the cache tree to + * be recomputed. + */ + cache_tree_free(&istate->cache_tree); + + /* + * If there is a problem creating the cache tree, then we + * need to expand to a full index since we cannot satisfy + * the current request as a sparse index. + */ + if (cache_tree_update(istate, WRITE_TREE_MISSING_OK)) + pl = NULL; + } if (!istate->repo) istate->repo = the_repository;@@ -317,8 +333,14 @@ void expand_to_pattern_list(struct index_state *istate, full = xcalloc(1, sizeof(struct index_state)); memcpy(full, istate, sizeof(struct index_state)); + /* + * This slightly-misnamed 'full' index might still be sparse if we + * are only modifying the list of sparse directories. This hinges + * on whether we have a non-NULL pattern list. + */ + full->sparse_index = pl ? PARTIALLY_SPARSE : COMPLETELY_FULL; + /* then change the necessary things */ - full->sparse_index = 0; full->cache_alloc = (3 * istate->cache_alloc) / 2; full->cache_nr = 0; ALLOC_ARRAY(full->cache, full->cache_alloc);@@ -330,11 +352,22 @@ void expand_to_pattern_list(struct index_state *istate, struct cache_entry *ce = istate->cache[i]; struct tree *tree; struct pathspec ps; + int dtype; if (!S_ISSPARSEDIR(ce->ce_mode)) { set_index_entry(full, full->cache_nr++, ce); continue; } + + /* We now have a sparse directory entry. Should we expand? */ + if (pl && + path_matches_pattern_list(ce->name, ce->ce_namelen, + NULL, &dtype, + pl, istate) <= 0) {
If I'm reading this correctly, what this is doing is:
- if we have a sparse directory entry
- ...and we're expanding only what matches the pattern list (i.e., not
'ensure_full_index')
- ...and that sparse directory path is either *not matching* or *undecided
whether it matches* the pattern list
- ...then we add the sparse directory to the result index and continue.
The part that's confusing me is the "<= 0", which means a return value of
'UNDECIDED' from 'path_matches_pattern_list' adds the sparse directory
as-is. At the moment, it looks like 'UNDECIDED' is only returned if not
using cone patterns (so it shouldn't make a functional difference at this
point), but wouldn't that return value indicate that the pattern *may or may
not* match the path, so we should continue on to 'read_tree_at()'?
All that to say, should the condition be:
/* We now have a sparse directory entry. Should we expand? */
if (pl &&
path_matches_pattern_list(ce->name, ce->ce_namelen,
NULL, &dtype,
pl, istate) == NOT_MATCHED) {
to reflect that a sparse directory should only be added to the index if we
*know* it isn't matched?
To be clear, this is ultimately a non-functional nit - my question is mostly
to make sure I understand the intent of the code.
quoted hunk ↗ jump to hunk
+ set_index_entry(full, full->cache_nr++, ce); + continue; + } + if (!(ce->ce_flags & CE_SKIP_WORKTREE)) warning(_("index entry is a directory, but not sparse (%08x)"), ce->ce_flags);@@ -360,7 +393,7 @@ void expand_to_pattern_list(struct index_state *istate, /* Copy back into original index. */ memcpy(&istate->name_hash, &full->name_hash, sizeof(full->name_hash)); memcpy(&istate->dir_hash, &full->dir_hash, sizeof(full->dir_hash)); - istate->sparse_index = 0; + istate->sparse_index = pl ? PARTIALLY_SPARSE : COMPLETELY_FULL; free(istate->cache); istate->cache = full->cache; istate->cache_nr = full->cache_nr;@@ -374,7 +407,7 @@ void expand_to_pattern_list(struct index_state *istate, /* Clear and recompute the cache-tree */ cache_tree_free(&istate->cache_tree); - cache_tree_update(istate, 0); + cache_tree_update(istate, WRITE_TREE_MISSING_OK); trace2_region_leave("index", pl ? "expand_to_pattern_list" : "ensure_full_index",