Hi Stolee, On Tue, 17 Aug 2021, Derrick Stolee via GitGitGadget wrote: > From: Derrick Stolee <dstolee@xxxxxxxxxxxxx> > > As we integrate the sparse index into more builtins, we occasionally > need to check the sparse-checkout patterns to see if a path is within > the sparse-checkout cone. Create some helper methods that help > initialize the patterns and check for pattern matching to make this > easier. > > The existing callers of commands like get_sparse_checkout_patterns() use > a custom 'struct pattern_list' that is not necessarily the one in the > 'struct index_state', so there are not many previous uses that could > adopt these helpers. There are just two in builtin/add.c and > sparse-index.c that can use path_in_sparse_checkout(). Very good! > > Signed-off-by: Derrick Stolee <dstolee@xxxxxxxxxxxxx> > --- > builtin/add.c | 8 ++------ > dir.c | 33 +++++++++++++++++++++++++++++++++ > dir.h | 6 ++++++ > sparse-index.c | 12 +++--------- > 4 files changed, 44 insertions(+), 15 deletions(-) > > diff --git a/builtin/add.c b/builtin/add.c > index 17528e8f922..f675bdeae4a 100644 > --- a/builtin/add.c > +++ b/builtin/add.c > @@ -190,8 +190,6 @@ static int refresh(int verbose, const struct pathspec *pathspec) > struct string_list only_match_skip_worktree = STRING_LIST_INIT_NODUP; > int flags = REFRESH_IGNORE_SKIP_WORKTREE | > (verbose ? REFRESH_IN_PORCELAIN : REFRESH_QUIET); > - struct pattern_list pl = { 0 }; > - int sparse_checkout_enabled = !get_sparse_checkout_patterns(&pl); > > seen = xcalloc(pathspec->nr, 1); > refresh_index(&the_index, flags, pathspec, seen, > @@ -199,12 +197,10 @@ static int refresh(int verbose, const struct pathspec *pathspec) > for (i = 0; i < pathspec->nr; i++) { > if (!seen[i]) { > const char *path = pathspec->items[i].original; > - int dtype = DT_REG; > > if (matches_skip_worktree(pathspec, i, &skip_worktree_seen) || > - (sparse_checkout_enabled && > - !path_matches_pattern_list(path, strlen(path), NULL, > - &dtype, &pl, &the_index))) { > + (core_apply_sparse_checkout && Do we need to test for `core_apply_sparse_checkout` here? Or does the `if (!istate->sparse_checkout_patterns) return MATCHED;` early return in `path_in_sparse_checkout()` suffice to catch this? The remainder of the patch looks good to me. Thank you, Dscho > + path_in_sparse_checkout(path, &the_index) == NOT_MATCHED)) { > string_list_append(&only_match_skip_worktree, > pathspec->items[i].original); > } else { > diff --git a/dir.c b/dir.c > index 03c4d212672..6fd4f2e0f27 100644 > --- a/dir.c > +++ b/dir.c > @@ -1439,6 +1439,39 @@ done: > return result; > } > > +int init_sparse_checkout_patterns(struct index_state *istate) > +{ > + if (!core_apply_sparse_checkout || > + istate->sparse_checkout_patterns) > + return 0; > + > + CALLOC_ARRAY(istate->sparse_checkout_patterns, 1); > + > + if (get_sparse_checkout_patterns(istate->sparse_checkout_patterns) < 0) { > + FREE_AND_NULL(istate->sparse_checkout_patterns); > + return -1; > + } > + > + return 0; > +} > + > +int path_in_sparse_checkout(const char *path, > + struct index_state *istate) > +{ > + const char *base; > + int dtype = DT_REG; > + init_sparse_checkout_patterns(istate); > + > + if (!istate->sparse_checkout_patterns) > + return MATCHED; > + > + base = strrchr(path, '/'); > + return path_matches_pattern_list(path, strlen(path), base ? base + 1 : path, > + &dtype, > + istate->sparse_checkout_patterns, > + istate) > 0; > +} > + > static struct path_pattern *last_matching_pattern_from_lists( > struct dir_struct *dir, struct index_state *istate, > const char *pathname, int pathlen, > diff --git a/dir.h b/dir.h > index b3e1a54a971..b899ee43d81 100644 > --- a/dir.h > +++ b/dir.h > @@ -394,6 +394,12 @@ enum pattern_match_result path_matches_pattern_list(const char *pathname, > const char *basename, int *dtype, > struct pattern_list *pl, > struct index_state *istate); > + > +int init_sparse_checkout_patterns(struct index_state *state); > + > +int path_in_sparse_checkout(const char *path, > + struct index_state *istate); > + > struct dir_entry *dir_add_ignored(struct dir_struct *dir, > struct index_state *istate, > const char *pathname, int len); > diff --git a/sparse-index.c b/sparse-index.c > index b6e90417556..2efc9fd4910 100644 > --- a/sparse-index.c > +++ b/sparse-index.c > @@ -34,17 +34,14 @@ static int convert_to_sparse_rec(struct index_state *istate, > int i, can_convert = 1; > int start_converted = num_converted; > enum pattern_match_result match; > - int dtype = DT_UNKNOWN; > struct strbuf child_path = STRBUF_INIT; > - struct pattern_list *pl = istate->sparse_checkout_patterns; > > /* > * Is the current path outside of the sparse cone? > * Then check if the region can be replaced by a sparse > * directory entry (everything is sparse and merged). > */ > - match = path_matches_pattern_list(ct_path, ct_pathlen, > - NULL, &dtype, pl, istate); > + match = path_in_sparse_checkout(ct_path, istate); > if (match != NOT_MATCHED) > can_convert = 0; > > @@ -153,11 +150,8 @@ int convert_to_sparse(struct index_state *istate) > if (!istate->repo->settings.sparse_index) > return 0; > > - if (!istate->sparse_checkout_patterns) { > - istate->sparse_checkout_patterns = xcalloc(1, sizeof(struct pattern_list)); > - if (get_sparse_checkout_patterns(istate->sparse_checkout_patterns) < 0) > - return 0; > - } > + if (init_sparse_checkout_patterns(istate) < 0) > + return 0; > > /* > * We need cone-mode patterns to use sparse-index. If a user edits > -- > gitgitgadget > >