Re: [PATCH] unpack-trees: add core.sparseCheckoutRmFiles config

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 2, 2021, 02:32 UTC
Message-ID: <xmqqmts9q9m2.fsf@gitster.g>
In-Reply-To: <f6d39636-308c-c846-55b5-3f16a155e69d@gmail.com>

Derrick Stolee <stolee@gmail.com> writes:

Show 8 quoted lines

>> This commit adds the above config item, defaulting to "true" to get the
>> old behavior. Set it to "false" to avoid removing such a file from the
>> worktree.
>
> I don't think config is necessary here. This behavior is niche
> enough to create a behavior-breaking change. However, we do want
> to ensure that the full flow of eventually clearing the file when
> clean is handled.

I didn't have a chance to get around to commenting on the patch (instead I was preparing for -rc3), but you covered pretty much everything I wanted to say. It is unusual for those who are using sparse checkout to have a modified (=tracked and dirty) file that shouldn't be there, and making sure the user notices these unusual files, instead of silently losing the changes to them, is probably a "bugfix".

An explicit request to destructively overwrite the path ("git restore -- path") or remove the working tree file along with modification ("git reset --hard") is a good thing to have, but the branch switching "git switch" is supposed to preserve local modification (or fail to switch), whether the dirty path is inside or outside the sparse checkout area.

Show 7 quoted lines

> If we _are_ going to go with a config option, then I'm not a big
> fan of this name. I've also been thinking that the sparse-checkout
> config has been squatting in the "core.*" space too long. Perhaps
> it is time to create its own section?
>
> What about something like sparseCheckout.keepDirtyFiles, which
> defaults to false?

What about not having a configuration?

Show 7 quoted lines

> 1. How does a user with a dirty, tracked, sparse file get back
>    into a state where that file is deleted? What commands do
>    they need to run? Can that be tested and added to the sparse-
>    checkout documentation?
>
> 2. What does 'git status' look like when a user is in this state?
>    Is that helpful?

Good points.

Thanks.