diff mbox series

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

Message ID 64358ec7ea2b3df4a8f1099452579a7568996921.1611161639.git.gitgitgadget@gmail.com (mailing list archive)
State New, archived
Headers show
Series More index cleanups | expand

Commit Message

Derrick Stolee Jan. 20, 2021, 4:53 p.m. UTC
From: Derrick Stolee <dstolee@microsoft.com>

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@microsoft.com>
---
 builtin/sparse-checkout.c |  5 -----
 dir.c                     | 17 +++++++++++++++++
 dir.h                     |  2 ++
 unpack-trees.c            |  6 +-----
 4 files changed, 20 insertions(+), 10 deletions(-)

Comments

Elijah Newren Jan. 20, 2021, 5:54 p.m. UTC | #1
On Wed, Jan 20, 2021 at 8:54 AM Derrick Stolee via GitGitGadget
<gitgitgadget@gmail.com> wrote:
>
> From: Derrick Stolee <dstolee@microsoft.com>
>
> 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@microsoft.com>
> ---
>  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!
diff mbox series

Patch

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);
 }