Re: [PATCH v4 10/10] sparse-checkout: add config to disable deleting dirs
From: Elijah Newren <hidden>
Date: 2021-08-27 20:58:31
On Tue, Aug 24, 2021 at 2:51 PM Derrick Stolee via GitGitGadget [off-list ref] wrote:
From: Derrick Stolee <redacted> 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. * 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? * 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?
quoted hunk ↗ jump to hunk
Signed-off-by: Derrick Stolee <redacted> --- Documentation/config/index.txt | 6 ++++++ builtin/sparse-checkout.c | 9 ++++++++- t/t1091-sparse-checkout-builtin.sh | 4 ++++ 3 files changed, 18 insertions(+), 1 deletion(-)diff --git a/Documentation/config/index.txt b/Documentation/config/index.txt index 75f3a2d1054..c65da20a931 100644 --- a/Documentation/config/index.txt +++ b/Documentation/config/index.txt@@ -1,3 +1,9 @@ +index.deleteSparseDirectories:: + When enabled, the cone mode sparse-checkout feature will delete + directories that are outside of the sparse-checkout cone, unless + such a directory contains an untracked, non-ignored file. Defaults + to true. + index.recordEndOfIndexEntries:: Specifies whether the index file should include an "End Of Index Entry" section. This reduces index load time on multiprocessordiff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c index d0f5c4702be..33ec729d9de 100644 --- a/builtin/sparse-checkout.c +++ b/builtin/sparse-checkout.c@@ -102,7 +102,7 @@ static int sparse_checkout_list(int argc, const char **argv) static void clean_tracked_sparse_directories(struct repository *r) { - int i, was_full = 0; + int i, value, was_full = 0; struct strbuf path = STRBUF_INIT; size_t pathlen; struct string_list_item *item;@@ -118,6 +118,13 @@ static void clean_tracked_sparse_directories(struct repository *r) !r->index->sparse_checkout_patterns->use_cone_patterns) return; + /* + * Users can disable this behavior. + */ + if (!repo_config_get_bool(r, "index.deletesparsedirectories", &value) && + !value) + return; + /* * Use the sparse index as a data structure to assist finding * directories that are safe to delete. This conversion to adiff --git a/t/t1091-sparse-checkout-builtin.sh b/t/t1091-sparse-checkout-builtin.sh index 71236981e64..e0f31186d89 100755 --- a/t/t1091-sparse-checkout-builtin.sh +++ b/t/t1091-sparse-checkout-builtin.sh@@ -666,6 +666,10 @@ test_expect_success 'cone mode clears ignored subdirectories' ' git -C repo status --porcelain=v2 >out && test_must_be_empty out && + git -C repo -c index.deleteSparseDirectories=false sparse-checkout reapply && + test_path_is_dir repo/folder1 && + test_path_is_dir repo/deep/deeper2 && + git -C repo sparse-checkout reapply && test_path_is_missing repo/folder1 && test_path_is_missing repo/deep/deeper2 && --gitgitgadget