git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/6] sparse-checkout: make set subsume init
@ 2021-12-04 20:00 Elijah Newren via GitGitGadget
  2021-12-04 20:00 ` [PATCH 1/6] sparse-checkout: pass use_stdin as a parameter instead of as a global Elijah Newren via GitGitGadget
                   ` (7 more replies)
  0 siblings, 8 replies; 63+ messages in thread
From: Elijah Newren via GitGitGadget @ 2021-12-04 20:00 UTC (permalink / raw)
  To: git; +Cc: Derrick Stolee, Lessley Dennington, Victoria Dye, Elijah Newren

As described at [1], the split of init and set subcommands in
sparse-checkout causes multiple issues:

 * Poor performance (deleting all tracked files, then later restoring many
   and maybe even most of them)
 * Poor UI (multiple progress bars in wrappers that hide both commands under
   1 user-facing command)
 * Loss of ignored files under directories the user wanted to keep

This series fixes this bug by providing a single command to switch to a
sparse-checkout: set. It does so by making set able to do the combined work
of init and set. It keeps init as-is to give folks time to adapt, but marks
it as deprecated.

A quick overview:

 * Patches 1-2: small preparatory refactorings
 * Patch 3: the crux of the series
 * Patches 4-5: documentation modifications (Patch 4 is worth reviewing; it
   marks init as deprecated -- are others okay with that?)
 * Patch 6: trivial modification of git clone --sparse to use git
   sparse-checkout set rather than git sparse-checkout init.

[1]
https://lore.kernel.org/git/CABPp-BE8TJ8QGAQWsSGT7S+9Xp-XmApcC9PSw3K=RQOP0rt+PQ@mail.gmail.com/

Elijah Newren (6):
  sparse-checkout: pass use_stdin as a parameter instead of as a global
  sparse-checkout: break apart functions for sparse_checkout_(set|add)
  sparse-checkout: enable `set` to initialize sparse-checkout mode
  git-sparse-checkout.txt: update to document that set handles init
  Documentation: clarify/correct a few sparsity related statements
  clone: avoid using deprecated `sparse-checkout init`

 Documentation/git-clone.txt           |   8 +-
 Documentation/git-sparse-checkout.txt |  94 ++++++++++---------
 builtin/clone.c                       |   2 +-
 builtin/sparse-checkout.c             | 130 +++++++++++++++++++++-----
 4 files changed, 160 insertions(+), 74 deletions(-)


base-commit: abe6bb3905392d5eb6b01fa6e54d7e784e0522aa
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1151%2Fnewren%2Fsparse-checkout-no-init-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1151/newren/sparse-checkout-no-init-v1
Pull-Request: https://github.com/git/git/pull/1151
-- 
gitgitgadget

^ permalink raw reply	[flat|nested] 63+ messages in thread

* [PATCH 1/6] sparse-checkout: pass use_stdin as a parameter instead of as a global
  2021-12-04 20:00 [PATCH 0/6] sparse-checkout: make set subsume init Elijah Newren via GitGitGadget
@ 2021-12-04 20:00 ` Elijah Newren via GitGitGadget
  2021-12-04 20:00 ` [PATCH 2/6] sparse-checkout: break apart functions for sparse_checkout_(set|add) Elijah Newren via GitGitGadget
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 63+ messages in thread
From: Elijah Newren via GitGitGadget @ 2021-12-04 20:00 UTC (permalink / raw)
  To: git
  Cc: Derrick Stolee, Lessley Dennington, Victoria Dye, Elijah Newren,
	Elijah Newren

From: Elijah Newren <newren@gmail.com>

add_patterns_from_input() has relied on a global variable,
set_opts.use_stdin, which has been used by both the `set` and `add`
subcommands of sparse-checkout.  Once we introduce an
add_opts.use_stdin, the hardcoding of set_opts.use_stdin will be
incorrect.  Pass the value as function parameter instead to allow us to
make subsequent changes.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 builtin/sparse-checkout.c | 28 ++++++++++++++++------------
 1 file changed, 16 insertions(+), 12 deletions(-)

diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c
index d0f5c4702be..8612328e5dd 100644
--- a/builtin/sparse-checkout.c
+++ b/builtin/sparse-checkout.c
@@ -525,7 +525,8 @@ static struct sparse_checkout_set_opts {
 } set_opts;
 
 static void add_patterns_from_input(struct pattern_list *pl,
-				    int argc, const char **argv)
+				    int argc, const char **argv,
+				    int use_stdin)
 {
 	int i;
 	if (core_sparse_checkout_cone) {
@@ -535,7 +536,7 @@ static void add_patterns_from_input(struct pattern_list *pl,
 		hashmap_init(&pl->parent_hashmap, pl_hashmap_cmp, NULL, 0);
 		pl->use_cone_patterns = 1;
 
-		if (set_opts.use_stdin) {
+		if (use_stdin) {
 			struct strbuf unquoted = STRBUF_INIT;
 			while (!strbuf_getline(&line, stdin)) {
 				if (line.buf[0] == '"') {
@@ -559,7 +560,7 @@ static void add_patterns_from_input(struct pattern_list *pl,
 			}
 		}
 	} else {
-		if (set_opts.use_stdin) {
+		if (use_stdin) {
 			struct strbuf line = STRBUF_INIT;
 
 			while (!strbuf_getline(&line, stdin)) {
@@ -580,7 +581,8 @@ enum modify_type {
 };
 
 static void add_patterns_cone_mode(int argc, const char **argv,
-				   struct pattern_list *pl)
+				   struct pattern_list *pl,
+				   int use_stdin)
 {
 	struct strbuf buffer = STRBUF_INIT;
 	struct pattern_entry *pe;
@@ -588,7 +590,7 @@ static void add_patterns_cone_mode(int argc, const char **argv,
 	struct pattern_list existing;
 	char *sparse_filename = get_sparse_checkout_filename();
 
-	add_patterns_from_input(pl, argc, argv);
+	add_patterns_from_input(pl, argc, argv, use_stdin);
 
 	memset(&existing, 0, sizeof(existing));
 	existing.use_cone_patterns = core_sparse_checkout_cone;
@@ -614,17 +616,19 @@ static void add_patterns_cone_mode(int argc, const char **argv,
 }
 
 static void add_patterns_literal(int argc, const char **argv,
-				 struct pattern_list *pl)
+				 struct pattern_list *pl,
+				 int use_stdin)
 {
 	char *sparse_filename = get_sparse_checkout_filename();
 	if (add_patterns_from_file_to_list(sparse_filename, "", 0,
 					   pl, NULL, 0))
 		die(_("unable to load existing sparse-checkout patterns"));
 	free(sparse_filename);
-	add_patterns_from_input(pl, argc, argv);
+	add_patterns_from_input(pl, argc, argv, use_stdin);
 }
 
-static int modify_pattern_list(int argc, const char **argv, enum modify_type m)
+static int modify_pattern_list(int argc, const char **argv, int use_stdin,
+			       enum modify_type m)
 {
 	int result;
 	int changed_config = 0;
@@ -633,13 +637,13 @@ static int modify_pattern_list(int argc, const char **argv, enum modify_type m)
 	switch (m) {
 	case ADD:
 		if (core_sparse_checkout_cone)
-			add_patterns_cone_mode(argc, argv, pl);
+			add_patterns_cone_mode(argc, argv, pl, use_stdin);
 		else
-			add_patterns_literal(argc, argv, pl);
+			add_patterns_literal(argc, argv, pl, use_stdin);
 		break;
 
 	case REPLACE:
-		add_patterns_from_input(pl, argc, argv);
+		add_patterns_from_input(pl, argc, argv, use_stdin);
 		break;
 	}
 
@@ -675,7 +679,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, m);
+	return modify_pattern_list(argc, argv, set_opts.use_stdin, m);
 }
 
 static char const * const builtin_sparse_checkout_reapply_usage[] = {
-- 
gitgitgadget


^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH 2/6] sparse-checkout: break apart functions for sparse_checkout_(set|add)
  2021-12-04 20:00 [PATCH 0/6] sparse-checkout: make set subsume init Elijah Newren via GitGitGadget
  2021-12-04 20:00 ` [PATCH 1/6] sparse-checkout: pass use_stdin as a parameter instead of as a global Elijah Newren via GitGitGadget
@ 2021-12-04 20:00 ` Elijah Newren via GitGitGadget
  2021-12-04 20:00 ` [PATCH 3/6] sparse-checkout: enable `set` to initialize sparse-checkout mode Elijah Newren via GitGitGadget
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 63+ messages in thread
From: Elijah Newren via GitGitGadget @ 2021-12-04 20:00 UTC (permalink / raw)
  To: git
  Cc: Derrick Stolee, Lessley Dennington, Victoria Dye, Elijah Newren,
	Elijah Newren

From: Elijah Newren <newren@gmail.com>

sparse_checkout_set() was reused by sparse_checkout_add() with the only
difference being a single parameter being passed to that function.
However, we would like sparse_checkout_set() to do the same work that
sparse_checkout_init() does if sparse checkouts are not already enabled.
To facilitate this transition, give each mode their own copy of the
function.  This does not introduce any behavioral changes; that will
come in a subsequent patch.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 builtin/sparse-checkout.c | 54 +++++++++++++++++++++++++++++----------
 1 file changed, 40 insertions(+), 14 deletions(-)

diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c
index 8612328e5dd..e252b82136e 100644
--- a/builtin/sparse-checkout.c
+++ b/builtin/sparse-checkout.c
@@ -515,15 +515,6 @@ static void strbuf_to_cone_pattern(struct strbuf *line, struct pattern_list *pl)
 	insert_recursive_pattern(pl, line);
 }
 
-static char const * const builtin_sparse_checkout_set_usage[] = {
-	N_("git sparse-checkout (set|add) (--stdin | <patterns>)"),
-	NULL
-};
-
-static struct sparse_checkout_set_opts {
-	int use_stdin;
-} set_opts;
-
 static void add_patterns_from_input(struct pattern_list *pl,
 				    int argc, const char **argv,
 				    int use_stdin)
@@ -663,8 +654,43 @@ static int modify_pattern_list(int argc, const char **argv, int use_stdin,
 	return result;
 }
 
-static int sparse_checkout_set(int argc, const char **argv, const char *prefix,
-			       enum modify_type m)
+static char const * const builtin_sparse_checkout_add_usage[] = {
+	N_("git sparse-checkout add (--stdin | <patterns>)"),
+	NULL
+};
+
+static struct sparse_checkout_add_opts {
+	int use_stdin;
+} add_opts;
+
+static int sparse_checkout_add(int argc, const char **argv, const char *prefix)
+{
+	static struct option builtin_sparse_checkout_add_options[] = {
+		OPT_BOOL(0, "stdin", &add_opts.use_stdin,
+			 N_("read patterns from standard in")),
+		OPT_END(),
+	};
+
+	repo_read_index(the_repository);
+
+	argc = parse_options(argc, argv, prefix,
+			     builtin_sparse_checkout_add_options,
+			     builtin_sparse_checkout_add_usage,
+			     PARSE_OPT_KEEP_UNKNOWN);
+
+	return modify_pattern_list(argc, argv, add_opts.use_stdin, ADD);
+}
+
+static char const * const builtin_sparse_checkout_set_usage[] = {
+	N_("git sparse-checkout set (--stdin | <patterns>)"),
+	NULL
+};
+
+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 struct option builtin_sparse_checkout_set_options[] = {
 		OPT_BOOL(0, "stdin", &set_opts.use_stdin,
@@ -679,7 +705,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, set_opts.use_stdin, m);
+	return modify_pattern_list(argc, argv, set_opts.use_stdin, REPLACE);
 }
 
 static char const * const builtin_sparse_checkout_reapply_usage[] = {
@@ -762,9 +788,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, REPLACE);
+			return sparse_checkout_set(argc, argv, prefix);
 		if (!strcmp(argv[0], "add"))
-			return sparse_checkout_set(argc, argv, prefix, ADD);
+			return sparse_checkout_add(argc, argv, prefix);
 		if (!strcmp(argv[0], "reapply"))
 			return sparse_checkout_reapply(argc, argv);
 		if (!strcmp(argv[0], "disable"))
-- 
gitgitgadget


^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH 3/6] sparse-checkout: enable `set` to initialize sparse-checkout mode
  2021-12-04 20:00 [PATCH 0/6] sparse-checkout: make set subsume init Elijah Newren via GitGitGadget
  2021-12-04 20:00 ` [PATCH 1/6] sparse-checkout: pass use_stdin as a parameter instead of as a global Elijah Newren via GitGitGadget
  2021-12-04 20:00 ` [PATCH 2/6] sparse-checkout: break apart functions for sparse_checkout_(set|add) Elijah Newren via GitGitGadget
@ 2021-12-04 20:00 ` Elijah Newren via GitGitGadget
  2021-12-04 21:40   ` Victoria Dye
  2021-12-07 16:42   ` Derrick Stolee
  2021-12-04 20:00 ` [PATCH 4/6] git-sparse-checkout.txt: update to document that set handles init Elijah Newren via GitGitGadget
                   ` (4 subsequent siblings)
  7 siblings, 2 replies; 63+ messages in thread
From: Elijah Newren via GitGitGadget @ 2021-12-04 20:00 UTC (permalink / raw)
  To: git
  Cc: Derrick Stolee, Lessley Dennington, Victoria Dye, Elijah Newren,
	Elijah Newren

From: Elijah Newren <newren@gmail.com>

The previously suggested workflow:
  git sparse-checkout init ...
  git sparse-checkout set ...

Suffered from three problems:
  1) It would delete nearly all files in the first step, then
     restore them in the second.  That was poor performance and
     forced unnecessary rebuilds.
  2) The two-step process resulted in two progress bars, which
     was suboptimal from a UI point of view for wrappers that
     invoked both of these commands but only exposed a single
     command to their end users.
  3) With cone mode, the first step would delete nearly all
     ignored files everywhere, because everything was considered
     to be outside of the specified sparsity paths.  (The user was
     not allowed to specify any sparsity paths in the `init` step.)

Avoid these problems by teaching `set` to understand the extra
parameters that `init` takes and performing any necessary initialization
if not already in a sparse checkout.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 builtin/sparse-checkout.c | 52 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 51 insertions(+), 1 deletion(-)

diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c
index e252b82136e..cf6a6c6c3a7 100644
--- a/builtin/sparse-checkout.c
+++ b/builtin/sparse-checkout.c
@@ -682,17 +682,26 @@ static int sparse_checkout_add(int argc, const char **argv, const char *prefix)
 }
 
 static char const * const builtin_sparse_checkout_set_usage[] = {
-	N_("git sparse-checkout set (--stdin | <patterns>)"),
+	N_("git sparse-checkout set [--cone] [--[no-]sparse-index] (--stdin | <patterns>)"),
 	NULL
 };
 
 static struct sparse_checkout_set_opts {
+	int cone_mode;
+	int sparse_index;
 	int use_stdin;
 } set_opts;
 
 static int sparse_checkout_set(int argc, const char **argv, const char *prefix)
 {
+	int mode, record_mode;
+	const char *default_patterns[] = {"/*", "!/*/"};
+
 	static struct option builtin_sparse_checkout_set_options[] = {
+		OPT_BOOL(0, "cone", &set_opts.cone_mode,
+			 N_("initialize the sparse-checkout in cone mode")),
+		OPT_BOOL(0, "sparse-index", &set_opts.sparse_index,
+			 N_("toggle the use of a sparse index")),
 		OPT_BOOL(0, "stdin", &set_opts.use_stdin,
 			 N_("read patterns from standard in")),
 		OPT_END(),
@@ -700,11 +709,52 @@ static int sparse_checkout_set(int argc, const char **argv, const char *prefix)
 
 	repo_read_index(the_repository);
 
+	set_opts.cone_mode = -1;
+	set_opts.sparse_index = -1;
+
 	argc = parse_options(argc, argv, prefix,
 			     builtin_sparse_checkout_set_options,
 			     builtin_sparse_checkout_set_usage,
 			     PARSE_OPT_KEEP_UNKNOWN);
 
+	/* Determine if we need to record the mode; ensure sparse checkout on */
+	record_mode = (set_opts.cone_mode != -1) || !core_apply_sparse_checkout;
+	core_apply_sparse_checkout = 1;
+
+	/* If not specified, use previous definition of cone mode */
+	if (set_opts.cone_mode == -1 && core_apply_sparse_checkout)
+		set_opts.cone_mode = core_sparse_checkout_cone;
+
+	/* Set cone/non-cone mode appropriately */
+	if (set_opts.cone_mode == 1) {
+		mode = MODE_CONE_PATTERNS;
+		core_sparse_checkout_cone = 1;
+	} else {
+		mode = MODE_ALL_PATTERNS;
+	}
+	if (record_mode && set_config(mode))
+		return 1;
+
+	/* Set sparse-index/non-sparse-index mode if specified */
+	if (set_opts.sparse_index >= 0) {
+		if (set_sparse_index_config(the_repository, set_opts.sparse_index) < 0)
+			die(_("failed to modify sparse-index config"));
+
+		/* force an index rewrite */
+		repo_read_index(the_repository);
+		the_repository->index->updated_workdir = 1;
+	}
+
+	/*
+	 * Cone mode automatically specifies the toplevel directory.  For
+	 * non-cone mode, if nothing is specified, manually select just the
+	 * top-level directory (much as 'init' would do).
+	 */
+	if (!core_sparse_checkout_cone && argc == 0) {
+		argv = default_patterns;
+		argc = 2;
+	}
+
 	return modify_pattern_list(argc, argv, set_opts.use_stdin, REPLACE);
 }
 
-- 
gitgitgadget


^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH 4/6] git-sparse-checkout.txt: update to document that set handles init
  2021-12-04 20:00 [PATCH 0/6] sparse-checkout: make set subsume init Elijah Newren via GitGitGadget
                   ` (2 preceding siblings ...)
  2021-12-04 20:00 ` [PATCH 3/6] sparse-checkout: enable `set` to initialize sparse-checkout mode Elijah Newren via GitGitGadget
@ 2021-12-04 20:00 ` Elijah Newren via GitGitGadget
  2021-12-04 21:48   ` Victoria Dye
  2021-12-07 20:42   ` Lessley Dennington
  2021-12-04 20:00 ` [PATCH 5/6] Documentation: clarify/correct a few sparsity related statements Elijah Newren via GitGitGadget
                   ` (3 subsequent siblings)
  7 siblings, 2 replies; 63+ messages in thread
From: Elijah Newren via GitGitGadget @ 2021-12-04 20:00 UTC (permalink / raw)
  To: git
  Cc: Derrick Stolee, Lessley Dennington, Victoria Dye, Elijah Newren,
	Elijah Newren

From: Elijah Newren <newren@gmail.com>

As noted in the previous commit, using separate `init` and `set` steps
with sparse-checkout result in a number of issues.  The previous commit
made `set` able to handle the work of both commands.  Update the
documentation to reflect this, and mark `init` as deprecated.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 Documentation/git-sparse-checkout.txt | 92 ++++++++++++++-------------
 1 file changed, 49 insertions(+), 43 deletions(-)

diff --git a/Documentation/git-sparse-checkout.txt b/Documentation/git-sparse-checkout.txt
index 42056ee9ff9..d22c925ecf8 100644
--- a/Documentation/git-sparse-checkout.txt
+++ b/Documentation/git-sparse-checkout.txt
@@ -30,28 +30,35 @@ COMMANDS
 'list'::
 	Describe the patterns in the sparse-checkout file.
 
-'init'::
-	Enable the `core.sparseCheckout` setting. If the
-	sparse-checkout file does not exist, then populate it with
-	patterns that match every file in the root directory and
-	no other directories, then will remove all directories tracked
-	by Git. Add patterns to the sparse-checkout file to
-	repopulate the working directory.
+'set'::
+	Enable the necessary config settings
+	(extensions.worktreeConfig, core.sparseCheckout,
+	core.sparseCheckoutCone) if they are not already enabled, and
+	write a set of patterns to the sparse-checkout file from the
+	list of arguments following the 'set' subcommand. Update the
+	working directory to match the new patterns.
 +
-To avoid interfering with other worktrees, it first enables the
-`extensions.worktreeConfig` setting and makes sure to set the
-`core.sparseCheckout` setting in the worktree-specific config file.
+When the `--stdin` option is provided, the patterns are read from
+standard in as a newline-delimited list instead of from the arguments.
 +
-When `--cone` is provided, the `core.sparseCheckoutCone` setting is
-also set, allowing for better performance with a limited set of
-patterns (see 'CONE PATTERN SET' below).
+When `--cone` is passed or `core.sparseCheckoutCone` is enabled, the
+input list is considered a list of directories instead of
+sparse-checkout patterns.  This allows for better performance with a
+limited set of patterns (see 'CONE PATTERN SET' below).  Note that the
+set command will write patterns to the sparse-checkout file to include
+all files contained in those directories (recursively) as well as
+files that are siblings of ancestor 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.
 +
-Use the `--[no-]sparse-index` option to toggle the use of the sparse
-index format. This reduces the size of the index to be more closely
-aligned with your sparse-checkout definition. This can have significant
-performance advantages for commands such as `git status` or `git add`.
-This feature is still experimental. Some commands might be slower with
-a sparse index until they are properly integrated with the feature.
+Use the `--[no-]sparse-index` option to use a sparse index (the
+default is to not use it).  A sparse index reduces the size of the
+index to be more closely aligned with your sparse-checkout
+definition. This can have significant performance advantages for
+commands such as `git status` or `git add`.  This feature is still
+experimental. Some commands might be slower with a sparse index until
+they are properly integrated with the feature.
 +
 **WARNING:** Using a sparse index requires modifying the index in a way
 that is not completely understood by external tools. If you have trouble
@@ -60,23 +67,6 @@ to rewrite your index to not be sparse. Older versions of Git will not
 understand the sparse directory entries index extension and may fail to
 interact with your repository until it is disabled.
 
-'set'::
-	Write a set of patterns to the sparse-checkout file, as given as
-	a list of arguments following the 'set' subcommand. Update the
-	working directory to match the new patterns. Enable the
-	core.sparseCheckout config setting if it is not already enabled.
-+
-When the `--stdin` option is provided, the patterns are read from
-standard in as a newline-delimited list instead of from the arguments.
-+
-When `core.sparseCheckoutCone` is enabled, the input list is considered a
-list of directories instead of sparse-checkout patterns. The command writes
-patterns to the sparse-checkout file to include all files contained in those
-directories (recursively) as well as files that are siblings of ancestor
-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,
@@ -96,9 +86,27 @@ C-style quoted strings.
 
 'disable'::
 	Disable the `core.sparseCheckout` config setting, and restore the
-	working directory to include all files. Leaves the sparse-checkout
-	file intact so a later 'git sparse-checkout init' command may
-	return the working directory to the same state.
+	working directory to include all files.
+
+'init'::
+	Deprecated command that behaves like `set` with no specified paths.
+	May be removed in the future.
++
+Historically, `set` did not used to handle all the necessary config
+settings, which meant that both `init` and `set` had to be called.
+Invoking both meant the `init` step would first remove nearly all
+tracked files (and in cone mode, ignored files too), then the `set`
+step would add many of the tracked files (but not ignored files) back.
+In addition to the lost files, the performance and UI of this
+combination was poor.
++
+Also, historically, `init` would not actually initialize the
+sparse-checkout file if it already existed.  This meant it was
+possible to return to a sparse-checkout without remembering which
+paths to pass to a subsequent 'set' or 'add' command.  However,
+`--cone` and `--sparse-index` options would not be remembered across
+the disable command, so the easy restore of calling a plain `init`
+decreased in utility.
 
 SPARSE CHECKOUT
 ---------------
@@ -117,10 +125,8 @@ directory, it updates the skip-worktree bits in the index based
 on this file. The files matching the patterns in the file will
 appear in the working directory, and the rest will not.
 
-To enable the sparse-checkout feature, run `git sparse-checkout init` to
-initialize a simple sparse-checkout file and enable the `core.sparseCheckout`
-config setting. Then, run `git sparse-checkout set` to modify the patterns in
-the sparse-checkout file.
+To enable the sparse-checkout feature, run `git sparse-checkout set` to
+set the patterns you want to use.
 
 To repopulate the working directory with all files, use the
 `git sparse-checkout disable` command.
-- 
gitgitgadget


^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH 5/6] Documentation: clarify/correct a few sparsity related statements
  2021-12-04 20:00 [PATCH 0/6] sparse-checkout: make set subsume init Elijah Newren via GitGitGadget
                   ` (3 preceding siblings ...)
  2021-12-04 20:00 ` [PATCH 4/6] git-sparse-checkout.txt: update to document that set handles init Elijah Newren via GitGitGadget
@ 2021-12-04 20:00 ` Elijah Newren via GitGitGadget
  2021-12-04 20:00 ` [PATCH 6/6] clone: avoid using deprecated `sparse-checkout init` Elijah Newren via GitGitGadget
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 63+ messages in thread
From: Elijah Newren via GitGitGadget @ 2021-12-04 20:00 UTC (permalink / raw)
  To: git
  Cc: Derrick Stolee, Lessley Dennington, Victoria Dye, Elijah Newren,
	Elijah Newren

From: Elijah Newren <newren@gmail.com>

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 Documentation/git-clone.txt           | 8 ++++----
 Documentation/git-sparse-checkout.txt | 2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt
index 3fe3810f1ce..b348a71fc68 100644
--- a/Documentation/git-clone.txt
+++ b/Documentation/git-clone.txt
@@ -167,10 +167,10 @@ objects from the source repository into a pack in the cloned repository.
 	configuration variables are created.
 
 --sparse::
-	Initialize the sparse-checkout file so the working
-	directory starts with only the files in the root
-	of the repository. The sparse-checkout file can be
-	modified to grow the working directory as needed.
+	Employ a sparse-checkout, with only files in the toplevel
+	directory initially being present.  The
+	linkgit:git-sparse-checkout[1] command can be used to grow the
+	working directory as needed.
 
 --filter=<filter-spec>::
 	Use the partial clone feature and request that the server sends
diff --git a/Documentation/git-sparse-checkout.txt b/Documentation/git-sparse-checkout.txt
index d22c925ecf8..73e7645b77a 100644
--- a/Documentation/git-sparse-checkout.txt
+++ b/Documentation/git-sparse-checkout.txt
@@ -115,7 +115,7 @@ SPARSE CHECKOUT
 It uses the skip-worktree bit (see linkgit:git-update-index[1]) to tell
 Git whether a file in the working directory is worth looking at. If
 the skip-worktree bit is set, then the file is ignored in the working
-directory. Git will not populate the contents of those files, which
+directory. Git will avoid populating the contents of those files, which
 makes a sparse checkout helpful when working in a repository with many
 files, but only a few are important to the current user.
 
-- 
gitgitgadget


^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH 6/6] clone: avoid using deprecated `sparse-checkout init`
  2021-12-04 20:00 [PATCH 0/6] sparse-checkout: make set subsume init Elijah Newren via GitGitGadget
                   ` (4 preceding siblings ...)
  2021-12-04 20:00 ` [PATCH 5/6] Documentation: clarify/correct a few sparsity related statements Elijah Newren via GitGitGadget
@ 2021-12-04 20:00 ` Elijah Newren via GitGitGadget
  2021-12-07 16:48 ` [PATCH 0/6] sparse-checkout: make set subsume init Derrick Stolee
  2021-12-07 20:20 ` [PATCH v2 00/10] " Elijah Newren via GitGitGadget
  7 siblings, 0 replies; 63+ messages in thread
From: Elijah Newren via GitGitGadget @ 2021-12-04 20:00 UTC (permalink / raw)
  To: git
  Cc: Derrick Stolee, Lessley Dennington, Victoria Dye, Elijah Newren,
	Elijah Newren

From: Elijah Newren <newren@gmail.com>

The previous commits marked `sparse-checkout init` as deprecated; we
can just use `set` instead here and pass it no paths.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 builtin/clone.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/clone.c b/builtin/clone.c
index fb377b27657..5bed37f8b51 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -633,7 +633,7 @@ static int git_sparse_checkout_init(const char *repo)
 {
 	struct strvec argv = STRVEC_INIT;
 	int result = 0;
-	strvec_pushl(&argv, "-C", repo, "sparse-checkout", "init", NULL);
+	strvec_pushl(&argv, "-C", repo, "sparse-checkout", "set", NULL);
 
 	/*
 	 * We must apply the setting in the current process
-- 
gitgitgadget

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* Re: [PATCH 3/6] sparse-checkout: enable `set` to initialize sparse-checkout mode
  2021-12-04 20:00 ` [PATCH 3/6] sparse-checkout: enable `set` to initialize sparse-checkout mode Elijah Newren via GitGitGadget
@ 2021-12-04 21:40   ` Victoria Dye
  2021-12-07 17:04     ` Elijah Newren
  2021-12-07 16:42   ` Derrick Stolee
  1 sibling, 1 reply; 63+ messages in thread
From: Victoria Dye @ 2021-12-04 21:40 UTC (permalink / raw)
  To: Elijah Newren via GitGitGadget, git
  Cc: Derrick Stolee, Lessley Dennington, Elijah Newren

Elijah Newren via GitGitGadget wrote:
> From: Elijah Newren <newren@gmail.com>
> 
> The previously suggested workflow:
>   git sparse-checkout init ...
>   git sparse-checkout set ...
> 
> Suffered from three problems:
>   1) It would delete nearly all files in the first step, then
>      restore them in the second.  That was poor performance and
>      forced unnecessary rebuilds.
>   2) The two-step process resulted in two progress bars, which
>      was suboptimal from a UI point of view for wrappers that
>      invoked both of these commands but only exposed a single
>      command to their end users.
>   3) With cone mode, the first step would delete nearly all
>      ignored files everywhere, because everything was considered
>      to be outside of the specified sparsity paths.  (The user was
>      not allowed to specify any sparsity paths in the `init` step.)
> 
> Avoid these problems by teaching `set` to understand the extra
> parameters that `init` takes and performing any necessary initialization
> if not already in a sparse checkout.
> 

I really like this change! It always seemed weird that `set` would
implicitly `init`, but without any of the options in `init`.

> Signed-off-by: Elijah Newren <newren@gmail.com>
> ---
>  builtin/sparse-checkout.c | 52 ++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 51 insertions(+), 1 deletion(-)
> 
> diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c
> index e252b82136e..cf6a6c6c3a7 100644
> --- a/builtin/sparse-checkout.c
> +++ b/builtin/sparse-checkout.c
> @@ -682,17 +682,26 @@ static int sparse_checkout_add(int argc, const char **argv, const char *prefix)
>  }
>  
>  static char const * const builtin_sparse_checkout_set_usage[] = {
> -	N_("git sparse-checkout set (--stdin | <patterns>)"),
> +	N_("git sparse-checkout set [--cone] [--[no-]sparse-index] (--stdin | <patterns>)"),

Since `cone` is an `OPT_BOOL`, shouldn't it appear in the usage string as
`--[no-]cone`? Without a `--no-cone` option, it's not clear how a user would
disable cone mode (since `set` preserves the existing cone mode setting if
`--cone` isn't given).

>  	NULL
>  };
>  
>  static struct sparse_checkout_set_opts {
> +	int cone_mode;
> +	int sparse_index;
>  	int use_stdin;
>  } set_opts;
>  
>  static int sparse_checkout_set(int argc, const char **argv, const char *prefix)
>  {
> +	int mode, record_mode;
> +	const char *default_patterns[] = {"/*", "!/*/"};
> +
>  	static struct option builtin_sparse_checkout_set_options[] = {
> +		OPT_BOOL(0, "cone", &set_opts.cone_mode,
> +			 N_("initialize the sparse-checkout in cone mode")),
> +		OPT_BOOL(0, "sparse-index", &set_opts.sparse_index,
> +			 N_("toggle the use of a sparse index")),
>  		OPT_BOOL(0, "stdin", &set_opts.use_stdin,
>  			 N_("read patterns from standard in")),

I know this isn't part of this patch, but is `stdin` really a bool (i.e.
someone might specify `--no-stdin`)? If not, I think `OPT_SET_INT` might be
more appropriate.

>  		OPT_END(),
> @@ -700,11 +709,52 @@ static int sparse_checkout_set(int argc, const char **argv, const char *prefix)
>  
>  	repo_read_index(the_repository);
>  
> +	set_opts.cone_mode = -1;
> +	set_opts.sparse_index = -1;
> +
>  	argc = parse_options(argc, argv, prefix,
>  			     builtin_sparse_checkout_set_options,
>  			     builtin_sparse_checkout_set_usage,
>  			     PARSE_OPT_KEEP_UNKNOWN);
>  
> +	/* Determine if we need to record the mode; ensure sparse checkout on */
> +	record_mode = (set_opts.cone_mode != -1) || !core_apply_sparse_checkout;
> +	core_apply_sparse_checkout = 1;
> +
> +	/* If not specified, use previous definition of cone mode */
> +	if (set_opts.cone_mode == -1 && core_apply_sparse_checkout)

I *think* this is supposed go before the `core_apply_sparse_checkout = 1;`
above it (if the intention is to only use the pre-existing cone mode setting
when already in a sparse checkout). If not, the `core_apply_sparse_checkout`
part of the condition is redundant and can be removed entirely.

> +		set_opts.cone_mode = core_sparse_checkout_cone;
> +
> +	/* Set cone/non-cone mode appropriately */
> +	if (set_opts.cone_mode == 1) {
> +		mode = MODE_CONE_PATTERNS;
> +		core_sparse_checkout_cone = 1;
> +	} else {
> +		mode = MODE_ALL_PATTERNS;
> +	}
> +	if (record_mode && set_config(mode))
> +		return 1;
> +
> +	/* Set sparse-index/non-sparse-index mode if specified */
> +	if (set_opts.sparse_index >= 0) {
> +		if (set_sparse_index_config(the_repository, set_opts.sparse_index) < 0)
> +			die(_("failed to modify sparse-index config"));
> +
> +		/* force an index rewrite */
> +		repo_read_index(the_repository);
> +		the_repository->index->updated_workdir = 1;
> +	}
> +
> +	/*
> +	 * Cone mode automatically specifies the toplevel directory.  For
> +	 * non-cone mode, if nothing is specified, manually select just the
> +	 * top-level directory (much as 'init' would do).
> +	 */
> +	if (!core_sparse_checkout_cone && argc == 0) {
> +		argv = default_patterns;
> +		argc = 2;
> +	}
> +
>  	return modify_pattern_list(argc, argv, set_opts.use_stdin, REPLACE);
>  }
>  
> 


^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [PATCH 4/6] git-sparse-checkout.txt: update to document that set handles init
  2021-12-04 20:00 ` [PATCH 4/6] git-sparse-checkout.txt: update to document that set handles init Elijah Newren via GitGitGadget
@ 2021-12-04 21:48   ` Victoria Dye
  2021-12-07 16:45     ` Derrick Stolee
  2021-12-07 17:06     ` Elijah Newren
  2021-12-07 20:42   ` Lessley Dennington
  1 sibling, 2 replies; 63+ messages in thread
From: Victoria Dye @ 2021-12-04 21:48 UTC (permalink / raw)
  To: Elijah Newren via GitGitGadget, git
  Cc: Derrick Stolee, Lessley Dennington, Elijah Newren

Elijah Newren via GitGitGadget wrote:
> From: Elijah Newren <newren@gmail.com>
> 
> As noted in the previous commit, using separate `init` and `set` steps
> with sparse-checkout result in a number of issues.  The previous commit
> made `set` able to handle the work of both commands.  Update the
> documentation to reflect this, and mark `init` as deprecated.
> 
> Signed-off-by: Elijah Newren <newren@gmail.com>
> ---
>  Documentation/git-sparse-checkout.txt | 92 ++++++++++++++-------------
>  1 file changed, 49 insertions(+), 43 deletions(-)
> 
> diff --git a/Documentation/git-sparse-checkout.txt b/Documentation/git-sparse-checkout.txt
> index 42056ee9ff9..d22c925ecf8 100644
> --- a/Documentation/git-sparse-checkout.txt
> +++ b/Documentation/git-sparse-checkout.txt
> @@ -30,28 +30,35 @@ COMMANDS
>  'list'::
>  	Describe the patterns in the sparse-checkout file.
>  
> -'init'::
> -	Enable the `core.sparseCheckout` setting. If the
> -	sparse-checkout file does not exist, then populate it with
> -	patterns that match every file in the root directory and
> -	no other directories, then will remove all directories tracked
> -	by Git. Add patterns to the sparse-checkout file to
> -	repopulate the working directory.
> +'set'::
> +	Enable the necessary config settings
> +	(extensions.worktreeConfig, core.sparseCheckout,
> +	core.sparseCheckoutCone) if they are not already enabled, and
> +	write a set of patterns to the sparse-checkout file from the
> +	list of arguments following the 'set' subcommand. Update the
> +	working directory to match the new patterns.
>  +
> -To avoid interfering with other worktrees, it first enables the
> -`extensions.worktreeConfig` setting and makes sure to set the
> -`core.sparseCheckout` setting in the worktree-specific config file.
> +When the `--stdin` option is provided, the patterns are read from
> +standard in as a newline-delimited list instead of from the arguments.
>  +
> -When `--cone` is provided, the `core.sparseCheckoutCone` setting is
> -also set, allowing for better performance with a limited set of
> -patterns (see 'CONE PATTERN SET' below).
> +When `--cone` is passed or `core.sparseCheckoutCone` is enabled, the
> +input list is considered a list of directories instead of
> +sparse-checkout patterns.  This allows for better performance with a
> +limited set of patterns (see 'CONE PATTERN SET' below).  Note that the
> +set command will write patterns to the sparse-checkout file to include
> +all files contained in those directories (recursively) as well as
> +files that are siblings of ancestor 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.
>  +
> -Use the `--[no-]sparse-index` option to toggle the use of the sparse
> -index format. This reduces the size of the index to be more closely
> -aligned with your sparse-checkout definition. This can have significant
> -performance advantages for commands such as `git status` or `git add`.
> -This feature is still experimental. Some commands might be slower with
> -a sparse index until they are properly integrated with the feature.
> +Use the `--[no-]sparse-index` option to use a sparse index (the
> +default is to not use it).  A sparse index reduces the size of the
> +index to be more closely aligned with your sparse-checkout
> +definition. This can have significant performance advantages for
> +commands such as `git status` or `git add`.  This feature is still
> +experimental. Some commands might be slower with a sparse index until
> +they are properly integrated with the feature.
>  +
>  **WARNING:** Using a sparse index requires modifying the index in a way
>  that is not completely understood by external tools. If you have trouble
> @@ -60,23 +67,6 @@ to rewrite your index to not be sparse. Older versions of Git will not
>  understand the sparse directory entries index extension and may fail to
>  interact with your repository until it is disabled.
>  
> -'set'::
> -	Write a set of patterns to the sparse-checkout file, as given as
> -	a list of arguments following the 'set' subcommand. Update the
> -	working directory to match the new patterns. Enable the
> -	core.sparseCheckout config setting if it is not already enabled.
> -+
> -When the `--stdin` option is provided, the patterns are read from
> -standard in as a newline-delimited list instead of from the arguments.
> -+
> -When `core.sparseCheckoutCone` is enabled, the input list is considered a
> -list of directories instead of sparse-checkout patterns. The command writes
> -patterns to the sparse-checkout file to include all files contained in those
> -directories (recursively) as well as files that are siblings of ancestor
> -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,
> @@ -96,9 +86,27 @@ C-style quoted strings.
>  
>  'disable'::
>  	Disable the `core.sparseCheckout` config setting, and restore the
> -	working directory to include all files. Leaves the sparse-checkout
> -	file intact so a later 'git sparse-checkout init' command may
> -	return the working directory to the same state.
> +	working directory to include all files.
> +
> +'init'::
> +	Deprecated command that behaves like `set` with no specified paths.
> +	May be removed in the future.

I'm on board with deprecating `init`, but one usage that's not covered by
the updated `set` is toggling the sparse index *without* modifying the
patterns. That likely won't matter to most users, but ones that assume `git
sparse-checkout set --[no-]sparse-index` works the same way as `git
sparse-checkout init --[no-]sparse-index` would find themselves losing their
existing pattern set.

Maybe `--[no-]sparse-index` should be added to `git sparse-checkout
reapply`? For changing settings without updating patterns, that probably
makes more sense than `init` or `set` anyway. If adding that option is
outside the scope of what you want to do in this series, though, I'd be
happy with a note somewhere in this documentation explicitly noting that
`set` (unlike `init`) will change your patterns, even when toggling
`index.sparse` (or `core.sparseCheckoutCone`).

> ++
> +Historically, `set` did not used to handle all the necessary config
> +settings, which meant that both `init` and `set` had to be called.
> +Invoking both meant the `init` step would first remove nearly all
> +tracked files (and in cone mode, ignored files too), then the `set`
> +step would add many of the tracked files (but not ignored files) back.
> +In addition to the lost files, the performance and UI of this
> +combination was poor.
> ++
> +Also, historically, `init` would not actually initialize the
> +sparse-checkout file if it already existed.  This meant it was
> +possible to return to a sparse-checkout without remembering which
> +paths to pass to a subsequent 'set' or 'add' command.  However,
> +`--cone` and `--sparse-index` options would not be remembered across
> +the disable command, so the easy restore of calling a plain `init`
> +decreased in utility.
>  
>  SPARSE CHECKOUT
>  ---------------
> @@ -117,10 +125,8 @@ directory, it updates the skip-worktree bits in the index based
>  on this file. The files matching the patterns in the file will
>  appear in the working directory, and the rest will not.
>  
> -To enable the sparse-checkout feature, run `git sparse-checkout init` to
> -initialize a simple sparse-checkout file and enable the `core.sparseCheckout`
> -config setting. Then, run `git sparse-checkout set` to modify the patterns in
> -the sparse-checkout file.
> +To enable the sparse-checkout feature, run `git sparse-checkout set` to
> +set the patterns you want to use.
>  
>  To repopulate the working directory with all files, use the
>  `git sparse-checkout disable` command.
> 


^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [PATCH 3/6] sparse-checkout: enable `set` to initialize sparse-checkout mode
  2021-12-04 20:00 ` [PATCH 3/6] sparse-checkout: enable `set` to initialize sparse-checkout mode Elijah Newren via GitGitGadget
  2021-12-04 21:40   ` Victoria Dye
@ 2021-12-07 16:42   ` Derrick Stolee
  2021-12-07 18:18     ` Elijah Newren
  1 sibling, 1 reply; 63+ messages in thread
From: Derrick Stolee @ 2021-12-07 16:42 UTC (permalink / raw)
  To: Elijah Newren via GitGitGadget, git
  Cc: Lessley Dennington, Victoria Dye, Elijah Newren

On 12/4/2021 3:00 PM, Elijah Newren via GitGitGadget wrote:
>  static int sparse_checkout_set(int argc, const char **argv, const char *prefix)
>  {
> +	int mode, record_mode;
> +	const char *default_patterns[] = {"/*", "!/*/"};

My gut reaction is that this array should be NULL terminated.

> +
>  	static struct option builtin_sparse_checkout_set_options[] = {
> +		OPT_BOOL(0, "cone", &set_opts.cone_mode,
> +			 N_("initialize the sparse-checkout in cone mode")),
> +		OPT_BOOL(0, "sparse-index", &set_opts.sparse_index,
> +			 N_("toggle the use of a sparse index")),
>  		OPT_BOOL(0, "stdin", &set_opts.use_stdin,
>  			 N_("read patterns from standard in")),
>  		OPT_END(),
> @@ -700,11 +709,52 @@ static int sparse_checkout_set(int argc, const char **argv, const char *prefix)
>  
>  	repo_read_index(the_repository);
>  
> +	set_opts.cone_mode = -1;
> +	set_opts.sparse_index = -1;
> +
>  	argc = parse_options(argc, argv, prefix,
>  			     builtin_sparse_checkout_set_options,
>  			     builtin_sparse_checkout_set_usage,
>  			     PARSE_OPT_KEEP_UNKNOWN);
>  
> +	/* Determine if we need to record the mode; ensure sparse checkout on */
> +	record_mode = (set_opts.cone_mode != -1) || !core_apply_sparse_checkout;
> +	core_apply_sparse_checkout = 1;
> +
> +	/* If not specified, use previous definition of cone mode */
> +	if (set_opts.cone_mode == -1 && core_apply_sparse_checkout)
> +		set_opts.cone_mode = core_sparse_checkout_cone;
> +
> +	/* Set cone/non-cone mode appropriately */
> +	if (set_opts.cone_mode == 1) {
> +		mode = MODE_CONE_PATTERNS;
> +		core_sparse_checkout_cone = 1;
> +	} else {
> +		mode = MODE_ALL_PATTERNS;
> +	}
> +	if (record_mode && set_config(mode))
> +		return 1;

I think it is appropriate to still have the init steps before the modification
of the worktree. I did have a brief pause to think about whether the set_config()
should be delayed until modify_pattern_list() succeeds.

> +	/* Set sparse-index/non-sparse-index mode if specified */
> +	if (set_opts.sparse_index >= 0) {
> +		if (set_sparse_index_config(the_repository, set_opts.sparse_index) < 0)
> +			die(_("failed to modify sparse-index config"));
> +
> +		/* force an index rewrite */
> +		repo_read_index(the_repository);
> +		the_repository->index->updated_workdir = 1;
> +	}
> +
> +	/*
> +	 * Cone mode automatically specifies the toplevel directory.  For
> +	 * non-cone mode, if nothing is specified, manually select just the
> +	 * top-level directory (much as 'init' would do).
> +	 */
> +	if (!core_sparse_checkout_cone && argc == 0) {
> +		argv = default_patterns;
> +		argc = 2;

Perhaps use 'default_patterns_nr' as a constant near the definition of
'default_patterns' so these numbers are not so far apart?

> +	}
> +
>  	return modify_pattern_list(argc, argv, set_opts.use_stdin, REPLACE);
>  }

Thanks,
-Stolee

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [PATCH 4/6] git-sparse-checkout.txt: update to document that set handles init
  2021-12-04 21:48   ` Victoria Dye
@ 2021-12-07 16:45     ` Derrick Stolee
  2021-12-07 17:06     ` Elijah Newren
  1 sibling, 0 replies; 63+ messages in thread
From: Derrick Stolee @ 2021-12-07 16:45 UTC (permalink / raw)
  To: Victoria Dye, Elijah Newren via GitGitGadget, git
  Cc: Lessley Dennington, Elijah Newren

On 12/4/2021 4:48 PM, Victoria Dye wrote:
> Elijah Newren via GitGitGadget wrote:

>> +'init'::
>> +	Deprecated command that behaves like `set` with no specified paths.
>> +	May be removed in the future.
> 
> I'm on board with deprecating `init`, but one usage that's not covered by
> the updated `set` is toggling the sparse index *without* modifying the
> patterns. That likely won't matter to most users, but ones that assume `git
> sparse-checkout set --[no-]sparse-index` works the same way as `git
> sparse-checkout init --[no-]sparse-index` would find themselves losing their
> existing pattern set.
> 
> Maybe `--[no-]sparse-index` should be added to `git sparse-checkout
> reapply`? For changing settings without updating patterns, that probably
> makes more sense than `init` or `set` anyway. If adding that option is
> outside the scope of what you want to do in this series, though, I'd be
> happy with a note somewhere in this documentation explicitly noting that
> `set` (unlike `init`) will change your patterns, even when toggling
> `index.sparse` (or `core.sparseCheckoutCone`).

I like 'reapply' as a good place for that functionality. Hopefully it
won't be long before the sparse index is on by default, but it will
still be good to have a user-facing way to toggle it, when necessary.

Thanks,
-Stolee

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [PATCH 0/6] sparse-checkout: make set subsume init
  2021-12-04 20:00 [PATCH 0/6] sparse-checkout: make set subsume init Elijah Newren via GitGitGadget
                   ` (5 preceding siblings ...)
  2021-12-04 20:00 ` [PATCH 6/6] clone: avoid using deprecated `sparse-checkout init` Elijah Newren via GitGitGadget
@ 2021-12-07 16:48 ` Derrick Stolee
  2021-12-07 20:20 ` [PATCH v2 00/10] " Elijah Newren via GitGitGadget
  7 siblings, 0 replies; 63+ messages in thread
From: Derrick Stolee @ 2021-12-07 16:48 UTC (permalink / raw)
  To: Elijah Newren via GitGitGadget, git
  Cc: Lessley Dennington, Victoria Dye, Elijah Newren

On 12/4/2021 3:00 PM, Elijah Newren via GitGitGadget wrote:
> As described at [1], the split of init and set subcommands in
> sparse-checkout causes multiple issues:
> 
>  * Poor performance (deleting all tracked files, then later restoring many
>    and maybe even most of them)
>  * Poor UI (multiple progress bars in wrappers that hide both commands under
>    1 user-facing command)
>  * Loss of ignored files under directories the user wanted to keep
> 
> This series fixes this bug by providing a single command to switch to a
> sparse-checkout: set. It does so by making set able to do the combined work
> of init and set. It keeps init as-is to give folks time to adapt, but marks
> it as deprecated.
> 
> A quick overview:
> 
>  * Patches 1-2: small preparatory refactorings

These were easy to read and obviously improvements. They make the later
work easier, too.

>  * Patch 3: the crux of the series

Victoria and I had some thoughts that might improve this patch, but it
is high quality without them.

>  * Patches 4-5: documentation modifications (Patch 4 is worth reviewing; it
>    marks init as deprecated -- are others okay with that?)
>  * Patch 6: trivial modification of git clone --sparse to use git
>    sparse-checkout set rather than git sparse-checkout init.

These are good immediate follow-ups. I notice you left out the "--sparse
implies --cone" change, and I think that is wise to leave for an independent
series.

Thanks,
-Stolee

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [PATCH 3/6] sparse-checkout: enable `set` to initialize sparse-checkout mode
  2021-12-04 21:40   ` Victoria Dye
@ 2021-12-07 17:04     ` Elijah Newren
  2021-12-07 20:36       ` Lessley Dennington
  0 siblings, 1 reply; 63+ messages in thread
From: Elijah Newren @ 2021-12-07 17:04 UTC (permalink / raw)
  To: Victoria Dye
  Cc: Elijah Newren via GitGitGadget, Git Mailing List, Derrick Stolee,
	Lessley Dennington

On Sat, Dec 4, 2021 at 1:40 PM Victoria Dye <vdye@github.com> wrote:
>
> Elijah Newren via GitGitGadget wrote:
> > From: Elijah Newren <newren@gmail.com>
> >
> > The previously suggested workflow:
> >   git sparse-checkout init ...
> >   git sparse-checkout set ...
> >
> > Suffered from three problems:
> >   1) It would delete nearly all files in the first step, then
> >      restore them in the second.  That was poor performance and
> >      forced unnecessary rebuilds.
> >   2) The two-step process resulted in two progress bars, which
> >      was suboptimal from a UI point of view for wrappers that
> >      invoked both of these commands but only exposed a single
> >      command to their end users.
> >   3) With cone mode, the first step would delete nearly all
> >      ignored files everywhere, because everything was considered
> >      to be outside of the specified sparsity paths.  (The user was
> >      not allowed to specify any sparsity paths in the `init` step.)
> >
> > Avoid these problems by teaching `set` to understand the extra
> > parameters that `init` takes and performing any necessary initialization
> > if not already in a sparse checkout.
> >
>
> I really like this change! It always seemed weird that `set` would
> implicitly `init`, but without any of the options in `init`.
>
> > Signed-off-by: Elijah Newren <newren@gmail.com>
> > ---
> >  builtin/sparse-checkout.c | 52 ++++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 51 insertions(+), 1 deletion(-)
> >
> > diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c
> > index e252b82136e..cf6a6c6c3a7 100644
> > --- a/builtin/sparse-checkout.c
> > +++ b/builtin/sparse-checkout.c
> > @@ -682,17 +682,26 @@ static int sparse_checkout_add(int argc, const char **argv, const char *prefix)
> >  }
> >
> >  static char const * const builtin_sparse_checkout_set_usage[] = {
> > -     N_("git sparse-checkout set (--stdin | <patterns>)"),
> > +     N_("git sparse-checkout set [--cone] [--[no-]sparse-index] (--stdin | <patterns>)"),
>
> Since `cone` is an `OPT_BOOL`, shouldn't it appear in the usage string as
> `--[no-]cone`? Without a `--no-cone` option, it's not clear how a user would
> disable cone mode (since `set` preserves the existing cone mode setting if
> `--cone` isn't given).

Yeah, fair point.  When copying from init I probably should have double checked.

Also, it makes me wonder if we should just make cone mode the default...

> >       NULL
> >  };
> >
> >  static struct sparse_checkout_set_opts {
> > +     int cone_mode;
> > +     int sparse_index;
> >       int use_stdin;
> >  } set_opts;
> >
> >  static int sparse_checkout_set(int argc, const char **argv, const char *prefix)
> >  {
> > +     int mode, record_mode;
> > +     const char *default_patterns[] = {"/*", "!/*/"};
> > +
> >       static struct option builtin_sparse_checkout_set_options[] = {
> > +             OPT_BOOL(0, "cone", &set_opts.cone_mode,
> > +                      N_("initialize the sparse-checkout in cone mode")),
> > +             OPT_BOOL(0, "sparse-index", &set_opts.sparse_index,
> > +                      N_("toggle the use of a sparse index")),
> >               OPT_BOOL(0, "stdin", &set_opts.use_stdin,
> >                        N_("read patterns from standard in")),
>
> I know this isn't part of this patch, but is `stdin` really a bool (i.e.
> someone might specify `--no-stdin`)? If not, I think `OPT_SET_INT` might be
> more appropriate.

Good point.  OPT_SET_INT() wouldn't fix it, though; you need to use
OPT_BOOL_F or OPT_SET_INT_F and pass PARSE_OPT_NONEG as a flag.  I can
include that.

> >               OPT_END(),
> > @@ -700,11 +709,52 @@ static int sparse_checkout_set(int argc, const char **argv, const char *prefix)
> >
> >       repo_read_index(the_repository);
> >
> > +     set_opts.cone_mode = -1;
> > +     set_opts.sparse_index = -1;
> > +
> >       argc = parse_options(argc, argv, prefix,
> >                            builtin_sparse_checkout_set_options,
> >                            builtin_sparse_checkout_set_usage,
> >                            PARSE_OPT_KEEP_UNKNOWN);
> >
> > +     /* Determine if we need to record the mode; ensure sparse checkout on */
> > +     record_mode = (set_opts.cone_mode != -1) || !core_apply_sparse_checkout;
> > +     core_apply_sparse_checkout = 1;
> > +
> > +     /* If not specified, use previous definition of cone mode */
> > +     if (set_opts.cone_mode == -1 && core_apply_sparse_checkout)
>
> I *think* this is supposed go before the `core_apply_sparse_checkout = 1;`
> above it (if the intention is to only use the pre-existing cone mode setting
> when already in a sparse checkout). If not, the `core_apply_sparse_checkout`
> part of the condition is redundant and can be removed entirely.

Eek.  I had it there originally, then moved it later while doing
various changes.  This definitely should be later; thanks for
catching.

> > +             set_opts.cone_mode = core_sparse_checkout_cone;
> > +
> > +     /* Set cone/non-cone mode appropriately */
> > +     if (set_opts.cone_mode == 1) {
> > +             mode = MODE_CONE_PATTERNS;
> > +             core_sparse_checkout_cone = 1;
> > +     } else {
> > +             mode = MODE_ALL_PATTERNS;
> > +     }
> > +     if (record_mode && set_config(mode))
> > +             return 1;
> > +
> > +     /* Set sparse-index/non-sparse-index mode if specified */
> > +     if (set_opts.sparse_index >= 0) {
> > +             if (set_sparse_index_config(the_repository, set_opts.sparse_index) < 0)
> > +                     die(_("failed to modify sparse-index config"));
> > +
> > +             /* force an index rewrite */
> > +             repo_read_index(the_repository);
> > +             the_repository->index->updated_workdir = 1;
> > +     }
> > +
> > +     /*
> > +      * Cone mode automatically specifies the toplevel directory.  For
> > +      * non-cone mode, if nothing is specified, manually select just the
> > +      * top-level directory (much as 'init' would do).
> > +      */
> > +     if (!core_sparse_checkout_cone && argc == 0) {
> > +             argv = default_patterns;
> > +             argc = 2;
> > +     }
> > +
> >       return modify_pattern_list(argc, argv, set_opts.use_stdin, REPLACE);
> >  }

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [PATCH 4/6] git-sparse-checkout.txt: update to document that set handles init
  2021-12-04 21:48   ` Victoria Dye
  2021-12-07 16:45     ` Derrick Stolee
@ 2021-12-07 17:06     ` Elijah Newren
  1 sibling, 0 replies; 63+ messages in thread
From: Elijah Newren @ 2021-12-07 17:06 UTC (permalink / raw)
  To: Victoria Dye
  Cc: Elijah Newren via GitGitGadget, Git Mailing List, Derrick Stolee,
	Lessley Dennington

On Sat, Dec 4, 2021 at 1:48 PM Victoria Dye <vdye@github.com> wrote:
>
> Elijah Newren via GitGitGadget wrote:
> > From: Elijah Newren <newren@gmail.com>
> >
> > As noted in the previous commit, using separate `init` and `set` steps
> > with sparse-checkout result in a number of issues.  The previous commit
> > made `set` able to handle the work of both commands.  Update the
> > documentation to reflect this, and mark `init` as deprecated.
> >
> > Signed-off-by: Elijah Newren <newren@gmail.com>
> > ---
> >  Documentation/git-sparse-checkout.txt | 92 ++++++++++++++-------------
> >  1 file changed, 49 insertions(+), 43 deletions(-)
> >
> > diff --git a/Documentation/git-sparse-checkout.txt b/Documentation/git-sparse-checkout.txt
> > index 42056ee9ff9..d22c925ecf8 100644
> > --- a/Documentation/git-sparse-checkout.txt
> > +++ b/Documentation/git-sparse-checkout.txt
> > @@ -30,28 +30,35 @@ COMMANDS
> >  'list'::
> >       Describe the patterns in the sparse-checkout file.
> >
> > -'init'::
> > -     Enable the `core.sparseCheckout` setting. If the
> > -     sparse-checkout file does not exist, then populate it with
> > -     patterns that match every file in the root directory and
> > -     no other directories, then will remove all directories tracked
> > -     by Git. Add patterns to the sparse-checkout file to
> > -     repopulate the working directory.
> > +'set'::
> > +     Enable the necessary config settings
> > +     (extensions.worktreeConfig, core.sparseCheckout,
> > +     core.sparseCheckoutCone) if they are not already enabled, and
> > +     write a set of patterns to the sparse-checkout file from the
> > +     list of arguments following the 'set' subcommand. Update the
> > +     working directory to match the new patterns.
> >  +
> > -To avoid interfering with other worktrees, it first enables the
> > -`extensions.worktreeConfig` setting and makes sure to set the
> > -`core.sparseCheckout` setting in the worktree-specific config file.
> > +When the `--stdin` option is provided, the patterns are read from
> > +standard in as a newline-delimited list instead of from the arguments.
> >  +
> > -When `--cone` is provided, the `core.sparseCheckoutCone` setting is
> > -also set, allowing for better performance with a limited set of
> > -patterns (see 'CONE PATTERN SET' below).
> > +When `--cone` is passed or `core.sparseCheckoutCone` is enabled, the
> > +input list is considered a list of directories instead of
> > +sparse-checkout patterns.  This allows for better performance with a
> > +limited set of patterns (see 'CONE PATTERN SET' below).  Note that the
> > +set command will write patterns to the sparse-checkout file to include
> > +all files contained in those directories (recursively) as well as
> > +files that are siblings of ancestor 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.
> >  +
> > -Use the `--[no-]sparse-index` option to toggle the use of the sparse
> > -index format. This reduces the size of the index to be more closely
> > -aligned with your sparse-checkout definition. This can have significant
> > -performance advantages for commands such as `git status` or `git add`.
> > -This feature is still experimental. Some commands might be slower with
> > -a sparse index until they are properly integrated with the feature.
> > +Use the `--[no-]sparse-index` option to use a sparse index (the
> > +default is to not use it).  A sparse index reduces the size of the
> > +index to be more closely aligned with your sparse-checkout
> > +definition. This can have significant performance advantages for
> > +commands such as `git status` or `git add`.  This feature is still
> > +experimental. Some commands might be slower with a sparse index until
> > +they are properly integrated with the feature.
> >  +
> >  **WARNING:** Using a sparse index requires modifying the index in a way
> >  that is not completely understood by external tools. If you have trouble
> > @@ -60,23 +67,6 @@ to rewrite your index to not be sparse. Older versions of Git will not
> >  understand the sparse directory entries index extension and may fail to
> >  interact with your repository until it is disabled.
> >
> > -'set'::
> > -     Write a set of patterns to the sparse-checkout file, as given as
> > -     a list of arguments following the 'set' subcommand. Update the
> > -     working directory to match the new patterns. Enable the
> > -     core.sparseCheckout config setting if it is not already enabled.
> > -+
> > -When the `--stdin` option is provided, the patterns are read from
> > -standard in as a newline-delimited list instead of from the arguments.
> > -+
> > -When `core.sparseCheckoutCone` is enabled, the input list is considered a
> > -list of directories instead of sparse-checkout patterns. The command writes
> > -patterns to the sparse-checkout file to include all files contained in those
> > -directories (recursively) as well as files that are siblings of ancestor
> > -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,
> > @@ -96,9 +86,27 @@ C-style quoted strings.
> >
> >  'disable'::
> >       Disable the `core.sparseCheckout` config setting, and restore the
> > -     working directory to include all files. Leaves the sparse-checkout
> > -     file intact so a later 'git sparse-checkout init' command may
> > -     return the working directory to the same state.
> > +     working directory to include all files.
> > +
> > +'init'::
> > +     Deprecated command that behaves like `set` with no specified paths.
> > +     May be removed in the future.
>
> I'm on board with deprecating `init`, but one usage that's not covered by
> the updated `set` is toggling the sparse index *without* modifying the
> patterns. That likely won't matter to most users, but ones that assume `git
> sparse-checkout set --[no-]sparse-index` works the same way as `git
> sparse-checkout init --[no-]sparse-index` would find themselves losing their
> existing pattern set.
>
> Maybe `--[no-]sparse-index` should be added to `git sparse-checkout
> reapply`? For changing settings without updating patterns, that probably
> makes more sense than `init` or `set` anyway. If adding that option is
> outside the scope of what you want to do in this series, though, I'd be
> happy with a note somewhere in this documentation explicitly noting that
> `set` (unlike `init`) will change your patterns, even when toggling
> `index.sparse` (or `core.sparseCheckoutCone`).

I like that idea; reapply seems like the right place to allow folks to
toggle options without updating sparsity paths.  I'll add it.

> > ++
> > +Historically, `set` did not used to handle all the necessary config
> > +settings, which meant that both `init` and `set` had to be called.
> > +Invoking both meant the `init` step would first remove nearly all
> > +tracked files (and in cone mode, ignored files too), then the `set`
> > +step would add many of the tracked files (but not ignored files) back.
> > +In addition to the lost files, the performance and UI of this
> > +combination was poor.
> > ++
> > +Also, historically, `init` would not actually initialize the
> > +sparse-checkout file if it already existed.  This meant it was
> > +possible to return to a sparse-checkout without remembering which
> > +paths to pass to a subsequent 'set' or 'add' command.  However,
> > +`--cone` and `--sparse-index` options would not be remembered across
> > +the disable command, so the easy restore of calling a plain `init`
> > +decreased in utility.
> >
> >  SPARSE CHECKOUT
> >  ---------------
> > @@ -117,10 +125,8 @@ directory, it updates the skip-worktree bits in the index based
> >  on this file. The files matching the patterns in the file will
> >  appear in the working directory, and the rest will not.
> >
> > -To enable the sparse-checkout feature, run `git sparse-checkout init` to
> > -initialize a simple sparse-checkout file and enable the `core.sparseCheckout`
> > -config setting. Then, run `git sparse-checkout set` to modify the patterns in
> > -the sparse-checkout file.
> > +To enable the sparse-checkout feature, run `git sparse-checkout set` to
> > +set the patterns you want to use.
> >
> >  To repopulate the working directory with all files, use the
> >  `git sparse-checkout disable` command.
> >
>

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [PATCH 3/6] sparse-checkout: enable `set` to initialize sparse-checkout mode
  2021-12-07 16:42   ` Derrick Stolee
@ 2021-12-07 18:18     ` Elijah Newren
  0 siblings, 0 replies; 63+ messages in thread
From: Elijah Newren @ 2021-12-07 18:18 UTC (permalink / raw)
  To: Derrick Stolee
  Cc: Elijah Newren via GitGitGadget, Git Mailing List,
	Lessley Dennington, Victoria Dye

On Tue, Dec 7, 2021 at 8:43 AM Derrick Stolee <stolee@gmail.com> wrote:
>
> On 12/4/2021 3:00 PM, Elijah Newren via GitGitGadget wrote:
> >  static int sparse_checkout_set(int argc, const char **argv, const char *prefix)
> >  {
> > +     int mode, record_mode;
> > +     const char *default_patterns[] = {"/*", "!/*/"};
>
> My gut reaction is that this array should be NULL terminated.

Sure, I can make that change.

> > +
> >       static struct option builtin_sparse_checkout_set_options[] = {
> > +             OPT_BOOL(0, "cone", &set_opts.cone_mode,
> > +                      N_("initialize the sparse-checkout in cone mode")),
> > +             OPT_BOOL(0, "sparse-index", &set_opts.sparse_index,
> > +                      N_("toggle the use of a sparse index")),
> >               OPT_BOOL(0, "stdin", &set_opts.use_stdin,
> >                        N_("read patterns from standard in")),
> >               OPT_END(),
> > @@ -700,11 +709,52 @@ static int sparse_checkout_set(int argc, const char **argv, const char *prefix)
> >
> >       repo_read_index(the_repository);
> >
> > +     set_opts.cone_mode = -1;
> > +     set_opts.sparse_index = -1;
> > +
> >       argc = parse_options(argc, argv, prefix,
> >                            builtin_sparse_checkout_set_options,
> >                            builtin_sparse_checkout_set_usage,
> >                            PARSE_OPT_KEEP_UNKNOWN);
> >
> > +     /* Determine if we need to record the mode; ensure sparse checkout on */
> > +     record_mode = (set_opts.cone_mode != -1) || !core_apply_sparse_checkout;
> > +     core_apply_sparse_checkout = 1;
> > +
> > +     /* If not specified, use previous definition of cone mode */
> > +     if (set_opts.cone_mode == -1 && core_apply_sparse_checkout)
> > +             set_opts.cone_mode = core_sparse_checkout_cone;
> > +
> > +     /* Set cone/non-cone mode appropriately */
> > +     if (set_opts.cone_mode == 1) {
> > +             mode = MODE_CONE_PATTERNS;
> > +             core_sparse_checkout_cone = 1;
> > +     } else {
> > +             mode = MODE_ALL_PATTERNS;
> > +     }
> > +     if (record_mode && set_config(mode))
> > +             return 1;
>
> I think it is appropriate to still have the init steps before the modification
> of the worktree. I did have a brief pause to think about whether the set_config()
> should be delayed until modify_pattern_list() succeeds.
>
> > +     /* Set sparse-index/non-sparse-index mode if specified */
> > +     if (set_opts.sparse_index >= 0) {
> > +             if (set_sparse_index_config(the_repository, set_opts.sparse_index) < 0)
> > +                     die(_("failed to modify sparse-index config"));
> > +
> > +             /* force an index rewrite */
> > +             repo_read_index(the_repository);
> > +             the_repository->index->updated_workdir = 1;
> > +     }
> > +
> > +     /*
> > +      * Cone mode automatically specifies the toplevel directory.  For
> > +      * non-cone mode, if nothing is specified, manually select just the
> > +      * top-level directory (much as 'init' would do).
> > +      */
> > +     if (!core_sparse_checkout_cone && argc == 0) {
> > +             argv = default_patterns;
> > +             argc = 2;
>
> Perhaps use 'default_patterns_nr' as a constant near the definition of
> 'default_patterns' so these numbers are not so far apart?

Sounds like a good idea; will do.

>
> > +     }
> > +
> >       return modify_pattern_list(argc, argv, set_opts.use_stdin, REPLACE);
> >  }
>
> Thanks,
> -Stolee

^ permalink raw reply	[flat|nested] 63+ messages in thread

* [PATCH v2 00/10] sparse-checkout: make set subsume init
  2021-12-04 20:00 [PATCH 0/6] sparse-checkout: make set subsume init Elijah Newren via GitGitGadget
                   ` (6 preceding siblings ...)
  2021-12-07 16:48 ` [PATCH 0/6] sparse-checkout: make set subsume init Derrick Stolee
@ 2021-12-07 20:20 ` Elijah Newren via GitGitGadget
  2021-12-07 20:20   ` [PATCH v2 01/10] sparse-checkout: pass use_stdin as a parameter instead of as a global Elijah Newren via GitGitGadget
                     ` (12 more replies)
  7 siblings, 13 replies; 63+ messages in thread
From: Elijah Newren via GitGitGadget @ 2021-12-07 20:20 UTC (permalink / raw)
  To: git
  Cc: Derrick Stolee, Lessley Dennington, Victoria Dye, Elijah Newren,
	Elijah Newren

As described at [1], the split of init and set subcommands in
sparse-checkout causes multiple issues:

 * Poor performance (deleting all tracked files, then later restoring many
   and maybe even most of them)
 * Poor UI (multiple progress bars in wrappers that hide both commands under
   1 user-facing command)
 * Loss of ignored files under directories the user wanted to keep

This series fixes this bug by providing a single command to switch to a
sparse-checkout: set. It does so by making set able to do the combined work
of init and set. It keeps init as-is to give folks time to adapt, but marks
it as deprecated. It also makes reapply able to toggle cone/non-cone mode
and sparse-index/non-sparse-index mode.

Changes since v1:

 * Inserted new patches 3 & 4 as additional preparatory cleanups
 * Took the new mode-toggling work code in sparse_checkout_set from the
   previous series and moved it into a new function, as a preparatory patch,
   and made it usable by init/set/reapply
 * Also updated reapply to allow mode-toggling
 * Updated the documentation as per above
 * Various other small items from review comments

A quick overview:

 * Patches 1-4: small preparatory refactorings
 * Patch 5: patch adding new function for toggling {cone,sparse-checkout}
   modes
 * Patch 6: the crux of the series; make set able to handle both init and
   set options
 * Patch 7: make reapply also able to do mode toggling
 * Patches 8-9: documentation modifications (Patch 4 is worth reviewing; it
   marks init as deprecated -- are others okay with that?)
 * Patch 10: trivial modification of git clone --sparse to use git
   sparse-checkout set rather than git sparse-checkout init.

[1]
https://lore.kernel.org/git/CABPp-BE8TJ8QGAQWsSGT7S+9Xp-XmApcC9PSw3K=RQOP0rt+PQ@mail.gmail.com/

Elijah Newren (10):
  sparse-checkout: pass use_stdin as a parameter instead of as a global
  sparse-checkout: break apart functions for sparse_checkout_(set|add)
  sparse-checkout: add sanity-checks on initial sparsity state
  sparse-checkout: disallow --no-stdin as an argument to set
  sparse-checkout: split out code for tweaking settings config
  sparse-checkout: enable `set` to initialize sparse-checkout mode
  sparse-checkout: enable reapply to take --[no-]{cone,sparse-index}
  git-sparse-checkout.txt: update to document init/set/reapply changes
  Documentation: clarify/correct a few sparsity related statements
  clone: avoid using deprecated `sparse-checkout init`

 Documentation/git-clone.txt           |   8 +-
 Documentation/git-sparse-checkout.txt | 100 +++++++------
 builtin/clone.c                       |   2 +-
 builtin/sparse-checkout.c             | 196 ++++++++++++++++++++------
 t/t1091-sparse-checkout-builtin.sh    |  10 +-
 5 files changed, 219 insertions(+), 97 deletions(-)


base-commit: abe6bb3905392d5eb6b01fa6e54d7e784e0522aa
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1151%2Fnewren%2Fsparse-checkout-no-init-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1151/newren/sparse-checkout-no-init-v2
Pull-Request: https://github.com/git/git/pull/1151

Range-diff vs v1:

  1:  e41cfe3c1bb =  1:  e41cfe3c1bb sparse-checkout: pass use_stdin as a parameter instead of as a global
  2:  6f7de8f5412 =  2:  6f7de8f5412 sparse-checkout: break apart functions for sparse_checkout_(set|add)
  -:  ----------- >  3:  bade5e52390 sparse-checkout: add sanity-checks on initial sparsity state
  -:  ----------- >  4:  0180bfc4a93 sparse-checkout: disallow --no-stdin as an argument to set
  -:  ----------- >  5:  3f5255eeef9 sparse-checkout: split out code for tweaking settings config
  3:  a90687eb4c1 !  6:  3c640f5bcef sparse-checkout: enable `set` to initialize sparse-checkout mode
     @@ builtin/sparse-checkout.c: static int sparse_checkout_add(int argc, const char *
       
       static char const * const builtin_sparse_checkout_set_usage[] = {
      -	N_("git sparse-checkout set (--stdin | <patterns>)"),
     -+	N_("git sparse-checkout set [--cone] [--[no-]sparse-index] (--stdin | <patterns>)"),
     ++	N_("git sparse-checkout set [--[no-]cone] [--[no-]sparse-index] (--stdin | <patterns>)"),
       	NULL
       };
       
     @@ builtin/sparse-checkout.c: static int sparse_checkout_add(int argc, const char *
       
       static int sparse_checkout_set(int argc, const char **argv, const char *prefix)
       {
     -+	int mode, record_mode;
     -+	const char *default_patterns[] = {"/*", "!/*/"};
     ++	int default_patterns_nr = 2;
     ++	const char *default_patterns[] = {"/*", "!/*/", NULL};
      +
       	static struct option builtin_sparse_checkout_set_options[] = {
      +		OPT_BOOL(0, "cone", &set_opts.cone_mode,
      +			 N_("initialize the sparse-checkout in cone mode")),
      +		OPT_BOOL(0, "sparse-index", &set_opts.sparse_index,
      +			 N_("toggle the use of a sparse index")),
     - 		OPT_BOOL(0, "stdin", &set_opts.use_stdin,
     - 			 N_("read patterns from standard in")),
     - 		OPT_END(),
     + 		OPT_BOOL_F(0, "stdin", &set_opts.use_stdin,
     + 			   N_("read patterns from standard in"),
     + 			   PARSE_OPT_NONEG),
      @@ builtin/sparse-checkout.c: static int sparse_checkout_set(int argc, const char **argv, const char *prefix)
       
       	repo_read_index(the_repository);
     @@ builtin/sparse-checkout.c: static int sparse_checkout_set(int argc, const char *
       			     builtin_sparse_checkout_set_usage,
       			     PARSE_OPT_KEEP_UNKNOWN);
       
     -+	/* Determine if we need to record the mode; ensure sparse checkout on */
     -+	record_mode = (set_opts.cone_mode != -1) || !core_apply_sparse_checkout;
     -+	core_apply_sparse_checkout = 1;
     -+
     -+	/* If not specified, use previous definition of cone mode */
     -+	if (set_opts.cone_mode == -1 && core_apply_sparse_checkout)
     -+		set_opts.cone_mode = core_sparse_checkout_cone;
     -+
     -+	/* Set cone/non-cone mode appropriately */
     -+	if (set_opts.cone_mode == 1) {
     -+		mode = MODE_CONE_PATTERNS;
     -+		core_sparse_checkout_cone = 1;
     -+	} else {
     -+		mode = MODE_ALL_PATTERNS;
     -+	}
     -+	if (record_mode && set_config(mode))
     ++	if (update_modes(&set_opts.cone_mode, &set_opts.sparse_index))
      +		return 1;
      +
     -+	/* Set sparse-index/non-sparse-index mode if specified */
     -+	if (set_opts.sparse_index >= 0) {
     -+		if (set_sparse_index_config(the_repository, set_opts.sparse_index) < 0)
     -+			die(_("failed to modify sparse-index config"));
     -+
     -+		/* force an index rewrite */
     -+		repo_read_index(the_repository);
     -+		the_repository->index->updated_workdir = 1;
     -+	}
     -+
      +	/*
      +	 * Cone mode automatically specifies the toplevel directory.  For
      +	 * non-cone mode, if nothing is specified, manually select just the
     @@ builtin/sparse-checkout.c: static int sparse_checkout_set(int argc, const char *
      +	 */
      +	if (!core_sparse_checkout_cone && argc == 0) {
      +		argv = default_patterns;
     -+		argc = 2;
     ++		argc = default_patterns_nr;
      +	}
      +
       	return modify_pattern_list(argc, argv, set_opts.use_stdin, REPLACE);
  -:  ----------- >  7:  acb10889a1f sparse-checkout: enable reapply to take --[no-]{cone,sparse-index}
  4:  95d3df78b2f !  8:  17b033caf4b git-sparse-checkout.txt: update to document that set handles init
     @@ Metadata
      Author: Elijah Newren <newren@gmail.com>
      
       ## Commit message ##
     -    git-sparse-checkout.txt: update to document that set handles init
     +    git-sparse-checkout.txt: update to document init/set/reapply changes
      
          As noted in the previous commit, using separate `init` and `set` steps
     -    with sparse-checkout result in a number of issues.  The previous commit
     -    made `set` able to handle the work of both commands.  Update the
     -    documentation to reflect this, and mark `init` as deprecated.
     +    with sparse-checkout result in a number of issues.  The previous commits
     +    made `set` able to handle the work of both commands, and enabled reapply
     +    to tweak the {cone,sparse-index} settings.  Update the documentation to
     +    reflect this, and mark `init` as deprecated.
      
          Signed-off-by: Elijah Newren <newren@gmail.com>
      
     @@ Documentation/git-sparse-checkout.txt: COMMANDS
      +files that are siblings of ancestor 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.
     ++quoted strings.  This may become the default in the future; --no-cone
     ++can be passed to request non-cone mode.
       +
      -Use the `--[no-]sparse-index` option to toggle the use of the sparse
      -index format. This reduces the size of the index to be more closely
     @@ Documentation/git-sparse-checkout.txt: to rewrite your index to not be sparse. O
       	Update the sparse-checkout file to include additional patterns.
       	By default, these patterns are read from the command-line arguments,
      @@ Documentation/git-sparse-checkout.txt: C-style quoted strings.
     + 	cases, it can make sense to run `git sparse-checkout reapply` later
     + 	after cleaning up affected paths (e.g. resolving conflicts, undoing
     + 	or committing changes, etc.).
     +++
     ++The `reapply` command can also take `--[no-]cone` and `--[no-]sparse-index`
     ++flags, with the same meaning as the flags from the `set` command, in order
     ++to change which sparsity mode you are using without needing to also respecify
     ++all sparsity paths.
       
       'disable'::
       	Disable the `core.sparseCheckout` config setting, and restore the
  5:  9d455f1fb51 =  9:  922a65b4019 Documentation: clarify/correct a few sparsity related statements
  6:  2ad404874ee = 10:  d47b2c88242 clone: avoid using deprecated `sparse-checkout init`

-- 
gitgitgadget

^ permalink raw reply	[flat|nested] 63+ messages in thread

* [PATCH v2 01/10] sparse-checkout: pass use_stdin as a parameter instead of as a global
  2021-12-07 20:20 ` [PATCH v2 00/10] " Elijah Newren via GitGitGadget
@ 2021-12-07 20:20   ` Elijah Newren via GitGitGadget
  2021-12-07 20:20   ` [PATCH v2 02/10] sparse-checkout: break apart functions for sparse_checkout_(set|add) Elijah Newren via GitGitGadget
                     ` (11 subsequent siblings)
  12 siblings, 0 replies; 63+ messages in thread
From: Elijah Newren via GitGitGadget @ 2021-12-07 20:20 UTC (permalink / raw)
  To: git
  Cc: Derrick Stolee, Lessley Dennington, Victoria Dye, Elijah Newren,
	Elijah Newren, Elijah Newren

From: Elijah Newren <newren@gmail.com>

add_patterns_from_input() has relied on a global variable,
set_opts.use_stdin, which has been used by both the `set` and `add`
subcommands of sparse-checkout.  Once we introduce an
add_opts.use_stdin, the hardcoding of set_opts.use_stdin will be
incorrect.  Pass the value as function parameter instead to allow us to
make subsequent changes.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 builtin/sparse-checkout.c | 28 ++++++++++++++++------------
 1 file changed, 16 insertions(+), 12 deletions(-)

diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c
index d0f5c4702be..8612328e5dd 100644
--- a/builtin/sparse-checkout.c
+++ b/builtin/sparse-checkout.c
@@ -525,7 +525,8 @@ static struct sparse_checkout_set_opts {
 } set_opts;
 
 static void add_patterns_from_input(struct pattern_list *pl,
-				    int argc, const char **argv)
+				    int argc, const char **argv,
+				    int use_stdin)
 {
 	int i;
 	if (core_sparse_checkout_cone) {
@@ -535,7 +536,7 @@ static void add_patterns_from_input(struct pattern_list *pl,
 		hashmap_init(&pl->parent_hashmap, pl_hashmap_cmp, NULL, 0);
 		pl->use_cone_patterns = 1;
 
-		if (set_opts.use_stdin) {
+		if (use_stdin) {
 			struct strbuf unquoted = STRBUF_INIT;
 			while (!strbuf_getline(&line, stdin)) {
 				if (line.buf[0] == '"') {
@@ -559,7 +560,7 @@ static void add_patterns_from_input(struct pattern_list *pl,
 			}
 		}
 	} else {
-		if (set_opts.use_stdin) {
+		if (use_stdin) {
 			struct strbuf line = STRBUF_INIT;
 
 			while (!strbuf_getline(&line, stdin)) {
@@ -580,7 +581,8 @@ enum modify_type {
 };
 
 static void add_patterns_cone_mode(int argc, const char **argv,
-				   struct pattern_list *pl)
+				   struct pattern_list *pl,
+				   int use_stdin)
 {
 	struct strbuf buffer = STRBUF_INIT;
 	struct pattern_entry *pe;
@@ -588,7 +590,7 @@ static void add_patterns_cone_mode(int argc, const char **argv,
 	struct pattern_list existing;
 	char *sparse_filename = get_sparse_checkout_filename();
 
-	add_patterns_from_input(pl, argc, argv);
+	add_patterns_from_input(pl, argc, argv, use_stdin);
 
 	memset(&existing, 0, sizeof(existing));
 	existing.use_cone_patterns = core_sparse_checkout_cone;
@@ -614,17 +616,19 @@ static void add_patterns_cone_mode(int argc, const char **argv,
 }
 
 static void add_patterns_literal(int argc, const char **argv,
-				 struct pattern_list *pl)
+				 struct pattern_list *pl,
+				 int use_stdin)
 {
 	char *sparse_filename = get_sparse_checkout_filename();
 	if (add_patterns_from_file_to_list(sparse_filename, "", 0,
 					   pl, NULL, 0))
 		die(_("unable to load existing sparse-checkout patterns"));
 	free(sparse_filename);
-	add_patterns_from_input(pl, argc, argv);
+	add_patterns_from_input(pl, argc, argv, use_stdin);
 }
 
-static int modify_pattern_list(int argc, const char **argv, enum modify_type m)
+static int modify_pattern_list(int argc, const char **argv, int use_stdin,
+			       enum modify_type m)
 {
 	int result;
 	int changed_config = 0;
@@ -633,13 +637,13 @@ static int modify_pattern_list(int argc, const char **argv, enum modify_type m)
 	switch (m) {
 	case ADD:
 		if (core_sparse_checkout_cone)
-			add_patterns_cone_mode(argc, argv, pl);
+			add_patterns_cone_mode(argc, argv, pl, use_stdin);
 		else
-			add_patterns_literal(argc, argv, pl);
+			add_patterns_literal(argc, argv, pl, use_stdin);
 		break;
 
 	case REPLACE:
-		add_patterns_from_input(pl, argc, argv);
+		add_patterns_from_input(pl, argc, argv, use_stdin);
 		break;
 	}
 
@@ -675,7 +679,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, m);
+	return modify_pattern_list(argc, argv, set_opts.use_stdin, m);
 }
 
 static char const * const builtin_sparse_checkout_reapply_usage[] = {
-- 
gitgitgadget


^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH v2 02/10] sparse-checkout: break apart functions for sparse_checkout_(set|add)
  2021-12-07 20:20 ` [PATCH v2 00/10] " Elijah Newren via GitGitGadget
  2021-12-07 20:20   ` [PATCH v2 01/10] sparse-checkout: pass use_stdin as a parameter instead of as a global Elijah Newren via GitGitGadget
@ 2021-12-07 20:20   ` Elijah Newren via GitGitGadget
  2021-12-07 20:20   ` [PATCH v2 03/10] sparse-checkout: add sanity-checks on initial sparsity state Elijah Newren via GitGitGadget
                     ` (10 subsequent siblings)
  12 siblings, 0 replies; 63+ messages in thread
From: Elijah Newren via GitGitGadget @ 2021-12-07 20:20 UTC (permalink / raw)
  To: git
  Cc: Derrick Stolee, Lessley Dennington, Victoria Dye, Elijah Newren,
	Elijah Newren, Elijah Newren

From: Elijah Newren <newren@gmail.com>

sparse_checkout_set() was reused by sparse_checkout_add() with the only
difference being a single parameter being passed to that function.
However, we would like sparse_checkout_set() to do the same work that
sparse_checkout_init() does if sparse checkouts are not already enabled.
To facilitate this transition, give each mode their own copy of the
function.  This does not introduce any behavioral changes; that will
come in a subsequent patch.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 builtin/sparse-checkout.c | 54 +++++++++++++++++++++++++++++----------
 1 file changed, 40 insertions(+), 14 deletions(-)

diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c
index 8612328e5dd..e252b82136e 100644
--- a/builtin/sparse-checkout.c
+++ b/builtin/sparse-checkout.c
@@ -515,15 +515,6 @@ static void strbuf_to_cone_pattern(struct strbuf *line, struct pattern_list *pl)
 	insert_recursive_pattern(pl, line);
 }
 
-static char const * const builtin_sparse_checkout_set_usage[] = {
-	N_("git sparse-checkout (set|add) (--stdin | <patterns>)"),
-	NULL
-};
-
-static struct sparse_checkout_set_opts {
-	int use_stdin;
-} set_opts;
-
 static void add_patterns_from_input(struct pattern_list *pl,
 				    int argc, const char **argv,
 				    int use_stdin)
@@ -663,8 +654,43 @@ static int modify_pattern_list(int argc, const char **argv, int use_stdin,
 	return result;
 }
 
-static int sparse_checkout_set(int argc, const char **argv, const char *prefix,
-			       enum modify_type m)
+static char const * const builtin_sparse_checkout_add_usage[] = {
+	N_("git sparse-checkout add (--stdin | <patterns>)"),
+	NULL
+};
+
+static struct sparse_checkout_add_opts {
+	int use_stdin;
+} add_opts;
+
+static int sparse_checkout_add(int argc, const char **argv, const char *prefix)
+{
+	static struct option builtin_sparse_checkout_add_options[] = {
+		OPT_BOOL(0, "stdin", &add_opts.use_stdin,
+			 N_("read patterns from standard in")),
+		OPT_END(),
+	};
+
+	repo_read_index(the_repository);
+
+	argc = parse_options(argc, argv, prefix,
+			     builtin_sparse_checkout_add_options,
+			     builtin_sparse_checkout_add_usage,
+			     PARSE_OPT_KEEP_UNKNOWN);
+
+	return modify_pattern_list(argc, argv, add_opts.use_stdin, ADD);
+}
+
+static char const * const builtin_sparse_checkout_set_usage[] = {
+	N_("git sparse-checkout set (--stdin | <patterns>)"),
+	NULL
+};
+
+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 struct option builtin_sparse_checkout_set_options[] = {
 		OPT_BOOL(0, "stdin", &set_opts.use_stdin,
@@ -679,7 +705,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, set_opts.use_stdin, m);
+	return modify_pattern_list(argc, argv, set_opts.use_stdin, REPLACE);
 }
 
 static char const * const builtin_sparse_checkout_reapply_usage[] = {
@@ -762,9 +788,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, REPLACE);
+			return sparse_checkout_set(argc, argv, prefix);
 		if (!strcmp(argv[0], "add"))
-			return sparse_checkout_set(argc, argv, prefix, ADD);
+			return sparse_checkout_add(argc, argv, prefix);
 		if (!strcmp(argv[0], "reapply"))
 			return sparse_checkout_reapply(argc, argv);
 		if (!strcmp(argv[0], "disable"))
-- 
gitgitgadget


^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH v2 03/10] sparse-checkout: add sanity-checks on initial sparsity state
  2021-12-07 20:20 ` [PATCH v2 00/10] " Elijah Newren via GitGitGadget
  2021-12-07 20:20   ` [PATCH v2 01/10] sparse-checkout: pass use_stdin as a parameter instead of as a global Elijah Newren via GitGitGadget
  2021-12-07 20:20   ` [PATCH v2 02/10] sparse-checkout: break apart functions for sparse_checkout_(set|add) Elijah Newren via GitGitGadget
@ 2021-12-07 20:20   ` Elijah Newren via GitGitGadget
  2021-12-07 20:20   ` [PATCH v2 04/10] sparse-checkout: disallow --no-stdin as an argument to set Elijah Newren via GitGitGadget
                     ` (9 subsequent siblings)
  12 siblings, 0 replies; 63+ messages in thread
From: Elijah Newren via GitGitGadget @ 2021-12-07 20:20 UTC (permalink / raw)
  To: git
  Cc: Derrick Stolee, Lessley Dennington, Victoria Dye, Elijah Newren,
	Elijah Newren, Elijah Newren

From: Elijah Newren <newren@gmail.com>

Most sparse-checkout subcommands (list, add, reapply, disable)
only make sense when already in a sparse state.  Add a quick check
that will error out early if this is not the case.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 builtin/sparse-checkout.c          | 12 ++++++++++++
 t/t1091-sparse-checkout-builtin.sh | 10 +++++++++-
 2 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c
index e252b82136e..e9f644ac362 100644
--- a/builtin/sparse-checkout.c
+++ b/builtin/sparse-checkout.c
@@ -56,6 +56,9 @@ static int sparse_checkout_list(int argc, const char **argv)
 	char *sparse_filename;
 	int res;
 
+	if (!core_apply_sparse_checkout)
+		die(_("this worktree is not sparse"));
+
 	argc = parse_options(argc, argv, NULL,
 			     builtin_sparse_checkout_list_options,
 			     builtin_sparse_checkout_list_usage, 0);
@@ -671,6 +674,9 @@ static int sparse_checkout_add(int argc, const char **argv, const char *prefix)
 		OPT_END(),
 	};
 
+	if (!core_apply_sparse_checkout)
+		die(_("no sparse-checkout to add to"));
+
 	repo_read_index(the_repository);
 
 	argc = parse_options(argc, argv, prefix,
@@ -719,6 +725,9 @@ static int sparse_checkout_reapply(int argc, const char **argv)
 		OPT_END(),
 	};
 
+	if (!core_apply_sparse_checkout)
+		die(_("must be in a sparse-checkout to reapply sparsity patterns"));
+
 	argc = parse_options(argc, argv, NULL,
 			     builtin_sparse_checkout_reapply_options,
 			     builtin_sparse_checkout_reapply_usage, 0);
@@ -740,6 +749,9 @@ static int sparse_checkout_disable(int argc, const char **argv)
 	struct pattern_list pl;
 	struct strbuf match_all = STRBUF_INIT;
 
+	if (!core_apply_sparse_checkout)
+		die(_("no active sparse-checkout to disable"));
+
 	argc = parse_options(argc, argv, NULL,
 			     builtin_sparse_checkout_disable_options,
 			     builtin_sparse_checkout_disable_usage, 0);
diff --git a/t/t1091-sparse-checkout-builtin.sh b/t/t1091-sparse-checkout-builtin.sh
index 272ba1b566b..90a281bcf64 100755
--- a/t/t1091-sparse-checkout-builtin.sh
+++ b/t/t1091-sparse-checkout-builtin.sh
@@ -41,7 +41,15 @@ test_expect_success 'setup' '
 	)
 '
 
-test_expect_success 'git sparse-checkout list (empty)' '
+test_expect_success 'git sparse-checkout list (not sparse)' '
+	test_must_fail git -C repo sparse-checkout list >list 2>err &&
+	test_must_be_empty list &&
+	test_i18ngrep "this worktree is not sparse" err
+'
+
+test_expect_success 'git sparse-checkout list (not sparse)' '
+	git -C repo sparse-checkout set &&
+	rm repo/.git/info/sparse-checkout &&
 	git -C repo sparse-checkout list >list 2>err &&
 	test_must_be_empty list &&
 	test_i18ngrep "this worktree is not sparse (sparse-checkout file may not exist)" err
-- 
gitgitgadget


^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH v2 04/10] sparse-checkout: disallow --no-stdin as an argument to set
  2021-12-07 20:20 ` [PATCH v2 00/10] " Elijah Newren via GitGitGadget
                     ` (2 preceding siblings ...)
  2021-12-07 20:20   ` [PATCH v2 03/10] sparse-checkout: add sanity-checks on initial sparsity state Elijah Newren via GitGitGadget
@ 2021-12-07 20:20   ` Elijah Newren via GitGitGadget
  2021-12-07 20:20   ` [PATCH v2 05/10] sparse-checkout: split out code for tweaking settings config Elijah Newren via GitGitGadget
                     ` (8 subsequent siblings)
  12 siblings, 0 replies; 63+ messages in thread
From: Elijah Newren via GitGitGadget @ 2021-12-07 20:20 UTC (permalink / raw)
  To: git
  Cc: Derrick Stolee, Lessley Dennington, Victoria Dye, Elijah Newren,
	Elijah Newren, Elijah Newren

From: Elijah Newren <newren@gmail.com>

We intentionally added --stdin as an option to `sparse-checkout set`,
but didn't intend for --no-stdin to be permitted as well.

Reported-by: Victoria Dye <vdye@github.com>
Signed-off-by: Elijah Newren <newren@gmail.com>
---
 builtin/sparse-checkout.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c
index e9f644ac362..0ee28f48134 100644
--- a/builtin/sparse-checkout.c
+++ b/builtin/sparse-checkout.c
@@ -699,8 +699,9 @@ static struct sparse_checkout_set_opts {
 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_BOOL_F(0, "stdin", &set_opts.use_stdin,
+			   N_("read patterns from standard in"),
+			   PARSE_OPT_NONEG),
 		OPT_END(),
 	};
 
-- 
gitgitgadget


^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH v2 05/10] sparse-checkout: split out code for tweaking settings config
  2021-12-07 20:20 ` [PATCH v2 00/10] " Elijah Newren via GitGitGadget
                     ` (3 preceding siblings ...)
  2021-12-07 20:20   ` [PATCH v2 04/10] sparse-checkout: disallow --no-stdin as an argument to set Elijah Newren via GitGitGadget
@ 2021-12-07 20:20   ` Elijah Newren via GitGitGadget
  2021-12-07 20:20   ` [PATCH v2 06/10] sparse-checkout: enable `set` to initialize sparse-checkout mode Elijah Newren via GitGitGadget
                     ` (7 subsequent siblings)
  12 siblings, 0 replies; 63+ messages in thread
From: Elijah Newren via GitGitGadget @ 2021-12-07 20:20 UTC (permalink / raw)
  To: git
  Cc: Derrick Stolee, Lessley Dennington, Victoria Dye, Elijah Newren,
	Elijah Newren, Elijah Newren

From: Elijah Newren <newren@gmail.com>

`init` has some code for handling updates to either cone mode or
the sparse-index setting.  We would like to be able to reuse this
elsewhere, namely in `set` and `reapply`.  Split this function out,
and make it slightly more general so it can handle being called from
the new callers.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 builtin/sparse-checkout.c | 56 ++++++++++++++++++++++++++-------------
 1 file changed, 37 insertions(+), 19 deletions(-)

diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c
index 0ee28f48134..f176435b6be 100644
--- a/builtin/sparse-checkout.c
+++ b/builtin/sparse-checkout.c
@@ -383,6 +383,41 @@ static int set_config(enum sparse_checkout_mode mode)
 	return 0;
 }
 
+static int update_modes(int *cone_mode, int *sparse_index)
+{
+	int mode, record_mode;
+
+	/* Determine if we need to record the mode; ensure sparse checkout on */
+	record_mode = (*cone_mode != -1) || !core_apply_sparse_checkout;
+
+	/* If not specified, use previous definition of cone mode */
+	if (*cone_mode == -1 && core_apply_sparse_checkout)
+		*cone_mode = core_sparse_checkout_cone;
+
+	/* Set cone/non-cone mode appropriately */
+	core_apply_sparse_checkout = 1;
+	if (*cone_mode == 1) {
+		mode = MODE_CONE_PATTERNS;
+		core_sparse_checkout_cone = 1;
+	} else {
+		mode = MODE_ALL_PATTERNS;
+	}
+	if (record_mode && set_config(mode))
+		return 1;
+
+	/* Set sparse-index/non-sparse-index mode if specified */
+	if (*sparse_index >= 0) {
+		if (set_sparse_index_config(the_repository, *sparse_index) < 0)
+			die(_("failed to modify sparse-index config"));
+
+		/* force an index rewrite */
+		repo_read_index(the_repository);
+		the_repository->index->updated_workdir = 1;
+	}
+
+	return 0;
+}
+
 static char const * const builtin_sparse_checkout_init_usage[] = {
 	N_("git sparse-checkout init [--cone] [--[no-]sparse-index]"),
 	NULL
@@ -399,7 +434,6 @@ static int sparse_checkout_init(int argc, const char **argv)
 	char *sparse_filename;
 	int res;
 	struct object_id oid;
-	int mode;
 	struct strbuf pattern = STRBUF_INIT;
 
 	static struct option builtin_sparse_checkout_init_options[] = {
@@ -412,19 +446,14 @@ static int sparse_checkout_init(int argc, const char **argv)
 
 	repo_read_index(the_repository);
 
+	init_opts.cone_mode = -1;
 	init_opts.sparse_index = -1;
 
 	argc = parse_options(argc, argv, NULL,
 			     builtin_sparse_checkout_init_options,
 			     builtin_sparse_checkout_init_usage, 0);
 
-	if (init_opts.cone_mode) {
-		mode = MODE_CONE_PATTERNS;
-		core_sparse_checkout_cone = 1;
-	} else
-		mode = MODE_ALL_PATTERNS;
-
-	if (set_config(mode))
+	if (update_modes(&init_opts.cone_mode, &init_opts.sparse_index))
 		return 1;
 
 	memset(&pl, 0, sizeof(pl));
@@ -432,17 +461,6 @@ static int sparse_checkout_init(int argc, const char **argv)
 	sparse_filename = get_sparse_checkout_filename();
 	res = add_patterns_from_file_to_list(sparse_filename, "", 0, &pl, NULL, 0);
 
-	if (init_opts.sparse_index >= 0) {
-		if (set_sparse_index_config(the_repository, init_opts.sparse_index) < 0)
-			die(_("failed to modify sparse-index config"));
-
-		/* force an index rewrite */
-		repo_read_index(the_repository);
-		the_repository->index->updated_workdir = 1;
-	}
-
-	core_apply_sparse_checkout = 1;
-
 	/* If we already have a sparse-checkout file, use it. */
 	if (res >= 0) {
 		free(sparse_filename);
-- 
gitgitgadget


^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH v2 06/10] sparse-checkout: enable `set` to initialize sparse-checkout mode
  2021-12-07 20:20 ` [PATCH v2 00/10] " Elijah Newren via GitGitGadget
                     ` (4 preceding siblings ...)
  2021-12-07 20:20   ` [PATCH v2 05/10] sparse-checkout: split out code for tweaking settings config Elijah Newren via GitGitGadget
@ 2021-12-07 20:20   ` Elijah Newren via GitGitGadget
  2021-12-07 20:20   ` [PATCH v2 07/10] sparse-checkout: enable reapply to take --[no-]{cone,sparse-index} Elijah Newren via GitGitGadget
                     ` (6 subsequent siblings)
  12 siblings, 0 replies; 63+ messages in thread
From: Elijah Newren via GitGitGadget @ 2021-12-07 20:20 UTC (permalink / raw)
  To: git
  Cc: Derrick Stolee, Lessley Dennington, Victoria Dye, Elijah Newren,
	Elijah Newren, Elijah Newren

From: Elijah Newren <newren@gmail.com>

The previously suggested workflow:
  git sparse-checkout init ...
  git sparse-checkout set ...

Suffered from three problems:
  1) It would delete nearly all files in the first step, then
     restore them in the second.  That was poor performance and
     forced unnecessary rebuilds.
  2) The two-step process resulted in two progress bars, which
     was suboptimal from a UI point of view for wrappers that
     invoked both of these commands but only exposed a single
     command to their end users.
  3) With cone mode, the first step would delete nearly all
     ignored files everywhere, because everything was considered
     to be outside of the specified sparsity paths.  (The user was
     not allowed to specify any sparsity paths in the `init` step.)

Avoid these problems by teaching `set` to understand the extra
parameters that `init` takes and performing any necessary initialization
if not already in a sparse checkout.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 builtin/sparse-checkout.c | 27 ++++++++++++++++++++++++++-
 1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c
index f176435b6be..1ecdc3ddd5a 100644
--- a/builtin/sparse-checkout.c
+++ b/builtin/sparse-checkout.c
@@ -706,17 +706,26 @@ static int sparse_checkout_add(int argc, const char **argv, const char *prefix)
 }
 
 static char const * const builtin_sparse_checkout_set_usage[] = {
-	N_("git sparse-checkout set (--stdin | <patterns>)"),
+	N_("git sparse-checkout set [--[no-]cone] [--[no-]sparse-index] (--stdin | <patterns>)"),
 	NULL
 };
 
 static struct sparse_checkout_set_opts {
+	int cone_mode;
+	int sparse_index;
 	int use_stdin;
 } set_opts;
 
 static int sparse_checkout_set(int argc, const char **argv, const char *prefix)
 {
+	int default_patterns_nr = 2;
+	const char *default_patterns[] = {"/*", "!/*/", NULL};
+
 	static struct option builtin_sparse_checkout_set_options[] = {
+		OPT_BOOL(0, "cone", &set_opts.cone_mode,
+			 N_("initialize the sparse-checkout in cone mode")),
+		OPT_BOOL(0, "sparse-index", &set_opts.sparse_index,
+			 N_("toggle the use of a sparse index")),
 		OPT_BOOL_F(0, "stdin", &set_opts.use_stdin,
 			   N_("read patterns from standard in"),
 			   PARSE_OPT_NONEG),
@@ -725,11 +734,27 @@ static int sparse_checkout_set(int argc, const char **argv, const char *prefix)
 
 	repo_read_index(the_repository);
 
+	set_opts.cone_mode = -1;
+	set_opts.sparse_index = -1;
+
 	argc = parse_options(argc, argv, prefix,
 			     builtin_sparse_checkout_set_options,
 			     builtin_sparse_checkout_set_usage,
 			     PARSE_OPT_KEEP_UNKNOWN);
 
+	if (update_modes(&set_opts.cone_mode, &set_opts.sparse_index))
+		return 1;
+
+	/*
+	 * Cone mode automatically specifies the toplevel directory.  For
+	 * non-cone mode, if nothing is specified, manually select just the
+	 * top-level directory (much as 'init' would do).
+	 */
+	if (!core_sparse_checkout_cone && argc == 0) {
+		argv = default_patterns;
+		argc = default_patterns_nr;
+	}
+
 	return modify_pattern_list(argc, argv, set_opts.use_stdin, REPLACE);
 }
 
-- 
gitgitgadget


^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH v2 07/10] sparse-checkout: enable reapply to take --[no-]{cone,sparse-index}
  2021-12-07 20:20 ` [PATCH v2 00/10] " Elijah Newren via GitGitGadget
                     ` (5 preceding siblings ...)
  2021-12-07 20:20   ` [PATCH v2 06/10] sparse-checkout: enable `set` to initialize sparse-checkout mode Elijah Newren via GitGitGadget
@ 2021-12-07 20:20   ` Elijah Newren via GitGitGadget
  2021-12-07 20:20   ` [PATCH v2 08/10] git-sparse-checkout.txt: update to document init/set/reapply changes Elijah Newren via GitGitGadget
                     ` (5 subsequent siblings)
  12 siblings, 0 replies; 63+ messages in thread
From: Elijah Newren via GitGitGadget @ 2021-12-07 20:20 UTC (permalink / raw)
  To: git
  Cc: Derrick Stolee, Lessley Dennington, Victoria Dye, Elijah Newren,
	Elijah Newren, Elijah Newren

From: Elijah Newren <newren@gmail.com>

Folks may want to switch to or from cone mode, or to or from a
sparse-index without changing their sparsity paths.  Allow them to do so
using the reapply command.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 builtin/sparse-checkout.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c
index 1ecdc3ddd5a..9d2a05677c4 100644
--- a/builtin/sparse-checkout.c
+++ b/builtin/sparse-checkout.c
@@ -759,13 +759,22 @@ static int sparse_checkout_set(int argc, const char **argv, const char *prefix)
 }
 
 static char const * const builtin_sparse_checkout_reapply_usage[] = {
-	N_("git sparse-checkout reapply"),
+	N_("git sparse-checkout reapply [--[no-]cone] [--[no-]sparse-index] "),
 	NULL
 };
 
+static struct sparse_checkout_reapply_opts {
+	int cone_mode;
+	int sparse_index;
+} reapply_opts;
+
 static int sparse_checkout_reapply(int argc, const char **argv)
 {
 	static struct option builtin_sparse_checkout_reapply_options[] = {
+		OPT_BOOL(0, "cone", &reapply_opts.cone_mode,
+			 N_("initialize the sparse-checkout in cone mode")),
+		OPT_BOOL(0, "sparse-index", &reapply_opts.sparse_index,
+			 N_("toggle the use of a sparse index")),
 		OPT_END(),
 	};
 
@@ -777,6 +786,13 @@ static int sparse_checkout_reapply(int argc, const char **argv)
 			     builtin_sparse_checkout_reapply_usage, 0);
 
 	repo_read_index(the_repository);
+
+	reapply_opts.cone_mode = -1;
+	reapply_opts.sparse_index = -1;
+
+	if (update_modes(&reapply_opts.cone_mode, &reapply_opts.sparse_index))
+		return 1;
+
 	return update_working_directory(NULL);
 }
 
-- 
gitgitgadget


^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH v2 08/10] git-sparse-checkout.txt: update to document init/set/reapply changes
  2021-12-07 20:20 ` [PATCH v2 00/10] " Elijah Newren via GitGitGadget
                     ` (6 preceding siblings ...)
  2021-12-07 20:20   ` [PATCH v2 07/10] sparse-checkout: enable reapply to take --[no-]{cone,sparse-index} Elijah Newren via GitGitGadget
@ 2021-12-07 20:20   ` Elijah Newren via GitGitGadget
  2021-12-07 20:20   ` [PATCH v2 09/10] Documentation: clarify/correct a few sparsity related statements Elijah Newren via GitGitGadget
                     ` (4 subsequent siblings)
  12 siblings, 0 replies; 63+ messages in thread
From: Elijah Newren via GitGitGadget @ 2021-12-07 20:20 UTC (permalink / raw)
  To: git
  Cc: Derrick Stolee, Lessley Dennington, Victoria Dye, Elijah Newren,
	Elijah Newren, Elijah Newren

From: Elijah Newren <newren@gmail.com>

As noted in the previous commit, using separate `init` and `set` steps
with sparse-checkout result in a number of issues.  The previous commits
made `set` able to handle the work of both commands, and enabled reapply
to tweak the {cone,sparse-index} settings.  Update the documentation to
reflect this, and mark `init` as deprecated.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 Documentation/git-sparse-checkout.txt | 98 +++++++++++++++------------
 1 file changed, 55 insertions(+), 43 deletions(-)

diff --git a/Documentation/git-sparse-checkout.txt b/Documentation/git-sparse-checkout.txt
index 42056ee9ff9..c7a25e282e7 100644
--- a/Documentation/git-sparse-checkout.txt
+++ b/Documentation/git-sparse-checkout.txt
@@ -30,28 +30,36 @@ COMMANDS
 'list'::
 	Describe the patterns in the sparse-checkout file.
 
-'init'::
-	Enable the `core.sparseCheckout` setting. If the
-	sparse-checkout file does not exist, then populate it with
-	patterns that match every file in the root directory and
-	no other directories, then will remove all directories tracked
-	by Git. Add patterns to the sparse-checkout file to
-	repopulate the working directory.
+'set'::
+	Enable the necessary config settings
+	(extensions.worktreeConfig, core.sparseCheckout,
+	core.sparseCheckoutCone) if they are not already enabled, and
+	write a set of patterns to the sparse-checkout file from the
+	list of arguments following the 'set' subcommand. Update the
+	working directory to match the new patterns.
 +
-To avoid interfering with other worktrees, it first enables the
-`extensions.worktreeConfig` setting and makes sure to set the
-`core.sparseCheckout` setting in the worktree-specific config file.
+When the `--stdin` option is provided, the patterns are read from
+standard in as a newline-delimited list instead of from the arguments.
 +
-When `--cone` is provided, the `core.sparseCheckoutCone` setting is
-also set, allowing for better performance with a limited set of
-patterns (see 'CONE PATTERN SET' below).
+When `--cone` is passed or `core.sparseCheckoutCone` is enabled, the
+input list is considered a list of directories instead of
+sparse-checkout patterns.  This allows for better performance with a
+limited set of patterns (see 'CONE PATTERN SET' below).  Note that the
+set command will write patterns to the sparse-checkout file to include
+all files contained in those directories (recursively) as well as
+files that are siblings of ancestor 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.  This may become the default in the future; --no-cone
+can be passed to request non-cone mode.
 +
-Use the `--[no-]sparse-index` option to toggle the use of the sparse
-index format. This reduces the size of the index to be more closely
-aligned with your sparse-checkout definition. This can have significant
-performance advantages for commands such as `git status` or `git add`.
-This feature is still experimental. Some commands might be slower with
-a sparse index until they are properly integrated with the feature.
+Use the `--[no-]sparse-index` option to use a sparse index (the
+default is to not use it).  A sparse index reduces the size of the
+index to be more closely aligned with your sparse-checkout
+definition. This can have significant performance advantages for
+commands such as `git status` or `git add`.  This feature is still
+experimental. Some commands might be slower with a sparse index until
+they are properly integrated with the feature.
 +
 **WARNING:** Using a sparse index requires modifying the index in a way
 that is not completely understood by external tools. If you have trouble
@@ -60,23 +68,6 @@ to rewrite your index to not be sparse. Older versions of Git will not
 understand the sparse directory entries index extension and may fail to
 interact with your repository until it is disabled.
 
-'set'::
-	Write a set of patterns to the sparse-checkout file, as given as
-	a list of arguments following the 'set' subcommand. Update the
-	working directory to match the new patterns. Enable the
-	core.sparseCheckout config setting if it is not already enabled.
-+
-When the `--stdin` option is provided, the patterns are read from
-standard in as a newline-delimited list instead of from the arguments.
-+
-When `core.sparseCheckoutCone` is enabled, the input list is considered a
-list of directories instead of sparse-checkout patterns. The command writes
-patterns to the sparse-checkout file to include all files contained in those
-directories (recursively) as well as files that are siblings of ancestor
-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,
@@ -93,12 +84,35 @@ C-style quoted strings.
 	cases, it can make sense to run `git sparse-checkout reapply` later
 	after cleaning up affected paths (e.g. resolving conflicts, undoing
 	or committing changes, etc.).
++
+The `reapply` command can also take `--[no-]cone` and `--[no-]sparse-index`
+flags, with the same meaning as the flags from the `set` command, in order
+to change which sparsity mode you are using without needing to also respecify
+all sparsity paths.
 
 'disable'::
 	Disable the `core.sparseCheckout` config setting, and restore the
-	working directory to include all files. Leaves the sparse-checkout
-	file intact so a later 'git sparse-checkout init' command may
-	return the working directory to the same state.
+	working directory to include all files.
+
+'init'::
+	Deprecated command that behaves like `set` with no specified paths.
+	May be removed in the future.
++
+Historically, `set` did not used to handle all the necessary config
+settings, which meant that both `init` and `set` had to be called.
+Invoking both meant the `init` step would first remove nearly all
+tracked files (and in cone mode, ignored files too), then the `set`
+step would add many of the tracked files (but not ignored files) back.
+In addition to the lost files, the performance and UI of this
+combination was poor.
++
+Also, historically, `init` would not actually initialize the
+sparse-checkout file if it already existed.  This meant it was
+possible to return to a sparse-checkout without remembering which
+paths to pass to a subsequent 'set' or 'add' command.  However,
+`--cone` and `--sparse-index` options would not be remembered across
+the disable command, so the easy restore of calling a plain `init`
+decreased in utility.
 
 SPARSE CHECKOUT
 ---------------
@@ -117,10 +131,8 @@ directory, it updates the skip-worktree bits in the index based
 on this file. The files matching the patterns in the file will
 appear in the working directory, and the rest will not.
 
-To enable the sparse-checkout feature, run `git sparse-checkout init` to
-initialize a simple sparse-checkout file and enable the `core.sparseCheckout`
-config setting. Then, run `git sparse-checkout set` to modify the patterns in
-the sparse-checkout file.
+To enable the sparse-checkout feature, run `git sparse-checkout set` to
+set the patterns you want to use.
 
 To repopulate the working directory with all files, use the
 `git sparse-checkout disable` command.
-- 
gitgitgadget


^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH v2 09/10] Documentation: clarify/correct a few sparsity related statements
  2021-12-07 20:20 ` [PATCH v2 00/10] " Elijah Newren via GitGitGadget
                     ` (7 preceding siblings ...)
  2021-12-07 20:20   ` [PATCH v2 08/10] git-sparse-checkout.txt: update to document init/set/reapply changes Elijah Newren via GitGitGadget
@ 2021-12-07 20:20   ` Elijah Newren via GitGitGadget
  2021-12-07 20:20   ` [PATCH v2 10/10] clone: avoid using deprecated `sparse-checkout init` Elijah Newren via GitGitGadget
                     ` (3 subsequent siblings)
  12 siblings, 0 replies; 63+ messages in thread
From: Elijah Newren via GitGitGadget @ 2021-12-07 20:20 UTC (permalink / raw)
  To: git
  Cc: Derrick Stolee, Lessley Dennington, Victoria Dye, Elijah Newren,
	Elijah Newren, Elijah Newren

From: Elijah Newren <newren@gmail.com>

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 Documentation/git-clone.txt           | 8 ++++----
 Documentation/git-sparse-checkout.txt | 2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt
index 3fe3810f1ce..b348a71fc68 100644
--- a/Documentation/git-clone.txt
+++ b/Documentation/git-clone.txt
@@ -167,10 +167,10 @@ objects from the source repository into a pack in the cloned repository.
 	configuration variables are created.
 
 --sparse::
-	Initialize the sparse-checkout file so the working
-	directory starts with only the files in the root
-	of the repository. The sparse-checkout file can be
-	modified to grow the working directory as needed.
+	Employ a sparse-checkout, with only files in the toplevel
+	directory initially being present.  The
+	linkgit:git-sparse-checkout[1] command can be used to grow the
+	working directory as needed.
 
 --filter=<filter-spec>::
 	Use the partial clone feature and request that the server sends
diff --git a/Documentation/git-sparse-checkout.txt b/Documentation/git-sparse-checkout.txt
index c7a25e282e7..aca36782cef 100644
--- a/Documentation/git-sparse-checkout.txt
+++ b/Documentation/git-sparse-checkout.txt
@@ -121,7 +121,7 @@ SPARSE CHECKOUT
 It uses the skip-worktree bit (see linkgit:git-update-index[1]) to tell
 Git whether a file in the working directory is worth looking at. If
 the skip-worktree bit is set, then the file is ignored in the working
-directory. Git will not populate the contents of those files, which
+directory. Git will avoid populating the contents of those files, which
 makes a sparse checkout helpful when working in a repository with many
 files, but only a few are important to the current user.
 
-- 
gitgitgadget


^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH v2 10/10] clone: avoid using deprecated `sparse-checkout init`
  2021-12-07 20:20 ` [PATCH v2 00/10] " Elijah Newren via GitGitGadget
                     ` (8 preceding siblings ...)
  2021-12-07 20:20   ` [PATCH v2 09/10] Documentation: clarify/correct a few sparsity related statements Elijah Newren via GitGitGadget
@ 2021-12-07 20:20   ` Elijah Newren via GitGitGadget
  2021-12-07 21:05   ` [PATCH v2 00/10] sparse-checkout: make set subsume init Derrick Stolee
                     ` (2 subsequent siblings)
  12 siblings, 0 replies; 63+ messages in thread
From: Elijah Newren via GitGitGadget @ 2021-12-07 20:20 UTC (permalink / raw)
  To: git
  Cc: Derrick Stolee, Lessley Dennington, Victoria Dye, Elijah Newren,
	Elijah Newren, Elijah Newren

From: Elijah Newren <newren@gmail.com>

The previous commits marked `sparse-checkout init` as deprecated; we
can just use `set` instead here and pass it no paths.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 builtin/clone.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/clone.c b/builtin/clone.c
index fb377b27657..5bed37f8b51 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -633,7 +633,7 @@ static int git_sparse_checkout_init(const char *repo)
 {
 	struct strvec argv = STRVEC_INIT;
 	int result = 0;
-	strvec_pushl(&argv, "-C", repo, "sparse-checkout", "init", NULL);
+	strvec_pushl(&argv, "-C", repo, "sparse-checkout", "set", NULL);
 
 	/*
 	 * We must apply the setting in the current process
-- 
gitgitgadget

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* Re: [PATCH 3/6] sparse-checkout: enable `set` to initialize sparse-checkout mode
  2021-12-07 17:04     ` Elijah Newren
@ 2021-12-07 20:36       ` Lessley Dennington
  0 siblings, 0 replies; 63+ messages in thread
From: Lessley Dennington @ 2021-12-07 20:36 UTC (permalink / raw)
  To: Elijah Newren, Victoria Dye
  Cc: Elijah Newren via GitGitGadget, Git Mailing List, Derrick Stolee



On 12/7/21 9:04 AM, Elijah Newren wrote:
> On Sat, Dec 4, 2021 at 1:40 PM Victoria Dye <vdye@github.com> wrote:
>>
>> Elijah Newren via GitGitGadget wrote:
>>> From: Elijah Newren <newren@gmail.com>
>>>
>>> The previously suggested workflow:
>>>    git sparse-checkout init ...
>>>    git sparse-checkout set ...
>>>
>>> Suffered from three problems:
>>>    1) It would delete nearly all files in the first step, then
>>>       restore them in the second.  That was poor performance and
>>>       forced unnecessary rebuilds.
>>>    2) The two-step process resulted in two progress bars, which
>>>       was suboptimal from a UI point of view for wrappers that
>>>       invoked both of these commands but only exposed a single
>>>       command to their end users.
>>>    3) With cone mode, the first step would delete nearly all
>>>       ignored files everywhere, because everything was considered
>>>       to be outside of the specified sparsity paths.  (The user was
>>>       not allowed to specify any sparsity paths in the `init` step.)
>>>
>>> Avoid these problems by teaching `set` to understand the extra
>>> parameters that `init` takes and performing any necessary initialization
>>> if not already in a sparse checkout.
>>>
>>
>> I really like this change! It always seemed weird that `set` would
>> implicitly `init`, but without any of the options in `init`.
>>
>>> Signed-off-by: Elijah Newren <newren@gmail.com>
>>> ---
>>>   builtin/sparse-checkout.c | 52 ++++++++++++++++++++++++++++++++++++++-
>>>   1 file changed, 51 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c
>>> index e252b82136e..cf6a6c6c3a7 100644
>>> --- a/builtin/sparse-checkout.c
>>> +++ b/builtin/sparse-checkout.c
>>> @@ -682,17 +682,26 @@ static int sparse_checkout_add(int argc, const char **argv, const char *prefix)
>>>   }
>>>
>>>   static char const * const builtin_sparse_checkout_set_usage[] = {
>>> -     N_("git sparse-checkout set (--stdin | <patterns>)"),
>>> +     N_("git sparse-checkout set [--cone] [--[no-]sparse-index] (--stdin | <patterns>)"),
>>
>> Since `cone` is an `OPT_BOOL`, shouldn't it appear in the usage string as
>> `--[no-]cone`? Without a `--no-cone` option, it's not clear how a user would
>> disable cone mode (since `set` preserves the existing cone mode setting if
>> `--cone` isn't given).
> 
> Yeah, fair point.  When copying from init I probably should have double checked.
> 
> Also, it makes me wonder if we should just make cone mode the default...
> 
That was actually my first thought when reviewing this change! It feels
like this would be a great moment to take the leap on that, but I can
also see it warranting a separate series.

>>>        NULL
>>>   };
>>>
>>>   static struct sparse_checkout_set_opts {
>>> +     int cone_mode;
>>> +     int sparse_index;
>>>        int use_stdin;
>>>   } set_opts;
>>>
>>>   static int sparse_checkout_set(int argc, const char **argv, const char *prefix)
>>>   {
>>> +     int mode, record_mode;
>>> +     const char *default_patterns[] = {"/*", "!/*/"};
>>> +
>>>        static struct option builtin_sparse_checkout_set_options[] = {
>>> +             OPT_BOOL(0, "cone", &set_opts.cone_mode,
>>> +                      N_("initialize the sparse-checkout in cone mode")),
>>> +             OPT_BOOL(0, "sparse-index", &set_opts.sparse_index,
>>> +                      N_("toggle the use of a sparse index")),
>>>                OPT_BOOL(0, "stdin", &set_opts.use_stdin,
>>>                         N_("read patterns from standard in")),
>>
>> I know this isn't part of this patch, but is `stdin` really a bool (i.e.
>> someone might specify `--no-stdin`)? If not, I think `OPT_SET_INT` might be
>> more appropriate.
> 
> Good point.  OPT_SET_INT() wouldn't fix it, though; you need to use
> OPT_BOOL_F or OPT_SET_INT_F and pass PARSE_OPT_NONEG as a flag.  I can
> include that.
> 
>>>                OPT_END(),
>>> @@ -700,11 +709,52 @@ static int sparse_checkout_set(int argc, const char **argv, const char *prefix)
>>>
>>>        repo_read_index(the_repository);
>>>
>>> +     set_opts.cone_mode = -1;
>>> +     set_opts.sparse_index = -1;
>>> +
>>>        argc = parse_options(argc, argv, prefix,
>>>                             builtin_sparse_checkout_set_options,
>>>                             builtin_sparse_checkout_set_usage,
>>>                             PARSE_OPT_KEEP_UNKNOWN);
>>>
>>> +     /* Determine if we need to record the mode; ensure sparse checkout on */
>>> +     record_mode = (set_opts.cone_mode != -1) || !core_apply_sparse_checkout;
>>> +     core_apply_sparse_checkout = 1;
>>> +
>>> +     /* If not specified, use previous definition of cone mode */
>>> +     if (set_opts.cone_mode == -1 && core_apply_sparse_checkout)
>>
>> I *think* this is supposed go before the `core_apply_sparse_checkout = 1;`
>> above it (if the intention is to only use the pre-existing cone mode setting
>> when already in a sparse checkout). If not, the `core_apply_sparse_checkout`
>> part of the condition is redundant and can be removed entirely.
> 
> Eek.  I had it there originally, then moved it later while doing
> various changes.  This definitely should be later; thanks for
> catching.
> 
>>> +             set_opts.cone_mode = core_sparse_checkout_cone;
>>> +
>>> +     /* Set cone/non-cone mode appropriately */
>>> +     if (set_opts.cone_mode == 1) {
>>> +             mode = MODE_CONE_PATTERNS;
>>> +             core_sparse_checkout_cone = 1;
>>> +     } else {
>>> +             mode = MODE_ALL_PATTERNS;
>>> +     }
>>> +     if (record_mode && set_config(mode))
>>> +             return 1;
>>> +
>>> +     /* Set sparse-index/non-sparse-index mode if specified */
>>> +     if (set_opts.sparse_index >= 0) {
>>> +             if (set_sparse_index_config(the_repository, set_opts.sparse_index) < 0)
>>> +                     die(_("failed to modify sparse-index config"));
>>> +
>>> +             /* force an index rewrite */
>>> +             repo_read_index(the_repository);
>>> +             the_repository->index->updated_workdir = 1;
>>> +     }
>>> +
>>> +     /*
>>> +      * Cone mode automatically specifies the toplevel directory.  For
>>> +      * non-cone mode, if nothing is specified, manually select just the
>>> +      * top-level directory (much as 'init' would do).
>>> +      */
>>> +     if (!core_sparse_checkout_cone && argc == 0) {
>>> +             argv = default_patterns;
>>> +             argc = 2;
>>> +     }
>>> +
>>>        return modify_pattern_list(argc, argv, set_opts.use_stdin, REPLACE);
>>>   }

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [PATCH 4/6] git-sparse-checkout.txt: update to document that set handles init
  2021-12-04 20:00 ` [PATCH 4/6] git-sparse-checkout.txt: update to document that set handles init Elijah Newren via GitGitGadget
  2021-12-04 21:48   ` Victoria Dye
@ 2021-12-07 20:42   ` Lessley Dennington
  2021-12-07 21:13     ` Elijah Newren
  1 sibling, 1 reply; 63+ messages in thread
From: Lessley Dennington @ 2021-12-07 20:42 UTC (permalink / raw)
  To: Elijah Newren via GitGitGadget, git
  Cc: Derrick Stolee, Victoria Dye, Elijah Newren



On 12/4/21 12:00 PM, Elijah Newren via GitGitGadget wrote:
> From: Elijah Newren <newren@gmail.com>
> 
> As noted in the previous commit, using separate `init` and `set` steps
> with sparse-checkout result in a number of issues.  The previous commit
> made `set` able to handle the work of both commands.  Update the
> documentation to reflect this, and mark `init` as deprecated.
> 
> Signed-off-by: Elijah Newren <newren@gmail.com>
> ---
>   Documentation/git-sparse-checkout.txt | 92 ++++++++++++++-------------
>   1 file changed, 49 insertions(+), 43 deletions(-)
> 
> diff --git a/Documentation/git-sparse-checkout.txt b/Documentation/git-sparse-checkout.txt
> index 42056ee9ff9..d22c925ecf8 100644
> --- a/Documentation/git-sparse-checkout.txt
> +++ b/Documentation/git-sparse-checkout.txt
> @@ -30,28 +30,35 @@ COMMANDS
>   'list'::
>   	Describe the patterns in the sparse-checkout file.
>   
> -'init'::
> -	Enable the `core.sparseCheckout` setting. If the
> -	sparse-checkout file does not exist, then populate it with
> -	patterns that match every file in the root directory and
> -	no other directories, then will remove all directories tracked
> -	by Git. Add patterns to the sparse-checkout file to
> -	repopulate the working directory.
> +'set'::
> +	Enable the necessary config settings
> +	(extensions.worktreeConfig, core.sparseCheckout,
> +	core.sparseCheckoutCone) if they are not already enabled, and
> +	write a set of patterns to the sparse-checkout file from the
> +	list of arguments following the 'set' subcommand. Update the
> +	working directory to match the new patterns.
>   +
> -To avoid interfering with other worktrees, it first enables the
> -`extensions.worktreeConfig` setting and makes sure to set the
> -`core.sparseCheckout` setting in the worktree-specific config file.
> +When the `--stdin` option is provided, the patterns are read from
> +standard in as a newline-delimited list instead of from the arguments.
>   +
> -When `--cone` is provided, the `core.sparseCheckoutCone` setting is
> -also set, allowing for better performance with a limited set of
> -patterns (see 'CONE PATTERN SET' below).
> +When `--cone` is passed or `core.sparseCheckoutCone` is enabled, the
> +input list is considered a list of directories instead of
> +sparse-checkout patterns.  This allows for better performance with a
> +limited set of patterns (see 'CONE PATTERN SET' below).  Note that the
> +set command will write patterns to the sparse-checkout file to include
> +all files contained in those directories (recursively) as well as
> +files that are siblings of ancestor 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.
>   +
> -Use the `--[no-]sparse-index` option to toggle the use of the sparse
> -index format. This reduces the size of the index to be more closely
> -aligned with your sparse-checkout definition. This can have significant
> -performance advantages for commands such as `git status` or `git add`.
> -This feature is still experimental. Some commands might be slower with
> -a sparse index until they are properly integrated with the feature.
> +Use the `--[no-]sparse-index` option to use a sparse index (the
> +default is to not use it).  A sparse index reduces the size of the
> +index to be more closely aligned with your sparse-checkout
> +definition. This can have significant performance advantages for
> +commands such as `git status` or `git add`.  This feature is still
> +experimental. Some commands might be slower with a sparse index until
> +they are properly integrated with the feature.
>   +
>   **WARNING:** Using a sparse index requires modifying the index in a way
>   that is not completely understood by external tools. If you have trouble
> @@ -60,23 +67,6 @@ to rewrite your index to not be sparse. Older versions of Git will not
>   understand the sparse directory entries index extension and may fail to
>   interact with your repository until it is disabled.
>   
> -'set'::
> -	Write a set of patterns to the sparse-checkout file, as given as
> -	a list of arguments following the 'set' subcommand. Update the
> -	working directory to match the new patterns. Enable the
> -	core.sparseCheckout config setting if it is not already enabled.
> -+
> -When the `--stdin` option is provided, the patterns are read from
> -standard in as a newline-delimited list instead of from the arguments.
> -+
> -When `core.sparseCheckoutCone` is enabled, the input list is considered a
> -list of directories instead of sparse-checkout patterns. The command writes
> -patterns to the sparse-checkout file to include all files contained in those
> -directories (recursively) as well as files that are siblings of ancestor
> -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,
> @@ -96,9 +86,27 @@ C-style quoted strings.
>   
>   'disable'::
>   	Disable the `core.sparseCheckout` config setting, and restore the
> -	working directory to include all files. Leaves the sparse-checkout
> -	file intact so a later 'git sparse-checkout init' command may
> -	return the working directory to the same state.
> +	working directory to include all files.
> +
> +'init'::
> +	Deprecated command that behaves like `set` with no specified paths.
> +	May be removed in the future.
> ++
> +Historically, `set` did not used to handle all the necessary config
> +settings, which meant that both `init` and `set` had to be called.
> +Invoking both meant the `init` step would first remove nearly all
> +tracked files (and in cone mode, ignored files too), then the `set`
> +step would add many of the tracked files (but not ignored files) back.
> +In addition to the lost files, the performance and UI of this
> +combination was poor.
> ++
Super nit: Perhaps update '`set` did not used to handle' to '`set` did not
handle'.
> +Also, historically, `init` would not actually initialize the
> +sparse-checkout file if it already existed.  This meant it was
> +possible to return to a sparse-checkout without remembering which
> +paths to pass to a subsequent 'set' or 'add' command.  However,
> +`--cone` and `--sparse-index` options would not be remembered across
> +the disable command, so the easy restore of calling a plain `init`
> +decreased in utility.
>   
>   SPARSE CHECKOUT
>   ---------------
> @@ -117,10 +125,8 @@ directory, it updates the skip-worktree bits in the index based
>   on this file. The files matching the patterns in the file will
>   appear in the working directory, and the rest will not.
>   
> -To enable the sparse-checkout feature, run `git sparse-checkout init` to
> -initialize a simple sparse-checkout file and enable the `core.sparseCheckout`
> -config setting. Then, run `git sparse-checkout set` to modify the patterns in
> -the sparse-checkout file.
> +To enable the sparse-checkout feature, run `git sparse-checkout set` to
> +set the patterns you want to use.
>   
>   To repopulate the working directory with all files, use the
>   `git sparse-checkout disable` command.
> 

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [PATCH v2 00/10] sparse-checkout: make set subsume init
  2021-12-07 20:20 ` [PATCH v2 00/10] " Elijah Newren via GitGitGadget
                     ` (9 preceding siblings ...)
  2021-12-07 20:20   ` [PATCH v2 10/10] clone: avoid using deprecated `sparse-checkout init` Elijah Newren via GitGitGadget
@ 2021-12-07 21:05   ` Derrick Stolee
  2021-12-08 14:16   ` Victoria Dye
  2021-12-10  3:56   ` [PATCH v3 " Elijah Newren via GitGitGadget
  12 siblings, 0 replies; 63+ messages in thread
From: Derrick Stolee @ 2021-12-07 21:05 UTC (permalink / raw)
  To: Elijah Newren via GitGitGadget, git
  Cc: Lessley Dennington, Victoria Dye, Elijah Newren

On 12/7/2021 3:20 PM, Elijah Newren via GitGitGadget wrote:
> As described at [1], the split of init and set subcommands in
> sparse-checkout causes multiple issues:
> 
>  * Poor performance (deleting all tracked files, then later restoring many
>    and maybe even most of them)
>  * Poor UI (multiple progress bars in wrappers that hide both commands under
>    1 user-facing command)
>  * Loss of ignored files under directories the user wanted to keep
> 
> This series fixes this bug by providing a single command to switch to a
> sparse-checkout: set. It does so by making set able to do the combined work
> of init and set. It keeps init as-is to give folks time to adapt, but marks
> it as deprecated. It also makes reapply able to toggle cone/non-cone mode
> and sparse-index/non-sparse-index mode.
> 
> Changes since v1:
> 
>  * Inserted new patches 3 & 4 as additional preparatory cleanups
>  * Took the new mode-toggling work code in sparse_checkout_set from the
>    previous series and moved it into a new function, as a preparatory patch,
>    and made it usable by init/set/reapply
>  * Also updated reapply to allow mode-toggling
>  * Updated the documentation as per above
>  * Various other small items from review comments

I reread this series and the new patch organization is an
improvement! I found this to be an easy read and have no
suggestions for changes.

Reviewed-by: Derrick Stolee <dstolee@microsoft.com>

Thanks,
-Stolee

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [PATCH 4/6] git-sparse-checkout.txt: update to document that set handles init
  2021-12-07 20:42   ` Lessley Dennington
@ 2021-12-07 21:13     ` Elijah Newren
  2021-12-07 22:36       ` Lessley Dennington
  0 siblings, 1 reply; 63+ messages in thread
From: Elijah Newren @ 2021-12-07 21:13 UTC (permalink / raw)
  To: Lessley Dennington
  Cc: Elijah Newren via GitGitGadget, Git Mailing List, Derrick Stolee,
	Victoria Dye

On Tue, Dec 7, 2021 at 12:42 PM Lessley Dennington
<lessleydennington@gmail.com> wrote:
>
> On 12/4/21 12:00 PM, Elijah Newren via GitGitGadget wrote:
> > From: Elijah Newren <newren@gmail.com>
> >
...
> > ++
> > +Historically, `set` did not used to handle all the necessary config
> > +settings, which meant that both `init` and `set` had to be called.
> > +Invoking both meant the `init` step would first remove nearly all
> > +tracked files (and in cone mode, ignored files too), then the `set`
> > +step would add many of the tracked files (but not ignored files) back.
> > +In addition to the lost files, the performance and UI of this
> > +combination was poor.
> > ++
> Super nit: Perhaps update '`set` did not used to handle' to '`set` did not
> handle'.

Yeah, that sounds better.  I'll wait a bit longer for other comments,
then send a fix for this along with any other needed fixes (even if
just included Acks & Reviewed-bys I get).  Thanks!

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [PATCH 4/6] git-sparse-checkout.txt: update to document that set handles init
  2021-12-07 21:13     ` Elijah Newren
@ 2021-12-07 22:36       ` Lessley Dennington
  0 siblings, 0 replies; 63+ messages in thread
From: Lessley Dennington @ 2021-12-07 22:36 UTC (permalink / raw)
  To: Elijah Newren
  Cc: Elijah Newren via GitGitGadget, Git Mailing List, Derrick Stolee,
	Victoria Dye



On 12/7/21 1:13 PM, Elijah Newren wrote:
> On Tue, Dec 7, 2021 at 12:42 PM Lessley Dennington
> <lessleydennington@gmail.com> wrote:
>>
>> On 12/4/21 12:00 PM, Elijah Newren via GitGitGadget wrote:
>>> From: Elijah Newren <newren@gmail.com>
>>>
> ...
>>> ++
>>> +Historically, `set` did not used to handle all the necessary config
>>> +settings, which meant that both `init` and `set` had to be called.
>>> +Invoking both meant the `init` step would first remove nearly all
>>> +tracked files (and in cone mode, ignored files too), then the `set`
>>> +step would add many of the tracked files (but not ignored files) back.
>>> +In addition to the lost files, the performance and UI of this
>>> +combination was poor.
>>> ++
>> Super nit: Perhaps update '`set` did not used to handle' to '`set` did not
>> handle'.
> 
> Yeah, that sounds better.  I'll wait a bit longer for other comments,
> then send a fix for this along with any other needed fixes (even if
> just included Acks & Reviewed-bys I get).  Thanks!
> 
Sounds good. Also apologies that I didn't add this to v2 - my Thunderbird
client hadn't loaded those messages at the time I sent my feedback.

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [PATCH v2 00/10] sparse-checkout: make set subsume init
  2021-12-07 20:20 ` [PATCH v2 00/10] " Elijah Newren via GitGitGadget
                     ` (10 preceding siblings ...)
  2021-12-07 21:05   ` [PATCH v2 00/10] sparse-checkout: make set subsume init Derrick Stolee
@ 2021-12-08 14:16   ` Victoria Dye
  2021-12-10  3:56   ` [PATCH v3 " Elijah Newren via GitGitGadget
  12 siblings, 0 replies; 63+ messages in thread
From: Victoria Dye @ 2021-12-08 14:16 UTC (permalink / raw)
  To: Elijah Newren via GitGitGadget, git
  Cc: Derrick Stolee, Lessley Dennington, Elijah Newren

Elijah Newren via GitGitGadget wrote:
> As described at [1], the split of init and set subcommands in
> sparse-checkout causes multiple issues:
> 
>  * Poor performance (deleting all tracked files, then later restoring many
>    and maybe even most of them)
>  * Poor UI (multiple progress bars in wrappers that hide both commands under
>    1 user-facing command)
>  * Loss of ignored files under directories the user wanted to keep
> 
> This series fixes this bug by providing a single command to switch to a
> sparse-checkout: set. It does so by making set able to do the combined work
> of init and set. It keeps init as-is to give folks time to adapt, but marks
> it as deprecated. It also makes reapply able to toggle cone/non-cone mode
> and sparse-index/non-sparse-index mode.
> 
> Changes since v1:
> 
>  * Inserted new patches 3 & 4 as additional preparatory cleanups
>  * Took the new mode-toggling work code in sparse_checkout_set from the
>    previous series and moved it into a new function, as a preparatory patch,
>    and made it usable by init/set/reapply
>  * Also updated reapply to allow mode-toggling
>  * Updated the documentation as per above
>  * Various other small items from review comments
> 
> A quick overview:
> 
>  * Patches 1-4: small preparatory refactorings
>  * Patch 5: patch adding new function for toggling {cone,sparse-checkout}
>    modes
>  * Patch 6: the crux of the series; make set able to handle both init and
>    set options
>  * Patch 7: make reapply also able to do mode toggling
>  * Patches 8-9: documentation modifications (Patch 4 is worth reviewing; it
>    marks init as deprecated -- are others okay with that?)
>  * Patch 10: trivial modification of git clone --sparse to use git
>    sparse-checkout set rather than git sparse-checkout init.
> 
> [1]
> https://lore.kernel.org/git/CABPp-BE8TJ8QGAQWsSGT7S+9Xp-XmApcC9PSw3K=RQOP0rt+PQ@mail.gmail.com/
> 
> Elijah Newren (10):
>   sparse-checkout: pass use_stdin as a parameter instead of as a global
>   sparse-checkout: break apart functions for sparse_checkout_(set|add)
>   sparse-checkout: add sanity-checks on initial sparsity state
>   sparse-checkout: disallow --no-stdin as an argument to set
>   sparse-checkout: split out code for tweaking settings config
>   sparse-checkout: enable `set` to initialize sparse-checkout mode
>   sparse-checkout: enable reapply to take --[no-]{cone,sparse-index}
>   git-sparse-checkout.txt: update to document init/set/reapply changes
>   Documentation: clarify/correct a few sparsity related statements
>   clone: avoid using deprecated `sparse-checkout init`
> 
>  Documentation/git-clone.txt           |   8 +-
>  Documentation/git-sparse-checkout.txt | 100 +++++++------
>  builtin/clone.c                       |   2 +-
>  builtin/sparse-checkout.c             | 196 ++++++++++++++++++++------
>  t/t1091-sparse-checkout-builtin.sh    |  10 +-
>  5 files changed, 219 insertions(+), 97 deletions(-)
> 
> 
> base-commit: abe6bb3905392d5eb6b01fa6e54d7e784e0522aa
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1151%2Fnewren%2Fsparse-checkout-no-init-v2
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1151/newren/sparse-checkout-no-init-v2
> Pull-Request: https://github.com/git/git/pull/1151
> 
> Range-diff vs v1:
> 
>   1:  e41cfe3c1bb =  1:  e41cfe3c1bb sparse-checkout: pass use_stdin as a parameter instead of as a global
>   2:  6f7de8f5412 =  2:  6f7de8f5412 sparse-checkout: break apart functions for sparse_checkout_(set|add)
>   -:  ----------- >  3:  bade5e52390 sparse-checkout: add sanity-checks on initial sparsity state
>   -:  ----------- >  4:  0180bfc4a93 sparse-checkout: disallow --no-stdin as an argument to set
>   -:  ----------- >  5:  3f5255eeef9 sparse-checkout: split out code for tweaking settings config
>   3:  a90687eb4c1 !  6:  3c640f5bcef sparse-checkout: enable `set` to initialize sparse-checkout mode
>      @@ builtin/sparse-checkout.c: static int sparse_checkout_add(int argc, const char *
>        
>        static char const * const builtin_sparse_checkout_set_usage[] = {
>       -	N_("git sparse-checkout set (--stdin | <patterns>)"),
>      -+	N_("git sparse-checkout set [--cone] [--[no-]sparse-index] (--stdin | <patterns>)"),
>      ++	N_("git sparse-checkout set [--[no-]cone] [--[no-]sparse-index] (--stdin | <patterns>)"),
>        	NULL
>        };
>        
>      @@ builtin/sparse-checkout.c: static int sparse_checkout_add(int argc, const char *
>        
>        static int sparse_checkout_set(int argc, const char **argv, const char *prefix)
>        {
>      -+	int mode, record_mode;
>      -+	const char *default_patterns[] = {"/*", "!/*/"};
>      ++	int default_patterns_nr = 2;
>      ++	const char *default_patterns[] = {"/*", "!/*/", NULL};
>       +
>        	static struct option builtin_sparse_checkout_set_options[] = {
>       +		OPT_BOOL(0, "cone", &set_opts.cone_mode,
>       +			 N_("initialize the sparse-checkout in cone mode")),
>       +		OPT_BOOL(0, "sparse-index", &set_opts.sparse_index,
>       +			 N_("toggle the use of a sparse index")),
>      - 		OPT_BOOL(0, "stdin", &set_opts.use_stdin,
>      - 			 N_("read patterns from standard in")),
>      - 		OPT_END(),
>      + 		OPT_BOOL_F(0, "stdin", &set_opts.use_stdin,
>      + 			   N_("read patterns from standard in"),
>      + 			   PARSE_OPT_NONEG),
>       @@ builtin/sparse-checkout.c: static int sparse_checkout_set(int argc, const char **argv, const char *prefix)
>        
>        	repo_read_index(the_repository);
>      @@ builtin/sparse-checkout.c: static int sparse_checkout_set(int argc, const char *
>        			     builtin_sparse_checkout_set_usage,
>        			     PARSE_OPT_KEEP_UNKNOWN);
>        
>      -+	/* Determine if we need to record the mode; ensure sparse checkout on */
>      -+	record_mode = (set_opts.cone_mode != -1) || !core_apply_sparse_checkout;
>      -+	core_apply_sparse_checkout = 1;
>      -+
>      -+	/* If not specified, use previous definition of cone mode */
>      -+	if (set_opts.cone_mode == -1 && core_apply_sparse_checkout)
>      -+		set_opts.cone_mode = core_sparse_checkout_cone;
>      -+
>      -+	/* Set cone/non-cone mode appropriately */
>      -+	if (set_opts.cone_mode == 1) {
>      -+		mode = MODE_CONE_PATTERNS;
>      -+		core_sparse_checkout_cone = 1;
>      -+	} else {
>      -+		mode = MODE_ALL_PATTERNS;
>      -+	}
>      -+	if (record_mode && set_config(mode))
>      ++	if (update_modes(&set_opts.cone_mode, &set_opts.sparse_index))
>       +		return 1;
>       +
>      -+	/* Set sparse-index/non-sparse-index mode if specified */
>      -+	if (set_opts.sparse_index >= 0) {
>      -+		if (set_sparse_index_config(the_repository, set_opts.sparse_index) < 0)
>      -+			die(_("failed to modify sparse-index config"));
>      -+
>      -+		/* force an index rewrite */
>      -+		repo_read_index(the_repository);
>      -+		the_repository->index->updated_workdir = 1;
>      -+	}
>      -+
>       +	/*
>       +	 * Cone mode automatically specifies the toplevel directory.  For
>       +	 * non-cone mode, if nothing is specified, manually select just the
>      @@ builtin/sparse-checkout.c: static int sparse_checkout_set(int argc, const char *
>       +	 */
>       +	if (!core_sparse_checkout_cone && argc == 0) {
>       +		argv = default_patterns;
>      -+		argc = 2;
>      ++		argc = default_patterns_nr;
>       +	}
>       +
>        	return modify_pattern_list(argc, argv, set_opts.use_stdin, REPLACE);
>   -:  ----------- >  7:  acb10889a1f sparse-checkout: enable reapply to take --[no-]{cone,sparse-index}
>   4:  95d3df78b2f !  8:  17b033caf4b git-sparse-checkout.txt: update to document that set handles init
>      @@ Metadata
>       Author: Elijah Newren <newren@gmail.com>
>       
>        ## Commit message ##
>      -    git-sparse-checkout.txt: update to document that set handles init
>      +    git-sparse-checkout.txt: update to document init/set/reapply changes
>       
>           As noted in the previous commit, using separate `init` and `set` steps
>      -    with sparse-checkout result in a number of issues.  The previous commit
>      -    made `set` able to handle the work of both commands.  Update the
>      -    documentation to reflect this, and mark `init` as deprecated.
>      +    with sparse-checkout result in a number of issues.  The previous commits
>      +    made `set` able to handle the work of both commands, and enabled reapply
>      +    to tweak the {cone,sparse-index} settings.  Update the documentation to
>      +    reflect this, and mark `init` as deprecated.
>       
>           Signed-off-by: Elijah Newren <newren@gmail.com>
>       
>      @@ Documentation/git-sparse-checkout.txt: COMMANDS
>       +files that are siblings of ancestor 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.
>      ++quoted strings.  This may become the default in the future; --no-cone
>      ++can be passed to request non-cone mode.
>        +
>       -Use the `--[no-]sparse-index` option to toggle the use of the sparse
>       -index format. This reduces the size of the index to be more closely
>      @@ Documentation/git-sparse-checkout.txt: to rewrite your index to not be sparse. O
>        	Update the sparse-checkout file to include additional patterns.
>        	By default, these patterns are read from the command-line arguments,
>       @@ Documentation/git-sparse-checkout.txt: C-style quoted strings.
>      + 	cases, it can make sense to run `git sparse-checkout reapply` later
>      + 	after cleaning up affected paths (e.g. resolving conflicts, undoing
>      + 	or committing changes, etc.).
>      +++
>      ++The `reapply` command can also take `--[no-]cone` and `--[no-]sparse-index`
>      ++flags, with the same meaning as the flags from the `set` command, in order
>      ++to change which sparsity mode you are using without needing to also respecify
>      ++all sparsity paths.
>        
>        'disable'::
>        	Disable the `core.sparseCheckout` config setting, and restore the
>   5:  9d455f1fb51 =  9:  922a65b4019 Documentation: clarify/correct a few sparsity related statements
>   6:  2ad404874ee = 10:  d47b2c88242 clone: avoid using deprecated `sparse-checkout init`
> 

These updates all look good to me, and the series as a whole represents a
huge improvement in the robustness & usability of `git sparse-checkout`. 

Reviewed-by: Victoria Dye <vdye@github.com>

Thanks!

^ permalink raw reply	[flat|nested] 63+ messages in thread

* [PATCH v3 00/10] sparse-checkout: make set subsume init
  2021-12-07 20:20 ` [PATCH v2 00/10] " Elijah Newren via GitGitGadget
                     ` (11 preceding siblings ...)
  2021-12-08 14:16   ` Victoria Dye
@ 2021-12-10  3:56   ` Elijah Newren via GitGitGadget
  2021-12-10  3:56     ` [PATCH v3 01/10] sparse-checkout: pass use_stdin as a parameter instead of as a global Elijah Newren via GitGitGadget
                       ` (10 more replies)
  12 siblings, 11 replies; 63+ messages in thread
From: Elijah Newren via GitGitGadget @ 2021-12-10  3:56 UTC (permalink / raw)
  To: git
  Cc: Derrick Stolee, Lessley Dennington, Victoria Dye, Elijah Newren,
	Elijah Newren

As described at [1], the split of init and set subcommands in
sparse-checkout causes multiple issues:

 * Poor performance (deleting all tracked files, then later restoring many
   and maybe even most of them)
 * Poor UI (multiple progress bars in wrappers that hide both commands under
   1 user-facing command)
 * Loss of ignored files under directories the user wanted to keep

This series fixes this bug by providing a single command to switch to a
sparse-checkout: set. It does so by making set able to do the combined work
of init and set. It keeps init as-is to give folks time to adapt, but marks
it as deprecated. It also makes reapply able to toggle cone/non-cone mode
and sparse-index/non-sparse-index mode.

Changes since v2:

 * Small documentation wording improvement
 * Added Reviewed-by's from Stolee and Victoria

Changes since v1:

 * Inserted new patches 3 & 4 as additional preparatory cleanups
 * Took the new mode-toggling work code in sparse_checkout_set from the
   previous series and moved it into a new function, as a preparatory patch,
   and made it usable by init/set/reapply
 * Also updated reapply to allow mode-toggling
 * Updated the documentation as per above
 * Various other small items from review comments

A quick overview:

 * Patches 1-4: small preparatory refactorings
 * Patch 5: patch adding new function for toggling {cone,sparse-checkout}
   modes
 * Patch 6: the crux of the series; make set able to handle both init and
   set options
 * Patch 7: make reapply also able to do mode toggling
 * Patches 8-9: documentation modifications (Patch 4 is worth reviewing; it
   marks init as deprecated -- are others okay with that?)
 * Patch 10: trivial modification of git clone --sparse to use git
   sparse-checkout set rather than git sparse-checkout init.

[1]
https://lore.kernel.org/git/CABPp-BE8TJ8QGAQWsSGT7S+9Xp-XmApcC9PSw3K=RQOP0rt+PQ@mail.gmail.com/

Elijah Newren (10):
  sparse-checkout: pass use_stdin as a parameter instead of as a global
  sparse-checkout: break apart functions for sparse_checkout_(set|add)
  sparse-checkout: add sanity-checks on initial sparsity state
  sparse-checkout: disallow --no-stdin as an argument to set
  sparse-checkout: split out code for tweaking settings config
  sparse-checkout: enable `set` to initialize sparse-checkout mode
  sparse-checkout: enable reapply to take --[no-]{cone,sparse-index}
  git-sparse-checkout.txt: update to document init/set/reapply changes
  Documentation: clarify/correct a few sparsity related statements
  clone: avoid using deprecated `sparse-checkout init`

 Documentation/git-clone.txt           |   8 +-
 Documentation/git-sparse-checkout.txt | 100 +++++++------
 builtin/clone.c                       |   2 +-
 builtin/sparse-checkout.c             | 196 ++++++++++++++++++++------
 t/t1091-sparse-checkout-builtin.sh    |  10 +-
 5 files changed, 219 insertions(+), 97 deletions(-)


base-commit: abe6bb3905392d5eb6b01fa6e54d7e784e0522aa
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1151%2Fnewren%2Fsparse-checkout-no-init-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1151/newren/sparse-checkout-no-init-v3
Pull-Request: https://github.com/git/git/pull/1151

Range-diff vs v2:

  1:  e41cfe3c1bb !  1:  814aed2d125 sparse-checkout: pass use_stdin as a parameter instead of as a global
     @@ Commit message
          incorrect.  Pass the value as function parameter instead to allow us to
          make subsequent changes.
      
     +    Reviewed-by: Derrick Stolee <dstolee@microsoft.com>
     +    Reviewed-by: Victoria Dye <vdye@github.com>
          Signed-off-by: Elijah Newren <newren@gmail.com>
      
       ## builtin/sparse-checkout.c ##
  2:  6f7de8f5412 !  2:  04cd57592e9 sparse-checkout: break apart functions for sparse_checkout_(set|add)
     @@ Commit message
          function.  This does not introduce any behavioral changes; that will
          come in a subsequent patch.
      
     +    Reviewed-by: Derrick Stolee <dstolee@microsoft.com>
     +    Reviewed-by: Victoria Dye <vdye@github.com>
          Signed-off-by: Elijah Newren <newren@gmail.com>
      
       ## builtin/sparse-checkout.c ##
  3:  bade5e52390 !  3:  f3af5edb25d sparse-checkout: add sanity-checks on initial sparsity state
     @@ Commit message
          only make sense when already in a sparse state.  Add a quick check
          that will error out early if this is not the case.
      
     +    Reviewed-by: Derrick Stolee <dstolee@microsoft.com>
     +    Reviewed-by: Victoria Dye <vdye@github.com>
          Signed-off-by: Elijah Newren <newren@gmail.com>
      
       ## builtin/sparse-checkout.c ##
  4:  0180bfc4a93 !  4:  82a13cc0197 sparse-checkout: disallow --no-stdin as an argument to set
     @@ Commit message
          but didn't intend for --no-stdin to be permitted as well.
      
          Reported-by: Victoria Dye <vdye@github.com>
     +    Reviewed-by: Derrick Stolee <dstolee@microsoft.com>
     +    Reviewed-by: Victoria Dye <vdye@github.com>
          Signed-off-by: Elijah Newren <newren@gmail.com>
      
       ## builtin/sparse-checkout.c ##
  5:  3f5255eeef9 !  5:  7a812e0222c sparse-checkout: split out code for tweaking settings config
     @@ Commit message
          and make it slightly more general so it can handle being called from
          the new callers.
      
     +    Reviewed-by: Derrick Stolee <dstolee@microsoft.com>
     +    Reviewed-by: Victoria Dye <vdye@github.com>
          Signed-off-by: Elijah Newren <newren@gmail.com>
      
       ## builtin/sparse-checkout.c ##
  6:  3c640f5bcef !  6:  7167a4b3118 sparse-checkout: enable `set` to initialize sparse-checkout mode
     @@ Commit message
          parameters that `init` takes and performing any necessary initialization
          if not already in a sparse checkout.
      
     +    Reviewed-by: Derrick Stolee <dstolee@microsoft.com>
     +    Reviewed-by: Victoria Dye <vdye@github.com>
          Signed-off-by: Elijah Newren <newren@gmail.com>
      
       ## builtin/sparse-checkout.c ##
  7:  acb10889a1f !  7:  3687637915f sparse-checkout: enable reapply to take --[no-]{cone,sparse-index}
     @@ Commit message
          sparse-index without changing their sparsity paths.  Allow them to do so
          using the reapply command.
      
     +    Reviewed-by: Derrick Stolee <dstolee@microsoft.com>
     +    Reviewed-by: Victoria Dye <vdye@github.com>
          Signed-off-by: Elijah Newren <newren@gmail.com>
      
       ## builtin/sparse-checkout.c ##
  8:  17b033caf4b !  8:  7483d1363e5 git-sparse-checkout.txt: update to document init/set/reapply changes
     @@ Commit message
          to tweak the {cone,sparse-index} settings.  Update the documentation to
          reflect this, and mark `init` as deprecated.
      
     +    Reviewed-by: Derrick Stolee <dstolee@microsoft.com>
     +    Reviewed-by: Victoria Dye <vdye@github.com>
          Signed-off-by: Elijah Newren <newren@gmail.com>
      
       ## Documentation/git-sparse-checkout.txt ##
     @@ Documentation/git-sparse-checkout.txt: C-style quoted strings.
      +	Deprecated command that behaves like `set` with no specified paths.
      +	May be removed in the future.
      ++
     -+Historically, `set` did not used to handle all the necessary config
     -+settings, which meant that both `init` and `set` had to be called.
     -+Invoking both meant the `init` step would first remove nearly all
     -+tracked files (and in cone mode, ignored files too), then the `set`
     -+step would add many of the tracked files (but not ignored files) back.
     -+In addition to the lost files, the performance and UI of this
     -+combination was poor.
     ++Historically, `set` did not handle all the necessary config settings,
     ++which meant that both `init` and `set` had to be called.  Invoking
     ++both meant the `init` step would first remove nearly all tracked files
     ++(and in cone mode, ignored files too), then the `set` step would add
     ++many of the tracked files (but not ignored files) back.  In addition
     ++to the lost files, the performance and UI of this combination was
     ++poor.
      ++
      +Also, historically, `init` would not actually initialize the
      +sparse-checkout file if it already existed.  This meant it was
  9:  922a65b4019 !  9:  11a45920602 Documentation: clarify/correct a few sparsity related statements
     @@ Metadata
       ## Commit message ##
          Documentation: clarify/correct a few sparsity related statements
      
     +    Reviewed-by: Derrick Stolee <dstolee@microsoft.com>
     +    Reviewed-by: Victoria Dye <vdye@github.com>
          Signed-off-by: Elijah Newren <newren@gmail.com>
      
       ## Documentation/git-clone.txt ##
 10:  d47b2c88242 ! 10:  395d9b194d3 clone: avoid using deprecated `sparse-checkout init`
     @@ Commit message
          The previous commits marked `sparse-checkout init` as deprecated; we
          can just use `set` instead here and pass it no paths.
      
     +    Reviewed-by: Derrick Stolee <dstolee@microsoft.com>
     +    Reviewed-by: Victoria Dye <vdye@github.com>
          Signed-off-by: Elijah Newren <newren@gmail.com>
      
       ## builtin/clone.c ##

-- 
gitgitgadget

^ permalink raw reply	[flat|nested] 63+ messages in thread

* [PATCH v3 01/10] sparse-checkout: pass use_stdin as a parameter instead of as a global
  2021-12-10  3:56   ` [PATCH v3 " Elijah Newren via GitGitGadget
@ 2021-12-10  3:56     ` Elijah Newren via GitGitGadget
  2021-12-10  3:56     ` [PATCH v3 02/10] sparse-checkout: break apart functions for sparse_checkout_(set|add) Elijah Newren via GitGitGadget
                       ` (9 subsequent siblings)
  10 siblings, 0 replies; 63+ messages in thread
From: Elijah Newren via GitGitGadget @ 2021-12-10  3:56 UTC (permalink / raw)
  To: git
  Cc: Derrick Stolee, Lessley Dennington, Victoria Dye, Elijah Newren,
	Elijah Newren, Elijah Newren

From: Elijah Newren <newren@gmail.com>

add_patterns_from_input() has relied on a global variable,
set_opts.use_stdin, which has been used by both the `set` and `add`
subcommands of sparse-checkout.  Once we introduce an
add_opts.use_stdin, the hardcoding of set_opts.use_stdin will be
incorrect.  Pass the value as function parameter instead to allow us to
make subsequent changes.

Reviewed-by: Derrick Stolee <dstolee@microsoft.com>
Reviewed-by: Victoria Dye <vdye@github.com>
Signed-off-by: Elijah Newren <newren@gmail.com>
---
 builtin/sparse-checkout.c | 28 ++++++++++++++++------------
 1 file changed, 16 insertions(+), 12 deletions(-)

diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c
index d0f5c4702be..8612328e5dd 100644
--- a/builtin/sparse-checkout.c
+++ b/builtin/sparse-checkout.c
@@ -525,7 +525,8 @@ static struct sparse_checkout_set_opts {
 } set_opts;
 
 static void add_patterns_from_input(struct pattern_list *pl,
-				    int argc, const char **argv)
+				    int argc, const char **argv,
+				    int use_stdin)
 {
 	int i;
 	if (core_sparse_checkout_cone) {
@@ -535,7 +536,7 @@ static void add_patterns_from_input(struct pattern_list *pl,
 		hashmap_init(&pl->parent_hashmap, pl_hashmap_cmp, NULL, 0);
 		pl->use_cone_patterns = 1;
 
-		if (set_opts.use_stdin) {
+		if (use_stdin) {
 			struct strbuf unquoted = STRBUF_INIT;
 			while (!strbuf_getline(&line, stdin)) {
 				if (line.buf[0] == '"') {
@@ -559,7 +560,7 @@ static void add_patterns_from_input(struct pattern_list *pl,
 			}
 		}
 	} else {
-		if (set_opts.use_stdin) {
+		if (use_stdin) {
 			struct strbuf line = STRBUF_INIT;
 
 			while (!strbuf_getline(&line, stdin)) {
@@ -580,7 +581,8 @@ enum modify_type {
 };
 
 static void add_patterns_cone_mode(int argc, const char **argv,
-				   struct pattern_list *pl)
+				   struct pattern_list *pl,
+				   int use_stdin)
 {
 	struct strbuf buffer = STRBUF_INIT;
 	struct pattern_entry *pe;
@@ -588,7 +590,7 @@ static void add_patterns_cone_mode(int argc, const char **argv,
 	struct pattern_list existing;
 	char *sparse_filename = get_sparse_checkout_filename();
 
-	add_patterns_from_input(pl, argc, argv);
+	add_patterns_from_input(pl, argc, argv, use_stdin);
 
 	memset(&existing, 0, sizeof(existing));
 	existing.use_cone_patterns = core_sparse_checkout_cone;
@@ -614,17 +616,19 @@ static void add_patterns_cone_mode(int argc, const char **argv,
 }
 
 static void add_patterns_literal(int argc, const char **argv,
-				 struct pattern_list *pl)
+				 struct pattern_list *pl,
+				 int use_stdin)
 {
 	char *sparse_filename = get_sparse_checkout_filename();
 	if (add_patterns_from_file_to_list(sparse_filename, "", 0,
 					   pl, NULL, 0))
 		die(_("unable to load existing sparse-checkout patterns"));
 	free(sparse_filename);
-	add_patterns_from_input(pl, argc, argv);
+	add_patterns_from_input(pl, argc, argv, use_stdin);
 }
 
-static int modify_pattern_list(int argc, const char **argv, enum modify_type m)
+static int modify_pattern_list(int argc, const char **argv, int use_stdin,
+			       enum modify_type m)
 {
 	int result;
 	int changed_config = 0;
@@ -633,13 +637,13 @@ static int modify_pattern_list(int argc, const char **argv, enum modify_type m)
 	switch (m) {
 	case ADD:
 		if (core_sparse_checkout_cone)
-			add_patterns_cone_mode(argc, argv, pl);
+			add_patterns_cone_mode(argc, argv, pl, use_stdin);
 		else
-			add_patterns_literal(argc, argv, pl);
+			add_patterns_literal(argc, argv, pl, use_stdin);
 		break;
 
 	case REPLACE:
-		add_patterns_from_input(pl, argc, argv);
+		add_patterns_from_input(pl, argc, argv, use_stdin);
 		break;
 	}
 
@@ -675,7 +679,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, m);
+	return modify_pattern_list(argc, argv, set_opts.use_stdin, m);
 }
 
 static char const * const builtin_sparse_checkout_reapply_usage[] = {
-- 
gitgitgadget


^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH v3 02/10] sparse-checkout: break apart functions for sparse_checkout_(set|add)
  2021-12-10  3:56   ` [PATCH v3 " Elijah Newren via GitGitGadget
  2021-12-10  3:56     ` [PATCH v3 01/10] sparse-checkout: pass use_stdin as a parameter instead of as a global Elijah Newren via GitGitGadget
@ 2021-12-10  3:56     ` Elijah Newren via GitGitGadget
  2021-12-10  3:56     ` [PATCH v3 03/10] sparse-checkout: add sanity-checks on initial sparsity state Elijah Newren via GitGitGadget
                       ` (8 subsequent siblings)
  10 siblings, 0 replies; 63+ messages in thread
From: Elijah Newren via GitGitGadget @ 2021-12-10  3:56 UTC (permalink / raw)
  To: git
  Cc: Derrick Stolee, Lessley Dennington, Victoria Dye, Elijah Newren,
	Elijah Newren, Elijah Newren

From: Elijah Newren <newren@gmail.com>

sparse_checkout_set() was reused by sparse_checkout_add() with the only
difference being a single parameter being passed to that function.
However, we would like sparse_checkout_set() to do the same work that
sparse_checkout_init() does if sparse checkouts are not already enabled.
To facilitate this transition, give each mode their own copy of the
function.  This does not introduce any behavioral changes; that will
come in a subsequent patch.

Reviewed-by: Derrick Stolee <dstolee@microsoft.com>
Reviewed-by: Victoria Dye <vdye@github.com>
Signed-off-by: Elijah Newren <newren@gmail.com>
---
 builtin/sparse-checkout.c | 54 +++++++++++++++++++++++++++++----------
 1 file changed, 40 insertions(+), 14 deletions(-)

diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c
index 8612328e5dd..e252b82136e 100644
--- a/builtin/sparse-checkout.c
+++ b/builtin/sparse-checkout.c
@@ -515,15 +515,6 @@ static void strbuf_to_cone_pattern(struct strbuf *line, struct pattern_list *pl)
 	insert_recursive_pattern(pl, line);
 }
 
-static char const * const builtin_sparse_checkout_set_usage[] = {
-	N_("git sparse-checkout (set|add) (--stdin | <patterns>)"),
-	NULL
-};
-
-static struct sparse_checkout_set_opts {
-	int use_stdin;
-} set_opts;
-
 static void add_patterns_from_input(struct pattern_list *pl,
 				    int argc, const char **argv,
 				    int use_stdin)
@@ -663,8 +654,43 @@ static int modify_pattern_list(int argc, const char **argv, int use_stdin,
 	return result;
 }
 
-static int sparse_checkout_set(int argc, const char **argv, const char *prefix,
-			       enum modify_type m)
+static char const * const builtin_sparse_checkout_add_usage[] = {
+	N_("git sparse-checkout add (--stdin | <patterns>)"),
+	NULL
+};
+
+static struct sparse_checkout_add_opts {
+	int use_stdin;
+} add_opts;
+
+static int sparse_checkout_add(int argc, const char **argv, const char *prefix)
+{
+	static struct option builtin_sparse_checkout_add_options[] = {
+		OPT_BOOL(0, "stdin", &add_opts.use_stdin,
+			 N_("read patterns from standard in")),
+		OPT_END(),
+	};
+
+	repo_read_index(the_repository);
+
+	argc = parse_options(argc, argv, prefix,
+			     builtin_sparse_checkout_add_options,
+			     builtin_sparse_checkout_add_usage,
+			     PARSE_OPT_KEEP_UNKNOWN);
+
+	return modify_pattern_list(argc, argv, add_opts.use_stdin, ADD);
+}
+
+static char const * const builtin_sparse_checkout_set_usage[] = {
+	N_("git sparse-checkout set (--stdin | <patterns>)"),
+	NULL
+};
+
+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 struct option builtin_sparse_checkout_set_options[] = {
 		OPT_BOOL(0, "stdin", &set_opts.use_stdin,
@@ -679,7 +705,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, set_opts.use_stdin, m);
+	return modify_pattern_list(argc, argv, set_opts.use_stdin, REPLACE);
 }
 
 static char const * const builtin_sparse_checkout_reapply_usage[] = {
@@ -762,9 +788,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, REPLACE);
+			return sparse_checkout_set(argc, argv, prefix);
 		if (!strcmp(argv[0], "add"))
-			return sparse_checkout_set(argc, argv, prefix, ADD);
+			return sparse_checkout_add(argc, argv, prefix);
 		if (!strcmp(argv[0], "reapply"))
 			return sparse_checkout_reapply(argc, argv);
 		if (!strcmp(argv[0], "disable"))
-- 
gitgitgadget


^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH v3 03/10] sparse-checkout: add sanity-checks on initial sparsity state
  2021-12-10  3:56   ` [PATCH v3 " Elijah Newren via GitGitGadget
  2021-12-10  3:56     ` [PATCH v3 01/10] sparse-checkout: pass use_stdin as a parameter instead of as a global Elijah Newren via GitGitGadget
  2021-12-10  3:56     ` [PATCH v3 02/10] sparse-checkout: break apart functions for sparse_checkout_(set|add) Elijah Newren via GitGitGadget
@ 2021-12-10  3:56     ` Elijah Newren via GitGitGadget
  2021-12-13 18:11       ` Junio C Hamano
  2021-12-10  3:56     ` [PATCH v3 04/10] sparse-checkout: disallow --no-stdin as an argument to set Elijah Newren via GitGitGadget
                       ` (7 subsequent siblings)
  10 siblings, 1 reply; 63+ messages in thread
From: Elijah Newren via GitGitGadget @ 2021-12-10  3:56 UTC (permalink / raw)
  To: git
  Cc: Derrick Stolee, Lessley Dennington, Victoria Dye, Elijah Newren,
	Elijah Newren, Elijah Newren

From: Elijah Newren <newren@gmail.com>

Most sparse-checkout subcommands (list, add, reapply, disable)
only make sense when already in a sparse state.  Add a quick check
that will error out early if this is not the case.

Reviewed-by: Derrick Stolee <dstolee@microsoft.com>
Reviewed-by: Victoria Dye <vdye@github.com>
Signed-off-by: Elijah Newren <newren@gmail.com>
---
 builtin/sparse-checkout.c          | 12 ++++++++++++
 t/t1091-sparse-checkout-builtin.sh | 10 +++++++++-
 2 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c
index e252b82136e..e9f644ac362 100644
--- a/builtin/sparse-checkout.c
+++ b/builtin/sparse-checkout.c
@@ -56,6 +56,9 @@ static int sparse_checkout_list(int argc, const char **argv)
 	char *sparse_filename;
 	int res;
 
+	if (!core_apply_sparse_checkout)
+		die(_("this worktree is not sparse"));
+
 	argc = parse_options(argc, argv, NULL,
 			     builtin_sparse_checkout_list_options,
 			     builtin_sparse_checkout_list_usage, 0);
@@ -671,6 +674,9 @@ static int sparse_checkout_add(int argc, const char **argv, const char *prefix)
 		OPT_END(),
 	};
 
+	if (!core_apply_sparse_checkout)
+		die(_("no sparse-checkout to add to"));
+
 	repo_read_index(the_repository);
 
 	argc = parse_options(argc, argv, prefix,
@@ -719,6 +725,9 @@ static int sparse_checkout_reapply(int argc, const char **argv)
 		OPT_END(),
 	};
 
+	if (!core_apply_sparse_checkout)
+		die(_("must be in a sparse-checkout to reapply sparsity patterns"));
+
 	argc = parse_options(argc, argv, NULL,
 			     builtin_sparse_checkout_reapply_options,
 			     builtin_sparse_checkout_reapply_usage, 0);
@@ -740,6 +749,9 @@ static int sparse_checkout_disable(int argc, const char **argv)
 	struct pattern_list pl;
 	struct strbuf match_all = STRBUF_INIT;
 
+	if (!core_apply_sparse_checkout)
+		die(_("no active sparse-checkout to disable"));
+
 	argc = parse_options(argc, argv, NULL,
 			     builtin_sparse_checkout_disable_options,
 			     builtin_sparse_checkout_disable_usage, 0);
diff --git a/t/t1091-sparse-checkout-builtin.sh b/t/t1091-sparse-checkout-builtin.sh
index 272ba1b566b..90a281bcf64 100755
--- a/t/t1091-sparse-checkout-builtin.sh
+++ b/t/t1091-sparse-checkout-builtin.sh
@@ -41,7 +41,15 @@ test_expect_success 'setup' '
 	)
 '
 
-test_expect_success 'git sparse-checkout list (empty)' '
+test_expect_success 'git sparse-checkout list (not sparse)' '
+	test_must_fail git -C repo sparse-checkout list >list 2>err &&
+	test_must_be_empty list &&
+	test_i18ngrep "this worktree is not sparse" err
+'
+
+test_expect_success 'git sparse-checkout list (not sparse)' '
+	git -C repo sparse-checkout set &&
+	rm repo/.git/info/sparse-checkout &&
 	git -C repo sparse-checkout list >list 2>err &&
 	test_must_be_empty list &&
 	test_i18ngrep "this worktree is not sparse (sparse-checkout file may not exist)" err
-- 
gitgitgadget


^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH v3 04/10] sparse-checkout: disallow --no-stdin as an argument to set
  2021-12-10  3:56   ` [PATCH v3 " Elijah Newren via GitGitGadget
                       ` (2 preceding siblings ...)
  2021-12-10  3:56     ` [PATCH v3 03/10] sparse-checkout: add sanity-checks on initial sparsity state Elijah Newren via GitGitGadget
@ 2021-12-10  3:56     ` Elijah Newren via GitGitGadget
  2021-12-10  3:56     ` [PATCH v3 05/10] sparse-checkout: split out code for tweaking settings config Elijah Newren via GitGitGadget
                       ` (6 subsequent siblings)
  10 siblings, 0 replies; 63+ messages in thread
From: Elijah Newren via GitGitGadget @ 2021-12-10  3:56 UTC (permalink / raw)
  To: git
  Cc: Derrick Stolee, Lessley Dennington, Victoria Dye, Elijah Newren,
	Elijah Newren, Elijah Newren

From: Elijah Newren <newren@gmail.com>

We intentionally added --stdin as an option to `sparse-checkout set`,
but didn't intend for --no-stdin to be permitted as well.

Reported-by: Victoria Dye <vdye@github.com>
Reviewed-by: Derrick Stolee <dstolee@microsoft.com>
Reviewed-by: Victoria Dye <vdye@github.com>
Signed-off-by: Elijah Newren <newren@gmail.com>
---
 builtin/sparse-checkout.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c
index e9f644ac362..0ee28f48134 100644
--- a/builtin/sparse-checkout.c
+++ b/builtin/sparse-checkout.c
@@ -699,8 +699,9 @@ static struct sparse_checkout_set_opts {
 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_BOOL_F(0, "stdin", &set_opts.use_stdin,
+			   N_("read patterns from standard in"),
+			   PARSE_OPT_NONEG),
 		OPT_END(),
 	};
 
-- 
gitgitgadget


^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH v3 05/10] sparse-checkout: split out code for tweaking settings config
  2021-12-10  3:56   ` [PATCH v3 " Elijah Newren via GitGitGadget
                       ` (3 preceding siblings ...)
  2021-12-10  3:56     ` [PATCH v3 04/10] sparse-checkout: disallow --no-stdin as an argument to set Elijah Newren via GitGitGadget
@ 2021-12-10  3:56     ` Elijah Newren via GitGitGadget
  2021-12-10  3:56     ` [PATCH v3 06/10] sparse-checkout: enable `set` to initialize sparse-checkout mode Elijah Newren via GitGitGadget
                       ` (5 subsequent siblings)
  10 siblings, 0 replies; 63+ messages in thread
From: Elijah Newren via GitGitGadget @ 2021-12-10  3:56 UTC (permalink / raw)
  To: git
  Cc: Derrick Stolee, Lessley Dennington, Victoria Dye, Elijah Newren,
	Elijah Newren, Elijah Newren

From: Elijah Newren <newren@gmail.com>

`init` has some code for handling updates to either cone mode or
the sparse-index setting.  We would like to be able to reuse this
elsewhere, namely in `set` and `reapply`.  Split this function out,
and make it slightly more general so it can handle being called from
the new callers.

Reviewed-by: Derrick Stolee <dstolee@microsoft.com>
Reviewed-by: Victoria Dye <vdye@github.com>
Signed-off-by: Elijah Newren <newren@gmail.com>
---
 builtin/sparse-checkout.c | 56 ++++++++++++++++++++++++++-------------
 1 file changed, 37 insertions(+), 19 deletions(-)

diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c
index 0ee28f48134..f176435b6be 100644
--- a/builtin/sparse-checkout.c
+++ b/builtin/sparse-checkout.c
@@ -383,6 +383,41 @@ static int set_config(enum sparse_checkout_mode mode)
 	return 0;
 }
 
+static int update_modes(int *cone_mode, int *sparse_index)
+{
+	int mode, record_mode;
+
+	/* Determine if we need to record the mode; ensure sparse checkout on */
+	record_mode = (*cone_mode != -1) || !core_apply_sparse_checkout;
+
+	/* If not specified, use previous definition of cone mode */
+	if (*cone_mode == -1 && core_apply_sparse_checkout)
+		*cone_mode = core_sparse_checkout_cone;
+
+	/* Set cone/non-cone mode appropriately */
+	core_apply_sparse_checkout = 1;
+	if (*cone_mode == 1) {
+		mode = MODE_CONE_PATTERNS;
+		core_sparse_checkout_cone = 1;
+	} else {
+		mode = MODE_ALL_PATTERNS;
+	}
+	if (record_mode && set_config(mode))
+		return 1;
+
+	/* Set sparse-index/non-sparse-index mode if specified */
+	if (*sparse_index >= 0) {
+		if (set_sparse_index_config(the_repository, *sparse_index) < 0)
+			die(_("failed to modify sparse-index config"));
+
+		/* force an index rewrite */
+		repo_read_index(the_repository);
+		the_repository->index->updated_workdir = 1;
+	}
+
+	return 0;
+}
+
 static char const * const builtin_sparse_checkout_init_usage[] = {
 	N_("git sparse-checkout init [--cone] [--[no-]sparse-index]"),
 	NULL
@@ -399,7 +434,6 @@ static int sparse_checkout_init(int argc, const char **argv)
 	char *sparse_filename;
 	int res;
 	struct object_id oid;
-	int mode;
 	struct strbuf pattern = STRBUF_INIT;
 
 	static struct option builtin_sparse_checkout_init_options[] = {
@@ -412,19 +446,14 @@ static int sparse_checkout_init(int argc, const char **argv)
 
 	repo_read_index(the_repository);
 
+	init_opts.cone_mode = -1;
 	init_opts.sparse_index = -1;
 
 	argc = parse_options(argc, argv, NULL,
 			     builtin_sparse_checkout_init_options,
 			     builtin_sparse_checkout_init_usage, 0);
 
-	if (init_opts.cone_mode) {
-		mode = MODE_CONE_PATTERNS;
-		core_sparse_checkout_cone = 1;
-	} else
-		mode = MODE_ALL_PATTERNS;
-
-	if (set_config(mode))
+	if (update_modes(&init_opts.cone_mode, &init_opts.sparse_index))
 		return 1;
 
 	memset(&pl, 0, sizeof(pl));
@@ -432,17 +461,6 @@ static int sparse_checkout_init(int argc, const char **argv)
 	sparse_filename = get_sparse_checkout_filename();
 	res = add_patterns_from_file_to_list(sparse_filename, "", 0, &pl, NULL, 0);
 
-	if (init_opts.sparse_index >= 0) {
-		if (set_sparse_index_config(the_repository, init_opts.sparse_index) < 0)
-			die(_("failed to modify sparse-index config"));
-
-		/* force an index rewrite */
-		repo_read_index(the_repository);
-		the_repository->index->updated_workdir = 1;
-	}
-
-	core_apply_sparse_checkout = 1;
-
 	/* If we already have a sparse-checkout file, use it. */
 	if (res >= 0) {
 		free(sparse_filename);
-- 
gitgitgadget


^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH v3 06/10] sparse-checkout: enable `set` to initialize sparse-checkout mode
  2021-12-10  3:56   ` [PATCH v3 " Elijah Newren via GitGitGadget
                       ` (4 preceding siblings ...)
  2021-12-10  3:56     ` [PATCH v3 05/10] sparse-checkout: split out code for tweaking settings config Elijah Newren via GitGitGadget
@ 2021-12-10  3:56     ` Elijah Newren via GitGitGadget
  2021-12-10  3:56     ` [PATCH v3 07/10] sparse-checkout: enable reapply to take --[no-]{cone,sparse-index} Elijah Newren via GitGitGadget
                       ` (4 subsequent siblings)
  10 siblings, 0 replies; 63+ messages in thread
From: Elijah Newren via GitGitGadget @ 2021-12-10  3:56 UTC (permalink / raw)
  To: git
  Cc: Derrick Stolee, Lessley Dennington, Victoria Dye, Elijah Newren,
	Elijah Newren, Elijah Newren

From: Elijah Newren <newren@gmail.com>

The previously suggested workflow:
  git sparse-checkout init ...
  git sparse-checkout set ...

Suffered from three problems:
  1) It would delete nearly all files in the first step, then
     restore them in the second.  That was poor performance and
     forced unnecessary rebuilds.
  2) The two-step process resulted in two progress bars, which
     was suboptimal from a UI point of view for wrappers that
     invoked both of these commands but only exposed a single
     command to their end users.
  3) With cone mode, the first step would delete nearly all
     ignored files everywhere, because everything was considered
     to be outside of the specified sparsity paths.  (The user was
     not allowed to specify any sparsity paths in the `init` step.)

Avoid these problems by teaching `set` to understand the extra
parameters that `init` takes and performing any necessary initialization
if not already in a sparse checkout.

Reviewed-by: Derrick Stolee <dstolee@microsoft.com>
Reviewed-by: Victoria Dye <vdye@github.com>
Signed-off-by: Elijah Newren <newren@gmail.com>
---
 builtin/sparse-checkout.c | 27 ++++++++++++++++++++++++++-
 1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c
index f176435b6be..1ecdc3ddd5a 100644
--- a/builtin/sparse-checkout.c
+++ b/builtin/sparse-checkout.c
@@ -706,17 +706,26 @@ static int sparse_checkout_add(int argc, const char **argv, const char *prefix)
 }
 
 static char const * const builtin_sparse_checkout_set_usage[] = {
-	N_("git sparse-checkout set (--stdin | <patterns>)"),
+	N_("git sparse-checkout set [--[no-]cone] [--[no-]sparse-index] (--stdin | <patterns>)"),
 	NULL
 };
 
 static struct sparse_checkout_set_opts {
+	int cone_mode;
+	int sparse_index;
 	int use_stdin;
 } set_opts;
 
 static int sparse_checkout_set(int argc, const char **argv, const char *prefix)
 {
+	int default_patterns_nr = 2;
+	const char *default_patterns[] = {"/*", "!/*/", NULL};
+
 	static struct option builtin_sparse_checkout_set_options[] = {
+		OPT_BOOL(0, "cone", &set_opts.cone_mode,
+			 N_("initialize the sparse-checkout in cone mode")),
+		OPT_BOOL(0, "sparse-index", &set_opts.sparse_index,
+			 N_("toggle the use of a sparse index")),
 		OPT_BOOL_F(0, "stdin", &set_opts.use_stdin,
 			   N_("read patterns from standard in"),
 			   PARSE_OPT_NONEG),
@@ -725,11 +734,27 @@ static int sparse_checkout_set(int argc, const char **argv, const char *prefix)
 
 	repo_read_index(the_repository);
 
+	set_opts.cone_mode = -1;
+	set_opts.sparse_index = -1;
+
 	argc = parse_options(argc, argv, prefix,
 			     builtin_sparse_checkout_set_options,
 			     builtin_sparse_checkout_set_usage,
 			     PARSE_OPT_KEEP_UNKNOWN);
 
+	if (update_modes(&set_opts.cone_mode, &set_opts.sparse_index))
+		return 1;
+
+	/*
+	 * Cone mode automatically specifies the toplevel directory.  For
+	 * non-cone mode, if nothing is specified, manually select just the
+	 * top-level directory (much as 'init' would do).
+	 */
+	if (!core_sparse_checkout_cone && argc == 0) {
+		argv = default_patterns;
+		argc = default_patterns_nr;
+	}
+
 	return modify_pattern_list(argc, argv, set_opts.use_stdin, REPLACE);
 }
 
-- 
gitgitgadget


^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH v3 07/10] sparse-checkout: enable reapply to take --[no-]{cone,sparse-index}
  2021-12-10  3:56   ` [PATCH v3 " Elijah Newren via GitGitGadget
                       ` (5 preceding siblings ...)
  2021-12-10  3:56     ` [PATCH v3 06/10] sparse-checkout: enable `set` to initialize sparse-checkout mode Elijah Newren via GitGitGadget
@ 2021-12-10  3:56     ` Elijah Newren via GitGitGadget
  2021-12-13 18:23       ` Junio C Hamano
  2021-12-10  3:56     ` [PATCH v3 08/10] git-sparse-checkout.txt: update to document init/set/reapply changes Elijah Newren via GitGitGadget
                       ` (3 subsequent siblings)
  10 siblings, 1 reply; 63+ messages in thread
From: Elijah Newren via GitGitGadget @ 2021-12-10  3:56 UTC (permalink / raw)
  To: git
  Cc: Derrick Stolee, Lessley Dennington, Victoria Dye, Elijah Newren,
	Elijah Newren, Elijah Newren

From: Elijah Newren <newren@gmail.com>

Folks may want to switch to or from cone mode, or to or from a
sparse-index without changing their sparsity paths.  Allow them to do so
using the reapply command.

Reviewed-by: Derrick Stolee <dstolee@microsoft.com>
Reviewed-by: Victoria Dye <vdye@github.com>
Signed-off-by: Elijah Newren <newren@gmail.com>
---
 builtin/sparse-checkout.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c
index 1ecdc3ddd5a..9d2a05677c4 100644
--- a/builtin/sparse-checkout.c
+++ b/builtin/sparse-checkout.c
@@ -759,13 +759,22 @@ static int sparse_checkout_set(int argc, const char **argv, const char *prefix)
 }
 
 static char const * const builtin_sparse_checkout_reapply_usage[] = {
-	N_("git sparse-checkout reapply"),
+	N_("git sparse-checkout reapply [--[no-]cone] [--[no-]sparse-index] "),
 	NULL
 };
 
+static struct sparse_checkout_reapply_opts {
+	int cone_mode;
+	int sparse_index;
+} reapply_opts;
+
 static int sparse_checkout_reapply(int argc, const char **argv)
 {
 	static struct option builtin_sparse_checkout_reapply_options[] = {
+		OPT_BOOL(0, "cone", &reapply_opts.cone_mode,
+			 N_("initialize the sparse-checkout in cone mode")),
+		OPT_BOOL(0, "sparse-index", &reapply_opts.sparse_index,
+			 N_("toggle the use of a sparse index")),
 		OPT_END(),
 	};
 
@@ -777,6 +786,13 @@ static int sparse_checkout_reapply(int argc, const char **argv)
 			     builtin_sparse_checkout_reapply_usage, 0);
 
 	repo_read_index(the_repository);
+
+	reapply_opts.cone_mode = -1;
+	reapply_opts.sparse_index = -1;
+
+	if (update_modes(&reapply_opts.cone_mode, &reapply_opts.sparse_index))
+		return 1;
+
 	return update_working_directory(NULL);
 }
 
-- 
gitgitgadget


^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH v3 08/10] git-sparse-checkout.txt: update to document init/set/reapply changes
  2021-12-10  3:56   ` [PATCH v3 " Elijah Newren via GitGitGadget
                       ` (6 preceding siblings ...)
  2021-12-10  3:56     ` [PATCH v3 07/10] sparse-checkout: enable reapply to take --[no-]{cone,sparse-index} Elijah Newren via GitGitGadget
@ 2021-12-10  3:56     ` Elijah Newren via GitGitGadget
  2021-12-10  3:56     ` [PATCH v3 09/10] Documentation: clarify/correct a few sparsity related statements Elijah Newren via GitGitGadget
                       ` (2 subsequent siblings)
  10 siblings, 0 replies; 63+ messages in thread
From: Elijah Newren via GitGitGadget @ 2021-12-10  3:56 UTC (permalink / raw)
  To: git
  Cc: Derrick Stolee, Lessley Dennington, Victoria Dye, Elijah Newren,
	Elijah Newren, Elijah Newren

From: Elijah Newren <newren@gmail.com>

As noted in the previous commit, using separate `init` and `set` steps
with sparse-checkout result in a number of issues.  The previous commits
made `set` able to handle the work of both commands, and enabled reapply
to tweak the {cone,sparse-index} settings.  Update the documentation to
reflect this, and mark `init` as deprecated.

Reviewed-by: Derrick Stolee <dstolee@microsoft.com>
Reviewed-by: Victoria Dye <vdye@github.com>
Signed-off-by: Elijah Newren <newren@gmail.com>
---
 Documentation/git-sparse-checkout.txt | 98 +++++++++++++++------------
 1 file changed, 55 insertions(+), 43 deletions(-)

diff --git a/Documentation/git-sparse-checkout.txt b/Documentation/git-sparse-checkout.txt
index 42056ee9ff9..72fcb6b2acc 100644
--- a/Documentation/git-sparse-checkout.txt
+++ b/Documentation/git-sparse-checkout.txt
@@ -30,28 +30,36 @@ COMMANDS
 'list'::
 	Describe the patterns in the sparse-checkout file.
 
-'init'::
-	Enable the `core.sparseCheckout` setting. If the
-	sparse-checkout file does not exist, then populate it with
-	patterns that match every file in the root directory and
-	no other directories, then will remove all directories tracked
-	by Git. Add patterns to the sparse-checkout file to
-	repopulate the working directory.
+'set'::
+	Enable the necessary config settings
+	(extensions.worktreeConfig, core.sparseCheckout,
+	core.sparseCheckoutCone) if they are not already enabled, and
+	write a set of patterns to the sparse-checkout file from the
+	list of arguments following the 'set' subcommand. Update the
+	working directory to match the new patterns.
 +
-To avoid interfering with other worktrees, it first enables the
-`extensions.worktreeConfig` setting and makes sure to set the
-`core.sparseCheckout` setting in the worktree-specific config file.
+When the `--stdin` option is provided, the patterns are read from
+standard in as a newline-delimited list instead of from the arguments.
 +
-When `--cone` is provided, the `core.sparseCheckoutCone` setting is
-also set, allowing for better performance with a limited set of
-patterns (see 'CONE PATTERN SET' below).
+When `--cone` is passed or `core.sparseCheckoutCone` is enabled, the
+input list is considered a list of directories instead of
+sparse-checkout patterns.  This allows for better performance with a
+limited set of patterns (see 'CONE PATTERN SET' below).  Note that the
+set command will write patterns to the sparse-checkout file to include
+all files contained in those directories (recursively) as well as
+files that are siblings of ancestor 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.  This may become the default in the future; --no-cone
+can be passed to request non-cone mode.
 +
-Use the `--[no-]sparse-index` option to toggle the use of the sparse
-index format. This reduces the size of the index to be more closely
-aligned with your sparse-checkout definition. This can have significant
-performance advantages for commands such as `git status` or `git add`.
-This feature is still experimental. Some commands might be slower with
-a sparse index until they are properly integrated with the feature.
+Use the `--[no-]sparse-index` option to use a sparse index (the
+default is to not use it).  A sparse index reduces the size of the
+index to be more closely aligned with your sparse-checkout
+definition. This can have significant performance advantages for
+commands such as `git status` or `git add`.  This feature is still
+experimental. Some commands might be slower with a sparse index until
+they are properly integrated with the feature.
 +
 **WARNING:** Using a sparse index requires modifying the index in a way
 that is not completely understood by external tools. If you have trouble
@@ -60,23 +68,6 @@ to rewrite your index to not be sparse. Older versions of Git will not
 understand the sparse directory entries index extension and may fail to
 interact with your repository until it is disabled.
 
-'set'::
-	Write a set of patterns to the sparse-checkout file, as given as
-	a list of arguments following the 'set' subcommand. Update the
-	working directory to match the new patterns. Enable the
-	core.sparseCheckout config setting if it is not already enabled.
-+
-When the `--stdin` option is provided, the patterns are read from
-standard in as a newline-delimited list instead of from the arguments.
-+
-When `core.sparseCheckoutCone` is enabled, the input list is considered a
-list of directories instead of sparse-checkout patterns. The command writes
-patterns to the sparse-checkout file to include all files contained in those
-directories (recursively) as well as files that are siblings of ancestor
-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,
@@ -93,12 +84,35 @@ C-style quoted strings.
 	cases, it can make sense to run `git sparse-checkout reapply` later
 	after cleaning up affected paths (e.g. resolving conflicts, undoing
 	or committing changes, etc.).
++
+The `reapply` command can also take `--[no-]cone` and `--[no-]sparse-index`
+flags, with the same meaning as the flags from the `set` command, in order
+to change which sparsity mode you are using without needing to also respecify
+all sparsity paths.
 
 'disable'::
 	Disable the `core.sparseCheckout` config setting, and restore the
-	working directory to include all files. Leaves the sparse-checkout
-	file intact so a later 'git sparse-checkout init' command may
-	return the working directory to the same state.
+	working directory to include all files.
+
+'init'::
+	Deprecated command that behaves like `set` with no specified paths.
+	May be removed in the future.
++
+Historically, `set` did not handle all the necessary config settings,
+which meant that both `init` and `set` had to be called.  Invoking
+both meant the `init` step would first remove nearly all tracked files
+(and in cone mode, ignored files too), then the `set` step would add
+many of the tracked files (but not ignored files) back.  In addition
+to the lost files, the performance and UI of this combination was
+poor.
++
+Also, historically, `init` would not actually initialize the
+sparse-checkout file if it already existed.  This meant it was
+possible to return to a sparse-checkout without remembering which
+paths to pass to a subsequent 'set' or 'add' command.  However,
+`--cone` and `--sparse-index` options would not be remembered across
+the disable command, so the easy restore of calling a plain `init`
+decreased in utility.
 
 SPARSE CHECKOUT
 ---------------
@@ -117,10 +131,8 @@ directory, it updates the skip-worktree bits in the index based
 on this file. The files matching the patterns in the file will
 appear in the working directory, and the rest will not.
 
-To enable the sparse-checkout feature, run `git sparse-checkout init` to
-initialize a simple sparse-checkout file and enable the `core.sparseCheckout`
-config setting. Then, run `git sparse-checkout set` to modify the patterns in
-the sparse-checkout file.
+To enable the sparse-checkout feature, run `git sparse-checkout set` to
+set the patterns you want to use.
 
 To repopulate the working directory with all files, use the
 `git sparse-checkout disable` command.
-- 
gitgitgadget


^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH v3 09/10] Documentation: clarify/correct a few sparsity related statements
  2021-12-10  3:56   ` [PATCH v3 " Elijah Newren via GitGitGadget
                       ` (7 preceding siblings ...)
  2021-12-10  3:56     ` [PATCH v3 08/10] git-sparse-checkout.txt: update to document init/set/reapply changes Elijah Newren via GitGitGadget
@ 2021-12-10  3:56     ` Elijah Newren via GitGitGadget
  2021-12-10  3:56     ` [PATCH v3 10/10] clone: avoid using deprecated `sparse-checkout init` Elijah Newren via GitGitGadget
  2021-12-14  4:09     ` [PATCH v4 00/10] sparse-checkout: make set subsume init Elijah Newren via GitGitGadget
  10 siblings, 0 replies; 63+ messages in thread
From: Elijah Newren via GitGitGadget @ 2021-12-10  3:56 UTC (permalink / raw)
  To: git
  Cc: Derrick Stolee, Lessley Dennington, Victoria Dye, Elijah Newren,
	Elijah Newren, Elijah Newren

From: Elijah Newren <newren@gmail.com>

Reviewed-by: Derrick Stolee <dstolee@microsoft.com>
Reviewed-by: Victoria Dye <vdye@github.com>
Signed-off-by: Elijah Newren <newren@gmail.com>
---
 Documentation/git-clone.txt           | 8 ++++----
 Documentation/git-sparse-checkout.txt | 2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt
index 3fe3810f1ce..b348a71fc68 100644
--- a/Documentation/git-clone.txt
+++ b/Documentation/git-clone.txt
@@ -167,10 +167,10 @@ objects from the source repository into a pack in the cloned repository.
 	configuration variables are created.
 
 --sparse::
-	Initialize the sparse-checkout file so the working
-	directory starts with only the files in the root
-	of the repository. The sparse-checkout file can be
-	modified to grow the working directory as needed.
+	Employ a sparse-checkout, with only files in the toplevel
+	directory initially being present.  The
+	linkgit:git-sparse-checkout[1] command can be used to grow the
+	working directory as needed.
 
 --filter=<filter-spec>::
 	Use the partial clone feature and request that the server sends
diff --git a/Documentation/git-sparse-checkout.txt b/Documentation/git-sparse-checkout.txt
index 72fcb6b2acc..9a4b43c105b 100644
--- a/Documentation/git-sparse-checkout.txt
+++ b/Documentation/git-sparse-checkout.txt
@@ -121,7 +121,7 @@ SPARSE CHECKOUT
 It uses the skip-worktree bit (see linkgit:git-update-index[1]) to tell
 Git whether a file in the working directory is worth looking at. If
 the skip-worktree bit is set, then the file is ignored in the working
-directory. Git will not populate the contents of those files, which
+directory. Git will avoid populating the contents of those files, which
 makes a sparse checkout helpful when working in a repository with many
 files, but only a few are important to the current user.
 
-- 
gitgitgadget


^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH v3 10/10] clone: avoid using deprecated `sparse-checkout init`
  2021-12-10  3:56   ` [PATCH v3 " Elijah Newren via GitGitGadget
                       ` (8 preceding siblings ...)
  2021-12-10  3:56     ` [PATCH v3 09/10] Documentation: clarify/correct a few sparsity related statements Elijah Newren via GitGitGadget
@ 2021-12-10  3:56     ` Elijah Newren via GitGitGadget
  2021-12-14  4:09     ` [PATCH v4 00/10] sparse-checkout: make set subsume init Elijah Newren via GitGitGadget
  10 siblings, 0 replies; 63+ messages in thread
From: Elijah Newren via GitGitGadget @ 2021-12-10  3:56 UTC (permalink / raw)
  To: git
  Cc: Derrick Stolee, Lessley Dennington, Victoria Dye, Elijah Newren,
	Elijah Newren, Elijah Newren

From: Elijah Newren <newren@gmail.com>

The previous commits marked `sparse-checkout init` as deprecated; we
can just use `set` instead here and pass it no paths.

Reviewed-by: Derrick Stolee <dstolee@microsoft.com>
Reviewed-by: Victoria Dye <vdye@github.com>
Signed-off-by: Elijah Newren <newren@gmail.com>
---
 builtin/clone.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/clone.c b/builtin/clone.c
index fb377b27657..5bed37f8b51 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -633,7 +633,7 @@ static int git_sparse_checkout_init(const char *repo)
 {
 	struct strvec argv = STRVEC_INIT;
 	int result = 0;
-	strvec_pushl(&argv, "-C", repo, "sparse-checkout", "init", NULL);
+	strvec_pushl(&argv, "-C", repo, "sparse-checkout", "set", NULL);
 
 	/*
 	 * We must apply the setting in the current process
-- 
gitgitgadget

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* Re: [PATCH v3 03/10] sparse-checkout: add sanity-checks on initial sparsity state
  2021-12-10  3:56     ` [PATCH v3 03/10] sparse-checkout: add sanity-checks on initial sparsity state Elijah Newren via GitGitGadget
@ 2021-12-13 18:11       ` Junio C Hamano
  2021-12-14  2:25         ` Elijah Newren
  0 siblings, 1 reply; 63+ messages in thread
From: Junio C Hamano @ 2021-12-13 18:11 UTC (permalink / raw)
  To: Elijah Newren via GitGitGadget
  Cc: git, Derrick Stolee, Lessley Dennington, Victoria Dye, Elijah Newren

"Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Elijah Newren <newren@gmail.com>
>
> Most sparse-checkout subcommands (list, add, reapply, disable)
> only make sense when already in a sparse state.  Add a quick check
> that will error out early if this is not the case.
>
> Reviewed-by: Derrick Stolee <dstolee@microsoft.com>
> Reviewed-by: Victoria Dye <vdye@github.com>
> Signed-off-by: Elijah Newren <newren@gmail.com>
> ---
>  builtin/sparse-checkout.c          | 12 ++++++++++++
>  t/t1091-sparse-checkout-builtin.sh | 10 +++++++++-
>  2 files changed, 21 insertions(+), 1 deletion(-)

I won't complain too much but some looks a bit questionable to die
on.  For example, when asked to "please disable" something that is
already disabled, I do not think the user's intention includes "if
already disabled, please die"; rather it is "I want the end result
to be in the disabled state", isn't it?

I think what is common among the ones that I find questionable to
die is that they do not use end-user input from argv.  "Please add X
to sparsity patterns" may not make much sense when we are not already
sparse", unlike "Please disable", for example.

    Side note. I suspect that it can be argued that we might just
    auto-enable sparse state upon the first request to add
    something, but I personally feel that is dwimming too much, as
    behaviours of git in normal mode and sparse mode are so
    different.

So, for the same reason, I think "list" that shows "there is
nothing" without an error, when sparse-checkout is not active, would
also be perfectly defensible, and some people may find that dying a
bit too much in such a situation.

Or does the system work differently between

 (A) core_apply_sparse_checkout is true and the sparsity pattern list is
     empty, and
 (B) sparse-checkout is not in effect at all.

If that is the case, please ignore all of the above.

> diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c
> index e252b82136e..e9f644ac362 100644
> --- a/builtin/sparse-checkout.c
> +++ b/builtin/sparse-checkout.c
> @@ -56,6 +56,9 @@ static int sparse_checkout_list(int argc, const char **argv)
>  	char *sparse_filename;
>  	int res;
>  
> +	if (!core_apply_sparse_checkout)
> +		die(_("this worktree is not sparse"));
> +
>  	argc = parse_options(argc, argv, NULL,
>  			     builtin_sparse_checkout_list_options,
>  			     builtin_sparse_checkout_list_usage, 0);
> @@ -671,6 +674,9 @@ static int sparse_checkout_add(int argc, const char **argv, const char *prefix)
>  		OPT_END(),
>  	};
>  
> +	if (!core_apply_sparse_checkout)
> +		die(_("no sparse-checkout to add to"));
> +
>  	repo_read_index(the_repository);
>  
>  	argc = parse_options(argc, argv, prefix,
> @@ -719,6 +725,9 @@ static int sparse_checkout_reapply(int argc, const char **argv)
>  		OPT_END(),
>  	};
>  
> +	if (!core_apply_sparse_checkout)
> +		die(_("must be in a sparse-checkout to reapply sparsity patterns"));
> +
>  	argc = parse_options(argc, argv, NULL,
>  			     builtin_sparse_checkout_reapply_options,
>  			     builtin_sparse_checkout_reapply_usage, 0);
> @@ -740,6 +749,9 @@ static int sparse_checkout_disable(int argc, const char **argv)
>  	struct pattern_list pl;
>  	struct strbuf match_all = STRBUF_INIT;
>  
> +	if (!core_apply_sparse_checkout)
> +		die(_("no active sparse-checkout to disable"));
> +
>  	argc = parse_options(argc, argv, NULL,
>  			     builtin_sparse_checkout_disable_options,
>  			     builtin_sparse_checkout_disable_usage, 0);
> diff --git a/t/t1091-sparse-checkout-builtin.sh b/t/t1091-sparse-checkout-builtin.sh
> index 272ba1b566b..90a281bcf64 100755
> --- a/t/t1091-sparse-checkout-builtin.sh
> +++ b/t/t1091-sparse-checkout-builtin.sh
> @@ -41,7 +41,15 @@ test_expect_success 'setup' '
>  	)
>  '
>  
> -test_expect_success 'git sparse-checkout list (empty)' '
> +test_expect_success 'git sparse-checkout list (not sparse)' '
> +	test_must_fail git -C repo sparse-checkout list >list 2>err &&
> +	test_must_be_empty list &&
> +	test_i18ngrep "this worktree is not sparse" err
> +'
> +
> +test_expect_success 'git sparse-checkout list (not sparse)' '
> +	git -C repo sparse-checkout set &&
> +	rm repo/.git/info/sparse-checkout &&
>  	git -C repo sparse-checkout list >list 2>err &&
>  	test_must_be_empty list &&
>  	test_i18ngrep "this worktree is not sparse (sparse-checkout file may not exist)" err

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [PATCH v3 07/10] sparse-checkout: enable reapply to take --[no-]{cone,sparse-index}
  2021-12-10  3:56     ` [PATCH v3 07/10] sparse-checkout: enable reapply to take --[no-]{cone,sparse-index} Elijah Newren via GitGitGadget
@ 2021-12-13 18:23       ` Junio C Hamano
  2021-12-14  2:39         ` Elijah Newren
  0 siblings, 1 reply; 63+ messages in thread
From: Junio C Hamano @ 2021-12-13 18:23 UTC (permalink / raw)
  To: Elijah Newren via GitGitGadget
  Cc: git, Derrick Stolee, Lessley Dennington, Victoria Dye, Elijah Newren

"Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Elijah Newren <newren@gmail.com>
>
> Folks may want to switch to or from cone mode, or to or from a
> sparse-index without changing their sparsity paths.  Allow them to do so
> using the reapply command.

Interesting.

Are there certain pattern that would make sense only in one of the
modes but not the other?  If there isn't any such pattern, this
feature perfectly makes sense, I would think.

If an existing pattern changes its meaning between the old mode and
the new mode, that is very much fine---that is what the user wanted
to achieve by switching between the modes with "reapply".

Thanks.

> Reviewed-by: Derrick Stolee <dstolee@microsoft.com>
> Reviewed-by: Victoria Dye <vdye@github.com>
> Signed-off-by: Elijah Newren <newren@gmail.com>
> ---
>  builtin/sparse-checkout.c | 18 +++++++++++++++++-
>  1 file changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c
> index 1ecdc3ddd5a..9d2a05677c4 100644
> --- a/builtin/sparse-checkout.c
> +++ b/builtin/sparse-checkout.c
> @@ -759,13 +759,22 @@ static int sparse_checkout_set(int argc, const char **argv, const char *prefix)
>  }
>  
>  static char const * const builtin_sparse_checkout_reapply_usage[] = {
> -	N_("git sparse-checkout reapply"),
> +	N_("git sparse-checkout reapply [--[no-]cone] [--[no-]sparse-index] "),
>  	NULL
>  };
>  
> +static struct sparse_checkout_reapply_opts {
> +	int cone_mode;
> +	int sparse_index;
> +} reapply_opts;
> +
>  static int sparse_checkout_reapply(int argc, const char **argv)
>  {
>  	static struct option builtin_sparse_checkout_reapply_options[] = {
> +		OPT_BOOL(0, "cone", &reapply_opts.cone_mode,
> +			 N_("initialize the sparse-checkout in cone mode")),
> +		OPT_BOOL(0, "sparse-index", &reapply_opts.sparse_index,
> +			 N_("toggle the use of a sparse index")),
>  		OPT_END(),
>  	};
>  
> @@ -777,6 +786,13 @@ static int sparse_checkout_reapply(int argc, const char **argv)
>  			     builtin_sparse_checkout_reapply_usage, 0);
>  
>  	repo_read_index(the_repository);
> +
> +	reapply_opts.cone_mode = -1;
> +	reapply_opts.sparse_index = -1;
> +
> +	if (update_modes(&reapply_opts.cone_mode, &reapply_opts.sparse_index))
> +		return 1;
> +
>  	return update_working_directory(NULL);
>  }

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [PATCH v3 03/10] sparse-checkout: add sanity-checks on initial sparsity state
  2021-12-13 18:11       ` Junio C Hamano
@ 2021-12-14  2:25         ` Elijah Newren
  0 siblings, 0 replies; 63+ messages in thread
From: Elijah Newren @ 2021-12-14  2:25 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Elijah Newren via GitGitGadget, Git Mailing List, Derrick Stolee,
	Lessley Dennington, Victoria Dye

On Mon, Dec 13, 2021 at 10:11 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> "Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > From: Elijah Newren <newren@gmail.com>
> >
> > Most sparse-checkout subcommands (list, add, reapply, disable)
> > only make sense when already in a sparse state.  Add a quick check
> > that will error out early if this is not the case.
> >
> > Reviewed-by: Derrick Stolee <dstolee@microsoft.com>
> > Reviewed-by: Victoria Dye <vdye@github.com>
> > Signed-off-by: Elijah Newren <newren@gmail.com>
> > ---
> >  builtin/sparse-checkout.c          | 12 ++++++++++++
> >  t/t1091-sparse-checkout-builtin.sh | 10 +++++++++-
> >  2 files changed, 21 insertions(+), 1 deletion(-)
>
> I won't complain too much but some looks a bit questionable to die
> on.  For example, when asked to "please disable" something that is
> already disabled, I do not think the user's intention includes "if
> already disabled, please die"; rather it is "I want the end result
> to be in the disabled state", isn't it?

Yeah, fair enough, I can change it to just print that it's already disabled.

However, I think add, list, and reapply should all die still.

> I think what is common among the ones that I find questionable to
> die is that they do not use end-user input from argv.  "Please add X
> to sparsity patterns" may not make much sense when we are not already
> sparse", unlike "Please disable", for example.
>
>     Side note. I suspect that it can be argued that we might just
>     auto-enable sparse state upon the first request to add
>     something, but I personally feel that is dwimming too much, as
>     behaviours of git in normal mode and sparse mode are so
>     different.
>
> So, for the same reason, I think "list" that shows "there is
> nothing" without an error, when sparse-checkout is not active, would
> also be perfectly defensible, and some people may find that dying a
> bit too much in such a situation.
>
> Or does the system work differently between
>
>  (A) core_apply_sparse_checkout is true and the sparsity pattern list is
>      empty, and

You can get empty output from
   git sparse-checkout list

after running
   git sparse-checkout --cone

(cone mode, no paths specified)

In this state, only files immediately in the toplevel directory will
be present; all subdirectories will be sparsified away.

(Granted, that's because cone mode by default gives you some pattern
lists behind the scenes.  If you actually delete all entries in
.git/info/sparse-checkout and do a git sparse-checkout reapply, then
you will have NO (tracked) files in your checkout; everything will be
SKIP_WORKTREE.)

>  (B) sparse-checkout is not in effect at all.

In this state, the tree is dense; all files are present.

> If that is the case, please ignore all of the above.

I think your comment about disable makes sense, though.

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [PATCH v3 07/10] sparse-checkout: enable reapply to take --[no-]{cone,sparse-index}
  2021-12-13 18:23       ` Junio C Hamano
@ 2021-12-14  2:39         ` Elijah Newren
  0 siblings, 0 replies; 63+ messages in thread
From: Elijah Newren @ 2021-12-14  2:39 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Elijah Newren via GitGitGadget, Git Mailing List, Derrick Stolee,
	Lessley Dennington, Victoria Dye

On Mon, Dec 13, 2021 at 10:23 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> "Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > From: Elijah Newren <newren@gmail.com>
> >
> > Folks may want to switch to or from cone mode, or to or from a
> > sparse-index without changing their sparsity paths.  Allow them to do so
> > using the reapply command.
>
> Interesting.
>
> Are there certain pattern that would make sense only in one of the
> modes but not the other?  If there isn't any such pattern, this
> feature perfectly makes sense, I would think.

Good question.

Valid cone mode patterns are a strict subset of the possible sparsity
patterns.  So, switching from cone to non-cone mode trivially makes
sense.  Understanding why switching in the other direction is okay
takes a bit more understanding...

Stolee designed cone mode such that when it's active, and someone
edits .git/info/sparse-checkout and adds patterns of their own (which
users might do because that had been suggested for several years in
the read-tree docs -- and still is), then the code will print a
warning ("disabling cone pattern matching") and operate in non-cone
mode.  As such, the sparsity does not "break" when they switch modes
if they have non-cone patterns; they'll just get warnings.

Further, we're making `reapply` consistent with `init` here.  Stolee
made `init` usable for switching modes in an active sparse-checkout
(though it wasn't well documented, and it's slightly confusing to
users who might worry that they'll lose their sparsity patterns by
using `init`).  We're just copying that ability from `init` over in
`reapply`, and recommending using the latter rather than the former.

> If an existing pattern changes its meaning between the old mode and
> the new mode, that is very much fine---that is what the user wanted
> to achieve by switching between the modes with "reapply".
>
> Thanks.

^ permalink raw reply	[flat|nested] 63+ messages in thread

* [PATCH v4 00/10] sparse-checkout: make set subsume init
  2021-12-10  3:56   ` [PATCH v3 " Elijah Newren via GitGitGadget
                       ` (9 preceding siblings ...)
  2021-12-10  3:56     ` [PATCH v3 10/10] clone: avoid using deprecated `sparse-checkout init` Elijah Newren via GitGitGadget
@ 2021-12-14  4:09     ` Elijah Newren via GitGitGadget
  2021-12-14  4:09       ` [PATCH v4 01/10] sparse-checkout: pass use_stdin as a parameter instead of as a global Elijah Newren via GitGitGadget
                         ` (9 more replies)
  10 siblings, 10 replies; 63+ messages in thread
From: Elijah Newren via GitGitGadget @ 2021-12-14  4:09 UTC (permalink / raw)
  To: git
  Cc: Derrick Stolee, Lessley Dennington, Victoria Dye, Elijah Newren,
	Elijah Newren

As described at [1], the split of init and set subcommands in
sparse-checkout causes multiple issues:

 * Poor performance (deleting all tracked files, then later restoring many
   and maybe even most of them)
 * Poor UI (multiple progress bars in wrappers that hide both commands under
   1 user-facing command)
 * Loss of ignored files under directories the user wanted to keep

This series fixes this bug by providing a single command to switch to a
sparse-checkout: set. It does so by making set able to do the combined work
of init and set. It keeps init as-is to give folks time to adapt, but marks
it as deprecated. It also makes reapply able to toggle cone/non-cone mode
and sparse-index/non-sparse-index mode.

Changes since v3:

 * Do not make git sparse-checkout disable throw an error when not in a
   sparse-checkout -- and document why we don't exit early either.

Changes since v2:

 * Small documentation wording improvement
 * Added Reviewed-by's from Stolee and Victoria

Changes since v1:

 * Inserted new patches 3 & 4 as additional preparatory cleanups
 * Took the new mode-toggling work code in sparse_checkout_set from the
   previous series and moved it into a new function, as a preparatory patch,
   and made it usable by init/set/reapply
 * Also updated reapply to allow mode-toggling
 * Updated the documentation as per above
 * Various other small items from review comments

A quick overview:

 * Patches 1-4: small preparatory refactorings
 * Patch 5: patch adding new function for toggling {cone,sparse-checkout}
   modes
 * Patch 6: the crux of the series; make set able to handle both init and
   set options
 * Patch 7: make reapply also able to do mode toggling
 * Patches 8-9: documentation modifications (Patch 4 is worth reviewing; it
   marks init as deprecated -- are others okay with that?)
 * Patch 10: trivial modification of git clone --sparse to use git
   sparse-checkout set rather than git sparse-checkout init.

[1]
https://lore.kernel.org/git/CABPp-BE8TJ8QGAQWsSGT7S+9Xp-XmApcC9PSw3K=RQOP0rt+PQ@mail.gmail.com/

Elijah Newren (10):
  sparse-checkout: pass use_stdin as a parameter instead of as a global
  sparse-checkout: break apart functions for sparse_checkout_(set|add)
  sparse-checkout: add sanity-checks on initial sparsity state
  sparse-checkout: disallow --no-stdin as an argument to set
  sparse-checkout: split out code for tweaking settings config
  sparse-checkout: enable `set` to initialize sparse-checkout mode
  sparse-checkout: enable reapply to take --[no-]{cone,sparse-index}
  git-sparse-checkout.txt: update to document init/set/reapply changes
  Documentation: clarify/correct a few sparsity related statements
  clone: avoid using deprecated `sparse-checkout init`

 Documentation/git-clone.txt           |   8 +-
 Documentation/git-sparse-checkout.txt | 100 +++++++------
 builtin/clone.c                       |   2 +-
 builtin/sparse-checkout.c             | 204 ++++++++++++++++++++------
 t/t1091-sparse-checkout-builtin.sh    |  10 +-
 5 files changed, 227 insertions(+), 97 deletions(-)


base-commit: abe6bb3905392d5eb6b01fa6e54d7e784e0522aa
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1151%2Fnewren%2Fsparse-checkout-no-init-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1151/newren/sparse-checkout-no-init-v4
Pull-Request: https://github.com/git/git/pull/1151

Range-diff vs v3:

  1:  814aed2d125 =  1:  814aed2d125 sparse-checkout: pass use_stdin as a parameter instead of as a global
  2:  04cd57592e9 =  2:  04cd57592e9 sparse-checkout: break apart functions for sparse_checkout_(set|add)
  3:  f3af5edb25d !  3:  a8fdda35b91 sparse-checkout: add sanity-checks on initial sparsity state
     @@ Metadata
       ## Commit message ##
          sparse-checkout: add sanity-checks on initial sparsity state
      
     -    Most sparse-checkout subcommands (list, add, reapply, disable)
     -    only make sense when already in a sparse state.  Add a quick check
     -    that will error out early if this is not the case.
     +    Most sparse-checkout subcommands (list, add, reapply) only make sense
     +    when already in a sparse state.  Add a quick check that will error out
     +    early if this is not the case.
     +
     +    Also document with a comment why we do not exit early in `disable` even
     +    when core.sparseCheckout starts as false.
      
          Reviewed-by: Derrick Stolee <dstolee@microsoft.com>
          Reviewed-by: Victoria Dye <vdye@github.com>
     @@ builtin/sparse-checkout.c: static int sparse_checkout_disable(int argc, const ch
       	struct pattern_list pl;
       	struct strbuf match_all = STRBUF_INIT;
       
     -+	if (!core_apply_sparse_checkout)
     -+		die(_("no active sparse-checkout to disable"));
     ++	/*
     ++	 * We do not exit early if !core_apply_sparse_checkout; due to the
     ++	 * ability for users to manually muck things up between
     ++	 *   direct editing of .git/info/sparse-checkout
     ++	 *   running read-tree -m u HEAD or update-index --skip-worktree
     ++	 *   direct toggling of config options
     ++	 * users might end up with an index with SKIP_WORKTREE bit set on
     ++	 * some files and not know how to undo it.  So, here we just
     ++	 * forcibly return to a dense checkout regardless of initial state.
     ++	 */
      +
       	argc = parse_options(argc, argv, NULL,
       			     builtin_sparse_checkout_disable_options,
  4:  82a13cc0197 =  4:  5882332b97f sparse-checkout: disallow --no-stdin as an argument to set
  5:  7a812e0222c =  5:  3e9e28c8dd2 sparse-checkout: split out code for tweaking settings config
  6:  7167a4b3118 =  6:  595ba138603 sparse-checkout: enable `set` to initialize sparse-checkout mode
  7:  3687637915f =  7:  09b13280c26 sparse-checkout: enable reapply to take --[no-]{cone,sparse-index}
  8:  7483d1363e5 =  8:  9d96da855ea git-sparse-checkout.txt: update to document init/set/reapply changes
  9:  11a45920602 =  9:  f669829a98b Documentation: clarify/correct a few sparsity related statements
 10:  395d9b194d3 = 10:  ae671aa615a clone: avoid using deprecated `sparse-checkout init`

-- 
gitgitgadget

^ permalink raw reply	[flat|nested] 63+ messages in thread

* [PATCH v4 01/10] sparse-checkout: pass use_stdin as a parameter instead of as a global
  2021-12-14  4:09     ` [PATCH v4 00/10] sparse-checkout: make set subsume init Elijah Newren via GitGitGadget
@ 2021-12-14  4:09       ` Elijah Newren via GitGitGadget
  2021-12-14  4:09       ` [PATCH v4 02/10] sparse-checkout: break apart functions for sparse_checkout_(set|add) Elijah Newren via GitGitGadget
                         ` (8 subsequent siblings)
  9 siblings, 0 replies; 63+ messages in thread
From: Elijah Newren via GitGitGadget @ 2021-12-14  4:09 UTC (permalink / raw)
  To: git
  Cc: Derrick Stolee, Lessley Dennington, Victoria Dye, Elijah Newren,
	Elijah Newren, Elijah Newren

From: Elijah Newren <newren@gmail.com>

add_patterns_from_input() has relied on a global variable,
set_opts.use_stdin, which has been used by both the `set` and `add`
subcommands of sparse-checkout.  Once we introduce an
add_opts.use_stdin, the hardcoding of set_opts.use_stdin will be
incorrect.  Pass the value as function parameter instead to allow us to
make subsequent changes.

Reviewed-by: Derrick Stolee <dstolee@microsoft.com>
Reviewed-by: Victoria Dye <vdye@github.com>
Signed-off-by: Elijah Newren <newren@gmail.com>
---
 builtin/sparse-checkout.c | 28 ++++++++++++++++------------
 1 file changed, 16 insertions(+), 12 deletions(-)

diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c
index d0f5c4702be..8612328e5dd 100644
--- a/builtin/sparse-checkout.c
+++ b/builtin/sparse-checkout.c
@@ -525,7 +525,8 @@ static struct sparse_checkout_set_opts {
 } set_opts;
 
 static void add_patterns_from_input(struct pattern_list *pl,
-				    int argc, const char **argv)
+				    int argc, const char **argv,
+				    int use_stdin)
 {
 	int i;
 	if (core_sparse_checkout_cone) {
@@ -535,7 +536,7 @@ static void add_patterns_from_input(struct pattern_list *pl,
 		hashmap_init(&pl->parent_hashmap, pl_hashmap_cmp, NULL, 0);
 		pl->use_cone_patterns = 1;
 
-		if (set_opts.use_stdin) {
+		if (use_stdin) {
 			struct strbuf unquoted = STRBUF_INIT;
 			while (!strbuf_getline(&line, stdin)) {
 				if (line.buf[0] == '"') {
@@ -559,7 +560,7 @@ static void add_patterns_from_input(struct pattern_list *pl,
 			}
 		}
 	} else {
-		if (set_opts.use_stdin) {
+		if (use_stdin) {
 			struct strbuf line = STRBUF_INIT;
 
 			while (!strbuf_getline(&line, stdin)) {
@@ -580,7 +581,8 @@ enum modify_type {
 };
 
 static void add_patterns_cone_mode(int argc, const char **argv,
-				   struct pattern_list *pl)
+				   struct pattern_list *pl,
+				   int use_stdin)
 {
 	struct strbuf buffer = STRBUF_INIT;
 	struct pattern_entry *pe;
@@ -588,7 +590,7 @@ static void add_patterns_cone_mode(int argc, const char **argv,
 	struct pattern_list existing;
 	char *sparse_filename = get_sparse_checkout_filename();
 
-	add_patterns_from_input(pl, argc, argv);
+	add_patterns_from_input(pl, argc, argv, use_stdin);
 
 	memset(&existing, 0, sizeof(existing));
 	existing.use_cone_patterns = core_sparse_checkout_cone;
@@ -614,17 +616,19 @@ static void add_patterns_cone_mode(int argc, const char **argv,
 }
 
 static void add_patterns_literal(int argc, const char **argv,
-				 struct pattern_list *pl)
+				 struct pattern_list *pl,
+				 int use_stdin)
 {
 	char *sparse_filename = get_sparse_checkout_filename();
 	if (add_patterns_from_file_to_list(sparse_filename, "", 0,
 					   pl, NULL, 0))
 		die(_("unable to load existing sparse-checkout patterns"));
 	free(sparse_filename);
-	add_patterns_from_input(pl, argc, argv);
+	add_patterns_from_input(pl, argc, argv, use_stdin);
 }
 
-static int modify_pattern_list(int argc, const char **argv, enum modify_type m)
+static int modify_pattern_list(int argc, const char **argv, int use_stdin,
+			       enum modify_type m)
 {
 	int result;
 	int changed_config = 0;
@@ -633,13 +637,13 @@ static int modify_pattern_list(int argc, const char **argv, enum modify_type m)
 	switch (m) {
 	case ADD:
 		if (core_sparse_checkout_cone)
-			add_patterns_cone_mode(argc, argv, pl);
+			add_patterns_cone_mode(argc, argv, pl, use_stdin);
 		else
-			add_patterns_literal(argc, argv, pl);
+			add_patterns_literal(argc, argv, pl, use_stdin);
 		break;
 
 	case REPLACE:
-		add_patterns_from_input(pl, argc, argv);
+		add_patterns_from_input(pl, argc, argv, use_stdin);
 		break;
 	}
 
@@ -675,7 +679,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, m);
+	return modify_pattern_list(argc, argv, set_opts.use_stdin, m);
 }
 
 static char const * const builtin_sparse_checkout_reapply_usage[] = {
-- 
gitgitgadget


^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH v4 02/10] sparse-checkout: break apart functions for sparse_checkout_(set|add)
  2021-12-14  4:09     ` [PATCH v4 00/10] sparse-checkout: make set subsume init Elijah Newren via GitGitGadget
  2021-12-14  4:09       ` [PATCH v4 01/10] sparse-checkout: pass use_stdin as a parameter instead of as a global Elijah Newren via GitGitGadget
@ 2021-12-14  4:09       ` Elijah Newren via GitGitGadget
  2021-12-14  4:09       ` [PATCH v4 03/10] sparse-checkout: add sanity-checks on initial sparsity state Elijah Newren via GitGitGadget
                         ` (7 subsequent siblings)
  9 siblings, 0 replies; 63+ messages in thread
From: Elijah Newren via GitGitGadget @ 2021-12-14  4:09 UTC (permalink / raw)
  To: git
  Cc: Derrick Stolee, Lessley Dennington, Victoria Dye, Elijah Newren,
	Elijah Newren, Elijah Newren

From: Elijah Newren <newren@gmail.com>

sparse_checkout_set() was reused by sparse_checkout_add() with the only
difference being a single parameter being passed to that function.
However, we would like sparse_checkout_set() to do the same work that
sparse_checkout_init() does if sparse checkouts are not already enabled.
To facilitate this transition, give each mode their own copy of the
function.  This does not introduce any behavioral changes; that will
come in a subsequent patch.

Reviewed-by: Derrick Stolee <dstolee@microsoft.com>
Reviewed-by: Victoria Dye <vdye@github.com>
Signed-off-by: Elijah Newren <newren@gmail.com>
---
 builtin/sparse-checkout.c | 54 +++++++++++++++++++++++++++++----------
 1 file changed, 40 insertions(+), 14 deletions(-)

diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c
index 8612328e5dd..e252b82136e 100644
--- a/builtin/sparse-checkout.c
+++ b/builtin/sparse-checkout.c
@@ -515,15 +515,6 @@ static void strbuf_to_cone_pattern(struct strbuf *line, struct pattern_list *pl)
 	insert_recursive_pattern(pl, line);
 }
 
-static char const * const builtin_sparse_checkout_set_usage[] = {
-	N_("git sparse-checkout (set|add) (--stdin | <patterns>)"),
-	NULL
-};
-
-static struct sparse_checkout_set_opts {
-	int use_stdin;
-} set_opts;
-
 static void add_patterns_from_input(struct pattern_list *pl,
 				    int argc, const char **argv,
 				    int use_stdin)
@@ -663,8 +654,43 @@ static int modify_pattern_list(int argc, const char **argv, int use_stdin,
 	return result;
 }
 
-static int sparse_checkout_set(int argc, const char **argv, const char *prefix,
-			       enum modify_type m)
+static char const * const builtin_sparse_checkout_add_usage[] = {
+	N_("git sparse-checkout add (--stdin | <patterns>)"),
+	NULL
+};
+
+static struct sparse_checkout_add_opts {
+	int use_stdin;
+} add_opts;
+
+static int sparse_checkout_add(int argc, const char **argv, const char *prefix)
+{
+	static struct option builtin_sparse_checkout_add_options[] = {
+		OPT_BOOL(0, "stdin", &add_opts.use_stdin,
+			 N_("read patterns from standard in")),
+		OPT_END(),
+	};
+
+	repo_read_index(the_repository);
+
+	argc = parse_options(argc, argv, prefix,
+			     builtin_sparse_checkout_add_options,
+			     builtin_sparse_checkout_add_usage,
+			     PARSE_OPT_KEEP_UNKNOWN);
+
+	return modify_pattern_list(argc, argv, add_opts.use_stdin, ADD);
+}
+
+static char const * const builtin_sparse_checkout_set_usage[] = {
+	N_("git sparse-checkout set (--stdin | <patterns>)"),
+	NULL
+};
+
+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 struct option builtin_sparse_checkout_set_options[] = {
 		OPT_BOOL(0, "stdin", &set_opts.use_stdin,
@@ -679,7 +705,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, set_opts.use_stdin, m);
+	return modify_pattern_list(argc, argv, set_opts.use_stdin, REPLACE);
 }
 
 static char const * const builtin_sparse_checkout_reapply_usage[] = {
@@ -762,9 +788,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, REPLACE);
+			return sparse_checkout_set(argc, argv, prefix);
 		if (!strcmp(argv[0], "add"))
-			return sparse_checkout_set(argc, argv, prefix, ADD);
+			return sparse_checkout_add(argc, argv, prefix);
 		if (!strcmp(argv[0], "reapply"))
 			return sparse_checkout_reapply(argc, argv);
 		if (!strcmp(argv[0], "disable"))
-- 
gitgitgadget


^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH v4 03/10] sparse-checkout: add sanity-checks on initial sparsity state
  2021-12-14  4:09     ` [PATCH v4 00/10] sparse-checkout: make set subsume init Elijah Newren via GitGitGadget
  2021-12-14  4:09       ` [PATCH v4 01/10] sparse-checkout: pass use_stdin as a parameter instead of as a global Elijah Newren via GitGitGadget
  2021-12-14  4:09       ` [PATCH v4 02/10] sparse-checkout: break apart functions for sparse_checkout_(set|add) Elijah Newren via GitGitGadget
@ 2021-12-14  4:09       ` Elijah Newren via GitGitGadget
  2021-12-14  4:09       ` [PATCH v4 04/10] sparse-checkout: disallow --no-stdin as an argument to set Elijah Newren via GitGitGadget
                         ` (6 subsequent siblings)
  9 siblings, 0 replies; 63+ messages in thread
From: Elijah Newren via GitGitGadget @ 2021-12-14  4:09 UTC (permalink / raw)
  To: git
  Cc: Derrick Stolee, Lessley Dennington, Victoria Dye, Elijah Newren,
	Elijah Newren, Elijah Newren

From: Elijah Newren <newren@gmail.com>

Most sparse-checkout subcommands (list, add, reapply) only make sense
when already in a sparse state.  Add a quick check that will error out
early if this is not the case.

Also document with a comment why we do not exit early in `disable` even
when core.sparseCheckout starts as false.

Reviewed-by: Derrick Stolee <dstolee@microsoft.com>
Reviewed-by: Victoria Dye <vdye@github.com>
Signed-off-by: Elijah Newren <newren@gmail.com>
---
 builtin/sparse-checkout.c          | 20 ++++++++++++++++++++
 t/t1091-sparse-checkout-builtin.sh | 10 +++++++++-
 2 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c
index e252b82136e..eb8fbd36b0b 100644
--- a/builtin/sparse-checkout.c
+++ b/builtin/sparse-checkout.c
@@ -56,6 +56,9 @@ static int sparse_checkout_list(int argc, const char **argv)
 	char *sparse_filename;
 	int res;
 
+	if (!core_apply_sparse_checkout)
+		die(_("this worktree is not sparse"));
+
 	argc = parse_options(argc, argv, NULL,
 			     builtin_sparse_checkout_list_options,
 			     builtin_sparse_checkout_list_usage, 0);
@@ -671,6 +674,9 @@ static int sparse_checkout_add(int argc, const char **argv, const char *prefix)
 		OPT_END(),
 	};
 
+	if (!core_apply_sparse_checkout)
+		die(_("no sparse-checkout to add to"));
+
 	repo_read_index(the_repository);
 
 	argc = parse_options(argc, argv, prefix,
@@ -719,6 +725,9 @@ static int sparse_checkout_reapply(int argc, const char **argv)
 		OPT_END(),
 	};
 
+	if (!core_apply_sparse_checkout)
+		die(_("must be in a sparse-checkout to reapply sparsity patterns"));
+
 	argc = parse_options(argc, argv, NULL,
 			     builtin_sparse_checkout_reapply_options,
 			     builtin_sparse_checkout_reapply_usage, 0);
@@ -740,6 +749,17 @@ static int sparse_checkout_disable(int argc, const char **argv)
 	struct pattern_list pl;
 	struct strbuf match_all = STRBUF_INIT;
 
+	/*
+	 * We do not exit early if !core_apply_sparse_checkout; due to the
+	 * ability for users to manually muck things up between
+	 *   direct editing of .git/info/sparse-checkout
+	 *   running read-tree -m u HEAD or update-index --skip-worktree
+	 *   direct toggling of config options
+	 * users might end up with an index with SKIP_WORKTREE bit set on
+	 * some files and not know how to undo it.  So, here we just
+	 * forcibly return to a dense checkout regardless of initial state.
+	 */
+
 	argc = parse_options(argc, argv, NULL,
 			     builtin_sparse_checkout_disable_options,
 			     builtin_sparse_checkout_disable_usage, 0);
diff --git a/t/t1091-sparse-checkout-builtin.sh b/t/t1091-sparse-checkout-builtin.sh
index 272ba1b566b..90a281bcf64 100755
--- a/t/t1091-sparse-checkout-builtin.sh
+++ b/t/t1091-sparse-checkout-builtin.sh
@@ -41,7 +41,15 @@ test_expect_success 'setup' '
 	)
 '
 
-test_expect_success 'git sparse-checkout list (empty)' '
+test_expect_success 'git sparse-checkout list (not sparse)' '
+	test_must_fail git -C repo sparse-checkout list >list 2>err &&
+	test_must_be_empty list &&
+	test_i18ngrep "this worktree is not sparse" err
+'
+
+test_expect_success 'git sparse-checkout list (not sparse)' '
+	git -C repo sparse-checkout set &&
+	rm repo/.git/info/sparse-checkout &&
 	git -C repo sparse-checkout list >list 2>err &&
 	test_must_be_empty list &&
 	test_i18ngrep "this worktree is not sparse (sparse-checkout file may not exist)" err
-- 
gitgitgadget


^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH v4 04/10] sparse-checkout: disallow --no-stdin as an argument to set
  2021-12-14  4:09     ` [PATCH v4 00/10] sparse-checkout: make set subsume init Elijah Newren via GitGitGadget
                         ` (2 preceding siblings ...)
  2021-12-14  4:09       ` [PATCH v4 03/10] sparse-checkout: add sanity-checks on initial sparsity state Elijah Newren via GitGitGadget
@ 2021-12-14  4:09       ` Elijah Newren via GitGitGadget
  2021-12-14  4:09       ` [PATCH v4 05/10] sparse-checkout: split out code for tweaking settings config Elijah Newren via GitGitGadget
                         ` (5 subsequent siblings)
  9 siblings, 0 replies; 63+ messages in thread
From: Elijah Newren via GitGitGadget @ 2021-12-14  4:09 UTC (permalink / raw)
  To: git
  Cc: Derrick Stolee, Lessley Dennington, Victoria Dye, Elijah Newren,
	Elijah Newren, Elijah Newren

From: Elijah Newren <newren@gmail.com>

We intentionally added --stdin as an option to `sparse-checkout set`,
but didn't intend for --no-stdin to be permitted as well.

Reported-by: Victoria Dye <vdye@github.com>
Reviewed-by: Derrick Stolee <dstolee@microsoft.com>
Reviewed-by: Victoria Dye <vdye@github.com>
Signed-off-by: Elijah Newren <newren@gmail.com>
---
 builtin/sparse-checkout.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c
index eb8fbd36b0b..387903eafe7 100644
--- a/builtin/sparse-checkout.c
+++ b/builtin/sparse-checkout.c
@@ -699,8 +699,9 @@ static struct sparse_checkout_set_opts {
 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_BOOL_F(0, "stdin", &set_opts.use_stdin,
+			   N_("read patterns from standard in"),
+			   PARSE_OPT_NONEG),
 		OPT_END(),
 	};
 
-- 
gitgitgadget


^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH v4 05/10] sparse-checkout: split out code for tweaking settings config
  2021-12-14  4:09     ` [PATCH v4 00/10] sparse-checkout: make set subsume init Elijah Newren via GitGitGadget
                         ` (3 preceding siblings ...)
  2021-12-14  4:09       ` [PATCH v4 04/10] sparse-checkout: disallow --no-stdin as an argument to set Elijah Newren via GitGitGadget
@ 2021-12-14  4:09       ` Elijah Newren via GitGitGadget
  2021-12-14  4:09       ` [PATCH v4 06/10] sparse-checkout: enable `set` to initialize sparse-checkout mode Elijah Newren via GitGitGadget
                         ` (4 subsequent siblings)
  9 siblings, 0 replies; 63+ messages in thread
From: Elijah Newren via GitGitGadget @ 2021-12-14  4:09 UTC (permalink / raw)
  To: git
  Cc: Derrick Stolee, Lessley Dennington, Victoria Dye, Elijah Newren,
	Elijah Newren, Elijah Newren

From: Elijah Newren <newren@gmail.com>

`init` has some code for handling updates to either cone mode or
the sparse-index setting.  We would like to be able to reuse this
elsewhere, namely in `set` and `reapply`.  Split this function out,
and make it slightly more general so it can handle being called from
the new callers.

Reviewed-by: Derrick Stolee <dstolee@microsoft.com>
Reviewed-by: Victoria Dye <vdye@github.com>
Signed-off-by: Elijah Newren <newren@gmail.com>
---
 builtin/sparse-checkout.c | 56 ++++++++++++++++++++++++++-------------
 1 file changed, 37 insertions(+), 19 deletions(-)

diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c
index 387903eafe7..3b74779bb48 100644
--- a/builtin/sparse-checkout.c
+++ b/builtin/sparse-checkout.c
@@ -383,6 +383,41 @@ static int set_config(enum sparse_checkout_mode mode)
 	return 0;
 }
 
+static int update_modes(int *cone_mode, int *sparse_index)
+{
+	int mode, record_mode;
+
+	/* Determine if we need to record the mode; ensure sparse checkout on */
+	record_mode = (*cone_mode != -1) || !core_apply_sparse_checkout;
+
+	/* If not specified, use previous definition of cone mode */
+	if (*cone_mode == -1 && core_apply_sparse_checkout)
+		*cone_mode = core_sparse_checkout_cone;
+
+	/* Set cone/non-cone mode appropriately */
+	core_apply_sparse_checkout = 1;
+	if (*cone_mode == 1) {
+		mode = MODE_CONE_PATTERNS;
+		core_sparse_checkout_cone = 1;
+	} else {
+		mode = MODE_ALL_PATTERNS;
+	}
+	if (record_mode && set_config(mode))
+		return 1;
+
+	/* Set sparse-index/non-sparse-index mode if specified */
+	if (*sparse_index >= 0) {
+		if (set_sparse_index_config(the_repository, *sparse_index) < 0)
+			die(_("failed to modify sparse-index config"));
+
+		/* force an index rewrite */
+		repo_read_index(the_repository);
+		the_repository->index->updated_workdir = 1;
+	}
+
+	return 0;
+}
+
 static char const * const builtin_sparse_checkout_init_usage[] = {
 	N_("git sparse-checkout init [--cone] [--[no-]sparse-index]"),
 	NULL
@@ -399,7 +434,6 @@ static int sparse_checkout_init(int argc, const char **argv)
 	char *sparse_filename;
 	int res;
 	struct object_id oid;
-	int mode;
 	struct strbuf pattern = STRBUF_INIT;
 
 	static struct option builtin_sparse_checkout_init_options[] = {
@@ -412,19 +446,14 @@ static int sparse_checkout_init(int argc, const char **argv)
 
 	repo_read_index(the_repository);
 
+	init_opts.cone_mode = -1;
 	init_opts.sparse_index = -1;
 
 	argc = parse_options(argc, argv, NULL,
 			     builtin_sparse_checkout_init_options,
 			     builtin_sparse_checkout_init_usage, 0);
 
-	if (init_opts.cone_mode) {
-		mode = MODE_CONE_PATTERNS;
-		core_sparse_checkout_cone = 1;
-	} else
-		mode = MODE_ALL_PATTERNS;
-
-	if (set_config(mode))
+	if (update_modes(&init_opts.cone_mode, &init_opts.sparse_index))
 		return 1;
 
 	memset(&pl, 0, sizeof(pl));
@@ -432,17 +461,6 @@ static int sparse_checkout_init(int argc, const char **argv)
 	sparse_filename = get_sparse_checkout_filename();
 	res = add_patterns_from_file_to_list(sparse_filename, "", 0, &pl, NULL, 0);
 
-	if (init_opts.sparse_index >= 0) {
-		if (set_sparse_index_config(the_repository, init_opts.sparse_index) < 0)
-			die(_("failed to modify sparse-index config"));
-
-		/* force an index rewrite */
-		repo_read_index(the_repository);
-		the_repository->index->updated_workdir = 1;
-	}
-
-	core_apply_sparse_checkout = 1;
-
 	/* If we already have a sparse-checkout file, use it. */
 	if (res >= 0) {
 		free(sparse_filename);
-- 
gitgitgadget


^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH v4 06/10] sparse-checkout: enable `set` to initialize sparse-checkout mode
  2021-12-14  4:09     ` [PATCH v4 00/10] sparse-checkout: make set subsume init Elijah Newren via GitGitGadget
                         ` (4 preceding siblings ...)
  2021-12-14  4:09       ` [PATCH v4 05/10] sparse-checkout: split out code for tweaking settings config Elijah Newren via GitGitGadget
@ 2021-12-14  4:09       ` Elijah Newren via GitGitGadget
  2021-12-14  4:09       ` [PATCH v4 07/10] sparse-checkout: enable reapply to take --[no-]{cone,sparse-index} Elijah Newren via GitGitGadget
                         ` (3 subsequent siblings)
  9 siblings, 0 replies; 63+ messages in thread
From: Elijah Newren via GitGitGadget @ 2021-12-14  4:09 UTC (permalink / raw)
  To: git
  Cc: Derrick Stolee, Lessley Dennington, Victoria Dye, Elijah Newren,
	Elijah Newren, Elijah Newren

From: Elijah Newren <newren@gmail.com>

The previously suggested workflow:
  git sparse-checkout init ...
  git sparse-checkout set ...

Suffered from three problems:
  1) It would delete nearly all files in the first step, then
     restore them in the second.  That was poor performance and
     forced unnecessary rebuilds.
  2) The two-step process resulted in two progress bars, which
     was suboptimal from a UI point of view for wrappers that
     invoked both of these commands but only exposed a single
     command to their end users.
  3) With cone mode, the first step would delete nearly all
     ignored files everywhere, because everything was considered
     to be outside of the specified sparsity paths.  (The user was
     not allowed to specify any sparsity paths in the `init` step.)

Avoid these problems by teaching `set` to understand the extra
parameters that `init` takes and performing any necessary initialization
if not already in a sparse checkout.

Reviewed-by: Derrick Stolee <dstolee@microsoft.com>
Reviewed-by: Victoria Dye <vdye@github.com>
Signed-off-by: Elijah Newren <newren@gmail.com>
---
 builtin/sparse-checkout.c | 27 ++++++++++++++++++++++++++-
 1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c
index 3b74779bb48..16daae84975 100644
--- a/builtin/sparse-checkout.c
+++ b/builtin/sparse-checkout.c
@@ -706,17 +706,26 @@ static int sparse_checkout_add(int argc, const char **argv, const char *prefix)
 }
 
 static char const * const builtin_sparse_checkout_set_usage[] = {
-	N_("git sparse-checkout set (--stdin | <patterns>)"),
+	N_("git sparse-checkout set [--[no-]cone] [--[no-]sparse-index] (--stdin | <patterns>)"),
 	NULL
 };
 
 static struct sparse_checkout_set_opts {
+	int cone_mode;
+	int sparse_index;
 	int use_stdin;
 } set_opts;
 
 static int sparse_checkout_set(int argc, const char **argv, const char *prefix)
 {
+	int default_patterns_nr = 2;
+	const char *default_patterns[] = {"/*", "!/*/", NULL};
+
 	static struct option builtin_sparse_checkout_set_options[] = {
+		OPT_BOOL(0, "cone", &set_opts.cone_mode,
+			 N_("initialize the sparse-checkout in cone mode")),
+		OPT_BOOL(0, "sparse-index", &set_opts.sparse_index,
+			 N_("toggle the use of a sparse index")),
 		OPT_BOOL_F(0, "stdin", &set_opts.use_stdin,
 			   N_("read patterns from standard in"),
 			   PARSE_OPT_NONEG),
@@ -725,11 +734,27 @@ static int sparse_checkout_set(int argc, const char **argv, const char *prefix)
 
 	repo_read_index(the_repository);
 
+	set_opts.cone_mode = -1;
+	set_opts.sparse_index = -1;
+
 	argc = parse_options(argc, argv, prefix,
 			     builtin_sparse_checkout_set_options,
 			     builtin_sparse_checkout_set_usage,
 			     PARSE_OPT_KEEP_UNKNOWN);
 
+	if (update_modes(&set_opts.cone_mode, &set_opts.sparse_index))
+		return 1;
+
+	/*
+	 * Cone mode automatically specifies the toplevel directory.  For
+	 * non-cone mode, if nothing is specified, manually select just the
+	 * top-level directory (much as 'init' would do).
+	 */
+	if (!core_sparse_checkout_cone && argc == 0) {
+		argv = default_patterns;
+		argc = default_patterns_nr;
+	}
+
 	return modify_pattern_list(argc, argv, set_opts.use_stdin, REPLACE);
 }
 
-- 
gitgitgadget


^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH v4 07/10] sparse-checkout: enable reapply to take --[no-]{cone,sparse-index}
  2021-12-14  4:09     ` [PATCH v4 00/10] sparse-checkout: make set subsume init Elijah Newren via GitGitGadget
                         ` (5 preceding siblings ...)
  2021-12-14  4:09       ` [PATCH v4 06/10] sparse-checkout: enable `set` to initialize sparse-checkout mode Elijah Newren via GitGitGadget
@ 2021-12-14  4:09       ` Elijah Newren via GitGitGadget
  2021-12-23  0:47         ` Jiang Xin
  2021-12-14  4:09       ` [PATCH v4 08/10] git-sparse-checkout.txt: update to document init/set/reapply changes Elijah Newren via GitGitGadget
                         ` (2 subsequent siblings)
  9 siblings, 1 reply; 63+ messages in thread
From: Elijah Newren via GitGitGadget @ 2021-12-14  4:09 UTC (permalink / raw)
  To: git
  Cc: Derrick Stolee, Lessley Dennington, Victoria Dye, Elijah Newren,
	Elijah Newren, Elijah Newren

From: Elijah Newren <newren@gmail.com>

Folks may want to switch to or from cone mode, or to or from a
sparse-index without changing their sparsity paths.  Allow them to do so
using the reapply command.

Reviewed-by: Derrick Stolee <dstolee@microsoft.com>
Reviewed-by: Victoria Dye <vdye@github.com>
Signed-off-by: Elijah Newren <newren@gmail.com>
---
 builtin/sparse-checkout.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c
index 16daae84975..0dae44c5759 100644
--- a/builtin/sparse-checkout.c
+++ b/builtin/sparse-checkout.c
@@ -759,13 +759,22 @@ static int sparse_checkout_set(int argc, const char **argv, const char *prefix)
 }
 
 static char const * const builtin_sparse_checkout_reapply_usage[] = {
-	N_("git sparse-checkout reapply"),
+	N_("git sparse-checkout reapply [--[no-]cone] [--[no-]sparse-index] "),
 	NULL
 };
 
+static struct sparse_checkout_reapply_opts {
+	int cone_mode;
+	int sparse_index;
+} reapply_opts;
+
 static int sparse_checkout_reapply(int argc, const char **argv)
 {
 	static struct option builtin_sparse_checkout_reapply_options[] = {
+		OPT_BOOL(0, "cone", &reapply_opts.cone_mode,
+			 N_("initialize the sparse-checkout in cone mode")),
+		OPT_BOOL(0, "sparse-index", &reapply_opts.sparse_index,
+			 N_("toggle the use of a sparse index")),
 		OPT_END(),
 	};
 
@@ -777,6 +786,13 @@ static int sparse_checkout_reapply(int argc, const char **argv)
 			     builtin_sparse_checkout_reapply_usage, 0);
 
 	repo_read_index(the_repository);
+
+	reapply_opts.cone_mode = -1;
+	reapply_opts.sparse_index = -1;
+
+	if (update_modes(&reapply_opts.cone_mode, &reapply_opts.sparse_index))
+		return 1;
+
 	return update_working_directory(NULL);
 }
 
-- 
gitgitgadget


^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH v4 08/10] git-sparse-checkout.txt: update to document init/set/reapply changes
  2021-12-14  4:09     ` [PATCH v4 00/10] sparse-checkout: make set subsume init Elijah Newren via GitGitGadget
                         ` (6 preceding siblings ...)
  2021-12-14  4:09       ` [PATCH v4 07/10] sparse-checkout: enable reapply to take --[no-]{cone,sparse-index} Elijah Newren via GitGitGadget
@ 2021-12-14  4:09       ` Elijah Newren via GitGitGadget
  2021-12-14  4:09       ` [PATCH v4 09/10] Documentation: clarify/correct a few sparsity related statements Elijah Newren via GitGitGadget
  2021-12-14  4:09       ` [PATCH v4 10/10] clone: avoid using deprecated `sparse-checkout init` Elijah Newren via GitGitGadget
  9 siblings, 0 replies; 63+ messages in thread
From: Elijah Newren via GitGitGadget @ 2021-12-14  4:09 UTC (permalink / raw)
  To: git
  Cc: Derrick Stolee, Lessley Dennington, Victoria Dye, Elijah Newren,
	Elijah Newren, Elijah Newren

From: Elijah Newren <newren@gmail.com>

As noted in the previous commit, using separate `init` and `set` steps
with sparse-checkout result in a number of issues.  The previous commits
made `set` able to handle the work of both commands, and enabled reapply
to tweak the {cone,sparse-index} settings.  Update the documentation to
reflect this, and mark `init` as deprecated.

Reviewed-by: Derrick Stolee <dstolee@microsoft.com>
Reviewed-by: Victoria Dye <vdye@github.com>
Signed-off-by: Elijah Newren <newren@gmail.com>
---
 Documentation/git-sparse-checkout.txt | 98 +++++++++++++++------------
 1 file changed, 55 insertions(+), 43 deletions(-)

diff --git a/Documentation/git-sparse-checkout.txt b/Documentation/git-sparse-checkout.txt
index 42056ee9ff9..72fcb6b2acc 100644
--- a/Documentation/git-sparse-checkout.txt
+++ b/Documentation/git-sparse-checkout.txt
@@ -30,28 +30,36 @@ COMMANDS
 'list'::
 	Describe the patterns in the sparse-checkout file.
 
-'init'::
-	Enable the `core.sparseCheckout` setting. If the
-	sparse-checkout file does not exist, then populate it with
-	patterns that match every file in the root directory and
-	no other directories, then will remove all directories tracked
-	by Git. Add patterns to the sparse-checkout file to
-	repopulate the working directory.
+'set'::
+	Enable the necessary config settings
+	(extensions.worktreeConfig, core.sparseCheckout,
+	core.sparseCheckoutCone) if they are not already enabled, and
+	write a set of patterns to the sparse-checkout file from the
+	list of arguments following the 'set' subcommand. Update the
+	working directory to match the new patterns.
 +
-To avoid interfering with other worktrees, it first enables the
-`extensions.worktreeConfig` setting and makes sure to set the
-`core.sparseCheckout` setting in the worktree-specific config file.
+When the `--stdin` option is provided, the patterns are read from
+standard in as a newline-delimited list instead of from the arguments.
 +
-When `--cone` is provided, the `core.sparseCheckoutCone` setting is
-also set, allowing for better performance with a limited set of
-patterns (see 'CONE PATTERN SET' below).
+When `--cone` is passed or `core.sparseCheckoutCone` is enabled, the
+input list is considered a list of directories instead of
+sparse-checkout patterns.  This allows for better performance with a
+limited set of patterns (see 'CONE PATTERN SET' below).  Note that the
+set command will write patterns to the sparse-checkout file to include
+all files contained in those directories (recursively) as well as
+files that are siblings of ancestor 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.  This may become the default in the future; --no-cone
+can be passed to request non-cone mode.
 +
-Use the `--[no-]sparse-index` option to toggle the use of the sparse
-index format. This reduces the size of the index to be more closely
-aligned with your sparse-checkout definition. This can have significant
-performance advantages for commands such as `git status` or `git add`.
-This feature is still experimental. Some commands might be slower with
-a sparse index until they are properly integrated with the feature.
+Use the `--[no-]sparse-index` option to use a sparse index (the
+default is to not use it).  A sparse index reduces the size of the
+index to be more closely aligned with your sparse-checkout
+definition. This can have significant performance advantages for
+commands such as `git status` or `git add`.  This feature is still
+experimental. Some commands might be slower with a sparse index until
+they are properly integrated with the feature.
 +
 **WARNING:** Using a sparse index requires modifying the index in a way
 that is not completely understood by external tools. If you have trouble
@@ -60,23 +68,6 @@ to rewrite your index to not be sparse. Older versions of Git will not
 understand the sparse directory entries index extension and may fail to
 interact with your repository until it is disabled.
 
-'set'::
-	Write a set of patterns to the sparse-checkout file, as given as
-	a list of arguments following the 'set' subcommand. Update the
-	working directory to match the new patterns. Enable the
-	core.sparseCheckout config setting if it is not already enabled.
-+
-When the `--stdin` option is provided, the patterns are read from
-standard in as a newline-delimited list instead of from the arguments.
-+
-When `core.sparseCheckoutCone` is enabled, the input list is considered a
-list of directories instead of sparse-checkout patterns. The command writes
-patterns to the sparse-checkout file to include all files contained in those
-directories (recursively) as well as files that are siblings of ancestor
-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,
@@ -93,12 +84,35 @@ C-style quoted strings.
 	cases, it can make sense to run `git sparse-checkout reapply` later
 	after cleaning up affected paths (e.g. resolving conflicts, undoing
 	or committing changes, etc.).
++
+The `reapply` command can also take `--[no-]cone` and `--[no-]sparse-index`
+flags, with the same meaning as the flags from the `set` command, in order
+to change which sparsity mode you are using without needing to also respecify
+all sparsity paths.
 
 'disable'::
 	Disable the `core.sparseCheckout` config setting, and restore the
-	working directory to include all files. Leaves the sparse-checkout
-	file intact so a later 'git sparse-checkout init' command may
-	return the working directory to the same state.
+	working directory to include all files.
+
+'init'::
+	Deprecated command that behaves like `set` with no specified paths.
+	May be removed in the future.
++
+Historically, `set` did not handle all the necessary config settings,
+which meant that both `init` and `set` had to be called.  Invoking
+both meant the `init` step would first remove nearly all tracked files
+(and in cone mode, ignored files too), then the `set` step would add
+many of the tracked files (but not ignored files) back.  In addition
+to the lost files, the performance and UI of this combination was
+poor.
++
+Also, historically, `init` would not actually initialize the
+sparse-checkout file if it already existed.  This meant it was
+possible to return to a sparse-checkout without remembering which
+paths to pass to a subsequent 'set' or 'add' command.  However,
+`--cone` and `--sparse-index` options would not be remembered across
+the disable command, so the easy restore of calling a plain `init`
+decreased in utility.
 
 SPARSE CHECKOUT
 ---------------
@@ -117,10 +131,8 @@ directory, it updates the skip-worktree bits in the index based
 on this file. The files matching the patterns in the file will
 appear in the working directory, and the rest will not.
 
-To enable the sparse-checkout feature, run `git sparse-checkout init` to
-initialize a simple sparse-checkout file and enable the `core.sparseCheckout`
-config setting. Then, run `git sparse-checkout set` to modify the patterns in
-the sparse-checkout file.
+To enable the sparse-checkout feature, run `git sparse-checkout set` to
+set the patterns you want to use.
 
 To repopulate the working directory with all files, use the
 `git sparse-checkout disable` command.
-- 
gitgitgadget


^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH v4 09/10] Documentation: clarify/correct a few sparsity related statements
  2021-12-14  4:09     ` [PATCH v4 00/10] sparse-checkout: make set subsume init Elijah Newren via GitGitGadget
                         ` (7 preceding siblings ...)
  2021-12-14  4:09       ` [PATCH v4 08/10] git-sparse-checkout.txt: update to document init/set/reapply changes Elijah Newren via GitGitGadget
@ 2021-12-14  4:09       ` Elijah Newren via GitGitGadget
  2021-12-14  7:38         ` Bagas Sanjaya
  2021-12-14  4:09       ` [PATCH v4 10/10] clone: avoid using deprecated `sparse-checkout init` Elijah Newren via GitGitGadget
  9 siblings, 1 reply; 63+ messages in thread
From: Elijah Newren via GitGitGadget @ 2021-12-14  4:09 UTC (permalink / raw)
  To: git
  Cc: Derrick Stolee, Lessley Dennington, Victoria Dye, Elijah Newren,
	Elijah Newren, Elijah Newren

From: Elijah Newren <newren@gmail.com>

Reviewed-by: Derrick Stolee <dstolee@microsoft.com>
Reviewed-by: Victoria Dye <vdye@github.com>
Signed-off-by: Elijah Newren <newren@gmail.com>
---
 Documentation/git-clone.txt           | 8 ++++----
 Documentation/git-sparse-checkout.txt | 2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt
index 3fe3810f1ce..b348a71fc68 100644
--- a/Documentation/git-clone.txt
+++ b/Documentation/git-clone.txt
@@ -167,10 +167,10 @@ objects from the source repository into a pack in the cloned repository.
 	configuration variables are created.
 
 --sparse::
-	Initialize the sparse-checkout file so the working
-	directory starts with only the files in the root
-	of the repository. The sparse-checkout file can be
-	modified to grow the working directory as needed.
+	Employ a sparse-checkout, with only files in the toplevel
+	directory initially being present.  The
+	linkgit:git-sparse-checkout[1] command can be used to grow the
+	working directory as needed.
 
 --filter=<filter-spec>::
 	Use the partial clone feature and request that the server sends
diff --git a/Documentation/git-sparse-checkout.txt b/Documentation/git-sparse-checkout.txt
index 72fcb6b2acc..9a4b43c105b 100644
--- a/Documentation/git-sparse-checkout.txt
+++ b/Documentation/git-sparse-checkout.txt
@@ -121,7 +121,7 @@ SPARSE CHECKOUT
 It uses the skip-worktree bit (see linkgit:git-update-index[1]) to tell
 Git whether a file in the working directory is worth looking at. If
 the skip-worktree bit is set, then the file is ignored in the working
-directory. Git will not populate the contents of those files, which
+directory. Git will avoid populating the contents of those files, which
 makes a sparse checkout helpful when working in a repository with many
 files, but only a few are important to the current user.
 
-- 
gitgitgadget


^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH v4 10/10] clone: avoid using deprecated `sparse-checkout init`
  2021-12-14  4:09     ` [PATCH v4 00/10] sparse-checkout: make set subsume init Elijah Newren via GitGitGadget
                         ` (8 preceding siblings ...)
  2021-12-14  4:09       ` [PATCH v4 09/10] Documentation: clarify/correct a few sparsity related statements Elijah Newren via GitGitGadget
@ 2021-12-14  4:09       ` Elijah Newren via GitGitGadget
  9 siblings, 0 replies; 63+ messages in thread
From: Elijah Newren via GitGitGadget @ 2021-12-14  4:09 UTC (permalink / raw)
  To: git
  Cc: Derrick Stolee, Lessley Dennington, Victoria Dye, Elijah Newren,
	Elijah Newren, Elijah Newren

From: Elijah Newren <newren@gmail.com>

The previous commits marked `sparse-checkout init` as deprecated; we
can just use `set` instead here and pass it no paths.

Reviewed-by: Derrick Stolee <dstolee@microsoft.com>
Reviewed-by: Victoria Dye <vdye@github.com>
Signed-off-by: Elijah Newren <newren@gmail.com>
---
 builtin/clone.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/clone.c b/builtin/clone.c
index fb377b27657..5bed37f8b51 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -633,7 +633,7 @@ static int git_sparse_checkout_init(const char *repo)
 {
 	struct strvec argv = STRVEC_INIT;
 	int result = 0;
-	strvec_pushl(&argv, "-C", repo, "sparse-checkout", "init", NULL);
+	strvec_pushl(&argv, "-C", repo, "sparse-checkout", "set", NULL);
 
 	/*
 	 * We must apply the setting in the current process
-- 
gitgitgadget

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* Re: [PATCH v4 09/10] Documentation: clarify/correct a few sparsity related statements
  2021-12-14  4:09       ` [PATCH v4 09/10] Documentation: clarify/correct a few sparsity related statements Elijah Newren via GitGitGadget
@ 2021-12-14  7:38         ` Bagas Sanjaya
  2021-12-14  7:48           ` Elijah Newren
  0 siblings, 1 reply; 63+ messages in thread
From: Bagas Sanjaya @ 2021-12-14  7:38 UTC (permalink / raw)
  To: Elijah Newren via GitGitGadget, git
  Cc: Derrick Stolee, Lessley Dennington, Victoria Dye, Elijah Newren

On 14/12/21 11.09, Elijah Newren via GitGitGadget wrote:
>   --sparse::
> -	Initialize the sparse-checkout file so the working
> -	directory starts with only the files in the root
> -	of the repository. The sparse-checkout file can be
> -	modified to grow the working directory as needed.
> +	Employ a sparse-checkout, with only files in the toplevel
> +	directory initially being present.  The
> +	linkgit:git-sparse-checkout[1] command can be used to grow the
> +	working directory as needed.
>   

s/toplevel/top-level/

>   It uses the skip-worktree bit (see linkgit:git-update-index[1]) to tell
>   Git whether a file in the working directory is worth looking at. If
>   the skip-worktree bit is set, then the file is ignored in the working
> -directory. Git will not populate the contents of those files, which
> +directory. Git will avoid populating the contents of those files, which
>   makes a sparse checkout helpful when working in a repository with many
>   files, but only a few are important to the current user.
>   

Looks OK.

-- 
An old man doll... just what I always wanted! - Clara

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [PATCH v4 09/10] Documentation: clarify/correct a few sparsity related statements
  2021-12-14  7:38         ` Bagas Sanjaya
@ 2021-12-14  7:48           ` Elijah Newren
  0 siblings, 0 replies; 63+ messages in thread
From: Elijah Newren @ 2021-12-14  7:48 UTC (permalink / raw)
  To: Bagas Sanjaya
  Cc: Elijah Newren via GitGitGadget, Git Mailing List, Derrick Stolee,
	Lessley Dennington, Victoria Dye

On Mon, Dec 13, 2021 at 11:38 PM Bagas Sanjaya <bagasdotme@gmail.com> wrote:
>
> On 14/12/21 11.09, Elijah Newren via GitGitGadget wrote:
> >   --sparse::
> > -     Initialize the sparse-checkout file so the working
> > -     directory starts with only the files in the root
> > -     of the repository. The sparse-checkout file can be
> > -     modified to grow the working directory as needed.
> > +     Employ a sparse-checkout, with only files in the toplevel
> > +     directory initially being present.  The
> > +     linkgit:git-sparse-checkout[1] command can be used to grow the
> > +     working directory as needed.
> >
>
> s/toplevel/top-level/

No, the only occurrence of either term in Documentation/glossary.txt
(though used incidentally) is 'toplevel' rather than 'top-level', git
rev-parse has a --show-toplevel flag with no hyphen between top and
level, and the occurrences of 'toplevel' in the codebase from a quick
grep outnumber top-level 2 to 1.  I'll keep 'toplevel'.

> >   It uses the skip-worktree bit (see linkgit:git-update-index[1]) to tell
> >   Git whether a file in the working directory is worth looking at. If
> >   the skip-worktree bit is set, then the file is ignored in the working
> > -directory. Git will not populate the contents of those files, which
> > +directory. Git will avoid populating the contents of those files, which
> >   makes a sparse checkout helpful when working in a repository with many
> >   files, but only a few are important to the current user.
> >
>
> Looks OK.

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [PATCH v4 07/10] sparse-checkout: enable reapply to take --[no-]{cone,sparse-index}
  2021-12-14  4:09       ` [PATCH v4 07/10] sparse-checkout: enable reapply to take --[no-]{cone,sparse-index} Elijah Newren via GitGitGadget
@ 2021-12-23  0:47         ` Jiang Xin
  2021-12-23 17:09           ` Elijah Newren
  0 siblings, 1 reply; 63+ messages in thread
From: Jiang Xin @ 2021-12-23  0:47 UTC (permalink / raw)
  To: Elijah Newren via GitGitGadget
  Cc: Git List, Derrick Stolee, Lessley Dennington, Victoria Dye,
	Elijah Newren

On Tue, Dec 14, 2021 at 3:24 PM Elijah Newren via GitGitGadget
<gitgitgadget@gmail.com> wrote:
>
> From: Elijah Newren <newren@gmail.com>
>
> Folks may want to switch to or from cone mode, or to or from a
> sparse-index without changing their sparsity paths.  Allow them to do so
> using the reapply command.
>
> Reviewed-by: Derrick Stolee <dstolee@microsoft.com>
> Reviewed-by: Victoria Dye <vdye@github.com>
> Signed-off-by: Elijah Newren <newren@gmail.com>
> ---
>  builtin/sparse-checkout.c | 18 +++++++++++++++++-
>  1 file changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c
> index 16daae84975..0dae44c5759 100644
> --- a/builtin/sparse-checkout.c
> +++ b/builtin/sparse-checkout.c
> @@ -759,13 +759,22 @@ static int sparse_checkout_set(int argc, const char **argv, const char *prefix)
>  }
>
>  static char const * const builtin_sparse_checkout_reapply_usage[] = {
> -       N_("git sparse-checkout reapply"),
> +       N_("git sparse-checkout reapply [--[no-]cone] [--[no-]sparse-index] "),

Found a trailing space in [1], which came from this commit.

[1]: https://github.com/git-l10n/git-po/blob/pot/next/2021-12-22.diff#L19

--
Jiang Xin

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [PATCH v4 07/10] sparse-checkout: enable reapply to take --[no-]{cone,sparse-index}
  2021-12-23  0:47         ` Jiang Xin
@ 2021-12-23 17:09           ` Elijah Newren
  2021-12-23 19:09             ` Junio C Hamano
  0 siblings, 1 reply; 63+ messages in thread
From: Elijah Newren @ 2021-12-23 17:09 UTC (permalink / raw)
  To: Jiang Xin
  Cc: Elijah Newren via GitGitGadget, Git List, Derrick Stolee,
	Lessley Dennington, Victoria Dye

On Wed, Dec 22, 2021 at 4:48 PM Jiang Xin <worldhello.net@gmail.com> wrote:
>
> On Tue, Dec 14, 2021 at 3:24 PM Elijah Newren via GitGitGadget
> <gitgitgadget@gmail.com> wrote:
> >
...
> >  static char const * const builtin_sparse_checkout_reapply_usage[] = {
> > -       N_("git sparse-checkout reapply"),
> > +       N_("git sparse-checkout reapply [--[no-]cone] [--[no-]sparse-index] "),
>
> Found a trailing space in [1], which came from this commit.
>
> [1]: https://github.com/git-l10n/git-po/blob/pot/next/2021-12-22.diff#L19

Sorry about that.  This series has already hit next, so I submitted a
new patch to correct this
(https://lore.kernel.org/git/pull.1106.git.1640279223893.gitgitgadget@gmail.com/).

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [PATCH v4 07/10] sparse-checkout: enable reapply to take --[no-]{cone,sparse-index}
  2021-12-23 17:09           ` Elijah Newren
@ 2021-12-23 19:09             ` Junio C Hamano
  0 siblings, 0 replies; 63+ messages in thread
From: Junio C Hamano @ 2021-12-23 19:09 UTC (permalink / raw)
  To: Elijah Newren
  Cc: Jiang Xin, Elijah Newren via GitGitGadget, Git List,
	Derrick Stolee, Lessley Dennington, Victoria Dye

Elijah Newren <newren@gmail.com> writes:

> On Wed, Dec 22, 2021 at 4:48 PM Jiang Xin <worldhello.net@gmail.com> wrote:
>>
>> On Tue, Dec 14, 2021 at 3:24 PM Elijah Newren via GitGitGadget
>> <gitgitgadget@gmail.com> wrote:
>> >
> ...
>> >  static char const * const builtin_sparse_checkout_reapply_usage[] = {
>> > -       N_("git sparse-checkout reapply"),
>> > +       N_("git sparse-checkout reapply [--[no-]cone] [--[no-]sparse-index] "),
>>
>> Found a trailing space in [1], which came from this commit.
>>
>> [1]: https://github.com/git-l10n/git-po/blob/pot/next/2021-12-22.diff#L19
>
> Sorry about that.  This series has already hit next, so I submitted a
> new patch to correct this
> (https://lore.kernel.org/git/pull.1106.git.1640279223893.gitgitgadget@gmail.com/).

Thanks, both.  Will take a look.

^ permalink raw reply	[flat|nested] 63+ messages in thread

end of thread, other threads:[~2021-12-23 19:09 UTC | newest]

Thread overview: 63+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-04 20:00 [PATCH 0/6] sparse-checkout: make set subsume init Elijah Newren via GitGitGadget
2021-12-04 20:00 ` [PATCH 1/6] sparse-checkout: pass use_stdin as a parameter instead of as a global Elijah Newren via GitGitGadget
2021-12-04 20:00 ` [PATCH 2/6] sparse-checkout: break apart functions for sparse_checkout_(set|add) Elijah Newren via GitGitGadget
2021-12-04 20:00 ` [PATCH 3/6] sparse-checkout: enable `set` to initialize sparse-checkout mode Elijah Newren via GitGitGadget
2021-12-04 21:40   ` Victoria Dye
2021-12-07 17:04     ` Elijah Newren
2021-12-07 20:36       ` Lessley Dennington
2021-12-07 16:42   ` Derrick Stolee
2021-12-07 18:18     ` Elijah Newren
2021-12-04 20:00 ` [PATCH 4/6] git-sparse-checkout.txt: update to document that set handles init Elijah Newren via GitGitGadget
2021-12-04 21:48   ` Victoria Dye
2021-12-07 16:45     ` Derrick Stolee
2021-12-07 17:06     ` Elijah Newren
2021-12-07 20:42   ` Lessley Dennington
2021-12-07 21:13     ` Elijah Newren
2021-12-07 22:36       ` Lessley Dennington
2021-12-04 20:00 ` [PATCH 5/6] Documentation: clarify/correct a few sparsity related statements Elijah Newren via GitGitGadget
2021-12-04 20:00 ` [PATCH 6/6] clone: avoid using deprecated `sparse-checkout init` Elijah Newren via GitGitGadget
2021-12-07 16:48 ` [PATCH 0/6] sparse-checkout: make set subsume init Derrick Stolee
2021-12-07 20:20 ` [PATCH v2 00/10] " Elijah Newren via GitGitGadget
2021-12-07 20:20   ` [PATCH v2 01/10] sparse-checkout: pass use_stdin as a parameter instead of as a global Elijah Newren via GitGitGadget
2021-12-07 20:20   ` [PATCH v2 02/10] sparse-checkout: break apart functions for sparse_checkout_(set|add) Elijah Newren via GitGitGadget
2021-12-07 20:20   ` [PATCH v2 03/10] sparse-checkout: add sanity-checks on initial sparsity state Elijah Newren via GitGitGadget
2021-12-07 20:20   ` [PATCH v2 04/10] sparse-checkout: disallow --no-stdin as an argument to set Elijah Newren via GitGitGadget
2021-12-07 20:20   ` [PATCH v2 05/10] sparse-checkout: split out code for tweaking settings config Elijah Newren via GitGitGadget
2021-12-07 20:20   ` [PATCH v2 06/10] sparse-checkout: enable `set` to initialize sparse-checkout mode Elijah Newren via GitGitGadget
2021-12-07 20:20   ` [PATCH v2 07/10] sparse-checkout: enable reapply to take --[no-]{cone,sparse-index} Elijah Newren via GitGitGadget
2021-12-07 20:20   ` [PATCH v2 08/10] git-sparse-checkout.txt: update to document init/set/reapply changes Elijah Newren via GitGitGadget
2021-12-07 20:20   ` [PATCH v2 09/10] Documentation: clarify/correct a few sparsity related statements Elijah Newren via GitGitGadget
2021-12-07 20:20   ` [PATCH v2 10/10] clone: avoid using deprecated `sparse-checkout init` Elijah Newren via GitGitGadget
2021-12-07 21:05   ` [PATCH v2 00/10] sparse-checkout: make set subsume init Derrick Stolee
2021-12-08 14:16   ` Victoria Dye
2021-12-10  3:56   ` [PATCH v3 " Elijah Newren via GitGitGadget
2021-12-10  3:56     ` [PATCH v3 01/10] sparse-checkout: pass use_stdin as a parameter instead of as a global Elijah Newren via GitGitGadget
2021-12-10  3:56     ` [PATCH v3 02/10] sparse-checkout: break apart functions for sparse_checkout_(set|add) Elijah Newren via GitGitGadget
2021-12-10  3:56     ` [PATCH v3 03/10] sparse-checkout: add sanity-checks on initial sparsity state Elijah Newren via GitGitGadget
2021-12-13 18:11       ` Junio C Hamano
2021-12-14  2:25         ` Elijah Newren
2021-12-10  3:56     ` [PATCH v3 04/10] sparse-checkout: disallow --no-stdin as an argument to set Elijah Newren via GitGitGadget
2021-12-10  3:56     ` [PATCH v3 05/10] sparse-checkout: split out code for tweaking settings config Elijah Newren via GitGitGadget
2021-12-10  3:56     ` [PATCH v3 06/10] sparse-checkout: enable `set` to initialize sparse-checkout mode Elijah Newren via GitGitGadget
2021-12-10  3:56     ` [PATCH v3 07/10] sparse-checkout: enable reapply to take --[no-]{cone,sparse-index} Elijah Newren via GitGitGadget
2021-12-13 18:23       ` Junio C Hamano
2021-12-14  2:39         ` Elijah Newren
2021-12-10  3:56     ` [PATCH v3 08/10] git-sparse-checkout.txt: update to document init/set/reapply changes Elijah Newren via GitGitGadget
2021-12-10  3:56     ` [PATCH v3 09/10] Documentation: clarify/correct a few sparsity related statements Elijah Newren via GitGitGadget
2021-12-10  3:56     ` [PATCH v3 10/10] clone: avoid using deprecated `sparse-checkout init` Elijah Newren via GitGitGadget
2021-12-14  4:09     ` [PATCH v4 00/10] sparse-checkout: make set subsume init Elijah Newren via GitGitGadget
2021-12-14  4:09       ` [PATCH v4 01/10] sparse-checkout: pass use_stdin as a parameter instead of as a global Elijah Newren via GitGitGadget
2021-12-14  4:09       ` [PATCH v4 02/10] sparse-checkout: break apart functions for sparse_checkout_(set|add) Elijah Newren via GitGitGadget
2021-12-14  4:09       ` [PATCH v4 03/10] sparse-checkout: add sanity-checks on initial sparsity state Elijah Newren via GitGitGadget
2021-12-14  4:09       ` [PATCH v4 04/10] sparse-checkout: disallow --no-stdin as an argument to set Elijah Newren via GitGitGadget
2021-12-14  4:09       ` [PATCH v4 05/10] sparse-checkout: split out code for tweaking settings config Elijah Newren via GitGitGadget
2021-12-14  4:09       ` [PATCH v4 06/10] sparse-checkout: enable `set` to initialize sparse-checkout mode Elijah Newren via GitGitGadget
2021-12-14  4:09       ` [PATCH v4 07/10] sparse-checkout: enable reapply to take --[no-]{cone,sparse-index} Elijah Newren via GitGitGadget
2021-12-23  0:47         ` Jiang Xin
2021-12-23 17:09           ` Elijah Newren
2021-12-23 19:09             ` Junio C Hamano
2021-12-14  4:09       ` [PATCH v4 08/10] git-sparse-checkout.txt: update to document init/set/reapply changes Elijah Newren via GitGitGadget
2021-12-14  4:09       ` [PATCH v4 09/10] Documentation: clarify/correct a few sparsity related statements Elijah Newren via GitGitGadget
2021-12-14  7:38         ` Bagas Sanjaya
2021-12-14  7:48           ` Elijah Newren
2021-12-14  4:09       ` [PATCH v4 10/10] clone: avoid using deprecated `sparse-checkout init` Elijah Newren via GitGitGadget

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).