diff mbox series

[v5,06/17] sparse-checkout: create 'disable' subcommand

Message ID c48535cd5c00310403ffccac7507f399fca5a8a4.1571666187.git.gitgitgadget@gmail.com (mailing list archive)
State New, archived
Headers show
Series New sparse-checkout builtin and "cone" mode | expand

Commit Message

John Passaro via GitGitGadget Oct. 21, 2019, 1:56 p.m. UTC
From: Derrick Stolee <dstolee@microsoft.com>

The instructions for disabling a sparse-checkout to a full
working directory are complicated and non-intuitive. Add a
subcommand, 'git sparse-checkout disable', to perform those
steps for the user.

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
---
 Documentation/git-sparse-checkout.txt | 27 ++++++++++++---------------
 builtin/sparse-checkout.c             | 26 +++++++++++++++++++++++++-
 t/t1091-sparse-checkout-builtin.sh    | 15 +++++++++++++++
 3 files changed, 52 insertions(+), 16 deletions(-)

Comments

SZEDER Gábor Nov. 19, 2019, 4:15 p.m. UTC | #1
On Mon, Oct 21, 2019 at 01:56:15PM +0000, Derrick Stolee via GitGitGadget wrote:
> From: Derrick Stolee <dstolee@microsoft.com>
> 
> The instructions for disabling a sparse-checkout to a full
> working directory are complicated and non-intuitive. Add a
> subcommand, 'git sparse-checkout disable', to perform those
> steps for the user.
> 
> Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
> ---
>  Documentation/git-sparse-checkout.txt | 27 ++++++++++++---------------
>  builtin/sparse-checkout.c             | 26 +++++++++++++++++++++++++-
>  t/t1091-sparse-checkout-builtin.sh    | 15 +++++++++++++++
>  3 files changed, 52 insertions(+), 16 deletions(-)
> 
> diff --git a/Documentation/git-sparse-checkout.txt b/Documentation/git-sparse-checkout.txt
> index b933043b3d..f794d4797a 100644
> --- a/Documentation/git-sparse-checkout.txt
> +++ b/Documentation/git-sparse-checkout.txt
> @@ -48,6 +48,10 @@ To avoid interfering with other worktrees, it first enables the
>  	working directory to match the new patterns. Enable the
>  	core.sparseCheckout config setting if it is not already enabled.
>  
> +'disable'::
> +	Remove the sparse-checkout file, set `core.sparseCheckout` to
> +	`false`, and restore the working directory to include all files.

