Re: [PATCH 6/9] sparse-checkout: load sparse-checkout patterns

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Jan 20, 2021 at 8:54 AM Derrick Stolee via GitGitGadget
<gitgitgadget@xxxxxxxxx> wrote:
>
> From: Derrick Stolee <dstolee@xxxxxxxxxxxxx>
>
> A future feature will want to load the sparse-checkout patterns into a
> pattern_list, but the current mechanism to do so is a bit complicated.
> This is made difficult due to needing to find the sparse-checkout file
> in different ways throughout the codebase.
>
> The logic implemented in the new get_sparse_checkout_patterns() was
> duplicated in populate_from_existing_patterns() in unpack-trees.c. Use
> the new method instead, keeping the logic around handling the struct
> unpack_trees_options.
>
> The callers to get_sparse_checkout_filename() in
> builtin/sparse-checkout.c manipulate the sparse-checkout file directly,
> so it is not appropriate to replace logic in that file with
> get_sparse_checkout_patterns().
>
> Signed-off-by: Derrick Stolee <dstolee@xxxxxxxxxxxxx>
> ---
>  builtin/sparse-checkout.c |  5 -----
>  dir.c                     | 17 +++++++++++++++++
>  dir.h                     |  2 ++
>  unpack-trees.c            |  6 +-----
>  4 files changed, 20 insertions(+), 10 deletions(-)
>
> diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c
> index e3140db2a0a..2306a9ad98e 100644
> --- a/builtin/sparse-checkout.c
> +++ b/builtin/sparse-checkout.c
> @@ -22,11 +22,6 @@ static char const * const builtin_sparse_checkout_usage[] = {
>         NULL
>  };
>
> -static char *get_sparse_checkout_filename(void)
> -{
> -       return git_pathdup("info/sparse-checkout");
> -}
> -
>  static void write_patterns_to_file(FILE *fp, struct pattern_list *pl)
>  {
>         int i;
> diff --git a/dir.c b/dir.c
> index d637461da5c..d153a63bbd1 100644
> --- a/dir.c
> +++ b/dir.c
> @@ -2998,6 +2998,23 @@ void setup_standard_excludes(struct dir_struct *dir)
>         }
>  }
>
> +char *get_sparse_checkout_filename(void)
> +{
> +       return git_pathdup("info/sparse-checkout");
> +}
> +
> +int get_sparse_checkout_patterns(struct pattern_list *pl)
> +{
> +       int res;
> +       char *sparse_filename = get_sparse_checkout_filename();
> +
> +       pl->use_cone_patterns = core_sparse_checkout_cone;
> +       res = add_patterns_from_file_to_list(sparse_filename, "", 0, pl, NULL);
> +
> +       free(sparse_filename);
> +       return res;
> +}
> +
>  int remove_path(const char *name)
>  {
>         char *slash;
> diff --git a/dir.h b/dir.h
> index a3c40dec516..facfae47402 100644
> --- a/dir.h
> +++ b/dir.h
> @@ -448,6 +448,8 @@ int is_empty_dir(const char *dir);
>
>  void setup_standard_excludes(struct dir_struct *dir);
>
> +char *get_sparse_checkout_filename(void);
> +int get_sparse_checkout_patterns(struct pattern_list *pl);
>
>  /* Constants for remove_dir_recursively: */
>
> diff --git a/unpack-trees.c b/unpack-trees.c
> index af6e9b9c2fd..837b8bb42fb 100644
> --- a/unpack-trees.c
> +++ b/unpack-trees.c
> @@ -1549,14 +1549,10 @@ static void mark_new_skip_worktree(struct pattern_list *pl,
>  static void populate_from_existing_patterns(struct unpack_trees_options *o,
>                                             struct pattern_list *pl)
>  {
> -       char *sparse = git_pathdup("info/sparse-checkout");
> -
> -       pl->use_cone_patterns = core_sparse_checkout_cone;
> -       if (add_patterns_from_file_to_list(sparse, "", 0, pl, NULL) < 0)
> +       if (get_sparse_checkout_patterns(pl) < 0)
>                 o->skip_sparse_checkout = 1;
>         else
>                 o->pl = pl;
> -       free(sparse);
>  }
>
>
> --
> gitgitgadget

Looks straightforward and well motivated to me.

But the cherry on top that really sells this patch is that more lines
of dir.c will blame to someone besides me.  Win-win!



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux