Subject: Re: [PATCH v5 02/17] sparse-checkout: create 'init' subcommand

From: SZEDER Gábor <[email-masked]>
Date: Thu, 21 Nov 2019 17:37:06 +0100

On Thu, Nov 21, 2019 at 10:36:10AM -0500, Derrick Stolee wrote:
> > But wait, I thought that only changes to files that are excluded from
> > the sparse-checkout are thrown away, but as it turns out it throws
> > away changes to files that are included in the sparse-checkout:

For completeness, 'git sparse-checkout disable' throws away staged
changes as well, as it, too, runs 'git read-tree -um HEAD' (or its
equivalent).

> Thanks for the additional details.
> 
> This series intended to make the existing sparse-checkout behavior
> more useful to users by not requiring manual edits of the sparse-checkout
> file followed by 'git read-tree' commands. However, there do appear
> to be some serious improvements that we can make in the future.
> 
> Keeping staged changes seems important, and we can address that in
> the near future.

I think that at least for now 'git sparse-checkout' should flat out
refuse to init/set/disable if the working tree is not clean (but still
allow 'list', as that's a read-only operation), like the patch below.
Yeah, that way it wouldn't work in cases that appear to be safe
(unstaged changes), but it would prevent the data loss until we
carefully consider the circumstances under which these operations can
be safely allowed.

  -- >8 --

diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c
index 76f65d8edd..4b05625c4c 100644
--- a/builtin/sparse-checkout.c
+++ b/builtin/sparse-checkout.c
@@ -12,6 +12,7 @@
 #include "lockfile.h"
 #include "resolve-undo.h"
 #include "unpack-trees.h"
+#include "wt-status.h"
 
 static char const * const builtin_sparse_checkout_usage[] = {
 	N_("git sparse-checkout (init|list|set|disable) <options>"),
@@ -188,6 +189,10 @@ static int sparse_checkout_init(int argc, const char **argv)
 			     builtin_sparse_checkout_init_options,
 			     builtin_sparse_checkout_init_usage, 0);
 
+	repo_read_index(the_repository);
+	require_clean_work_tree(the_repository,
+				N_("initialize sparse-checkout"), NULL, 1, 0);
+
 	if (init_opts.cone_mode) {
 		mode = MODE_CONE_PATTERNS;
 		core_sparse_checkout_cone = 1;
@@ -386,6 +391,10 @@ static int sparse_checkout_set(int argc, const char **argv, const char *prefix)
 			     builtin_sparse_checkout_set_usage,
 			     PARSE_OPT_KEEP_UNKNOWN);
 
+	repo_read_index(the_repository);
+	require_clean_work_tree(the_repository,
+				N_("set up sparse-checkout"), NULL, 1, 0);
+
 	if (core_sparse_checkout_cone) {
 		struct strbuf line = STRBUF_INIT;
 		hashmap_init(&pl.recursive_hashmap, pl_hashmap_cmp, NULL, 0);
@@ -437,6 +446,10 @@ static int sparse_checkout_disable(int argc, const char **argv)
 	char *sparse_filename;
 	FILE *fp;
 
+	repo_read_index(the_repository);
+	require_clean_work_tree(the_repository,
+				N_("disable sparse-checkout"), NULL, 1, 0);
+
 	if (sc_set_config(MODE_ALL_PATTERNS))
 		die(_("failed to change config"));