From 287fd17e3a1daec2b0d0d8e26d2b2318511ec153 Mon Sep 17 00:00:00 2001 From: Victoria Dye Date: Tue, 1 Mar 2022 20:24:24 +0000 Subject: sparse-index: prevent repo root from becoming sparse Prevent the repository root from being collapsed into a sparse directory by treating an empty path as "inside the sparse-checkout". When collapsing a sparse index (e.g. in 'git sparse-checkout reapply'), the root directory typically could not become a sparse directory due to the presence of in-cone root-level files and directories. However, if no such in-cone files or directories were present, there was no explicit check signaling that the "repository root path" (an empty string, in the case of 'convert_to_sparse(...)') was in-cone, and a sparse directory index entry would be created from the repository root directory. The documentation in Documentation/git-sparse-checkout.txt explicitly states that the files in the root directory are expected to be in-cone for a cone-mode sparse-checkout. Collapsing the root into a sparse directory entry violates that assumption, as sparse directory entries are expected to be outside the sparse cone and have SKIP_WORKTREE enabled. This invalid state in turn causes issues with commands that interact with the index, e.g. 'git status'. Treating an empty (root) path as in-cone prevents the creation of a root sparse directory in 'convert_to_sparse(...)'. Because the repository root is otherwise never compared with sparse patterns (in both cone-mode and non-cone sparse-checkouts), the new check does not cause additional changes to how sparse patterns are applied. Helped-by: Derrick Stolee Signed-off-by: Victoria Dye Signed-off-by: Junio C Hamano diff --git a/dir.c b/dir.c index d91295f..a136377 100644 --- a/dir.c +++ b/dir.c @@ -1463,10 +1463,11 @@ static int path_in_sparse_checkout_1(const char *path, const char *end, *slash; /* - * We default to accepting a path if there are no patterns or - * they are of the wrong type. + * We default to accepting a path if the path is empty, there are no + * patterns, or the patterns are of the wrong type. */ - if (init_sparse_checkout_patterns(istate) || + if (!*path || + init_sparse_checkout_patterns(istate) || (require_cone_mode && !istate->sparse_checkout_patterns->use_cone_patterns)) return 1; diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh index f3a059e..9ef7cd8 100755 --- a/t/t1092-sparse-checkout-compatibility.sh +++ b/t/t1092-sparse-checkout-compatibility.sh @@ -244,6 +244,24 @@ test_expect_success 'expanded in-memory index matches full index' ' test_sparse_match git ls-files --stage ' +test_expect_success 'root directory cannot be sparse' ' + init_repos && + + # Remove all in-cone files and directories from the index, collapse index + # with `git sparse-checkout reapply` + git -C sparse-index rm -r . && + git -C sparse-index sparse-checkout reapply && + + # Verify sparse directories still present, root directory is not sparse + cat >expect <<-EOF && + folder1/ + folder2/ + x/ + EOF + git -C sparse-index ls-files --sparse >actual && + test_cmp expect actual +' + test_expect_success 'status with options' ' init_repos && test_sparse_match ls && -- cgit v0.10.2-6-g49f6 From 2c521b0e4900d8e7ff1f611ed956cfdd67f03eb0 Mon Sep 17 00:00:00 2001 From: Victoria Dye Date: Tue, 1 Mar 2022 20:24:25 +0000 Subject: status: fix nested sparse directory diff in sparse index Enable the 'recursive' diff option for the diff executed as part of 'git status'. Without the 'recursive' enabled, 'git status' reports index changes incorrectly when the following conditions were met: * sparse index is enabled * there is a difference between the index and HEAD in a file inside a *subdirectory* of a sparse directory * the sparse directory index entry is *not* expanded in-core Because it is not recursive by default, the diff in 'git status' reports changes only at the level of files and directories that are immediate children of a sparse directory, rather than recursing into directories with changes to identify the modified file(s). As a result, 'git status' reports the immediate subdirectory itself as "modified". Example: $ git init $ mkdir -p sparse/sub $ echo test >sparse/sub/foo $ git add . $ git commit -m "commit 1" $ echo somethingelse >sparse/sub/foo $ git add . $ git commit -a -m "commit 2" $ git sparse-checkout set --cone --sparse-index 'sparse' $ git reset --soft HEAD~1 $ git status On branch master You are in a sparse checkout. Changes to be committed: (use "git restore --staged ..." to unstage) modified: sparse/sub Enabling the 'recursive' diff option in 'wt_status_collect_changes_index' corrects this issue by allowing the diff to recurse into subdirectories of sparse directories to find modified files. Given the same repository setup as the example above, the corrected result of `git status` is: $ git status On branch master You are in a sparse checkout. Changes to be committed: (use "git restore --staged ..." to unstage) modified: sparse/sub/foo Signed-off-by: Victoria Dye Signed-off-by: Junio C Hamano diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh index 9ef7cd8..b1dcaa0 100755 --- a/t/t1092-sparse-checkout-compatibility.sh +++ b/t/t1092-sparse-checkout-compatibility.sh @@ -278,6 +278,13 @@ test_expect_success 'status with options' ' test_all_match git status --porcelain=v2 -uno ' +test_expect_success 'status with diff in unexpanded sparse directory' ' + init_repos && + test_all_match git checkout rename-base && + test_all_match git reset --soft rename-out-to-out && + test_all_match git status --porcelain=v2 +' + test_expect_success 'status reports sparse-checkout' ' init_repos && git -C sparse-checkout status >full && diff --git a/wt-status.c b/wt-status.c index 335e723..7da8bbe 100644 --- a/wt-status.c +++ b/wt-status.c @@ -651,6 +651,15 @@ static void wt_status_collect_changes_index(struct wt_status *s) rev.diffopt.detect_rename = s->detect_rename >= 0 ? s->detect_rename : rev.diffopt.detect_rename; rev.diffopt.rename_limit = s->rename_limit >= 0 ? s->rename_limit : rev.diffopt.rename_limit; rev.diffopt.rename_score = s->rename_score >= 0 ? s->rename_score : rev.diffopt.rename_score; + + /* + * The `recursive` option must be enabled to allow the diff to recurse + * into subdirectories of sparse directory index entries. If it is not + * enabled, a subdirectory containing file(s) with changes is reported + * as "modified", rather than the modified files themselves. + */ + rev.diffopt.flags.recursive = 1; + copy_pathspec(&rev.prune_data, &s->pathspec); run_diff_index(&rev, 1); object_array_clear(&rev.pending); -- cgit v0.10.2-6-g49f6 From cc89331ddc92cd89012eaf7937d167b3e0beaecc Mon Sep 17 00:00:00 2001 From: Victoria Dye Date: Tue, 1 Mar 2022 20:24:26 +0000 Subject: read-tree: explicitly disallow prefixes with a leading '/' Exit with an error if a prefix provided to `git read-tree --prefix` begins with '/'. In most cases, prefixes like this result in an "invalid path" error; however, the repository root would be interpreted as valid when specified as '--prefix=/'. This is due to leniency around trailing directory separators on prefixes (e.g., allowing both '--prefix=my-dir' and '--prefix=my-dir/') - the '/' in the prefix is actually the *trailing* slash, although it could be misinterpreted as a *leading* slash. To remove the confusing repo root-as-'/' case and make it clear that prefixes should not begin with '/', exit with an error if the first character of the provided prefix is '/'. Helped-by: Elijah Newren Signed-off-by: Victoria Dye Signed-off-by: Junio C Hamano diff --git a/builtin/read-tree.c b/builtin/read-tree.c index 2109c4c..c1a4019 100644 --- a/builtin/read-tree.c +++ b/builtin/read-tree.c @@ -166,6 +166,10 @@ int cmd_read_tree(int argc, const char **argv, const char *cmd_prefix) if (1 < opts.merge + opts.reset + prefix_set) die("Which one? -m, --reset, or --prefix?"); + /* Prefix should not start with a directory separator */ + if (opts.prefix && opts.prefix[0] == '/') + die("Invalid prefix, prefix cannot start with '/'"); + if (opts.reset) opts.reset = UNPACK_RESET_OVERWRITE_UNTRACKED; diff --git a/t/t1003-read-tree-prefix.sh b/t/t1003-read-tree-prefix.sh index e0db206..c860c08 100755 --- a/t/t1003-read-tree-prefix.sh +++ b/t/t1003-read-tree-prefix.sh @@ -25,4 +25,14 @@ test_expect_success 'read-tree --prefix' ' cmp expect actual ' +test_expect_success 'read-tree --prefix with leading slash exits with error' ' + git rm -rf . && + test_must_fail git read-tree --prefix=/two/ $tree && + git read-tree --prefix=two/ $tree && + + git rm -rf . && + test_must_fail git read-tree --prefix=/ $tree && + git read-tree --prefix= $tree +' + test_done -- cgit v0.10.2-6-g49f6 From 14bf38cfcff0b68468444e85bc09c7d5007be34b Mon Sep 17 00:00:00 2001 From: Victoria Dye Date: Tue, 1 Mar 2022 20:24:27 +0000 Subject: read-tree: expand sparse checkout test coverage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add tests focused on how 'git read-tree' behaves in sparse checkouts. Extra emphasis is placed on interactions with files outside the sparse cone, e.g. merges with out-of-cone conflicts. Helped-by: Ævar Arnfjörð Bjarmason Signed-off-by: Victoria Dye Signed-off-by: Junio C Hamano diff --git a/t/perf/p2000-sparse-operations.sh b/t/perf/p2000-sparse-operations.sh index 2a7106b..382716c 100755 --- a/t/perf/p2000-sparse-operations.sh +++ b/t/perf/p2000-sparse-operations.sh @@ -117,6 +117,7 @@ test_perf_on_all git diff test_perf_on_all git diff --cached test_perf_on_all git blame $SPARSE_CONE/a 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 diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh index b1dcaa0..9bb5aeb 100755 --- a/t/t1092-sparse-checkout-compatibility.sh +++ b/t/t1092-sparse-checkout-compatibility.sh @@ -819,6 +819,93 @@ test_expect_success 'update-index --cacheinfo' ' test_cmp expect sparse-checkout-out ' +for MERGE_TREES in "base HEAD update-folder2" \ + "update-folder1 update-folder2" \ + "update-folder2" +do + test_expect_success "'read-tree -mu $MERGE_TREES' with files outside sparse definition" ' + init_repos && + + # Although the index matches, without --no-sparse-checkout, outside-of- + # definition files will not exist on disk for sparse checkouts + test_all_match git read-tree -mu $MERGE_TREES && + test_all_match git status --porcelain=v2 && + test_path_is_missing sparse-checkout/folder2 && + test_path_is_missing sparse-index/folder2 && + + test_all_match git read-tree --reset -u HEAD && + test_all_match git status --porcelain=v2 && + + test_all_match git read-tree -mu --no-sparse-checkout $MERGE_TREES && + test_all_match git status --porcelain=v2 && + test_cmp sparse-checkout/folder2/a sparse-index/folder2/a && + test_cmp sparse-checkout/folder2/a full-checkout/folder2/a + + ' +done + +test_expect_success 'read-tree --merge with edit/edit conflicts in sparse directories' ' + init_repos && + + # Merge of multiple changes to same directory (but not same files) should + # succeed + test_all_match git read-tree -mu base rename-base update-folder1 && + test_all_match git status --porcelain=v2 && + + test_all_match git reset --hard && + + test_all_match git read-tree -mu rename-base update-folder2 && + test_all_match git status --porcelain=v2 && + + test_all_match git reset --hard && + + test_all_match test_must_fail git read-tree -mu base update-folder1 rename-out-to-in && + test_all_match test_must_fail git read-tree -mu rename-out-to-in update-folder1 +' + +test_expect_success 'read-tree --prefix' ' + init_repos && + + # If files differing between the index and target exist + # inside the prefix, `read-tree --prefix` should fail + test_all_match test_must_fail git read-tree --prefix=deep/ deepest && + test_all_match test_must_fail git read-tree --prefix=folder1/ update-folder1 && + + # If no differing index entries exist matching the prefix, + # `read-tree --prefix` updates the index successfully + test_all_match git rm -rf deep/deeper1/deepest/ && + test_all_match git read-tree --prefix=deep/deeper1/deepest -u deepest && + test_all_match git status --porcelain=v2 && + + test_all_match git rm -rf --sparse folder1/ && + test_all_match git read-tree --prefix=folder1/ -u update-folder1 && + test_all_match git status --porcelain=v2 && + + test_all_match git rm -rf --sparse folder2/0 && + test_all_match git read-tree --prefix=folder2/0/ -u rename-out-to-out && + test_all_match git status --porcelain=v2 +' + +test_expect_success 'read-tree --merge with directory-file conflicts' ' + init_repos && + + test_all_match git checkout -b test-branch rename-base && + + # Although the index matches, without --no-sparse-checkout, outside-of- + # definition files will not exist on disk for sparse checkouts + test_sparse_match git read-tree -mu rename-out-to-out && + test_sparse_match git status --porcelain=v2 && + test_path_is_missing sparse-checkout/folder2 && + test_path_is_missing sparse-index/folder2 && + + test_sparse_match git read-tree --reset -u HEAD && + test_sparse_match git status --porcelain=v2 && + + test_sparse_match git read-tree -mu --no-sparse-checkout rename-out-to-out && + test_sparse_match git status --porcelain=v2 && + test_cmp sparse-checkout/folder2/0/1 sparse-index/folder2/0/1 +' + test_expect_success 'merge, cherry-pick, and rebase' ' init_repos && -- cgit v0.10.2-6-g49f6 From 2c66a7c8cefa27ef56efbc76bdfe13696b9dac3a Mon Sep 17 00:00:00 2001 From: Victoria Dye Date: Tue, 1 Mar 2022 20:24:28 +0000 Subject: read-tree: integrate with sparse index Enable use of sparse index in 'git read-tree'. The integration in this patch is limited only to usage of 'read-tree' that does not need additional functional changes for the sparse index to behave as expected (i.e., produce the same user-facing results as a non-sparse index sparse-checkout). To ensure no unexpected behavior occurs, the index is explicitly expanded when: * '--no-sparse-checkout' is specified (because it disables sparse-checkout) * '--prefix' is specified (if the prefix is inside a sparse directory, the prefixed tree cannot be properly traversed) * two or more arguments are specified ('twoway_merge' and 'threeway_merge' do not yet support merging sparse directories) Signed-off-by: Victoria Dye Signed-off-by: Junio C Hamano diff --git a/builtin/read-tree.c b/builtin/read-tree.c index c1a4019..0a52cab 100644 --- a/builtin/read-tree.c +++ b/builtin/read-tree.c @@ -160,8 +160,6 @@ int cmd_read_tree(int argc, const char **argv, const char *cmd_prefix) argc = parse_options(argc, argv, cmd_prefix, read_tree_options, read_tree_usage, 0); - hold_locked_index(&lock_file, LOCK_DIE_ON_ERROR); - prefix_set = opts.prefix ? 1 : 0; if (1 < opts.merge + opts.reset + prefix_set) die("Which one? -m, --reset, or --prefix?"); @@ -173,6 +171,11 @@ int cmd_read_tree(int argc, const char **argv, const char *cmd_prefix) if (opts.reset) opts.reset = UNPACK_RESET_OVERWRITE_UNTRACKED; + prepare_repo_settings(the_repository); + the_repository->settings.command_requires_full_index = 0; + + hold_locked_index(&lock_file, LOCK_DIE_ON_ERROR); + /* * NEEDSWORK * @@ -214,6 +217,10 @@ int cmd_read_tree(int argc, const char **argv, const char *cmd_prefix) if (opts.merge && !opts.index_only) setup_work_tree(); + /* TODO: audit sparse index behavior in unpack_trees */ + if (opts.skip_sparse_checkout || opts.prefix) + ensure_full_index(&the_index); + if (opts.merge) { switch (stage - 1) { case 0: @@ -223,11 +230,21 @@ int cmd_read_tree(int argc, const char **argv, const char *cmd_prefix) opts.fn = opts.prefix ? bind_merge : oneway_merge; break; case 2: + /* + * TODO: update twoway_merge to handle edit/edit conflicts in + * sparse directories. + */ + ensure_full_index(&the_index); opts.fn = twoway_merge; opts.initial_checkout = is_cache_unborn(); break; case 3: default: + /* + * TODO: update threeway_merge to handle edit/edit conflicts in + * sparse directories. + */ + ensure_full_index(&the_index); opts.fn = threeway_merge; break; } diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh index 9bb5aeb..86241b0 100755 --- a/t/t1092-sparse-checkout-compatibility.sh +++ b/t/t1092-sparse-checkout-compatibility.sh @@ -1409,6 +1409,17 @@ test_expect_success 'sparse index is not expanded: fetch/pull' ' ensure_not_expanded pull full base ' +test_expect_success 'sparse index is not expanded: read-tree' ' + init_repos && + + ensure_not_expanded checkout -b test-branch update-folder1 && + for MERGE_TREES in "update-folder2" + do + ensure_not_expanded read-tree -mu $MERGE_TREES && + ensure_not_expanded reset --hard || return 1 + done +' + test_expect_success 'ls-files' ' init_repos && -- cgit v0.10.2-6-g49f6 From 749703924121b9eb2750b4313b2c769113c8b310 Mon Sep 17 00:00:00 2001 From: Victoria Dye Date: Tue, 1 Mar 2022 20:24:29 +0000 Subject: read-tree: narrow scope of index expansion for '--prefix' When 'git read-tree' is provided with a prefix, expand the index only if the prefix is equivalent to a sparse directory or contained within one. If the index is not expanded in these cases, 'ce_in_traverse_path' will indicate that the relevant sparse directory is not in the prefix/traverse path, skipping past it and not unpacking the appropriate tree(s). If the prefix is in-cone, its sparse subdirectories (if any) will be traversed correctly without index expansion. The behavior of 'git read-tree' with prefixes 1) inside of cone, 2) equal to a sparse directory, and 3) inside a sparse directory are all tested as part of the 't/t1092-sparse-checkout-compatibility.sh' test 'read-tree --prefix', ensuring that the sparse index case works the way it did prior to this change as well as matching non-sparse index sparse-checkout. Helped-by: Elijah Newren Signed-off-by: Victoria Dye Signed-off-by: Junio C Hamano diff --git a/builtin/read-tree.c b/builtin/read-tree.c index 0a52cab..ec6d038 100644 --- a/builtin/read-tree.c +++ b/builtin/read-tree.c @@ -217,8 +217,7 @@ int cmd_read_tree(int argc, const char **argv, const char *cmd_prefix) if (opts.merge && !opts.index_only) setup_work_tree(); - /* TODO: audit sparse index behavior in unpack_trees */ - if (opts.skip_sparse_checkout || opts.prefix) + if (opts.skip_sparse_checkout) ensure_full_index(&the_index); if (opts.merge) { diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh index 86241b0..d98558f 100755 --- a/t/t1092-sparse-checkout-compatibility.sh +++ b/t/t1092-sparse-checkout-compatibility.sh @@ -1417,7 +1417,13 @@ test_expect_success 'sparse index is not expanded: read-tree' ' do ensure_not_expanded read-tree -mu $MERGE_TREES && ensure_not_expanded reset --hard || return 1 - done + done && + + rm -rf sparse-index/deep/deeper2 && + ensure_not_expanded add . && + ensure_not_expanded commit -m "test" && + + ensure_not_expanded read-tree --prefix=deep/deeper2 -u deepest ' test_expect_success 'ls-files' ' diff --git a/unpack-trees.c b/unpack-trees.c index 360844b..f3667d8 100644 --- a/unpack-trees.c +++ b/unpack-trees.c @@ -1693,6 +1693,41 @@ static void populate_from_existing_patterns(struct unpack_trees_options *o, o->pl = pl; } +static void update_sparsity_for_prefix(const char *prefix, + struct index_state *istate) +{ + int prefix_len = strlen(prefix); + struct strbuf ce_prefix = STRBUF_INIT; + + if (!istate->sparse_index) + return; + + while (prefix_len > 0 && prefix[prefix_len - 1] == '/') + prefix_len--; + + if (prefix_len <= 0) + BUG("Invalid prefix passed to update_sparsity_for_prefix"); + + strbuf_grow(&ce_prefix, prefix_len + 1); + strbuf_add(&ce_prefix, prefix, prefix_len); + strbuf_addch(&ce_prefix, '/'); + + /* + * If the prefix points to a sparse directory or a path inside a sparse + * directory, the index should be expanded. This is accomplished in one + * of two ways: + * - if the prefix is inside a sparse directory, it will be expanded by + * the 'ensure_full_index(...)' call in 'index_name_pos(...)'. + * - if the prefix matches an existing sparse directory entry, + * 'index_name_pos(...)' will return its index position, triggering + * the 'ensure_full_index(...)' below. + */ + if (!path_in_cone_mode_sparse_checkout(ce_prefix.buf, istate) && + index_name_pos(istate, ce_prefix.buf, ce_prefix.len) >= 0) + ensure_full_index(istate); + + strbuf_release(&ce_prefix); +} static int verify_absent(const struct cache_entry *, enum unpack_trees_error_types, @@ -1739,6 +1774,9 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options setup_standard_excludes(o->dir); } + if (o->prefix) + update_sparsity_for_prefix(o->prefix, o->src_index); + if (!core_apply_sparse_checkout || !o->update) o->skip_sparse_checkout = 1; if (!o->skip_sparse_checkout && !o->pl) { -- cgit v0.10.2-6-g49f6 From ab81047a6c5c4f98640f8a2f56e138367cfdada9 Mon Sep 17 00:00:00 2001 From: Victoria Dye Date: Tue, 1 Mar 2022 20:24:30 +0000 Subject: read-tree: make two-way merge sparse-aware Enable two-way merge with 'git read-tree' without expanding the sparse index. When in a sparse index, a two-way merge will trivially succeed as long as there are not changes to the same sparse directory in multiple trees (i.e., sparse directory-level "edit-edit" conflicts). If there are such conflicts, the merge will fail despite the possibility that individual files could merge cleanly. In order to resolve these "edit-edit" conflicts, "conflicted" sparse directories are - rather than rejected - merged by traversing their associated trees by OID. For each child of the sparse directory: 1. Files are merged as normal (see Documentation/git-read-tree.txt for details). 2. Subdirectories are treated as sparse directories and merged in 'twoway_merge'. If there are no conflicts, they are merged according to the rules in Documentation/git-read-tree.txt; otherwise, the subdirectory is recursively traversed and merged. This process allows sparse directories to be individually merged at the necessary depth *without* expanding a full index. The 't/t1092-sparse-checkout-compatibility.sh' test 'read-tree --merge with edit/edit conflicts in sparse directories' tests two-way merges with 1) changes inside sparse directories that do not conflict and 2) changes that do conflict (with the correct file(s) reported in the error message). Additionally, add two-way merge cases to 'sparse index is not expanded: read-tree' to confirm that the index is not expanded regardless of whether edit/edit conflicts are present in a sparse directory. Signed-off-by: Victoria Dye Signed-off-by: Junio C Hamano diff --git a/builtin/read-tree.c b/builtin/read-tree.c index ec6d038..9227f07 100644 --- a/builtin/read-tree.c +++ b/builtin/read-tree.c @@ -229,11 +229,6 @@ int cmd_read_tree(int argc, const char **argv, const char *cmd_prefix) opts.fn = opts.prefix ? bind_merge : oneway_merge; break; case 2: - /* - * TODO: update twoway_merge to handle edit/edit conflicts in - * sparse directories. - */ - ensure_full_index(&the_index); opts.fn = twoway_merge; opts.initial_checkout = is_cache_unborn(); break; diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh index d98558f..61dc2ea 100755 --- a/t/t1092-sparse-checkout-compatibility.sh +++ b/t/t1092-sparse-checkout-compatibility.sh @@ -1413,7 +1413,9 @@ test_expect_success 'sparse index is not expanded: read-tree' ' init_repos && ensure_not_expanded checkout -b test-branch update-folder1 && - for MERGE_TREES in "update-folder2" + for MERGE_TREES in "base update-folder2" \ + "base rename-base" \ + "update-folder2" do ensure_not_expanded read-tree -mu $MERGE_TREES && ensure_not_expanded reset --hard || return 1 diff --git a/unpack-trees.c b/unpack-trees.c index f3667d8..0c2a678 100644 --- a/unpack-trees.c +++ b/unpack-trees.c @@ -1360,6 +1360,42 @@ static int is_sparse_directory_entry(struct cache_entry *ce, return sparse_dir_matches_path(ce, info, name); } +static int unpack_sparse_callback(int n, unsigned long mask, unsigned long dirmask, struct name_entry *names, struct traverse_info *info) +{ + struct cache_entry *src[MAX_UNPACK_TREES + 1] = { NULL, }; + struct unpack_trees_options *o = info->data; + int ret; + + assert(o->merge); + + /* + * Unlike in 'unpack_callback', where src[0] is derived from the index when + * merging, src[0] is a transient cache entry derived from the first tree + * provided. Create the temporary entry as if it came from a non-sparse index. + */ + if (!is_null_oid(&names[0].oid)) { + src[0] = create_ce_entry(info, &names[0], 0, + &o->result, 1, + dirmask & (1ul << 0)); + src[0]->ce_flags |= (CE_SKIP_WORKTREE | CE_NEW_SKIP_WORKTREE); + } + + /* + * 'unpack_single_entry' assumes that src[0] is derived directly from + * the index, rather than from an entry in 'names'. This is *not* true when + * merging a sparse directory, in which case names[0] is the "index" source + * entry. To match the expectations of 'unpack_single_entry', shift past the + * "index" tree (i.e., names[0]) and adjust 'names', 'n', 'mask', and + * 'dirmask' accordingly. + */ + ret = unpack_single_entry(n - 1, mask >> 1, dirmask >> 1, src, names + 1, info); + + if (src[0]) + discard_cache_entry(src[0]); + + return ret >= 0 ? mask : -1; +} + /* * Note that traverse_by_cache_tree() duplicates some logic in this function * without actually calling it. If you change the logic here you may need to @@ -2472,6 +2508,37 @@ static int merged_entry(const struct cache_entry *ce, return 1; } +static int merged_sparse_dir(const struct cache_entry * const *src, int n, + struct unpack_trees_options *o) +{ + struct tree_desc t[MAX_UNPACK_TREES + 1]; + void * tree_bufs[MAX_UNPACK_TREES + 1]; + struct traverse_info info; + int i, ret; + + /* + * Create the tree traversal information for traversing into *only* the + * sparse directory. + */ + setup_traverse_info(&info, src[0]->name); + info.fn = unpack_sparse_callback; + info.data = o; + info.show_all_errors = o->show_all_errors; + info.pathspec = o->pathspec; + + /* Get the tree descriptors of the sparse directory in each of the merging trees */ + for (i = 0; i < n; i++) + tree_bufs[i] = fill_tree_descriptor(o->src_index->repo, &t[i], + src[i] && !is_null_oid(&src[i]->oid) ? &src[i]->oid : NULL); + + ret = traverse_trees(o->src_index, n, t, &info); + + for (i = 0; i < n; i++) + free(tree_bufs[i]); + + return ret; +} + static int deleted_entry(const struct cache_entry *ce, const struct cache_entry *old, struct unpack_trees_options *o) @@ -2742,6 +2809,14 @@ int twoway_merge(const struct cache_entry * const *src, * reject the merge instead. */ return merged_entry(newtree, current, o); + } else if (S_ISSPARSEDIR(current->ce_mode)) { + /* + * The sparse directories differ, but we don't know whether that's + * because of two different files in the directory being modified + * (can be trivially merged) or if there is a real file conflict. + * Merge the sparse directory by OID to compare file-by-file. + */ + return merged_sparse_dir(src, 3, o); } else return reject_merge(current, o); } -- cgit v0.10.2-6-g49f6 From f27c170f645e6b8ed642c49c503964893ee26a4f Mon Sep 17 00:00:00 2001 From: Victoria Dye Date: Tue, 1 Mar 2022 20:24:31 +0000 Subject: read-tree: make three-way merge sparse-aware Enable use of 'merged_sparse_dir' in 'threeway_merge'. As with two-way merge, the contents of each conflicted sparse directory are merged without referencing the index, avoiding sparse index expansion. As with two-way merge, the 't/t1092-sparse-checkout-compatibility.sh' test 'read-tree --merge with edit/edit conflicts in sparse directories' confirms that three-way merges with edit/edit changes (both with and without conflicts) inside a sparse directory result in the correct index state or error message. To ensure the index is not unnecessarily expanded, add three-way merge cases to 'sparse index is not expanded: read-tree'. Signed-off-by: Victoria Dye Signed-off-by: Junio C Hamano diff --git a/builtin/read-tree.c b/builtin/read-tree.c index 9227f07..9f1f33e 100644 --- a/builtin/read-tree.c +++ b/builtin/read-tree.c @@ -234,11 +234,6 @@ int cmd_read_tree(int argc, const char **argv, const char *cmd_prefix) break; case 3: default: - /* - * TODO: update threeway_merge to handle edit/edit conflicts in - * sparse directories. - */ - ensure_full_index(&the_index); opts.fn = threeway_merge; break; } diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh index 61dc2ea..8be8e3c 100755 --- a/t/t1092-sparse-checkout-compatibility.sh +++ b/t/t1092-sparse-checkout-compatibility.sh @@ -1413,7 +1413,9 @@ test_expect_success 'sparse index is not expanded: read-tree' ' init_repos && ensure_not_expanded checkout -b test-branch update-folder1 && - for MERGE_TREES in "base update-folder2" \ + for MERGE_TREES in "base HEAD update-folder2" \ + "base HEAD rename-base" \ + "base update-folder2" \ "base rename-base" \ "update-folder2" do diff --git a/unpack-trees.c b/unpack-trees.c index 0c2a678..b876cac 100644 --- a/unpack-trees.c +++ b/unpack-trees.c @@ -2643,16 +2643,24 @@ int threeway_merge(const struct cache_entry * const *stages, */ /* #14, #14ALT, #2ALT */ if (remote && !df_conflict_head && head_match && !remote_match) { - if (index && !same(index, remote) && !same(index, head)) - return reject_merge(index, o); + if (index && !same(index, remote) && !same(index, head)) { + if (S_ISSPARSEDIR(index->ce_mode)) + return merged_sparse_dir(stages, 4, o); + else + return reject_merge(index, o); + } return merged_entry(remote, index, o); } /* * If we have an entry in the index cache, then we want to * make sure that it matches head. */ - if (index && !same(index, head)) - return reject_merge(index, o); + if (index && !same(index, head)) { + if (S_ISSPARSEDIR(index->ce_mode)) + return merged_sparse_dir(stages, 4, o); + else + return reject_merge(index, o); + } if (head) { /* #5ALT, #15 */ @@ -2714,11 +2722,21 @@ int threeway_merge(const struct cache_entry * const *stages, } - /* Below are "no merge" cases, which require that the index be - * up-to-date to avoid the files getting overwritten with - * conflict resolution files. - */ + /* Handle "no merge" cases (see t/t1000-read-tree-m-3way.sh) */ if (index) { + /* + * If we've reached the "no merge" cases and we're merging + * a sparse directory, we may have an "edit/edit" conflict that + * can be resolved by individually merging directory contents. + */ + if (S_ISSPARSEDIR(index->ce_mode)) + return merged_sparse_dir(stages, 4, o); + + /* + * If we're not merging a sparse directory, ensure the index is + * up-to-date to avoid files getting overwritten with conflict + * resolution files + */ if (verify_uptodate(index, o)) return -1; } -- cgit v0.10.2-6-g49f6