From 009f63f46d25f4e33539a2f2af1b14040d6666c8 Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Sun, 22 Aug 2021 14:55:59 -0400 Subject: [PATCH 01/10] 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 23c7985eb40974..404e7bd37f80f3 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 5c8ffb1f7598b0..75295951eb7974 100644 --- a/builtin/sparse-checkout.c +++ b/builtin/sparse-checkout.c @@ -111,7 +111,7 @@ static int sparse_checkout_list(int argc, const char **argv, const char *prefix) 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; @@ -127,6 +127,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 9ceb17f9118919..8a9ae66d578fda 100755 --- a/t/t1091-sparse-checkout-builtin.sh +++ b/t/t1091-sparse-checkout-builtin.sh @@ -782,6 +782,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 53d2460f99b0f2cba64813e20bd0d60f124514cb Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Mon, 26 Jul 2021 15:43:05 -0400 Subject: [PATCH 02/10] 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 ee3eb629e3dc5e..9797ad009ceb68 100644 --- a/diff.c +++ b/diff.c @@ -3994,6 +3994,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 1915faf0ea3d392bfc78ff74bcb75cb436271a31 Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Thu, 23 Sep 2021 09:59:06 -0400 Subject: [PATCH 03/10] 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 adc9cfb4df39f2..1039fedec79599 100644 --- a/sequencer.c +++ b/sequencer.c @@ -699,7 +699,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 ? repo_get_commit_tree(r, next) : empty_tree(r); From 5b4db8ba1976653dc4ad0dbb6005ab587ff0304e Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Tue, 29 Jun 2021 11:12:56 -0400 Subject: [PATCH 04/10] 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 | 20 +++++++++++++++----- builtin/rm.c | 9 +++++++-- read-cache.c | 2 +- 3 files changed, 23 insertions(+), 8 deletions(-) diff --git a/builtin/add.c b/builtin/add.c index 03ee5a3a7a2e35..620430cabf6f3d 100644 --- a/builtin/add.c +++ b/builtin/add.c @@ -5,6 +5,7 @@ */ #define USE_THE_INDEX_VARIABLE #include "builtin.h" +#include "environment.h" #include "advice.h" #include "config.h" #include "lockfile.h" @@ -48,6 +49,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; @@ -128,8 +130,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 { @@ -139,7 +142,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; } @@ -517,7 +524,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); @@ -541,7 +552,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 dff819ae5098ff..82501a3df8ada1 100644 --- a/builtin/rm.c +++ b/builtin/rm.c @@ -5,6 +5,7 @@ */ #define USE_THE_INDEX_VARIABLE #include "builtin.h" +#include "environment.h" #include "advice.h" #include "config.h" #include "lockfile.h" @@ -312,7 +313,7 @@ int cmd_rm(int argc, const char **argv, const char *prefix) for (i = 0; i < the_index.cache_nr; i++) { const struct cache_entry *ce = the_index.cache[i]; - if (!include_sparse && + if (!include_sparse && !core_virtualfilesystem && (ce_skip_worktree(ce) || !path_in_sparse_checkout(ce->name, &the_index))) continue; @@ -349,7 +350,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; } diff --git a/read-cache.c b/read-cache.c index a08daf87dcd1ae..ea7e08f350b35e 100644 --- a/read-cache.c +++ b/read-cache.c @@ -3943,7 +3943,7 @@ static void update_callback(struct diff_queue_struct *q, struct diff_filepair *p = q->queue[i]; const char *path = p->one->path; - if (!data->include_sparse && + if (!data->include_sparse && !core_virtualfilesystem && !path_in_sparse_checkout(path, data->index)) continue; From 67fe9e3e630bd97b93a5405596ca3628b027eb10 Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Tue, 15 Jun 2021 11:07:11 -0400 Subject: [PATCH 05/10] 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/perf/p2000-sparse-operations.sh | 4 ++-- t/t1092-sparse-checkout-compatibility.sh | 1 + t/t7817-grep-sparse-checkout.sh | 2 +- 4 files changed, 5 insertions(+), 4 deletions(-) diff --git a/repo-settings.c b/repo-settings.c index fb76ee4c69ca7e..b11ce98a5499bf 100644 --- a/repo-settings.c +++ b/repo-settings.c @@ -87,7 +87,7 @@ void prepare_repo_settings(struct repository *r) /* Boolean config or default, does not cascade (simple) */ 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); repo_cfg_bool(r, "index.skiphash", &r->settings.index_skip_hash, r->settings.index_skip_hash); repo_cfg_bool(r, "pack.readreverseindex", &r->settings.pack_read_reverse_index, 1); repo_cfg_bool(r, "pack.usebitmapboundarytraversal", diff --git a/t/perf/p2000-sparse-operations.sh b/t/perf/p2000-sparse-operations.sh index 96ed3e1d698815..376d3c6abc9c77 100755 --- a/t/perf/p2000-sparse-operations.sh +++ b/t/perf/p2000-sparse-operations.sh @@ -56,7 +56,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 && @@ -65,7 +65,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 && diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh index 8a95adf4b50de6..f40b5914d70223 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 eb595645657fad..db3004c4fe71c0 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 c474fcf3d91ee0f56b2b73380e6db34cac80e857 Mon Sep 17 00:00:00 2001 From: Lessley Dennington Date: Fri, 10 Sep 2021 13:57:25 -0700 Subject: [PATCH 06/10] 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 f40b5914d70223..2866ba098756e0 100755 --- a/t/t1092-sparse-checkout-compatibility.sh +++ b/t/t1092-sparse-checkout-compatibility.sh @@ -518,6 +518,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' ' @@ -1453,6 +1490,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 33f26b6ecfd909ee5f561cee05b9a16cfea47d51 Mon Sep 17 00:00:00 2001 From: Victoria Dye Date: Wed, 22 Sep 2021 14:02:21 -0400 Subject: [PATCH 07/10] 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 2866ba098756e0..ce27112f1fe219 100755 --- a/t/t1092-sparse-checkout-compatibility.sh +++ b/t/t1092-sparse-checkout-compatibility.sh @@ -1815,6 +1815,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 e166a2c3839e90461c9e66adb89a221e8c310548 Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Tue, 16 Nov 2021 10:26:08 -0500 Subject: [PATCH 08/10] 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 cf5a1e279b9cb3..d2d10b34e515e6 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -1309,6 +1309,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. * @@ -1319,6 +1321,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 487e326b3fac12..5ab6263b792d3f 100755 --- a/t/t7900-maintenance.sh +++ b/t/t7900-maintenance.sh @@ -54,6 +54,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: Sat, 30 Oct 2021 20:41:32 -0400 Subject: [PATCH 09/10] 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 3aacd5987e2dcb..095637ab356ee9 100644 --- a/dir.c +++ b/dir.c @@ -1533,6 +1533,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 ce27112f1fe219..542bb96c82982a 100755 --- a/t/t1092-sparse-checkout-compatibility.sh +++ b/t/t1092-sparse-checkout-compatibility.sh @@ -1029,7 +1029,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 779c87cf0efcd1974806121afcad3e5ca3f21c7e Mon Sep 17 00:00:00 2001 From: Kevin Willford Date: Wed, 15 Mar 2017 16:36:53 -0600 Subject: [PATCH 10/10] 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 7a3e08bd1388b2..349a9d0d036c79 100644 --- a/builtin/reset.c +++ b/builtin/reset.c @@ -40,6 +40,8 @@ #include "add-interactive.h" #include "strbuf.h" #include "quote.h" +#include "dir.h" +#include "entry.h" #define REFRESH_INDEX_DELAY_WARNING_IN_MS (2 * 1000) @@ -156,9 +158,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 = index_name_pos(&the_index, two->path, strlen(two->path)); + if ((pos >= 0 && ce_skip_worktree(the_index.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_index(&the_index, one->path); @@ -177,8 +217,14 @@ static void update_index_from_diff(struct diff_queue_struct *q, * to properly construct the reset sparse directory. */ pos = index_name_pos(&the_index, one->path, strlen(one->path)); - if ((pos >= 0 && ce_skip_worktree(the_index.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(the_index.cache[pos])) || + (pos < 0 && !path_in_sparse_checkout(one->path, &the_index)))) ce->ce_flags |= CE_SKIP_WORKTREE; if (!ce)