| Message ID | 5d28570c82af19b4bda4253e72ace3760dfe2606.1629206603.git.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Sparse index: delete ignored files outside sparse cone | expand |
Hi Stolee, On Tue, 17 Aug 2021, Derrick Stolee via GitGitGadget wrote: > From: Derrick Stolee <dstolee@microsoft.com> > > 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@microsoft.com> > --- > 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 > >
On 8/19/2021 4:07 AM, Johannes Schindelin wrote: ... >> 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)) { Thank you for pointing out this. This is actually a stale change from an earlier version where path_in_sparse_checkout returned an 'enum pattern_match_result' but now casts down to an 'int', meaning '0' is not in the sparse-checkout and '1' is that it _is_ in the sparse-checkout. >> +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; So here, if we do not have sparse-checkout patterns (for example, if core_apply_sparse_checkout is false), then this returns MATCHED (== 1). To be extra clear, this should just be 'return 1;'. >> + base = strrchr(path, '/'); >> + return path_matches_pattern_list(path, strlen(path), base ? base + 1 : path, >> + &dtype, >> + istate->sparse_checkout_patterns, >> + istate) > 0; Here, we are selecting the portion of 'enum pattern_match_result' that we care about (MATCHED and MATCHED_RECURSIVE). The UNMATCHED (0) and UNDECIDED (-1) are the other possibilities, but file paths will not return UNDECIDED, that is instead for directories in non-cone mode patterns. Thanks, -Stolee
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 && + 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