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@xxxxxxxxxxxxx>
>>
>> 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@xxxxxxxxxxxxx>
>> ---
>> 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
