From 47f47bf28c191ebc03fa25f6525751ca41a3b796 Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Sun, 22 Aug 2021 14:55:59 -0400 Subject: [PATCH 01/15] sparse-checkout: add config to disable deleting dirs 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. Signed-off-by: Derrick Stolee --- 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 75f3a2d105..c65da20a93 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 multiprocessor diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c index f91e29b56a..ca20d01f3f 100644 --- a/builtin/sparse-checkout.c +++ b/builtin/sparse-checkout.c @@ -106,7 +106,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; @@ -122,6 +122,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 a diff --git a/t/t1091-sparse-checkout-builtin.sh b/t/t1091-sparse-checkout-builtin.sh index de1ec89007..5528fe486c 100755 --- a/t/t1091-sparse-checkout-builtin.sh +++ b/t/t1091-sparse-checkout-builtin.sh @@ -761,6 +761,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 && From 52213fc7a7e29afc45bbf239f2b7ba510e61fe75 Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Mon, 26 Jul 2021 15:43:05 -0400 Subject: [PATCH 02/15] diff: ignore sparse paths in diffstat The diff_populate_filespec() method is used to describe the diff after a merge operation is complete, especially when a conflict appears. In order to avoid expanding a sparse index, the reuse_worktree_file() needs to be adapted to ignore files that are outside of the sparse-checkout cone. The file names and OIDs used for this check come from the merged tree in the case of the ORT strategy, not the index, hence the ability to look into these paths without having already expanded the index. Signed-off-by: Derrick Stolee --- diff.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/diff.c b/diff.c index e71cf75886..046d1e340c 100644 --- a/diff.c +++ b/diff.c @@ -3893,6 +3893,13 @@ static int reuse_worktree_file(struct index_state *istate, if (!FAST_WORKING_DIRECTORY && !want_file && has_object_pack(oid)) return 0; + /* + * If this path does not match our sparse-checkout definition, + * then the file will not be in the working directory. + */ + if (!path_in_sparse_checkout(name, istate)) + return 0; + /* * Similarly, if we'd have to convert the file contents anyway, that * makes the optimization not worthwhile. From de7a241b0029dee87efc562f574c11ca7e7f56df Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Tue, 29 Jun 2021 11:12:56 -0400 Subject: [PATCH 03/15] add/rm: allow adding sparse entries when virtual Upstream, a20f704 (add: warn when asked to update SKIP_WORKTREE entries, 2021-04-08) modified how 'git add ' works with cache entries marked with the SKIP_WORKTREE bit. The intention is to prevent a user from accidentally adding a path that is outside their sparse-checkout definition but somehow matches an existing index entry. A similar change for 'git rm' happened in d5f4b82 (rm: honor sparse checkout patterns, 2021-04-08). This breaks when using the virtual filesystem in VFS for Git. It is rare, but we could be in a scenario where the user has staged a change and then the file is projected away. If the user re-adds the file, then this warning causes the command to fail with the advise message. Disable this logic when core_virtualfilesystem is enabled. Signed-off-by: Derrick Stolee --- builtin/add.c | 22 ++++++++++++++++------ builtin/rm.c | 8 ++++++-- 2 files changed, 22 insertions(+), 8 deletions(-) diff --git a/builtin/add.c b/builtin/add.c index 76d5ad1f5d..9a10501558 100644 --- a/builtin/add.c +++ b/builtin/add.c @@ -47,6 +47,7 @@ static int chmod_pathspec(struct pathspec *pathspec, char flip, int show_only) int err; if (!include_sparse && + !core_virtualfilesystem && (ce_skip_worktree(ce) || !path_in_sparse_checkout(ce->name, &the_index))) continue; @@ -97,7 +98,8 @@ static void update_callback(struct diff_queue_struct *q, struct diff_filepair *p = q->queue[i]; const char *path = p->one->path; - if (!include_sparse && !path_in_sparse_checkout(path, &the_index)) + if (!include_sparse && !core_virtualfilesystem && + !path_in_sparse_checkout(path, &the_index)) continue; switch (fix_unmerged_status(p, data)) { @@ -215,8 +217,9 @@ static int refresh(int verbose, const struct pathspec *pathspec) if (!seen[i]) { const char *path = pathspec->items[i].original; - if (matches_skip_worktree(pathspec, i, &skip_worktree_seen) || - !path_in_sparse_checkout(path, &the_index)) { + if (!core_virtualfilesystem && + (matches_skip_worktree(pathspec, i, &skip_worktree_seen) || + !path_in_sparse_checkout(path, &the_index))) { string_list_append(&only_match_skip_worktree, pathspec->items[i].original); } else { @@ -226,7 +229,11 @@ static int refresh(int verbose, const struct pathspec *pathspec) } } - if (only_match_skip_worktree.nr) { + /* + * When using a virtual filesystem, we might re-add a path + * that is currently virtual and we want that to succeed. + */ + if (!core_virtualfilesystem && only_match_skip_worktree.nr) { advise_on_updating_sparse_paths(&only_match_skip_worktree); ret = 1; } @@ -644,7 +651,11 @@ int cmd_add(int argc, const char **argv, const char *prefix) if (seen[i]) continue; - if (!include_sparse && + /* + * When using a virtual filesystem, we might re-add a path + * that is currently virtual and we want that to succeed. + */ + if (!include_sparse && !core_virtualfilesystem && matches_skip_worktree(&pathspec, i, &skip_worktree_seen)) { string_list_append(&only_match_skip_worktree, pathspec.items[i].original); @@ -668,7 +679,6 @@ int cmd_add(int argc, const char **argv, const char *prefix) } } - if (only_match_skip_worktree.nr) { advise_on_updating_sparse_paths(&only_match_skip_worktree); exit_status = 1; diff --git a/builtin/rm.c b/builtin/rm.c index 84a935a16e..fc6266bc03 100644 --- a/builtin/rm.c +++ b/builtin/rm.c @@ -301,7 +301,7 @@ int cmd_rm(int argc, const char **argv, const char *prefix) for (i = 0; i < active_nr; i++) { const struct cache_entry *ce = active_cache[i]; - if (!include_sparse && + if (!include_sparse && !core_virtualfilesystem && (ce_skip_worktree(ce) || !path_in_sparse_checkout(ce->name, &the_index))) continue; @@ -338,7 +338,11 @@ int cmd_rm(int argc, const char **argv, const char *prefix) *original ? original : "."); } - if (only_match_skip_worktree.nr) { + /* + * When using a virtual filesystem, we might re-add a path + * that is currently virtual and we want that to succeed. + */ + if (!core_virtualfilesystem && only_match_skip_worktree.nr) { advise_on_updating_sparse_paths(&only_match_skip_worktree); ret = 1; } From 922a5c5df5c1cc19059ba55ed2dfa34453355c81 Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Sun, 22 Aug 2021 19:42:47 -0400 Subject: [PATCH 04/15] t7524: test no longer fails Signed-off-by: Derrick Stolee --- t/t7522-serialized-status.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t7522-serialized-status.sh b/t/t7522-serialized-status.sh index 6b9f91a6d7..230e1e24cf 100755 --- a/t/t7522-serialized-status.sh +++ b/t/t7522-serialized-status.sh @@ -400,7 +400,7 @@ EOF ' -test_expect_failure 'ensure deserialize -v does not crash' ' +test_expect_success 'ensure deserialize -v does not crash' ' git init -b main verbose_test && touch verbose_test/a && From 14f74af9d46daf7cee505c1ee73c61642a0f702b Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Tue, 15 Jun 2021 11:07:11 -0400 Subject: [PATCH 05/15] repo-settings: enable sparse index by default There is some strangeness when expanding a sparse-index that exists within a submodule. We will need to resolve that later, but for now, let's do a better job of explicitly disabling the sparse-index when requested, and do so in t7817. Signed-off-by: Derrick Stolee --- repo-settings.c | 2 +- t/t1092-sparse-checkout-compatibility.sh | 1 + t/t7817-grep-sparse-checkout.sh | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/repo-settings.c b/repo-settings.c index 459041f65c..ffeee85de0 100644 --- a/repo-settings.c +++ b/repo-settings.c @@ -74,7 +74,7 @@ void prepare_repo_settings(struct repository *r) repo_cfg_bool(r, "fetch.writecommitgraph", &r->settings.fetch_write_commit_graph, 0); repo_cfg_bool(r, "pack.usesparse", &r->settings.pack_use_sparse, 1); repo_cfg_bool(r, "core.multipackindex", &r->settings.core_multi_pack_index, 1); - repo_cfg_bool(r, "index.sparse", &r->settings.sparse_index, 0); + repo_cfg_bool(r, "index.sparse", &r->settings.sparse_index, 1); /* * The GIT_TEST_MULTI_PACK_INDEX variable is special in that diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh index f9f8c988bb..f5d25027f9 100755 --- a/t/t1092-sparse-checkout-compatibility.sh +++ b/t/t1092-sparse-checkout-compatibility.sh @@ -155,6 +155,7 @@ init_repos () { git -C sparse-index reset --hard && # initialize sparse-checkout definitions + git -C sparse-checkout config index.sparse false && git -C sparse-checkout sparse-checkout init --cone && git -C sparse-checkout sparse-checkout set deep && git -C sparse-index sparse-checkout init --cone --sparse-index && diff --git a/t/t7817-grep-sparse-checkout.sh b/t/t7817-grep-sparse-checkout.sh index eb59564565..db3004c4fe 100755 --- a/t/t7817-grep-sparse-checkout.sh +++ b/t/t7817-grep-sparse-checkout.sh @@ -49,7 +49,7 @@ test_expect_success 'setup' ' echo "text" >B/b && git add A B && git commit -m sub && - git sparse-checkout init --cone && + git sparse-checkout init --cone --no-sparse-index && git sparse-checkout set B ) && From eca95d8609cbbad7d379473d68704e8e4c090f25 Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Fri, 3 Sep 2021 09:55:13 -0400 Subject: [PATCH 06/15] dir: add path_matches_cone_mode_pattern_list() When matching against a generic pattern list, the 'basename' is important for some patterns. However, it and the 'dtype' parameter are irrelevant for cone mode sparse-checkout patterns. If we know that we are working with cone mode patterns from the start, then we can speed up the pattern check slightly by not computing the 'basename'. In many existing consumers, the 'basename' is already known from context, but some new consumers we compute this on-demand. A future change will add more calls that do not have the 'basename' from context and would need to compute it for many cache entries in a tight loop. Avoid this problem by creating the new path_matches_cone_mode_pattern_list() method. Signed-off-by: Derrick Stolee --- dir.c | 78 +++++++++++++++++++++++++++++++++-------------------------- dir.h | 9 +++++++ 2 files changed, 53 insertions(+), 34 deletions(-) diff --git a/dir.c b/dir.c index 4339715ae5..28da33f875 100644 --- a/dir.c +++ b/dir.c @@ -1400,46 +1400,16 @@ static struct path_pattern *last_matching_pattern_from_list(const char *pathname return res; } -/* - * Scan the list of patterns to determine if the ordered list - * of patterns matches on 'pathname'. - * - * Return 1 for a match, 0 for not matched and -1 for undecided. - */ -enum pattern_match_result path_matches_pattern_list( +enum pattern_match_result path_matches_cone_mode_pattern_list( const char *pathname, int pathlen, - const char *basename, int *dtype, - struct pattern_list *pl, - struct index_state *istate) + struct pattern_list *pl) { - struct path_pattern *pattern; struct strbuf parent_pathname = STRBUF_INIT; int result = NOT_MATCHED; size_t slash_pos; - /* - * The virtual file system data is used to prevent git from traversing - * any part of the tree that is not in the virtual file system. Return - * 1 to exclude the entry if it is not found in the virtual file system, - * else fall through to the regular excludes logic as it may further exclude. - */ - if (*dtype == DT_UNKNOWN) - *dtype = resolve_dtype(DT_UNKNOWN, istate, pathname, pathlen); - if (is_excluded_from_virtualfilesystem(pathname, pathlen, *dtype) > 0) - return 1; - - if (!pl->use_cone_patterns) { - pattern = last_matching_pattern_from_list(pathname, pathlen, basename, - dtype, pl, istate); - if (pattern) { - if (pattern->flags & PATTERN_FLAG_NEGATIVE) - return NOT_MATCHED; - else - return MATCHED; - } - - return UNDECIDED; - } + if (!pl->use_cone_patterns) + BUG("path_matches_cone_mode_pattern_list requires cone mode patterns"); if (pl->full_cone) return MATCHED; @@ -1492,6 +1462,46 @@ done: return result; } +/* + * Scan the list of patterns to determine if the ordered list + * of patterns matches on 'pathname'. + * + * Return 1 for a match, 0 for not matched and -1 for undecided. + */ +enum pattern_match_result path_matches_pattern_list( + const char *pathname, int pathlen, + const char *basename, int *dtype, + struct pattern_list *pl, + struct index_state *istate) +{ + /* + * The virtual file system data is used to prevent git from traversing + * any part of the tree that is not in the virtual file system. Return + * 1 to exclude the entry if it is not found in the virtual file system, + * else fall through to the regular excludes logic as it may further exclude. + */ + if (*dtype == DT_UNKNOWN) + *dtype = resolve_dtype(DT_UNKNOWN, istate, pathname, pathlen); + if (is_excluded_from_virtualfilesystem(pathname, pathlen, *dtype) > 0) + return 1; + + if (!pl->use_cone_patterns) { + struct path_pattern *pattern = last_matching_pattern_from_list( + pathname, pathlen, basename, + dtype, pl, istate); + if (pattern) { + if (pattern->flags & PATTERN_FLAG_NEGATIVE) + return NOT_MATCHED; + else + return MATCHED; + } + + return UNDECIDED; + } + + return path_matches_cone_mode_pattern_list(pathname, pathlen, pl); +} + int init_sparse_checkout_patterns(struct index_state *istate) { if (!core_apply_sparse_checkout) diff --git a/dir.h b/dir.h index 7bc862030c..3e73d11f31 100644 --- a/dir.h +++ b/dir.h @@ -383,6 +383,15 @@ enum pattern_match_result { MATCHED_RECURSIVE = 2, }; +/* + * Test if a given path is contained in the given pattern list. + * + * The given pattern list _must_ use cone mode patterns. + */ +enum pattern_match_result path_matches_cone_mode_pattern_list( + const char *pathname, int pathlen, + struct pattern_list *pl); + /* * Scan the list of patterns to determine if the ordered list * of patterns matches on 'pathname'. From f12d7660e85a3e064c1b6d42af4b9ba80657135e Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Mon, 13 Sep 2021 13:50:43 -0400 Subject: [PATCH 07/15] p2000: explicitly turn off sparse index The full index test cases in p2000-sparse-operations.sh relied on the fact that the sparse index was off by default. When this default was flipped, the performance test was not altered. Do so now with the --no-sparse-index flag. Signed-off-by: Derrick Stolee --- t/perf/p2000-sparse-operations.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/t/perf/p2000-sparse-operations.sh b/t/perf/p2000-sparse-operations.sh index c181110a43..3847b2b2d0 100755 --- a/t/perf/p2000-sparse-operations.sh +++ b/t/perf/p2000-sparse-operations.sh @@ -55,7 +55,7 @@ test_expect_success 'setup repo and indexes' ' git -c core.sparseCheckoutCone=true clone --branch=wide --sparse . full-v3 && ( cd full-v3 && - git sparse-checkout init --cone && + git sparse-checkout init --cone --no-sparse-index && git sparse-checkout set $SPARSE_CONE && git config index.version 3 && git update-index --index-version=3 && @@ -64,7 +64,7 @@ test_expect_success 'setup repo and indexes' ' git -c core.sparseCheckoutCone=true clone --branch=wide --sparse . full-v4 && ( cd full-v4 && - git sparse-checkout init --cone && + git sparse-checkout init --cone --no-sparse-index && git sparse-checkout set $SPARSE_CONE && git config index.version 4 && git update-index --index-version=4 && From 7770d81aff2c47977b1db3c9a64eb4f041d25263 Mon Sep 17 00:00:00 2001 From: Lessley Dennington Date: Fri, 10 Sep 2021 13:57:25 -0700 Subject: [PATCH 08/15] diff(sparse-index): verify with partially-sparse This verifies that `diff` and `diff --staged` behave the same in sparse index repositories in the following partially-staged scenarios (i.e. the index, HEAD, and working directory differ at a given path): 1. Path is within sparse-checkout cone. 2. Path is outside sparse-checkout cone. 3. A merge conflict exists for paths outside sparse-checkout cone. Signed-off-by: Lessley Dennington --- t/t1092-sparse-checkout-compatibility.sh | 42 ++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh index f5d25027f9..0147a16bb0 100755 --- a/t/t1092-sparse-checkout-compatibility.sh +++ b/t/t1092-sparse-checkout-compatibility.sh @@ -488,6 +488,43 @@ test_expect_success 'diff --cached' ' test_all_match git diff --cached ' +test_expect_success 'diff partially-staged' ' + init_repos && + + write_script edit-contents <<-\EOF && + echo text >>$1 + EOF + + # Add file within cone + test_all_match git sparse-checkout set deep && + run_on_all ../edit-contents deep/testfile && + test_all_match git add deep/testfile && + run_on_all ../edit-contents deep/testfile && + + test_all_match git diff && + test_all_match git diff --staged && + + # Add file outside cone + test_all_match git reset --hard && + run_on_all mkdir newdirectory && + run_on_all ../edit-contents newdirectory/testfile && + test_all_match git sparse-checkout set newdirectory && + test_all_match git add newdirectory/testfile && + run_on_all ../edit-contents newdirectory/testfile && + test_all_match git sparse-checkout set && + + test_all_match git diff && + test_all_match git diff --staged && + + # Merge conflict outside cone + test_all_match git reset --hard && + test_all_match git checkout merge-left && + test_all_match test_must_fail git merge merge-right && + + test_all_match git diff && + test_all_match git diff --staged +' + # NEEDSWORK: sparse-checkout behaves differently from full-checkout when # running this test with 'df-conflict-2' after 'df-conflict-1'. test_expect_success 'diff with renames and conflicts' ' @@ -1391,6 +1428,11 @@ test_expect_success 'sparse-index is not expanded' ' ensure_not_expanded reset --merge update-deep && ensure_not_expanded reset --hard && + echo a test change >>sparse-index/README.md && + ensure_not_expanded diff && + git -C sparse-index add README.md && + ensure_not_expanded diff --staged && + ensure_not_expanded reset base -- deep/a && ensure_not_expanded reset base -- nonexistent-file && ensure_not_expanded reset deepest -- deep && From ba4255302a9528c7722366399d0bf70b42c2b14f Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Mon, 13 Sep 2021 13:51:40 -0400 Subject: [PATCH 09/15] p2000: add new commands Signed-off-by: Derrick Stolee --- t/perf/p2000-sparse-operations.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/t/perf/p2000-sparse-operations.sh b/t/perf/p2000-sparse-operations.sh index 3847b2b2d0..fd83a42a87 100755 --- a/t/perf/p2000-sparse-operations.sh +++ b/t/perf/p2000-sparse-operations.sh @@ -123,5 +123,6 @@ test_perf_on_all git blame $SPARSE_CONE/f3/a test_perf_on_all git read-tree -mu HEAD test_perf_on_all git checkout-index -f --all test_perf_on_all git update-index --add --remove $SPARSE_CONE/a +test_perf_on_all git sparse-checkout reapply test_done From df696c1101e884f04fdde05bd4261b0a96c84ea7 Mon Sep 17 00:00:00 2001 From: Victoria Dye Date: Wed, 22 Sep 2021 14:02:21 -0400 Subject: [PATCH 10/15] stash: expand testing for `git stash -u` Test cases specific to handling untracked files in `git stash` a) ensure that files outside the sparse checkout definition are handled as-expected and b) document the index expansion inside of `git stash -u`. Note that, in b), it is not the full repository index that is expanded - it is the temporary, standalone index containing the stashed untracked files only. Signed-off-by: Victoria Dye --- t/t1092-sparse-checkout-compatibility.sh | 40 ++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh index 0147a16bb0..f15394b131 100755 --- a/t/t1092-sparse-checkout-compatibility.sh +++ b/t/t1092-sparse-checkout-compatibility.sh @@ -1728,6 +1728,46 @@ test_expect_success 'sparse index is not expanded: sparse-checkout' ' ensure_not_expanded sparse-checkout set ' +# NEEDSWORK: although the full repository's index is _not_ expanded as part of +# stash, a temporary index, which is _not_ sparse, is created when stashing and +# applying a stash of untracked files. As a result, the test reports that it +# finds an instance of `ensure_full_index`, but it does not carry with it the +# performance implications of expanding the full repository index. +test_expect_success 'sparse index is not expanded: stash -u' ' + init_repos && + + mkdir -p sparse-index/folder1 && + echo >>sparse-index/README.md && + echo >>sparse-index/a && + echo >>sparse-index/folder1/new && + + GIT_TRACE2_EVENT="$(pwd)/trace2.txt" GIT_TRACE2_EVENT_NESTING=10 \ + git -C sparse-index stash -u && + test_region index ensure_full_index trace2.txt && + + GIT_TRACE2_EVENT="$(pwd)/trace2.txt" GIT_TRACE2_EVENT_NESTING=10 \ + git -C sparse-index stash pop && + test_region index ensure_full_index trace2.txt +' + +# NEEDSWORK: similar to `git add`, untracked files outside of the sparse +# checkout definition are successfully stashed and unstashed. +test_expect_success 'stash -u outside sparse checkout definition' ' + init_repos && + + write_script edit-contents <<-\EOF && + echo text >>$1 + EOF + + run_on_sparse mkdir -p folder1 && + run_on_all ../edit-contents folder1/new && + test_all_match git stash -u && + test_all_match git status --porcelain=v2 && + + test_all_match git stash pop -q && + test_all_match git status --porcelain=v2 +' + # NEEDSWORK: a sparse-checkout behaves differently from a full checkout # in this scenario, but it shouldn't. test_expect_success 'reset mixed and checkout orphan' ' From e44c5952fab005ec86cc9ec4a82dd2491d74ec15 Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Thu, 23 Sep 2021 09:59:06 -0400 Subject: [PATCH 11/15] sequencer: avoid progress when stderr is redirected During a run of the Scalar functional tests, we hit a case where the inexact rename detection of a 'git cherry-pick' command slowed to the point of writing its delayed progress, failing the test because stderr differed from the control case. Showing progress like this when stderr is not a terminal is non-standard for Git, so inject an isatty(2) when initializing the progress option in sequencer.c. Unfortunately, there is no '--quiet' option in 'git cherry-pick' currently wired up. This could be considered in the future, and the isatty(2) could be moved to that position. This would also be needed for commands like 'git rebase', so we leave that for another time. Reported-by: Victoria Dye Signed-off-by: Derrick Stolee --- sequencer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sequencer.c b/sequencer.c index 61a8e0020d..b35d275d86 100644 --- a/sequencer.c +++ b/sequencer.c @@ -643,7 +643,7 @@ static int do_recursive_merge(struct repository *r, o.branch2 = next ? next_label : "(empty tree)"; if (is_rebase_i(opts)) o.buffer_output = 2; - o.show_rename_progress = 1; + o.show_rename_progress = isatty(2); head_tree = parse_tree_indirect(head); next_tree = next ? get_commit_tree(next) : empty_tree(r); From 3c1e0a85a6e11e602d7286b4f48620ba95dfbf1b Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Sat, 30 Oct 2021 20:41:32 -0400 Subject: [PATCH 12/15] sparse: add vfs-specific precautions * t1092: remove the 'git update-index' test that currently fails because the command ignores the bad path, but doesn't return a failure. * dir.c: prevent matching against sparse-checkout patterns when the virtual filesystem is enabled. Should prevent some corner case issues. * t1092: add quiet mode for some rebase tests because the stderr output can change in some of the modes. Signed-off-by: Derrick Stolee --- dir.c | 7 +++++++ t/t1092-sparse-checkout-compatibility.sh | 4 +++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/dir.c b/dir.c index 28da33f875..8b2634e8a5 100644 --- a/dir.c +++ b/dir.c @@ -1527,6 +1527,13 @@ static int path_in_sparse_checkout_1(const char *path, enum pattern_match_result match = UNDECIDED; const char *end, *slash; + /* + * When using a virtual filesystem, there aren't really patterns + * to follow, but be extra careful to skip this check. + */ + if (core_virtualfilesystem) + return 1; + /* * We default to accepting a path if the path is empty, there are no * patterns, or the patterns are of the wrong type. diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh index f15394b131..268f7b350b 100755 --- a/t/t1092-sparse-checkout-compatibility.sh +++ b/t/t1092-sparse-checkout-compatibility.sh @@ -982,7 +982,9 @@ test_expect_success 'read-tree --merge with directory-file conflicts' ' test_expect_success 'merge, cherry-pick, and rebase' ' init_repos && - for OPERATION in "merge -m merge" cherry-pick "rebase --apply" "rebase --merge" + # microsoft/git specific: we need to use "quiet" mode + # to avoid different stderr for some rebases. + for OPERATION in "merge -m merge" cherry-pick "rebase -q --apply" "rebase -q --merge" do test_all_match git checkout -B temp update-deep && test_all_match git $OPERATION update-folder1 && From c32dd5b0065098b01891970233297e0e7d5fce51 Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Tue, 16 Nov 2021 10:26:08 -0500 Subject: [PATCH 13/15] maintenance: delete stale lock files The maintenance.lock file exists to prevent concurrent maintenance processes from writing to a repository at the same time. However, it has the downside of causing maintenance to start failing without recovery if there is any reason why the maintenance command failed without cleaning up the lock-file. This change makes it such that maintenance will delete a lock file that was modified over 6 hours ago. This will auto-heal repositories that are stuck with failed maintenance (and maybe it will fail again, but we will get a message other than the lock file exists). Signed-off-by: Derrick Stolee --- builtin/gc.c | 21 +++++++++++++++++++++ t/t7900-maintenance.sh | 17 +++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/builtin/gc.c b/builtin/gc.c index 02be8f5551..0ce8cfeb7b 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -1284,6 +1284,8 @@ static int maintenance_run_tasks(struct maintenance_run_opts *opts) char *lock_path = xstrfmt("%s/maintenance", r->objects->odb->path); if (hold_lock_file_for_update(&lk, lock_path, LOCK_NO_DEREF) < 0) { + struct stat st; + struct strbuf lock_dot_lock = STRBUF_INIT; /* * Another maintenance command is running. * @@ -1294,6 +1296,25 @@ static int maintenance_run_tasks(struct maintenance_run_opts *opts) if (!opts->auto_flag && !opts->quiet) warning(_("lock file '%s' exists, skipping maintenance"), lock_path); + + /* + * Check timestamp on .lock file to see if we should + * delete it to recover from a fail state. + */ + strbuf_addstr(&lock_dot_lock, lock_path); + strbuf_addstr(&lock_dot_lock, ".lock"); + if (lstat(lock_dot_lock.buf, &st)) + warning_errno(_("unable to stat '%s'"), lock_dot_lock.buf); + else { + if (st.st_mtime < time(NULL) - (6 * 60 * 60)) { + if (unlink(lock_dot_lock.buf)) + warning_errno(_("unable to delete stale lock file")); + else + warning(_("deleted stale lock file")); + } + } + + strbuf_release(&lock_dot_lock); free(lock_path); return 0; } diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh index 74aa638475..0ab49b0ecc 100755 --- a/t/t7900-maintenance.sh +++ b/t/t7900-maintenance.sh @@ -52,6 +52,23 @@ test_expect_success 'run [--auto|--quiet]' ' test_subcommand git gc --no-quiet err && + grep "lock file .* exists, skipping maintenance" err && + + test-tool chmtime =-22000 .git/objects/maintenance.lock && + git maintenance run --schedule=hourly --no-quiet 2>err && + grep "deleted stale lock file" err && + test_path_is_missing .git/objects/maintenance.lock && + + git maintenance run --schedule=hourly 2>err && + test_must_be_empty err +' + test_expect_success 'maintenance.auto config option' ' GIT_TRACE2_EVENT="$(pwd)/default" git commit --quiet --allow-empty -m 1 && test_subcommand git maintenance run --auto --quiet Date: Sun, 31 Oct 2021 11:47:02 -0400 Subject: [PATCH 14/15] t0001: drop memory-leak check Signed-off-by: Derrick Stolee --- t/t0001-init.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/t/t0001-init.sh b/t/t0001-init.sh index d479303efa..61232bcfbd 100755 --- a/t/t0001-init.sh +++ b/t/t0001-init.sh @@ -2,7 +2,8 @@ test_description='git init' -TEST_PASSES_SANITIZE_LEAK=true +# Drop this leak check because it doesn't work on every platform. +# TEST_PASSES_SANITIZE_LEAK=true . ./test-lib.sh check_config () { From 737c66e955fbbb20e018486230e7fb03f573631b Mon Sep 17 00:00:00 2001 From: Kevin Willford Date: Wed, 15 Mar 2017 16:36:53 -0600 Subject: [PATCH 15/15] reset: fix mixed reset when using virtual filesystem During the 2.35.0 rebase, we ejected 570f64b (Fix reset when using the sparse-checkout feature., 2017-03-15) because of a similar change upstream that actually works with the expected behavior of sparse-checkout. That commit only ever existed in microsoft/git, but when it was considered for upstream we realized that it behaved strangely for a sparse-checkout scenario. The root problem is that during a mixed reset, 'git reset ' updates the index to aggree with but leaves the worktree the same as it was before. The issue with sparse-checkout is that some files might not be in the worktree and thus the information from those files would be "lost". The upstream decision was to leave these files as ignored, because that's what the SKIP_WORKTREE bit means: don't put these files in the worktree and ignore their contents. If there already were files in the worktree, then Git does not change them. The case for "losing" data is if a committed change outside of the sparse-checkout was in the previous HEAD position. However, this information could be recovered from the reflog. The case where this is different is in a virtualized filesystem. The virtualization is projecting the index contents onto the filesystem, so we need to do something different here. In a virtual environment, every file is considered "important" and we abuse the SKIP_WORKTREE bit to indicate that Git does not need to process a projected file. When a file is populated, the virtual filesystem hook provides the information for removing the SKIP_WORKTREE bit. In the case of these mixed resets, we have the issue where we change the projection of the worktree for these cache entries that change. If a file is populated in the worktree, then the populated file will persist and appear in a follow-up 'git status'. However, if the file is not populated and only projected, we change the projection from the current value to the new value, leaving a clean 'git status'. The previous version of this commit includes a call to checkout_entry(), which populates the file. This causes the file to be actually in the working tree and no longer projected. To make this work with the upstream changes, stop setting the skip-worktree bit for the new cache entry. This seemed to work fine without this change, but it's likely due to some indirection with the virtual filesystem. Better to do the best-possible thing here so we don't hide a corner-case bug by accident. Helped-by: Victoria Dye Signed-off-by: Kevin Willford Signed-off-by: Derrick Stolee --- builtin/reset.c | 50 +++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 48 insertions(+), 2 deletions(-) diff --git a/builtin/reset.c b/builtin/reset.c index ef514f861a..680a419fed 100644 --- a/builtin/reset.c +++ b/builtin/reset.c @@ -28,6 +28,8 @@ #include "dir.h" #include "strbuf.h" #include "quote.h" +#include "dir.h" +#include "entry.h" #define REFRESH_INDEX_DELAY_WARNING_IN_MS (2 * 1000) @@ -141,9 +143,47 @@ static void update_index_from_diff(struct diff_queue_struct *q, for (i = 0; i < q->nr; i++) { int pos; + int respect_skip_worktree = 1; struct diff_filespec *one = q->queue[i]->one; + struct diff_filespec *two = q->queue[i]->two; int is_in_reset_tree = one->mode && !is_null_oid(&one->oid); + int is_missing = !(one->mode && !is_null_oid(&one->oid)); + int was_missing = !two->mode && is_null_oid(&two->oid); struct cache_entry *ce; + struct cache_entry *ceBefore; + struct checkout state = CHECKOUT_INIT; + + /* + * When using the virtual filesystem feature, the cache entries that are + * added here will not have the skip-worktree bit set. + * + * Without this code there is data that is lost because the files that + * would normally be in the working directory are not there and show as + * deleted for the next status or in the case of added files just disappear. + * We need to create the previous version of the files in the working + * directory so that they will have the right content and the next + * status call will show modified or untracked files correctly. + */ + if (core_virtualfilesystem && !file_exists(two->path)) + { + pos = cache_name_pos(two->path, strlen(two->path)); + if ((pos >= 0 && ce_skip_worktree(active_cache[pos])) && + (is_missing || !was_missing)) + { + state.force = 1; + state.refresh_cache = 1; + state.istate = &the_index; + ceBefore = make_cache_entry(&the_index, two->mode, + &two->oid, two->path, + 0, 0); + if (!ceBefore) + die(_("make_cache_entry failed for path '%s'"), + two->path); + + checkout_entry(ceBefore, &state, NULL, NULL); + respect_skip_worktree = 0; + } + } if (!is_in_reset_tree && !intent_to_add) { remove_file_from_cache(one->path); @@ -162,8 +202,14 @@ static void update_index_from_diff(struct diff_queue_struct *q, * to properly construct the reset sparse directory. */ pos = cache_name_pos(one->path, strlen(one->path)); - if ((pos >= 0 && ce_skip_worktree(active_cache[pos])) || - (pos < 0 && !path_in_sparse_checkout(one->path, &the_index))) + + /* + * Do not add the SKIP_WORKTREE bit back if we populated the + * file on purpose in a virtual filesystem scenario. + */ + if (respect_skip_worktree && + ((pos >= 0 && ce_skip_worktree(active_cache[pos])) || + (pos < 0 && !path_in_sparse_checkout(one->path, &the_index)))) ce->ce_flags |= CE_SKIP_WORKTREE; if (!ce)