I think it would make sense to leave the sparse-checkout file behind
as-is, and only update the coonfiguration and the working tree.  That
way users could quickly go back to their last sparse checkout setup by
simply running 'git sparse-checkout init'.

  
> +static int sparse_checkout_disable(int argc, const char **argv)
> +{
> +	char *sparse_filename;
> +	FILE *fp;
> +
> +	if (sc_set_config(MODE_ALL_PATTERNS))
> +		die(_("failed to change config"));
> +
> +	sparse_filename = get_sparse_checkout_filename();
> +	fp = xfopen(sparse_filename, "w");
> +	fprintf(fp, "/*\n");
> +	fclose(fp);
> +
> +	if (update_working_directory())
> +		die(_("error while refreshing working directory"));
> +
> +	unlink(sparse_filename);
> +	free(sparse_filename);
> +
> +	return sc_set_config(MODE_NO_PATTERNS);

Hrm.  So disabling the sparse-checkout calls the same helper function
to write the configuration as initializing or setting, but with a
different parameter.  However, the error message in that function is:

  error(_("failed to enable core.sparseCheckout"));

So if something goes wrong while disabling the sparse-checkout, the
user might get an error saying "error: failed to enable
core.sparseCheckout".  That doesn't look quite right, does it?

> +}
Derrick Stolee Nov. 21, 2019, 1:17 p.m. UTC | #2
On 11/19/2019 11:15 AM, SZEDER Gábor wrote:
> On Mon, Oct 21, 2019 at 01:56:15PM +0000, Derrick Stolee via GitGitGadget wrote:
>> From: Derrick Stolee <dstolee@microsoft.com>
>>
>> The instructions for disabling a sparse-checkout to a full
>> working directory are complicated and non-intuitive. Add a
>> subcommand, 'git sparse-checkout disable', to perform those
>> steps for the user.
>>
>> Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
>> ---
>>  Documentation/git-sparse-checkout.txt | 27 ++++++++++++---------------
>>  builtin/sparse-checkout.c             | 26 +++++++++++++++++++++++++-
>>  t/t1091-sparse-checkout-builtin.sh    | 15 +++++++++++++++
>>  3 files changed, 52 insertions(+), 16 deletions(-)
>>
>> diff --git a/Documentation/git-sparse-checkout.txt b/Documentation/git-sparse-checkout.txt
>> index b933043b3d..f794d4797a 100644
>> --- a/Documentation/git-sparse-checkout.txt
>> +++ b/Documentation/git-sparse-checkout.txt
>> @@ -48,6 +48,10 @@ To avoid interfering with other worktrees, it first enables the
>>  	working directory to match the new patterns. Enable the
>>  	core.sparseCheckout config setting if it is not already enabled.
>>  
>> +'disable'::
>> +	Remove the sparse-checkout file, set `core.sparseCheckout` to
>> +	`false`, and restore the working directory to include all files.
> 
> I think it would make sense to leave the sparse-checkout file behind
> as-is, and only update the coonfiguration and the working tree.  That
> way users could quickly go back to their last sparse checkout setup by
> simply running 'git sparse-checkout init'.
> 
>   
>> +static int sparse_checkout_disable(int argc, const char **argv)
>> +{
>> +	char *sparse_filename;
>> +	FILE *fp;
>> +
>> +	if (sc_set_config(MODE_ALL_PATTERNS))
>> +		die(_("failed to change config"));
>> +
>> +	sparse_filename = get_sparse_checkout_filename();
>> +	fp = xfopen(sparse_filename, "w");
>> +	fprintf(fp, "/*\n");
>> +	fclose(fp);
>> +
>> +	if (update_working_directory())
>> +		die(_("error while refreshing working directory"));
>> +
>> +	unlink(sparse_filename);
>> +	free(sparse_filename);
>> +
>> +	return sc_set_config(MODE_NO_PATTERNS);
> 
> Hrm.  So disabling the sparse-checkout calls the same helper function
> to write the configuration as initializing or setting, but with a
> different parameter.  However, the error message in that function is:
> 
>   error(_("failed to enable core.sparseCheckout"));
> 
> So if something goes wrong while disabling the sparse-checkout, the
> user might get an error saying "error: failed to enable
> core.sparseCheckout".  That doesn't look quite right, does it?

Both of your comments are valid, but will be easier to implement
later in the series, after "sparse-checkout: update working directory
in-process" which previously did not interact with the "disable"
command. I'll add a new commit that adds that integration point
and should resolve your concerns raised here.

-Stolee
diff mbox series

Patch

diff --git a/Documentation/git-sparse-checkout.txt b/Documentation/git-sparse-checkout.txt
index b933043b3d..f794d4797a 100644
--- a/Documentation/git-sparse-checkout.txt
+++ b/Documentation/git-sparse-checkout.txt
@@ -48,6 +48,10 @@  To avoid interfering with other worktrees, it first enables the
 	working directory to match the new patterns. Enable the
 	core.sparseCheckout config setting if it is not already enabled.
 
+'disable'::
+	Remove the sparse-checkout file, set `core.sparseCheckout` to
+	`false`, and restore the working directory to include all files.
+
 SPARSE CHECKOUT
 ---------------
 
@@ -65,6 +69,14 @@  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 repopulate the working directory with all files, use the
+`git sparse-checkout disable` command.
+
 ## FULL PATTERN SET
 
 By default, the sparse-checkout file uses the same syntax as `.gitignore`
@@ -79,21 +91,6 @@  using negative patterns. For example, to remove the file `unwanted`:
 !unwanted
 ----------------
 
-Another tricky thing is fully repopulating the working directory when you
-no longer want sparse checkout. You cannot just disable "sparse
-checkout" because skip-worktree bits are still in the index and your working
-directory is still sparsely populated. You should re-populate the working
-directory with the `$GIT_DIR/info/sparse-checkout` file content as
-follows:
-
-----------------
-/*
-----------------
-
-Then you can disable sparse checkout. Sparse checkout support in 'git
-checkout' and similar commands is disabled by default. You need to
-set `core.sparseCheckout` to `true` in order to have sparse checkout
-support.
 
 SEE ALSO
 --------
diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c
index f2e2bd772d..9fdcc6c4ef 100644
--- a/builtin/sparse-checkout.c
+++ b/builtin/sparse-checkout.c
@@ -8,7 +8,7 @@ 
 #include "strbuf.h"
 
 static char const * const builtin_sparse_checkout_usage[] = {
-	N_("git sparse-checkout (init|list|set) <options>"),
+	N_("git sparse-checkout (init|list|set|disable) <options>"),
 	NULL
 };
 
@@ -212,6 +212,28 @@  static int sparse_checkout_set(int argc, const char **argv, const char *prefix)
 	return result;
 }
 
+static int sparse_checkout_disable(int argc, const char **argv)
+{
+	char *sparse_filename;
+	FILE *fp;
+
+	if (sc_set_config(MODE_ALL_PATTERNS))
+		die(_("failed to change config"));
+
+	sparse_filename = get_sparse_checkout_filename();
+	fp = xfopen(sparse_filename, "w");
+	fprintf(fp, "/*\n");
+	fclose(fp);
+
+	if (update_working_directory())
+		die(_("error while refreshing working directory"));
+
+	unlink(sparse_filename);
+	free(sparse_filename);
+
+	return sc_set_config(MODE_NO_PATTERNS);
+}
+
 int cmd_sparse_checkout(int argc, const char **argv, const char *prefix)
 {
 	static struct option builtin_sparse_checkout_options[] = {
@@ -236,6 +258,8 @@  int cmd_sparse_checkout(int argc, const char **argv, const char *prefix)
 			return sparse_checkout_init(argc, argv);
 		if (!strcmp(argv[0], "set"))
 			return sparse_checkout_set(argc, argv, prefix);
+		if (!strcmp(argv[0], "disable"))
+			return sparse_checkout_disable(argc, argv);
 	}
 
 	usage_with_options(builtin_sparse_checkout_usage,
diff --git a/t/t1091-sparse-checkout-builtin.sh b/t/t1091-sparse-checkout-builtin.sh
index a9ff5eb9ec..583c232e9e 100755
--- a/t/t1091-sparse-checkout-builtin.sh
+++ b/t/t1091-sparse-checkout-builtin.sh
@@ -148,4 +148,19 @@  test_expect_success 'set sparse-checkout using --stdin' '
 	test_cmp expect dir
 '
 
+test_expect_success 'sparse-checkout disable' '
+	git -C repo sparse-checkout disable &&
+	test_path_is_missing repo/.git/info/sparse-checkout &&
+	git -C repo config --list >config &&
+	test_i18ngrep "core.sparsecheckout=false" config &&
+	ls repo >dir &&
+	cat >expect <<-EOF &&
+		a
+		deep
+		folder1
+		folder2
+	EOF
+	test_cmp expect dir
+'
+
 test_done