threads / patch / 52780

patch, 4 partsSparse-checkout: Add subcommand and Windows paths

Subject: [PATCH 0/4] Sparse-checkout: Add subcommand and Windows paths

## tl;dr

9 messages between Feb 11, 2020 and Feb 11, 2020. Diffs are folded; open one to read it.

replies: 8people: 3as markdown or json

Derrick Stolee via GitGitGadget· Feb 11, 2020, 15:02 UTC · lore
This is based on ds/sparse-checkout-harden.

The sparse-checkout builtin currently lets users modify their sparse-checkout file with the all-or-nothing "set" subcommand. It may be easier for a user to expand their sparse cone using a "git sparse-checkout add <pattern/path> ..." subcommand. To achieve this while reusing as much code as possible from the "set" subcommand, the first two patches extract methods from sparse_checkout_set().

As part of this "add" subcommand, I created a modify_pattern_list() method that serves as the core interaction with the sparse-checkout file and working directory. It changes how it constructs the in-memory pattern list based on an "enum modify_type" that only has two options: REPLACE and ADD. This could be extended with REMOVE in a later update. I started building the REMOVE logic, but it was more complicated than ADD, and I didn't have time to finish it right now due to other obligations.

Finally, a Windows user contacted me about using Windows-style paths when in cone mode to add a nested directory. This case is fixed and tested in the final patch.

Thanks, -Stolee
Derrick Stolee (4):
  sparse-checkout: extract add_patterns_from_input()
  sparse-checkout: extract pattern update from 'set' subcommand
  sparse-checkout: create 'add' subcommand
  sparse-checkout: work with Windows paths
 Documentation/git-sparse-checkout.txt |   7 ++
 builtin/sparse-checkout.c             | 141 ++++++++++++++++++++------
 t/t1091-sparse-checkout-builtin.sh    |  73 +++++++++++++
 3 files changed, 189 insertions(+), 32 deletions(-)
base-commit: f998a3f1e588d73ed7285cb14ac4839f63f6dc82
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-546%2Fderrickstolee%2Fsparse-add-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-546/derrickstolee/sparse-add-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/546
-- 
gitgitgadget
Derrick Stolee via GitGitGadget· Feb 11, 2020, 15:02 UTC · re: Derrick Stolee via GitGitGadget · lore

[PATCH 2/4] sparse-checkout: extract pattern update from 'set' subcommand

From: Derrick Stolee <dstolee@microsoft.com>

In anticipation of adding "add" and "remove" subcommands to the sparse-checkout builtin, extract a modify_pattern_list() method from the sparse_checkout_set() method. This command will read input from the command-line or stdin to construct a set of patterns, then modify the existing sparse-checkout patterns after a successful update of the working directory.

Currently, the only way to modify the patterns is to replace all of the patterns. This will be extended in a later update.

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
---
 builtin/sparse-checkout.c | 44 +++++++++++++++++++++++----------------
 1 file changed, 26 insertions(+), 18 deletions(-)
Show changes to builtin/sparse-checkout.c +26 −18
diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c
index 41d8aaf9a2..03915dd729 100644
--- a/builtin/sparse-checkout.c
+++ b/builtin/sparse-checkout.c
@@ -462,29 +462,17 @@ static void add_patterns_from_input(struct pattern_list *pl,
 	}
 }
 
