From 88ab367df7bdfefc3f0cc5db8a70d054e7edc467 Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Sun, 4 Jul 2021 16:02:19 -0400 Subject: [PATCH 1/4] t1092: test ignored files outside of cone When a sparse directory leaves the sparse-checkout definition, we leave the ignored files in place. This then requires commands like 'git status' to expand a sparse index to find contained '.gitignore' files within the sparse directory. This test is marked as an expected failure, and the failures happen within the ensure_not_expanded lines. Signed-off-by: Derrick Stolee --- t/t1092-sparse-checkout-compatibility.sh | 45 ++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh index b7dc5569b6e43f..dd3b72afc769f6 100755 --- a/t/t1092-sparse-checkout-compatibility.sh +++ b/t/t1092-sparse-checkout-compatibility.sh @@ -42,6 +42,12 @@ test_expect_success 'setup' ' cp -r deep/deeper1/0 folder2 && echo >>folder1/0/0/0 && echo >>folder2/0/1 && + + cat >.gitignore <<-\EOF && + obj/ + *.o + EOF + git add . && git commit -m "initial commit" && git checkout -b base && @@ -62,6 +68,7 @@ test_expect_success 'setup' ' EOF cp folder1/larger-content folder2/ && cp folder1/larger-content deep/deeper1/ && + git add . && git commit -m "add interesting rename content" && @@ -587,6 +594,44 @@ test_expect_success 'sparse-index is not expanded' ' ensure_not_expanded add . ' +test_expect_failure 'sparse-index is not expanded (with ignored files outside cone)' ' + init_repos && + + write_script adjust_repo <<-\EOF && + mkdir folder1 obj folder1/obj && + echo ignored >folder1/obj/a && + echo ignored >obj/a &&c + echo ignored >folder1/file.o && + echo ignored >folder1.o + EOF + + run_on_all ../adjust_repo && + + ensure_not_expanded status && + ensure_not_expanded commit --allow-empty -m empty && + echo >>sparse-index/a && + ensure_not_expanded commit -a -m a && + echo >>sparse-index/a && + ensure_not_expanded commit --include a -m a && + echo >>sparse-index/deep/deeper1/a && + ensure_not_expanded commit --include deep/deeper1/a -m deeper && + ensure_not_expanded checkout rename-out-to-out && + ensure_not_expanded checkout - && + ensure_not_expanded switch rename-out-to-out && + ensure_not_expanded switch - && + git -C sparse-index reset --hard && + ensure_not_expanded checkout rename-out-to-out -- deep/deeper1 && + git -C sparse-index reset --hard && + ensure_not_expanded restore -s rename-out-to-out -- deep/deeper1 && + + echo >>sparse-index/README.md && + ensure_not_expanded add -A && + echo >>sparse-index/extra.txt && + ensure_not_expanded add extra.txt && + echo >>sparse-index/untracked.txt && + ensure_not_expanded add . +' + test_expect_success 'reset mixed and checkout orphan' ' init_repos && From 52d645120d800975819b6ee858acf228c31340f1 Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Mon, 5 Jul 2021 11:57:06 -0400 Subject: [PATCH 2/4] t7519: rewrite sparse index test This test was very fragile because it shared an index across sparse and non-sparse behavior. Since that expansion and contraction could cause the index to lose its FS Monitor bitmap and token, behavior is fragile to changes in 'git sparse-checkout set'. Rewrite the test to use two clones of the original repo: full and sparse. This allows us to also keep the test files (actual, expect, trace2.txt) out of the repos we are testing with 'git status'. Signed-off-by: Derrick Stolee --- t/t7519-status-fsmonitor.sh | 38 ++++++++++++++++++++----------------- 1 file changed, 21 insertions(+), 17 deletions(-) diff --git a/t/t7519-status-fsmonitor.sh b/t/t7519-status-fsmonitor.sh index deea88d4431d23..6f2cf306f66588 100755 --- a/t/t7519-status-fsmonitor.sh +++ b/t/t7519-status-fsmonitor.sh @@ -389,43 +389,47 @@ test_expect_success 'status succeeds after staging/unstaging' ' # If "!" is supplied, then we verify that we do not call ensure_full_index # during a call to 'git status'. Otherwise, we verify that we _do_ call it. check_sparse_index_behavior () { - git status --porcelain=v2 >expect && - git sparse-checkout init --cone --sparse-index && - git sparse-checkout set dir1 dir2 && + git -C full status --porcelain=v2 >expect && GIT_TRACE2_EVENT="$(pwd)/trace2.txt" GIT_TRACE2_EVENT_NESTING=10 \ - git status --porcelain=v2 >actual && + git -C sparse status --porcelain=v2 >actual && test_region $1 index ensure_full_index trace2.txt && test_region fsm_hook query trace2.txt && test_cmp expect actual && - rm trace2.txt && - git sparse-checkout disable + rm trace2.txt } test_expect_success 'status succeeds with sparse index' ' - git reset --hard && + git clone . full && + git clone . sparse && + git -C sparse sparse-checkout init --cone --sparse-index && + git -C sparse sparse-checkout set dir1 dir2 && - test_config core.fsmonitor "$TEST_DIRECTORY/t7519/fsmonitor-all" && - check_sparse_index_behavior ! && - - write_script .git/hooks/fsmonitor-test<<-\EOF && + write_script .git/hooks/fsmonitor-test <<-\EOF && printf "last_update_token\0" EOF - git config core.fsmonitor .git/hooks/fsmonitor-test && + git -C full config core.fsmonitor ../.git/hooks/fsmonitor-test && + git -C sparse config core.fsmonitor ../.git/hooks/fsmonitor-test && check_sparse_index_behavior ! && - write_script .git/hooks/fsmonitor-test<<-\EOF && + write_script .git/hooks/fsmonitor-test <<-\EOF && printf "last_update_token\0" printf "dir1/modified\0" EOF check_sparse_index_behavior ! && - cp -r dir1 dir1a && - git add dir1a && - git commit -m "add dir1a" && + git -C sparse sparse-checkout add dir1a && + + for repo in full sparse + do + cp -r $repo/dir1 $repo/dir1a && + git -C $repo add dir1a && + git -C $repo commit -m "add dir1a" || return 1 + done && + git -C sparse sparse-checkout set dir1 dir2 && # This one modifies outside the sparse-checkout definition # and hence we expect to expand the sparse-index. - write_script .git/hooks/fsmonitor-test<<-\EOF && + write_script .git/hooks/fsmonitor-test <<-\EOF && printf "last_update_token\0" printf "dir1a/modified\0" EOF From 4ad0348335f7b91397790884b9de6538353f3148 Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Sun, 4 Jul 2021 17:06:12 -0400 Subject: [PATCH 3/4] sparse-checkout: clear tracked sparse dirs When changing the scope of a sparse-checkout using cone mode, we might have some tracked directories go out of scope. The current logic removes the tracked files from within those directories, but leaves the ignored files within those directories. This is a bit unexpected to users who have given input to Git saying they don't need those directories anymore. Since these ignored files are typically build output or helper files from IDEs, the users should not need the files now that the tracked files are removed. If the tracked files reappear, then they will have newer timestamps than the build artifacts, so the artifacts will need to be regenerated anyway. Leaving these ignored files in the sparse directories makes it impossible to gain performance benefits in the sparse index. When we track into these directories, we need to know if the files are ignored or not, which might depend on the _tracked_ .gitignore file(s) within the sparse directory. This depends on the indexed version of the file, so the sparse directory must be expanded. By deleting the sparse directories when changing scope (or running 'git sparse-checkout reapply') we regain these performance benefits as if the repository was in a clean state. If users depend on ignored files within the sparse directories, then they have created a bad shape in their repository. This shape makes it impossible to get performance benefits using the sparse index, so they can workaround it (currently) by disabling the sparse index. Signed-off-by: Derrick Stolee --- builtin/sparse-checkout.c | 78 ++++++++++++++++++++++++ t/t1091-sparse-checkout-builtin.sh | 35 +++++++++++ t/t1092-sparse-checkout-compatibility.sh | 4 +- 3 files changed, 115 insertions(+), 2 deletions(-) diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c index a4bdd7c4940260..4448daba27e0cb 100644 --- a/builtin/sparse-checkout.c +++ b/builtin/sparse-checkout.c @@ -15,6 +15,7 @@ #include "wt-status.h" #include "quote.h" #include "sparse-index.h" +#include "run-command.h" static const char *empty_base = ""; @@ -100,6 +101,76 @@ static int sparse_checkout_list(int argc, const char **argv) return 0; } +static void clean_tracked_sparse_directories(struct repository *r) +{ + int i; + struct strvec args = STRVEC_INIT; + + /* + * If we are not using cone mode patterns, then we cannot + * delete directories outside of the sparse cone. + */ + if (!r || !r->index || !r->index->sparse_checkout_patterns || + !r->index->sparse_checkout_patterns->use_cone_patterns) + return; + /* + * NEEDSWORK: For now, only use this behavior when index.sparse + * is enabled. We may want this behavior enabled whenever using + * cone mode patterns. + */ + prepare_repo_settings(r); + if (!r->settings.sparse_index) + return; + + strvec_pushl(&args, "clean", "-dfx", "--", NULL); + + /* + * Since we now depend on the sparse index to enable this + * behavior, use it to our advantage. This process is more + * complicated without it. + */ + convert_to_sparse(r->index); + + for (i = 0; i < r->index->cache_nr; i++) { + struct cache_entry *ce = r->index->cache[i]; + + /* + * Is this a sparse directory? If so, then definitely + * include it. All contained content is outside of the + * patterns. + */ + if (S_ISSPARSEDIR(ce->ce_mode) && + repo_file_exists(r, ce->name)) { + strvec_push(&args, ce->name); + continue; + } + } + + /* + * Only run if we found an existing sparse directory, otherwise + * the clean will be across the entire worktree! + */ + if (args.nr > 3) + run_command_v_opt(args.v, RUN_GIT_CMD); + + /* + * The 'git clean -dfx -- ...' command empties the + * tracked directories outside of the sparse cone, but does not + * delete the directories themselves. Remove them now. + */ + for (i = 3; i < args.nr; i++) + rmdir_or_warn(args.v[i]); + + strvec_clear(&args); + + /* + * This is temporary: the sparse-checkout builtin is not + * integrated with the sparse-index yet, so we need to keep + * it full during the process. + */ + ensure_full_index(r->index); +} + static int update_working_directory(struct pattern_list *pl) { enum update_sparsity_result result; @@ -141,6 +212,8 @@ static int update_working_directory(struct pattern_list *pl) else rollback_lock_file(&lock_file); + clean_tracked_sparse_directories(r); + r->index->sparse_checkout_patterns = NULL; return result; } @@ -540,8 +613,11 @@ static int modify_pattern_list(int argc, const char **argv, enum modify_type m) { int result; int changed_config = 0; + struct pattern_list *old_pl = xcalloc(1, sizeof(*old_pl)); struct pattern_list *pl = xcalloc(1, sizeof(*pl)); + get_sparse_checkout_patterns(old_pl); + switch (m) { case ADD: if (core_sparse_checkout_cone) @@ -567,7 +643,9 @@ static int modify_pattern_list(int argc, const char **argv, enum modify_type m) set_config(MODE_NO_PATTERNS); clear_pattern_list(pl); + clear_pattern_list(old_pl); free(pl); + free(old_pl); return result; } diff --git a/t/t1091-sparse-checkout-builtin.sh b/t/t1091-sparse-checkout-builtin.sh index 92613f381c7d23..3095fc10d0510b 100755 --- a/t/t1091-sparse-checkout-builtin.sh +++ b/t/t1091-sparse-checkout-builtin.sh @@ -642,4 +642,39 @@ test_expect_success MINGW 'cone mode replaces backslashes with slashes' ' check_files repo/deep a deeper1 ' +test_expect_success 'cone mode clears ignored subdirectories' ' + rm repo/.git/info/sparse-checkout && + + # NEEDSWORK: --sparse-index is required for now + git -C repo sparse-checkout init --cone --sparse-index && + git -C repo sparse-checkout set deep/deeper1 && + + cat >repo/.gitignore <<-\EOF && + obj/ + *.o + EOF + + git -C repo add .gitignore && + git -C repo commit -m ".gitignore" && + + mkdir -p repo/obj repo/folder1/obj repo/deep/deeper2/obj && + for file in folder1/obj/a obj/a folder1/file.o folder1.o \ + deep/deeper2/obj/a deep/deeper2/file.o file.o + do + echo ignored >repo/$file || return 1 + done && + + git -C repo status --porcelain=v2 >out && + test_must_be_empty out && + + git -C repo sparse-checkout reapply && + test_path_is_missing repo/folder1 && + test_path_is_missing repo/deep/deeper2 && + test_path_is_dir repo/obj && + test_path_is_file repo/file.o && + + git -C repo status --porcelain=v2 >out && + test_must_be_empty out +' + test_done diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh index dd3b72afc769f6..afb27cb57548a6 100755 --- a/t/t1092-sparse-checkout-compatibility.sh +++ b/t/t1092-sparse-checkout-compatibility.sh @@ -594,7 +594,7 @@ test_expect_success 'sparse-index is not expanded' ' ensure_not_expanded add . ' -test_expect_failure 'sparse-index is not expanded (with ignored files outside cone)' ' +test_expect_success 'sparse-index is not expanded (with ignored files outside cone)' ' init_repos && write_script adjust_repo <<-\EOF && @@ -606,6 +606,7 @@ test_expect_failure 'sparse-index is not expanded (with ignored files outside co EOF run_on_all ../adjust_repo && + git -C sparse-index sparse-checkout reapply && ensure_not_expanded status && ensure_not_expanded commit --allow-empty -m empty && @@ -623,7 +624,6 @@ test_expect_failure 'sparse-index is not expanded (with ignored files outside co ensure_not_expanded checkout rename-out-to-out -- deep/deeper1 && git -C sparse-index reset --hard && ensure_not_expanded restore -s rename-out-to-out -- deep/deeper1 && - echo >>sparse-index/README.md && ensure_not_expanded add -A && echo >>sparse-index/extra.txt && From 2d3b5b459ed27b1cc756ba2f388b701b92af54e9 Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Mon, 5 Jul 2021 12:43:20 -0400 Subject: [PATCH 4/4] unpack-trees: fix nested sparse-dir search The iterated search in find_cache_entry() was recently modified to include a loop that searches backwards for a sparse directory entry that matches the given traverse_info and name_entry. However, the string comparison failed to actually concatenate those two strings, so this failed to find a sparse directory when it was not a top-level directory. This caused some errors in rare cases where a 'git checkout' spanned a diff that modified files within the sparse directory entry, but we could not correctly find the entry. Signed-off-by: Derrick Stolee --- unpack-trees.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/unpack-trees.c b/unpack-trees.c index 5f1d00d2d247e7..0a2c0df0eb381b 100644 --- a/unpack-trees.c +++ b/unpack-trees.c @@ -1286,9 +1286,10 @@ static int sparse_dir_matches_path(const struct cache_entry *ce, static struct cache_entry *find_cache_entry(struct traverse_info *info, const struct name_entry *p) { - struct cache_entry *ce; + struct cache_entry *ce = NULL; int pos = find_cache_pos(info, p->path, p->pathlen); struct unpack_trees_options *o = info->data; + struct strbuf full_path = STRBUF_INIT; if (0 <= pos) return o->src_index->cache[pos]; @@ -1304,6 +1305,10 @@ static struct cache_entry *find_cache_entry(struct traverse_info *info, if (pos < 0 || pos >= o->src_index->cache_nr) return NULL; + strbuf_addstr(&full_path, info->traverse_path); + strbuf_add(&full_path, p->path, p->pathlen); + strbuf_addch(&full_path, '/'); + /* * We might have multiple entries between 'pos' and * the actual sparse-directory entry, so start walking @@ -1315,17 +1320,20 @@ static struct cache_entry *find_cache_entry(struct traverse_info *info, /* * Have we walked too far? */ - if (strncmp(ce->name, p->path, p->pathlen)) - return NULL; + if (strncmp(ce->name, full_path.buf, full_path.len)) { + ce = NULL; + break; + } if (S_ISSPARSEDIR(ce->ce_mode) && sparse_dir_matches_path(ce, info, p)) - return ce; + break; pos--; } - return NULL; + strbuf_release(&full_path); + return ce; } static void debug_path(struct traverse_info *info)