On Mon, Aug 30, 2021 at 6:30 AM Derrick Stolee <stolee@xxxxxxxxx> wrote: > > On 8/27/2021 4:58 PM, Elijah Newren wrote: > > On Tue, Aug 24, 2021 at 2:51 PM Derrick Stolee via GitGitGadget > > <gitgitgadget@xxxxxxxxx> wrote: > >> > >> From: Derrick Stolee <dstolee@xxxxxxxxxxxxx> > >> > >> The clean_tracked_sparse_directories() method deletes the tracked > >> directories that go out of scope when the sparse-checkout cone changes, > >> at least in cone mode. This is new behavior, but is recommended based on > >> our understanding of how users are interacting with the feature in most > >> cases. > >> > >> It is possible that some users will object to the new behavior, so > >> create a new configuration option 'index.deleteSparseDirectories' that > >> can be set to 'false' to make clean_tracked_sparse_directories() do > >> nothing. This will keep all untracked files in the working tree and > >> cause performance problems with the sparse index, but those trade-offs > >> are for the user to decide. > > > > I'm not sold that we need anything here, and it sounds like this is > > all being added based on a theoretical concern. I might not object > > too much normally to trying to address theoretical conerns with new > > features, but: > > > > * I'm a little concerned that we're adding a configuration option > > (which live forever and are harder to work with > > backward-compatibility-wise) rather than a command line flag. > > The issue with a command-line flag is that it would need to be added > to all the commands that reapply the sparse-checkout definition. > Maybe that's just the 'git sparse-checkout (init|set|add|reapply)' > and 'git read-tree' commands, but are there other places where this > logic might be applied in the future? > > Also, as more third-party tools integrate with sparse-checkout, I > don't expect users to be directly involved in changing their cone, > so any use of a command-line flag would need to be integrated into > those tools. A config option applies this logic universally, when > needed. > > > * It's not clear to me that the option name is what we want. For > > example, git checkout has a --[no-]overwrite-ignored for overwriting > > ignored files (or not) when switching to a different branch. git > > merge has the same flags (though only the fast-forwarding backend > > heeds it currently). This behavior is quite similar to that flag, and > > has the same default of treating ignored files as expendable. Should > > the naming be more similar? > > I'm open to changing the name to more closely match existing naming > conventions, once we've decided if this should be included at all. > > > * It's not clear to me that the option has the right level of > > implementation granularity. If someone wants ignored files to be > > treated as important, should they need to set a > > merge.no_overwrite_ignored AND a checkout.no_overwrite_ignored AND a > > index.deleteSparseDirectories AND other names, or have one high-level > > option that sets all of these behaviors? > > These cases have different meanings for why an ignored file is > important. For merge and checkout, the argument is saying "don't > overwrite ignored files if they are to be replaced by a tracked > file". For sparse-checkout, the config is saying "don't delete > ignored files that are leaving the sparse-checkout scope". I think > it is meaningful to say that files that match the .gitignore > patterns but are still tracked are deleted, because sparse-checkout > removes all tracked files. > > I'm less convinced that there needs to be a meta-setting that covers > all of these cases simultaneously. > > --- > > As for moving forward, I'm fine skipping this patch if there is no > support for it. I just wanted to demonstrate that it could be done. That's fair, and you certainly achieved that. > Perhaps we wait for the theoretical concern to be a real one, as > requested by a user with a legitimate reason to care? Sounds good to me. Then we'll have a better frame of reference for judging how the feature should be tailored as well.
