Re: [PATCH v3 5/8] sparse-checkout: create helper methods
- From
Johannes Schindelin <johannes.schindelin@gmx.de>
- Date
- Aug 19, 2021, 08:07 UTC
- Message-ID
- <nycvar.QRO.7.76.6.2108191005560.55@tvgsbejvaqbjf.bet>
- In-Reply-To
- <5d28570c82af19b4bda4253e72ace3760dfe2606.1629206603.git.gitgitgadget@gmail.com>
Hi Stolee,
On Tue, 17 Aug 2021, Derrick Stolee via GitGitGadget wrote:
Show 13 quoted lines
> 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!
Show 33 quoted lines
>
> 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
Show 106 quoted lines
> + 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
>
>