-static int sparse_checkout_set(int argc, const char **argv, const char *prefix)
+enum modify_type {
+	REPLACE,
+};
+
+static int modify_pattern_list(int argc, const char **argv, enum modify_type m)
 {
-	struct pattern_list pl;
 	int result;
 	int changed_config = 0;
-
-	static struct option builtin_sparse_checkout_set_options[] = {
-		OPT_BOOL(0, "stdin", &set_opts.use_stdin,
-			 N_("read patterns from standard in")),
-		OPT_END(),
-	};
-
-	repo_read_index(the_repository);
-	require_clean_work_tree(the_repository,
-				N_("set sparse-checkout patterns"), NULL, 1, 0);
-
+	struct pattern_list pl;
 	memset(&pl, 0, sizeof(pl));
 
-	argc = parse_options(argc, argv, prefix,
-			     builtin_sparse_checkout_set_options,
-			     builtin_sparse_checkout_set_usage,
-			     PARSE_OPT_KEEP_UNKNOWN);
-
 	add_patterns_from_input(&pl, argc, argv);
 
 	if (!core_apply_sparse_checkout) {
@@ -502,6 +490,26 @@ static int sparse_checkout_set(int argc, const char **argv, const char *prefix)
 	return result;
 }
 
+static int sparse_checkout_set(int argc, const char **argv, const char *prefix)
+{
+	static struct option builtin_sparse_checkout_set_options[] = {
+		OPT_BOOL(0, "stdin", &set_opts.use_stdin,
+			 N_("read patterns from standard in")),
+		OPT_END(),
+	};
+
+	repo_read_index(the_repository);
+	require_clean_work_tree(the_repository,
+				N_("set sparse-checkout patterns"), NULL, 1, 0);
+
+	argc = parse_options(argc, argv, prefix,
+			     builtin_sparse_checkout_set_options,
+			     builtin_sparse_checkout_set_usage,
+			     PARSE_OPT_KEEP_UNKNOWN);
+
+	return modify_pattern_list(argc, argv, REPLACE);
+}
+
 static int sparse_checkout_disable(int argc, const char **argv)
 {
 	struct pattern_list pl;
-- 
gitgitgadget
Derrick Stolee via GitGitGadget· Feb 11, 2020, 15:02 UTC · re: Derrick Stolee via GitGitGadget · lore

[PATCH 3/4] sparse-checkout: create 'add' subcommand

From: Derrick Stolee <dstolee@microsoft.com>

When using the sparse-checkout feature, a user may want to incrementally grow their sparse-checkout pattern set. Allow adding patterns using a new 'add' subcommand. This is not much different from the 'set' subcommand, because we still want to allow the '--stdin' option and interpret inputs as directories when in cone mode and patterns otherwise.

When in cone mode, we are growing the cone. This may actually reduce the set of patterns when adding directory A when A/B is already a directory in the cone. Test the different cases: siblings, parents, ancestors.

When not in cone mode, we can only assume the patterns should be appended to the sparse-checkout file.

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
---
 Documentation/git-sparse-checkout.txt |  7 +++
 builtin/sparse-checkout.c             | 72 ++++++++++++++++++++++++---
 t/t1091-sparse-checkout-builtin.sh    | 59 ++++++++++++++++++++++
 3 files changed, 132 insertions(+), 6 deletions(-)
Show changes to 3 files +132 −6

Documentation/git-sparse-checkout.txt, builtin/sparse-checkout.c, t/t1091-sparse-checkout-builtin.sh

diff --git a/Documentation/git-sparse-checkout.txt b/Documentation/git-sparse-checkout.txt
index 0914619881..746f920d71 100644
--- a/Documentation/git-sparse-checkout.txt
+++ b/Documentation/git-sparse-checkout.txt
@@ -59,6 +59,13 @@ directories. The input format matches the output of `git ls-tree --name-only`.
 This includes interpreting pathnames that begin with a double quote (") as
 C-style quoted strings.
 
+'add'::
+	Update the sparse-checkout file to include additional patterns.
+	By default, these patterns are read from the command-line arguments,
+	but they can be read from stdin using the `--stdin` option. When
+	`core.sparseCheckoutCone` is enabled, the given patterns are interpreted
+	as directory names as in the 'set' subcommand.
+
 'disable'::
 	Disable the `core.sparseCheckout` config setting, and restore the
 	working directory to include all files. Leaves the sparse-checkout
diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c
index 03915dd729..af9e3e5123 100644
--- a/builtin/sparse-checkout.c
+++ b/builtin/sparse-checkout.c
@@ -18,7 +18,7 @@
 static const char *empty_base = "";
 
 static char const * const builtin_sparse_checkout_usage[] = {
-	N_("git sparse-checkout (init|list|set|disable) <options>"),
+	N_("git sparse-checkout (init|list|set|add|disable) <options>"),
 	NULL
 };
 
@@ -404,7 +404,7 @@ static void strbuf_to_cone_pattern(struct strbuf *line, struct pattern_list *pl)
 }
 
 static char const * const builtin_sparse_checkout_set_usage[] = {
-	N_("git sparse-checkout set (--stdin | <patterns>)"),
+	N_("git sparse-checkout (set|add) (--stdin | <patterns>)"),
 	NULL
 };
 
@@ -464,8 +464,54 @@ static void add_patterns_from_input(struct pattern_list *pl,
 
 enum modify_type {
 	REPLACE,
+	ADD,
 };
 
+static void add_patterns_cone_mode(int argc, const char **argv,
+				   struct pattern_list *pl)
+{
+	struct strbuf buffer = STRBUF_INIT;
+	struct pattern_entry *pe;
+	struct hashmap_iter iter;
+	struct pattern_list existing;
+	char *sparse_filename = get_sparse_checkout_filename();
+
+	add_patterns_from_input(pl, argc, argv);
+
+	memset(&existing, 0, sizeof(existing));
+	existing.use_cone_patterns = core_sparse_checkout_cone;
+
+	if (add_patterns_from_file_to_list(sparse_filename, "", 0,
+					   &existing, NULL))
+		die(_("unable to load existing sparse-checkout patterns"));
+	free(sparse_filename);
+
+	hashmap_for_each_entry(&existing.recursive_hashmap, &iter, pe, ent) {
+		if (!hashmap_contains_parent(&pl->recursive_hashmap,
+					pe->pattern, &buffer) ||
+		    !hashmap_contains_parent(&pl->parent_hashmap,
+					pe->pattern, &buffer)) {
+			strbuf_reset(&buffer);
+			strbuf_addstr(&buffer, pe->pattern);
+			insert_recursive_pattern(pl, &buffer);
+		}
+	}
+
+	clear_pattern_list(&existing);
+	strbuf_release(&buffer);
+}
+
+static void add_patterns_literal(int argc, const char **argv,
+				 struct pattern_list *pl)
+{
+	char *sparse_filename = get_sparse_checkout_filename();
+	if (add_patterns_from_file_to_list(sparse_filename, "", 0,
+					   pl, NULL))
+		die(_("unable to load existing sparse-checkout patterns"));
+	free(sparse_filename);
+	add_patterns_from_input(pl, argc, argv);
+}
+
 static int modify_pattern_list(int argc, const char **argv, enum modify_type m)
 {
 	int result;
@@ -473,7 +519,18 @@ static int modify_pattern_list(int argc, const char **argv, enum modify_type m)
 	struct pattern_list pl;
 	memset(&pl, 0, sizeof(pl));
 
-	add_patterns_from_input(&pl, argc, argv);
+	switch (m) {
+	case ADD:
+		if (core_sparse_checkout_cone)
+			add_patterns_cone_mode(argc, argv, &pl);
+		else
+			add_patterns_literal(argc, argv, &pl);
+		break;
+
+	case REPLACE:
+		add_patterns_from_input(&pl, argc, argv);
+		break;
+	}
 
 	if (!core_apply_sparse_checkout) {
 		set_config(MODE_ALL_PATTERNS);
@@ -490,7 +547,8 @@ static int modify_pattern_list(int argc, const char **argv, enum modify_type m)
 	return result;
 }
 
-static int sparse_checkout_set(int argc, const char **argv, const char *prefix)
+static int sparse_checkout_set(int argc, const char **argv, const char *prefix,
+			       enum modify_type m)
 {
 	static struct option builtin_sparse_checkout_set_options[] = {
 		OPT_BOOL(0, "stdin", &set_opts.use_stdin,
@@ -507,7 +565,7 @@ static int sparse_checkout_set(int argc, const char **argv, const char *prefix)
 			     builtin_sparse_checkout_set_usage,
 			     PARSE_OPT_KEEP_UNKNOWN);
 
-	return modify_pattern_list(argc, argv, REPLACE);
+	return modify_pattern_list(argc, argv, m);
 }
 
 static int sparse_checkout_disable(int argc, const char **argv)
@@ -558,7 +616,9 @@ int cmd_sparse_checkout(int argc, const char **argv, const char *prefix)
 		if (!strcmp(argv[0], "init"))
 			return sparse_checkout_init(argc, argv);
 		if (!strcmp(argv[0], "set"))
-			return sparse_checkout_set(argc, argv, prefix);
+			return sparse_checkout_set(argc, argv, prefix, REPLACE);
+		if (!strcmp(argv[0], "add"))
+			return sparse_checkout_set(argc, argv, prefix, ADD);
 		if (!strcmp(argv[0], "disable"))
 			return sparse_checkout_disable(argc, argv);
 	}
diff --git a/t/t1091-sparse-checkout-builtin.sh b/t/t1091-sparse-checkout-builtin.sh
index 7d982096fb..f9265de5e8 100755
--- a/t/t1091-sparse-checkout-builtin.sh
+++ b/t/t1091-sparse-checkout-builtin.sh
@@ -141,6 +141,21 @@ test_expect_success 'set sparse-checkout using --stdin' '
 	check_files repo "a folder1 folder2"
 '
 
+test_expect_success 'add to sparse-checkout' '
+	cat repo/.git/info/sparse-checkout >expect &&
+	cat >add <<-\EOF &&
+	pattern1
+	/folder1/
+	pattern2
+	EOF
+	cat add >>expect &&
+	git -C repo sparse-checkout add --stdin <add &&
+	git -C repo sparse-checkout list >actual &&
+	test_cmp expect actual &&
+	test_cmp expect repo/.git/info/sparse-checkout &&
+	check_files repo "a folder1 folder2"
+'
+
 test_expect_success 'cone mode: match patterns' '
 	git -C repo config --worktree core.sparseCheckoutCone true &&
 	rm -rf repo/a repo/folder1 repo/folder2 &&
@@ -219,8 +234,52 @@ test_expect_success 'cone mode: set with nested folders' '
 	test_cmp repo/.git/info/sparse-checkout expect
 '
 
+test_expect_success 'cone mode: add independent path' '
+	git -C repo sparse-checkout set deep/deeper1 &&
+	git -C repo sparse-checkout add folder1 &&
+	cat >expect <<-\EOF &&
+	/*
+	!/*/
+	/deep/
+	!/deep/*/
+	/deep/deeper1/
+	/folder1/
+	EOF
+	test_cmp expect repo/.git/info/sparse-checkout &&
+	check_files repo a deep folder1
+'
+
+test_expect_success 'cone mode: add sibling path' '
+	git -C repo sparse-checkout set deep/deeper1 &&
+	git -C repo sparse-checkout add deep/deeper2 &&
+	cat >expect <<-\EOF &&
+	/*
+	!/*/
+	/deep/
+	!/deep/*/
+	/deep/deeper1/
+	/deep/deeper2/
+	EOF
+	test_cmp expect repo/.git/info/sparse-checkout &&
+	check_files repo a deep
+'
+
+test_expect_success 'cone mode: add parent path' '
+	git -C repo sparse-checkout set deep/deeper1 folder1 &&
+	git -C repo sparse-checkout add deep &&
+	cat >expect <<-\EOF &&
+	/*
+	!/*/
+	/deep/
+	/folder1/
+	EOF
+	test_cmp expect repo/.git/info/sparse-checkout &&
+	check_files repo a deep folder1
+'
+
 test_expect_success 'revert to old sparse-checkout on bad update' '
 	test_when_finished git -C repo reset --hard &&
+	git -C repo sparse-checkout set deep &&
 	echo update >repo/deep/deeper2/a &&
 	cp repo/.git/info/sparse-checkout expect &&
 	test_must_fail git -C repo sparse-checkout set deep/deeper1 2>err &&
-- 
gitgitgadget
Junio C Hamano· Feb 11, 2020, 17:05 UTC · re: Derrick Stolee via GitGitGadget · lore

Re: [PATCH 3/4] sparse-checkout: create 'add' subcommand

"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:
Show 12 quoted lines
> +	switch (m) {
> +	case ADD:
> +		if (core_sparse_checkout_cone)
> +			add_patterns_cone_mode(argc, argv, &pl);
> +		else
> +			add_patterns_literal(argc, argv, &pl);
> +		break;
> +
> +	case REPLACE:
> +		add_patterns_from_input(&pl, argc, argv);
> +		break;
> +	}

Is it just me or do readers find it irritating to see the order of the arguments seem a bit random? Those who like Pseudo-OO-in-C probably would want to consistently see &pl as the first parameter, which lets them pretend various flavours of add_patterns_*() are all "methods" to the pattern list object.

Show 10 quoted lines
> +static void add_patterns_literal(int argc, const char **argv,
> +				 struct pattern_list *pl)
> +{
> +	char *sparse_filename = get_sparse_checkout_filename();
> +	if (add_patterns_from_file_to_list(sparse_filename, "", 0,
> +					   pl, NULL))
> +		die(_("unable to load existing sparse-checkout patterns"));
> +	free(sparse_filename);
> +	add_patterns_from_input(pl, argc, argv);
> +}

It may make it easier to read the caller if this helper were replaced with one that does not add anything but just read from the existing file, i.e.

		if (core_sparse_checkout_cone)
			add_patterns_cone_mode(argc, argv, &pl);
		else {
			read_existing_patterns(&pl);
			add_patterns_from_input(&pl, argc, argv);
		}
Derrick Stolee via GitGitGadget· Feb 11, 2020, 15:02 UTC · re: Derrick Stolee via GitGitGadget · lore

[PATCH 1/4] sparse-checkout: extract add_patterns_from_input()

From: Derrick Stolee <dstolee@microsoft.com>

In anticipation of extending the sparse-checkout builtin with "add" and "remove" subcommands, extract the code that fills a pattern list based on the input values. The input changes depending on the presence of "--stdin" or the value of core.sparseCheckoutCone.

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
---
 builtin/sparse-checkout.c | 64 +++++++++++++++++++++------------------
 1 file changed, 35 insertions(+), 29 deletions(-)
Show changes to builtin/sparse-checkout.c +35 −29
diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c
index 7aeb384362..41d8aaf9a2 100644
--- a/builtin/sparse-checkout.c
+++ b/builtin/sparse-checkout.c
@@ -412,36 +412,16 @@ static struct sparse_checkout_set_opts {
 	int use_stdin;
 } set_opts;
 
-static int sparse_checkout_set(int argc, const char **argv, const char *prefix)
+static void add_patterns_from_input(struct pattern_list *pl,
+				    int argc, const char **argv)
 {
 	int i;
-	struct pattern_list pl;
-	int result;
-	int changed_config = 0;
-
-	static struct option builtin_sparse_checkout_set_options[] = {
-		OPT_BOOL(0, "stdin", &set_opts.use_stdin,
-			 N_("read patterns from standard in")),
-		OPT_END(),
-	};
-
-	repo_read_index(the_repository);
-	require_clean_work_tree(the_repository,
-				N_("set sparse-checkout patterns"), NULL, 1, 0);
-
-	memset(&pl, 0, sizeof(pl));
-
-	argc = parse_options(argc, argv, prefix,
-			     builtin_sparse_checkout_set_options,
-			     builtin_sparse_checkout_set_usage,
-			     PARSE_OPT_KEEP_UNKNOWN);
-
 	if (core_sparse_checkout_cone) {
 		struct strbuf line = STRBUF_INIT;
 
-		hashmap_init(&pl.recursive_hashmap, pl_hashmap_cmp, NULL, 0);
-		hashmap_init(&pl.parent_hashmap, pl_hashmap_cmp, NULL, 0);
-		pl.use_cone_patterns = 1;
+		hashmap_init(&pl->recursive_hashmap, pl_hashmap_cmp, NULL, 0);
+		hashmap_init(&pl->parent_hashmap, pl_hashmap_cmp, NULL, 0);
+		pl->use_cone_patterns = 1;
 
 		if (set_opts.use_stdin) {
 			struct strbuf unquoted = STRBUF_INIT;
@@ -455,7 +435,7 @@ static int sparse_checkout_set(int argc, const char **argv, const char *prefix)
 					strbuf_swap(&unquoted, &line);
 				}
 
-				strbuf_to_cone_pattern(&line, &pl);
+				strbuf_to_cone_pattern(&line, pl);
 			}
 
 			strbuf_release(&unquoted);
@@ -463,7 +443,7 @@ static int sparse_checkout_set(int argc, const char **argv, const char *prefix)
 			for (i = 0; i < argc; i++) {
 				strbuf_setlen(&line, 0);
 				strbuf_addstr(&line, argv[i]);
-				strbuf_to_cone_pattern(&line, &pl);
+				strbuf_to_cone_pattern(&line, pl);
 			}
 		}
 	} else {
@@ -473,13 +453,39 @@ static int sparse_checkout_set(int argc, const char **argv, const char *prefix)
 			while (!strbuf_getline(&line, stdin)) {
 				size_t len;
 				char *buf = strbuf_detach(&line, &len);
-				add_pattern(buf, empty_base, 0, &pl, 0);
+				add_pattern(buf, empty_base, 0, pl, 0);
 			}
 		} else {
 			for (i = 0; i < argc; i++)
-				add_pattern(argv[i], empty_base, 0, &pl, 0);
+				add_pattern(argv[i], empty_base, 0, pl, 0);
 		}
 	}
+}
+
+static int sparse_checkout_set(int argc, const char **argv, const char *prefix)
+{
+	struct pattern_list pl;
+	int result;
+	int changed_config = 0;
+
+	static struct option builtin_sparse_checkout_set_options[] = {
+		OPT_BOOL(0, "stdin", &set_opts.use_stdin,
+			 N_("read patterns from standard in")),
+		OPT_END(),
+	};
+
+	repo_read_index(the_repository);
+	require_clean_work_tree(the_repository,
+				N_("set sparse-checkout patterns"), NULL, 1, 0);
+
+	memset(&pl, 0, sizeof(pl));
+
+	argc = parse_options(argc, argv, prefix,
+			     builtin_sparse_checkout_set_options,
+			     builtin_sparse_checkout_set_usage,
+			     PARSE_OPT_KEEP_UNKNOWN);
+
+	add_patterns_from_input(&pl, argc, argv);
 
 	if (!core_apply_sparse_checkout) {
 		set_config(MODE_ALL_PATTERNS);
-- 
gitgitgadget
Junio C Hamano· Feb 11, 2020, 16:56 UTC · re: Derrick Stolee via GitGitGadget · lore

Re: [PATCH 1/4] sparse-checkout: extract add_patterns_from_input()

"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:
> -static int sparse_checkout_set(int argc, const char **argv, const char *prefix)
> +static void add_patterns_from_input(struct pattern_list *pl,
> +				    int argc, const char **argv)

Separating out what happens _after_ parse_options into its own helper function makes sense.

Everything, not limited to this function, works on its input, so the name suffix does not add much value to the readers. IOW, I do not think I, as a new reader of this code, would be confused if this function were called add_patterns().

If we wanted to add more words to the name, perhaps I'd use them to describe the shape of the input (e.g. "add_patterns_from_acav") but that is obvious from the list of input parameter, so...

I haven't read the rest of the series, but will we ever call this helper function with an array we manufacture ourselves? If the input array is known to be NULL terminated (and at this step in the series, it certainly is, as we are using the ac/av supplied by the C runtime entry point), perhaps we can omit argc from the parameter to simplify the calling convention a bit?

Show 33 quoted lines
>  {
>  	int i;
> -	struct pattern_list pl;
> -	int result;
> -	int changed_config = 0;
> -
> -	static struct option builtin_sparse_checkout_set_options[] = {
> -		OPT_BOOL(0, "stdin", &set_opts.use_stdin,
> -			 N_("read patterns from standard in")),
> -		OPT_END(),
> -	};
> -
> -	repo_read_index(the_repository);
> -	require_clean_work_tree(the_repository,
> -				N_("set sparse-checkout patterns"), NULL, 1, 0);
> -
> -	memset(&pl, 0, sizeof(pl));
> -
> -	argc = parse_options(argc, argv, prefix,
> -			     builtin_sparse_checkout_set_options,
> -			     builtin_sparse_checkout_set_usage,
> -			     PARSE_OPT_KEEP_UNKNOWN);
> -
>  	if (core_sparse_checkout_cone) {
>  		struct strbuf line = STRBUF_INIT;
>  
> -		hashmap_init(&pl.recursive_hashmap, pl_hashmap_cmp, NULL, 0);
> -		hashmap_init(&pl.parent_hashmap, pl_hashmap_cmp, NULL, 0);
> -		pl.use_cone_patterns = 1;
> +		hashmap_init(&pl->recursive_hashmap, pl_hashmap_cmp, NULL, 0);
> +		hashmap_init(&pl->parent_hashmap, pl_hashmap_cmp, NULL, 0);
> +		pl->use_cone_patterns = 1;
> ...
Derrick Stolee via GitGitGadget· Feb 11, 2020, 15:02 UTC · re: Derrick Stolee via GitGitGadget · lore

[PATCH 4/4] sparse-checkout: work with Windows paths

From: Derrick Stolee <dstolee@microsoft.com>

When using Windows, a user may run 'git sparse-checkout set A\B\C' to add the Unix-style path A/B/C to their sparse-checkout patterns. Normalizing the input path converts the backslashes to slashes before we add the string 'A/B/C' to the recursive hashset.

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
---
 builtin/sparse-checkout.c          |  3 +++
 t/t1091-sparse-checkout-builtin.sh | 14 ++++++++++++++
 2 files changed, 17 insertions(+)
Show changes to 2 files +17 −0

builtin/sparse-checkout.c, t/t1091-sparse-checkout-builtin.sh

diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c
index af9e3e5123..3e314e3358 100644
--- a/builtin/sparse-checkout.c
+++ b/builtin/sparse-checkout.c
@@ -394,6 +394,9 @@ static void strbuf_to_cone_pattern(struct strbuf *line, struct pattern_list *pl)
 
 	strbuf_trim_trailing_dir_sep(line);
 
+	if (strbuf_normalize_path(line))
+		die(_("could not normalize path %s"), line->buf);
+
 	if (!line->len)
 		return;
 
diff --git a/t/t1091-sparse-checkout-builtin.sh b/t/t1091-sparse-checkout-builtin.sh
index f9265de5e8..c35cbdef45 100755
--- a/t/t1091-sparse-checkout-builtin.sh
+++ b/t/t1091-sparse-checkout-builtin.sh
@@ -497,4 +497,18 @@ test_expect_success BSLASHPSPEC 'pattern-checks: escaped characters' '
 	test_cmp list-expect list-actual
 '
 
+test_expect_success MINGW 'cone mode replaces backslashes with slashes' '
+	git -C repo sparse-checkout set deep\\deeper1 &&
+	cat >expect <<-\EOF &&
+	/*
+	!/*/
+	/deep/
+	!/deep/*/
+	/deep/deeper1/
+	EOF
+	test_cmp expect repo/.git/info/sparse-checkout &&
+	check_files repo a deep &&
+	check_files repo/deep a deeper1
+'
+
 test_done
-- 
gitgitgadget
Junio C Hamano· Feb 11, 2020, 17:06 UTC · re: Derrick Stolee via GitGitGadget · lore

Re: [PATCH 0/4] Sparse-checkout: Add subcommand and Windows paths

"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:
Show 6 quoted lines
> This is based on ds/sparse-checkout-harden.
>
> The sparse-checkout builtin currently lets users modify their
> sparse-checkout file with the all-or-nothing "set" subcommand. It may be
> easier for a user to expand their sparse cone using a "git sparse-checkout
> add <pattern/path> ..." subcommand.
Makes sense.  

I suspect that people may get greedy and start saying "now we can add a new directory (say, Documentation/) without repeating what we already configured, I want a way to add a new directory without some of the paths in that directory (say, Documentation/ but not Documentation/technical/)", but that won't happen until people can use "add", so let's take small incremental steps, and addition of "add" is such a good first step.

Will queue.  Thanks.
Jeff King· Feb 11, 2020, 19:48 UTC · re: Derrick Stolee via GitGitGadget · lore

Re: [PATCH 0/4] Sparse-checkout: Add subcommand and Windows paths

On Tue, Feb 11, 2020 at 03:02:20PM +0000, Derrick Stolee via GitGitGadget wrote:
Show 8 quoted lines
> This is based on ds/sparse-checkout-harden.
> 
> The sparse-checkout builtin currently lets users modify their
> sparse-checkout file with the all-or-nothing "set" subcommand. It may be
> easier for a user to expand their sparse cone using a "git sparse-checkout
> add <pattern/path> ..." subcommand. To achieve this while reusing as much
> code as possible from the "set" subcommand, the first two patches extract
> methods from sparse_checkout_set().

FWIW, this makes perfect sense to me. I actually expected "set" to behave like "add" the first time I tried it out.

-Peff

← back to recent threads