{"thread":{"id":"56023","subject":"[PATCH 1/5] p2000: add 'git checkout -' test and decrease depth","startedAt":"2021-06-29T02:13:11Z","lastAt":"2021-07-22T04:22:51Z","messageCount":21,"participants":["Derrick Stolee via GitGitGadget","Elijah Newren","Derrick Stolee"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"428701","messageId":"bb3dd1fdd4891f660b0721fd9e42252a86062fff.1624932786.git.gitgitgadget@gmail.com","threadId":"56023","inReplyTo":"pull.973.git.1624932786.gitgitgadget@gmail.com","subject":"[PATCH 1/5] p2000: add 'git checkout -' test and decrease depth","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-06-29T02:13:02Z","receivedAt":"2021-06-29T02:13:11Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nAs we increase our list of commands to test in\np2000-sparse-operations.sh, we will want to have a slightly smaller test\nrepository. Reduce the size by a factor of four by reducing the depth of\nthe step that creates a big index around a moderately-sized repository.\n\nAlso add a step to run 'git checkout -' on repeat. This requires having\na previous location in the reflog, so add that to the initialization\nsteps.\n\nSigned-off-by: Derrick Stolee \n---\n t/perf/p2000-sparse-operations.sh | 27 ++++++++++++++++++++-------\n 1 file changed, 20 insertions(+), 7 deletions(-)\n\ndiff --git a/t/perf/p2000-sparse-operations.sh b/t/perf/p2000-sparse-operations.sh\nindex 94513c97748..f7f8c012103 100755\n--- a/t/perf/p2000-sparse-operations.sh\n+++ b/t/perf/p2000-sparse-operations.sh\n@@ -6,7 +6,7 @@ test_description=\"test performance of Git operations using the index\"\n \n test_perf_default_repo\n \n-SPARSE_CONE=f2/f4/f1\n+SPARSE_CONE=f2/f4\n \n test_expect_success 'setup repo and indexes' '\n \tgit reset --hard HEAD &&\n@@ -27,7 +27,7 @@ test_expect_success 'setup repo and indexes' '\n \tOLD_COMMIT=$(git rev-parse HEAD) &&\n \tOLD_TREE=$(git rev-parse HEAD^{tree}) &&\n \n-\tfor i in $(test_seq 1 4)\n+\tfor i in $(test_seq 1 3)\n \tdo\n \t\tcat >in <<-EOF &&\n \t\t\t100755 blob $BLOB\ta\n@@ -43,14 +43,23 @@ test_expect_success 'setup repo and indexes' '\n \tdone &&\n \n \tgit sparse-checkout init --cone &&\n-\tgit branch -f wide $OLD_COMMIT &&\n+\tgit sparse-checkout set $SPARSE_CONE &&\n+\tgit checkout -b wide $OLD_COMMIT &&\n+\n+\tfor l2 in f1 f2 f3 f4\n+\tdo\n+\t\techo more bogus >>$SPARSE_CONE/$l2/a &&\n+\t\tgit commit -a -m \"edit $SPARSE_CONE/$l2/a\" || return 1\n+\tdone &&\n+\n \tgit -c core.sparseCheckoutCone=true clone --branch=wide --sparse . full-index-v3 &&\n \t(\n \t\tcd full-index-v3 &&\n \t\tgit sparse-checkout init --cone &&\n \t\tgit sparse-checkout set $SPARSE_CONE &&\n \t\tgit config index.version 3 &&\n-\t\tgit update-index --index-version=3\n+\t\tgit update-index --index-version=3 &&\n+\t\tgit checkout HEAD~4\n \t) &&\n \tgit -c core.sparseCheckoutCone=true clone --branch=wide --sparse . full-index-v4 &&\n \t(\n@@ -58,7 +67,8 @@ test_expect_success 'setup repo and indexes' '\n \t\tgit sparse-checkout init --cone &&\n \t\tgit sparse-checkout set $SPARSE_CONE &&\n \t\tgit config index.version 4 &&\n-\t\tgit update-index --index-version=4\n+\t\tgit update-index --index-version=4 &&\n+\t\tgit checkout HEAD~4\n \t) &&\n \tgit -c core.sparseCheckoutCone=true clone --branch=wide --sparse . sparse-index-v3 &&\n \t(\n@@ -66,7 +76,8 @@ test_expect_success 'setup repo and indexes' '\n \t\tgit sparse-checkout init --cone --sparse-index &&\n \t\tgit sparse-checkout set $SPARSE_CONE &&\n \t\tgit config index.version 3 &&\n-\t\tgit update-index --index-version=3\n+\t\tgit update-index --index-version=3 &&\n+\t\tgit checkout HEAD~4\n \t) &&\n \tgit -c core.sparseCheckoutCone=true clone --branch=wide --sparse . sparse-index-v4 &&\n \t(\n@@ -74,7 +85,8 @@ test_expect_success 'setup repo and indexes' '\n \t\tgit sparse-checkout init --cone --sparse-index &&\n \t\tgit sparse-checkout set $SPARSE_CONE &&\n \t\tgit config index.version 4 &&\n-\t\tgit update-index --index-version=4\n+\t\tgit update-index --index-version=4 &&\n+\t\tgit checkout HEAD~4\n \t)\n '\n \n@@ -97,5 +109,6 @@ test_perf_on_all git status\n test_perf_on_all git add -A\n test_perf_on_all git add .\n test_perf_on_all git commit -a -m A\n+test_perf_on_all git checkout -f -\n \n test_done\n-- \ngitgitgadget\n\n"},{"id":"428702","messageId":"pull.973.git.1624932786.gitgitgadget@gmail.com","threadId":"56023","inReplyTo":null,"subject":"[PATCH 0/5] Sparse index: integrate with commit and checkout","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-06-29T02:13:01Z","receivedAt":"2021-06-29T02:13:12Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"This series extends our integration of sparse-index to 'git commit' and 'git\ncheckout'.\n\nThis is based on ds/status-with-sparse-index (v7) and v2.32.0. The hard work\nwas already done in that topic, so these changes are simple.\n\nRecall that we have delayed our integration with 'git add' until we can work\nout the concerns about how to deal with pathspecs outside of the\nsparse-checkout definition. Those concerns might have some overlap with how\n'git commit' takes a pathspec, but this seems like a rare enough case to\nhandle here and we can be more careful with the behavior change in the next\nseries which will integrate with git add.\n\nIn addition to the tests that already exist in t1092, I have integrated\nthese changes in microsoft/git and tested them against the Scalar functional\ntests, which go through quite a few complicated scenarios, verifying that\nthings work the same across the full index and sparse-index cases.\n\nThanks, -Stolee\n\nDerrick Stolee (5):\n p2000: add 'git checkout -' test and decrease depth\n p2000: compress repo names\n commit: integrate with sparse-index\n sparse-index: recompute cache-tree\n checkout: stop expanding sparse indexes\n\n builtin/checkout.c | 8 ++--\n builtin/commit.c | 3 ++\n cache-tree.c | 2 -\n sparse-index.c | 2 +\n t/perf/p2000-sparse-operations.sh | 47 ++++++++++++--------\n t/t1092-sparse-checkout-compatibility.sh | 55 ++++++++++++++++++++++--\n 6 files changed, 89 insertions(+), 28 deletions(-)\n\n\nbase-commit: 1d744848ee6b58ccaf3a30f20abe9797ed5d2ce7\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-973%2Fderrickstolee%2Fsparse-index%2Fcommit-and-checkout-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-973/derrickstolee/sparse-index/commit-and-checkout-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/973\n-- \ngitgitgadget\n"},{"id":"428703","messageId":"eb15bf37685b1f75255451c9fa8aba7d69fc2b14.1624932786.git.gitgitgadget@gmail.com","threadId":"56023","inReplyTo":"pull.973.git.1624932786.gitgitgadget@gmail.com","subject":"[PATCH 2/5] p2000: compress repo names","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-06-29T02:13:03Z","receivedAt":"2021-06-29T02:13:12Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nBy using shorter names for the test repos, we will get a slightly more\ncompressed performance summary without comprimising clarity.\n\nSigned-off-by: Derrick Stolee \n---\n t/perf/p2000-sparse-operations.sh | 20 ++++++++++----------\n 1 file changed, 10 insertions(+), 10 deletions(-)\n\ndiff --git a/t/perf/p2000-sparse-operations.sh b/t/perf/p2000-sparse-operations.sh\nindex f7f8c012103..597626276fb 100755\n--- a/t/perf/p2000-sparse-operations.sh\n+++ b/t/perf/p2000-sparse-operations.sh\n@@ -52,36 +52,36 @@ test_expect_success 'setup repo and indexes' '\n \t\tgit commit -a -m \"edit $SPARSE_CONE/$l2/a\" || return 1\n \tdone &&\n \n-\tgit -c core.sparseCheckoutCone=true clone --branch=wide --sparse . full-index-v3 &&\n+\tgit -c core.sparseCheckoutCone=true clone --branch=wide --sparse . full-v3 &&\n \t(\n-\t\tcd full-index-v3 &&\n+\t\tcd full-v3 &&\n \t\tgit sparse-checkout init --cone &&\n \t\tgit sparse-checkout set $SPARSE_CONE &&\n \t\tgit config index.version 3 &&\n \t\tgit update-index --index-version=3 &&\n \t\tgit checkout HEAD~4\n \t) &&\n-\tgit -c core.sparseCheckoutCone=true clone --branch=wide --sparse . full-index-v4 &&\n+\tgit -c core.sparseCheckoutCone=true clone --branch=wide --sparse . full-v4 &&\n \t(\n-\t\tcd full-index-v4 &&\n+\t\tcd full-v4 &&\n \t\tgit sparse-checkout init --cone &&\n \t\tgit sparse-checkout set $SPARSE_CONE &&\n \t\tgit config index.version 4 &&\n \t\tgit update-index --index-version=4 &&\n \t\tgit checkout HEAD~4\n \t) &&\n-\tgit -c core.sparseCheckoutCone=true clone --branch=wide --sparse . sparse-index-v3 &&\n+\tgit -c core.sparseCheckoutCone=true clone --branch=wide --sparse . sparse-v3 &&\n \t(\n-\t\tcd sparse-index-v3 &&\n+\t\tcd sparse-v3 &&\n \t\tgit sparse-checkout init --cone --sparse-index &&\n \t\tgit sparse-checkout set $SPARSE_CONE &&\n \t\tgit config index.version 3 &&\n \t\tgit update-index --index-version=3 &&\n \t\tgit checkout HEAD~4\n \t) &&\n-\tgit -c core.sparseCheckoutCone=true clone --branch=wide --sparse . sparse-index-v4 &&\n+\tgit -c core.sparseCheckoutCone=true clone --branch=wide --sparse . sparse-v4 &&\n \t(\n-\t\tcd sparse-index-v4 &&\n+\t\tcd sparse-v4 &&\n \t\tgit sparse-checkout init --cone --sparse-index &&\n \t\tgit sparse-checkout set $SPARSE_CONE &&\n \t\tgit config index.version 4 &&\n@@ -92,8 +92,8 @@ test_expect_success 'setup repo and indexes' '\n \n test_perf_on_all () {\n \tcommand=\"$@\"\n-\tfor repo in full-index-v3 full-index-v4 \\\n-\t\t sparse-index-v3 sparse-index-v4\n+\tfor repo in full-v3 full-v4 \\\n+\t\t sparse-v3 sparse-v4\n \tdo\n \t\ttest_perf \"$command ($repo)\" \"\n \t\t\t(\n-- \ngitgitgadget\n\n"},{"id":"428704","messageId":"413babe6e774eb4e09b091532219980e1cca94cb.1624932786.git.gitgitgadget@gmail.com","threadId":"56023","inReplyTo":"pull.973.git.1624932786.gitgitgadget@gmail.com","subject":"[PATCH 3/5] commit: integrate with sparse-index","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-06-29T02:13:04Z","receivedAt":"2021-06-29T02:13:13Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nUpdate 'git commit' to allow using the sparse-index in memory without\nexpanding to a full one. The only place that had an ensure_full_index()\ncall was in cache_tree_update(). The recursive algorithm for\nupdate_one() was already updated in 2de37c536 (cache-tree: integrate\nwith sparse directory entries, 2021-03-03) to handle sparse directory\nentries in the index.\n\nMost of this change involves testing different command-line options that\nallow specifying which on-disk changes should be included in the commit.\nThis includes no options (only take currently-staged changes), -a (take\nall tracked changes), and --include (take a list of specific changes).\nTo simplify testing that these options do not expand the index, update\nthe test that previously verified that 'git status' does not expand the\nindex with a helper method, ensure_not_expanded().\n\nThis allows 'git commit' to operate much faster when the sparse-checkout\ncone is much smaller than the full list of files at HEAD.\n\nHere are the relevant lines from p2000-sparse-operations.sh:\n\nTest HEAD~1 HEAD\n----------------------------------------------------------------------------------\n2000.14: git commit -a -m A (full-v3) 0.35(0.26+0.06) 0.36(0.28+0.07) +2.9%\n2000.15: git commit -a -m A (full-v4) 0.32(0.26+0.05) 0.34(0.28+0.06) +6.3%\n2000.16: git commit -a -m A (sparse-v3) 0.63(0.59+0.06) 0.04(0.05+0.05) -93.7%\n2000.17: git commit -a -m A (sparse-v4) 0.64(0.59+0.08) 0.04(0.04+0.04) -93.8%\n\nIt is important to compare the full-index case to the sparse-index case,\nso the improvement for index version v4 is actually an 88% improvement in\nthis synthetic example.\n\nIn a real repository with over two million files at HEAD and 60,000\nfiles in the sparse-checkout definition, the time for 'git commit -a'\nwent from 2.61 seconds to 134ms. I compared this to the result if the\nindex only contained the paths in the sparse-checkout definition and\nfound the theoretical optimum to be 120ms, so the out-of-cone paths only\nadd a 12% overhead.\n\nSigned-off-by: Derrick Stolee \n---\n builtin/commit.c | 3 ++\n cache-tree.c | 2 -\n t/t1092-sparse-checkout-compatibility.sh | 47 ++++++++++++++++++++++--\n 3 files changed, 46 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 12f51db158a..0bc64892505 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -1682,6 +1682,9 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \tif (argc == 2 && !strcmp(argv[1], \"-h\"))\n \t\tusage_with_options(builtin_commit_usage, builtin_commit_options);\n \n+\tprepare_repo_settings(the_repository);\n+\tthe_repository->settings.command_requires_full_index = 0;\n+\n \tstatus_init_config(&s, git_commit_config);\n \ts.commit_template = 1;\n \tstatus_format = STATUS_FORMAT_NONE; /* Ignore status.short */\ndiff --git a/cache-tree.c b/cache-tree.c\nindex 45e58666afc..577b18d8811 100644\n--- a/cache-tree.c\n+++ b/cache-tree.c\n@@ -461,8 +461,6 @@ int cache_tree_update(struct index_state *istate, int flags)\n \tif (i)\n \t\treturn i;\n \n-\tensure_full_index(istate);\n-\n \tif (!istate->cache_tree)\n \t\tistate->cache_tree = cache_tree();\n \ndiff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh\nindex df217a2d10b..11c6dc8ec1f 100755\n--- a/t/t1092-sparse-checkout-compatibility.sh\n+++ b/t/t1092-sparse-checkout-compatibility.sh\n@@ -275,6 +275,34 @@ test_expect_success 'add, commit, checkout' '\n \ttest_all_match git checkout -\n '\n \n+test_expect_success 'commit including unstaged changes' '\n+\tinit_repos &&\n+\n+\twrite_script edit-file <<-\\EOF &&\n+\techo $1 >$2\n+\tEOF\n+\n+\trun_on_all ../edit-file 1 a &&\n+\trun_on_all ../edit-file 1 deep/a &&\n+\n+\ttest_all_match git commit -m \"-a\" -a &&\n+\ttest_all_match git status --porcelain=v2 &&\n+\n+\trun_on_all ../edit-file 2 a &&\n+\trun_on_all ../edit-file 2 deep/a &&\n+\n+\ttest_all_match git commit -m \"--include\" --include deep/a &&\n+\ttest_all_match git status --porcelain=v2 &&\n+\ttest_all_match git commit -m \"--include\" --include a &&\n+\ttest_all_match git status --porcelain=v2 &&\n+\n+\trun_on_all ../edit-file 3 a &&\n+\trun_on_all ../edit-file 3 deep/a &&\n+\n+\ttest_all_match git commit -m \"--amend\" -a --amend &&\n+\ttest_all_match git status --porcelain=v2\n+'\n+\n test_expect_success 'status/add: outside sparse cone' '\n \tinit_repos &&\n \n@@ -535,14 +563,25 @@ test_expect_success 'sparse-index is expanded and converted back' '\n \ttest_region index ensure_full_index trace2.txt\n '\n \n-test_expect_success 'sparse-index is not expanded' '\n-\tinit_repos &&\n-\n+ensure_not_expanded () {\n \trm -f trace2.txt &&\n \techo >>sparse-index/untracked.txt &&\n \tGIT_TRACE2_EVENT=\"$(pwd)/trace2.txt\" GIT_TRACE2_EVENT_NESTING=10 \\\n-\t\tgit -C sparse-index status &&\n+\t\tgit -C sparse-index \"$@\" &&\n \ttest_region ! index ensure_full_index trace2.txt\n+}\n+\n+test_expect_success 'sparse-index is not expanded' '\n+\tinit_repos &&\n+\n+\tensure_not_expanded status &&\n+\tensure_not_expanded commit --allow-empty -m empty &&\n+\techo >>sparse-index/a &&\n+\tensure_not_expanded commit -a -m a &&\n+\techo >>sparse-index/a &&\n+\tensure_not_expanded commit --include a -m a &&\n+\techo >>sparse-index/deep/deeper1/a &&\n+\tensure_not_expanded commit --include deep/deeper1/a -m deeper\n '\n \n test_expect_success 'reset mixed and checkout orphan' '\n-- \ngitgitgadget\n\n"},{"id":"428706","messageId":"ffe8473caab5189961ec29464629e13abbe310ed.1624932787.git.gitgitgadget@gmail.com","threadId":"56023","inReplyTo":"pull.973.git.1624932786.gitgitgadget@gmail.com","subject":"[PATCH 4/5] sparse-index: recompute cache-tree","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-06-29T02:13:05Z","receivedAt":"2021-06-29T02:13:15Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nWhen some commands run with command_requires_full_index=1, then the\nindex can get in a state where the in-memory cache tree is actually\nequal to the sparse index's cache tree instead of the full one.\n\nThis results in incorrect entry_count values. By clearing the cache\ntree before converting to sparse, we avoid this issue.\n\nSigned-off-by: Derrick Stolee \n---\n sparse-index.c | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/sparse-index.c b/sparse-index.c\nindex 53c8f711ccc..c6b4feec413 100644\n--- a/sparse-index.c\n+++ b/sparse-index.c\n@@ -170,6 +170,8 @@ int convert_to_sparse(struct index_state *istate)\n \tif (index_has_unmerged_entries(istate))\n \t\treturn 0;\n \n+\t/* Clear and recompute the cache-tree */\n+\tcache_tree_free(&istate->cache_tree);\n \tif (cache_tree_update(istate, 0)) {\n \t\twarning(_(\"unable to update cache-tree, staying full\"));\n \t\treturn -1;\n-- \ngitgitgadget\n\n"},{"id":"428707","messageId":"8710fee36b7821e0c36196b8e7b9a27d5994ae72.1624932787.git.gitgitgadget@gmail.com","threadId":"56023","inReplyTo":"pull.973.git.1624932786.gitgitgadget@gmail.com","subject":"[PATCH 5/5] checkout: stop expanding sparse indexes","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-06-29T02:13:06Z","receivedAt":"2021-06-29T02:13:16Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nPrevious changes did the necessary improvements to unpack-trees.c and\ndiff-lib.c in order to modify a sparse index based on its comparision\nwith a tree. The only remaining work is to remove some\nensure_full_index() calls and add tests that verify that the index is\nnot expanded in our interesting cases. Include 'switch' and 'restore' in\nthese tests, as they share a base implementation with 'checkout'.\n\nHere are the relevant performance results from\np2000-sparse-operations.sh:\n\nTest HEAD~1 HEAD\n--------------------------------------------------------------------------------\n2000.18: git checkout -f - (full-v3) 0.49(0.43+0.03) 0.47(0.39+0.05) -4.1%\n2000.19: git checkout -f - (full-v4) 0.45(0.37+0.06) 0.42(0.37+0.05) -6.7%\n2000.20: git checkout -f - (sparse-v3) 0.76(0.71+0.07) 0.04(0.03+0.04) -94.7%\n2000.21: git checkout -f - (sparse-v4) 0.75(0.72+0.04) 0.05(0.06+0.04) -93.3%\n\nIt is important to compare the full index case to the sparse index case,\nas the previous results for the sparse index were inflated by the index\nexpansion. For index v4, this is an 88% improvement.\n\nOn an internal repository with over two million paths at HEAD and a\nsparse-checkout definition containing ~60,000 of those paths, 'git\ncheckout' went from 3.5s to 297ms with this change. The theoretical\noptimum where only those ~60,000 paths exist was 275ms, so the extra\nsparse directory entries contribute a 22ms overhead.\n\nSigned-off-by: Derrick Stolee \n---\n builtin/checkout.c | 8 +++-----\n t/t1092-sparse-checkout-compatibility.sh | 10 +++++++++-\n 2 files changed, 12 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex f4cd7747d35..b5d477919a7 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -378,9 +378,6 @@ static int checkout_worktree(const struct checkout_opts *opts,\n \tif (pc_workers > 1)\n \t\tinit_parallel_checkout();\n \n-\t/* TODO: audit for interaction with sparse-index. */\n-\tensure_full_index(&the_index);\n-\n \tfor (pos = 0; pos < active_nr; pos++) {\n \t\tstruct cache_entry *ce = active_cache[pos];\n \t\tif (ce->ce_flags & CE_MATCHED) {\n@@ -530,8 +527,6 @@ static int checkout_paths(const struct checkout_opts *opts,\n \t * Make sure all pathspecs participated in locating the paths\n \t * to be checked out.\n \t */\n-\t/* TODO: audit for interaction with sparse-index. */\n-\tensure_full_index(&the_index);\n \tfor (pos = 0; pos < active_nr; pos++)\n \t\tif (opts->overlay_mode)\n \t\t\tmark_ce_for_checkout_overlay(active_cache[pos],\n@@ -1593,6 +1588,9 @@ static int checkout_main(int argc, const char **argv, const char *prefix,\n \n \tgit_config(git_checkout_config, opts);\n \n+\tprepare_repo_settings(the_repository);\n+\tthe_repository->settings.command_requires_full_index = 0;\n+\n \topts->track = BRANCH_TRACK_UNSPECIFIED;\n \n \tif (!opts->accept_pathspec && !opts->accept_ref)\ndiff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh\nindex 11c6dc8ec1f..1d7dd735d7d 100755\n--- a/t/t1092-sparse-checkout-compatibility.sh\n+++ b/t/t1092-sparse-checkout-compatibility.sh\n@@ -581,7 +581,15 @@ test_expect_success 'sparse-index is not expanded' '\n \techo >>sparse-index/a &&\n \tensure_not_expanded commit --include a -m a &&\n \techo >>sparse-index/deep/deeper1/a &&\n-\tensure_not_expanded commit --include deep/deeper1/a -m deeper\n+\tensure_not_expanded commit --include deep/deeper1/a -m deeper &&\n+\tensure_not_expanded checkout rename-out-to-out &&\n+\tensure_not_expanded checkout - &&\n+\tensure_not_expanded switch rename-out-to-out &&\n+\tensure_not_expanded switch - &&\n+\tgit -C sparse-index reset --hard &&\n+\tensure_not_expanded checkout rename-out-to-out -- deep/deeper1 &&\n+\tgit -C sparse-index reset --hard &&\n+\tensure_not_expanded restore -s rename-out-to-out -- deep/deeper1\n '\n \n test_expect_success 'reset mixed and checkout orphan' '\n-- \ngitgitgadget\n"},{"id":"429585","messageId":"CABPp-BF_i1QRCXaeKzqoc6Q2=3un-wku7aKUEdBbXfeVfTG8xg@mail.gmail.com","threadId":"56023","inReplyTo":"pull.973.git.1624932786.gitgitgadget@gmail.com","subject":"Re: [PATCH 0/5] Sparse index: integrate with commit and checkout","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2021-07-09T21:26:00Z","receivedAt":"2021-07-09T21:26:15Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Mon, Jun 28, 2021 at 7:13 PM Derrick Stolee via GitGitGadget\n wrote:\n>\n> This series extends our integration of sparse-index to 'git commit' and 'git\n> checkout'.\n>\n> This is based on ds/status-with-sparse-index (v7) and v2.32.0. The hard work\n> was already done in that topic, so these changes are simple.\n>\n> Recall that we have delayed our integration with 'git add' until we can work\n> out the concerns about how to deal with pathspecs outside of the\n> sparse-checkout definition. Those concerns might have some overlap with how\n> 'git commit' takes a pathspec, but this seems like a rare enough case to\n> handle here and we can be more careful with the behavior change in the next\n> series which will integrate with git add.\n>\n> In addition to the tests that already exist in t1092, I have integrated\n> these changes in microsoft/git and tested them against the Scalar functional\n> tests, which go through quite a few complicated scenarios, verifying that\n> things work the same across the full index and sparse-index cases.\n>\n> Thanks, -Stolee\n>\n> Derrick Stolee (5):\n> p2000: add 'git checkout -' test and decrease depth\n> p2000: compress repo names\n> commit: integrate with sparse-index\n> sparse-index: recompute cache-tree\n> checkout: stop expanding sparse indexes\n>\n> builtin/checkout.c | 8 ++--\n> builtin/commit.c | 3 ++\n> cache-tree.c | 2 -\n> sparse-index.c | 2 +\n> t/perf/p2000-sparse-operations.sh | 47 ++++++++++++--------\n> t/t1092-sparse-checkout-compatibility.sh | 55 ++++++++++++++++++++++--\n> 6 files changed, 89 insertions(+), 28 deletions(-)\n>\n>\n> base-commit: 1d744848ee6b58ccaf3a30f20abe9797ed5d2ce7\n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-973%2Fderrickstolee%2Fsparse-index%2Fcommit-and-checkout-v1\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-973/derrickstolee/sparse-index/commit-and-checkout-v1\n> Pull-Request: https://github.com/gitgitgadget/git/pull/973\n\nI've read over these patches and didn't find any problems in them in\nmy reading; however, since it builds upon ds/status-with-sparse-index\n(v7)...\n\nI decided to retry some of my ideas and testing on Patch 10/16 of v7,\nover at https://lore.kernel.org/git/CABPp-BHwTAKwFiWQ0-2P=_g+7HLK5FfOAz-uujRjLou1fXT3zw@mail.gmail.com/\n\nIt turns out that the block you added there is now triggered by t1092\nafter this series, and the testcase won't pass without that block. It\nmight be clearer to move that code fragment, or perhaps the whole\npatch, into this series...though the code fragment as is has\nintroduced a bug. If you take t1092 test 12 (\"diff with\ndirectory/file conflicts\") and modify it so that before the\n\n git checkout df-conflict\n\ninvocation from sparse-index, you first run:\n\n $ git sparse-checkout disable\n $ echo more stuff >>folder1/edited-content\n $ git add -u\n $ git diff HEAD # note the changes\n $ git sparse-checkout init --cone --sparse-index\n $ git diff HEAD # note the changes are still there\n $ git checkout df-conflict # no error?? What about the\nconflicting changes?\n $ git diff HEAD\n\nthen the last command will show that the staged changes from before\nthe commit have simply been discarded. In short, this makes the\nseries behave like --force was passed with sparse directory entries,\nwhen --force wasn't passed.\n\nSo we've still got some directory/file conflict issues.\n"},{"id":"429812","messageId":"b362c428-eec9-39e3-55a0-0738431e1d98@gmail.com","threadId":"56023","inReplyTo":"CABPp-BF_i1QRCXaeKzqoc6Q2=3un-wku7aKUEdBbXfeVfTG8xg@mail.gmail.com","subject":"Re: [PATCH 0/5] Sparse index: integrate with commit and checkout","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2021-07-12T18:46:10Z","receivedAt":"2021-07-12T18:46:17Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 7/9/2021 5:26 PM, Elijah Newren wrote:\n> On Mon, Jun 28, 2021 at 7:13 PM Derrick Stolee via GitGitGadget\n> wrote:\n>>\n...\n> I've read over these patches and didn't find any problems in them in\n> my reading; however, since it builds upon ds/status-with-sparse-index\n> (v7)...\n> \n> I decided to retry some of my ideas and testing on Patch 10/16 of v7,\n> over at https://lore.kernel.org/git/CABPp-BHwTAKwFiWQ0-2P=_g+7HLK5FfOAz-uujRjLou1fXT3zw@mail.gmail.com/\n> \n> It turns out that the block you added there is now triggered by t1092\n> after this series, and the testcase won't pass without that block. It\n> might be clearer to move that code fragment, or perhaps the whole\n> patch, into this series...though the code fragment as is has\n> introduced a bug. If you take t1092 test 12 (\"diff with\n> directory/file conflicts\") and modify it so that before the\n> \n> git checkout df-conflict\n> \n> invocation from sparse-index, you first run:\n> \n> $ git sparse-checkout disable\n> $ echo more stuff >>folder1/edited-content\n> $ git add -u\n> $ git diff HEAD # note the changes\n> $ git sparse-checkout init --cone --sparse-index\n> $ git diff HEAD # note the changes are still there\n> $ git checkout df-conflict # no error?? What about the\n> conflicting changes?\n> $ git diff HEAD\n> \n> then the last command will show that the staged changes from before\n> the commit have simply been discarded. In short, this makes the\n> series behave like --force was passed with sparse directory entries,\n> when --force wasn't passed.\n> \n> So we've still got some directory/file conflict issues.\n\nYou are absolutely right that this seems strange. In fact, there\nis a behavior change during 'git checkout' for sparse-checkouts\nin general, but also my sparse-index change creates an additional\nchange in this case.\n\nHere is a test that demonstrates the issue:\n\ntest_expect_success 'staged directory/file conflict' '\n\tinit_repos &&\n\n\ttest_sparse_match git sparse-checkout disable &&\n\twrite_script edit-contents <<-\\EOF &&\n\techo text >>folder1/edited-content\n\tEOF\n\trun_on_all ../edit-contents &&\n\n\ttest_all_match git add folder1/edited-content &&\n\ttest_all_match git diff HEAD &&\n\tgit -C sparse-checkout sparse-checkout init --cone --no-sparse-index &&\n\tgit -C sparse-index sparse-checkout init --cone --sparse-index &&\n\ttest_all_match git diff HEAD &&\n\n\t# Sparse-checkouts handle this conflict differently than\n\t# full checkouts, as they consider the file \"folder1\" to\n\t# be deleted in favor of the staged file\n\t# \"folder1/edited-content\".\n\ttest_sparse_match git checkout df-conflict &&\n\ttest_sparse_match git diff HEAD\n'\n\nThe sparse-index case drops all staged changes during the\n'git checkout df-conflict' command, so the test fails on\nthat line.\n\nThat final diff looks like this in the sparse-checkout\nrepo (no sparse index):\n\ndiff --git a/folder1 b/folder1\ndeleted file mode 100644\nindex d95f3ad..0000000\n--- a/folder1\n+++ /dev/null\n@@ -1 +0,0 @@\n-content\ndiff --git a/folder1/edited-content b/folder1/edited-content\nnew file mode 100644\nindex 0000000..8e27be7\n--- /dev/null\n+++ b/folder1/edited-content\n@@ -0,0 +1 @@\n+text\n\nThis is a strange case in that we have a staged tree that is\noutside of the sparse-checkout cone. When running the 'git\ncheckout df-conflict' command, the twoway_merge() method\nreceives the following values:\n\n current: \"folder1/\" (tree OID)\n oldtree: \"\" (NULL OID)\n newtree: \"folder1\" (blob OID)\n\nIs this value for 'oldtree' correct? It seems strange to me,\nso I'll look further into it.\n\nClearly, the resolution that was presented in the previous\npatch was incorrect so I will try to understand this\nsituation better.\n\nFurther, I expect it to be simpler to modify the behavior\nhere to match the full checkout case than to make the\nsparse-index case match the normal sparse-checkout case.\nThe \"natural\" thing would be to keep the staged \"folder1/\"\ndirectory, but that would present as adding all contained\ncontent, not just the single staged entry.\n\nThanks,\n-Stolee\n"},{"id":"430318","messageId":"c127ceed-10fd-9ad0-e858-db79bec0cf8d@gmail.com","threadId":"56023","inReplyTo":"b362c428-eec9-39e3-55a0-0738431e1d98@gmail.com","subject":"Re: [PATCH 0/5] Sparse index: integrate with commit and checkout","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2021-07-16T13:59:21Z","receivedAt":"2021-07-16T13:59:37Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 7/12/2021 2:46 PM, Derrick Stolee wrote:\n> On 7/9/2021 5:26 PM, Elijah Newren wrote:\n>> On Mon, Jun 28, 2021 at 7:13 PM Derrick Stolee via GitGitGadget\n>> wrote:\n>>>\n...\n> This is a strange case in that we have a staged tree that is\n> outside of the sparse-checkout cone. When running the 'git\n> checkout df-conflict' command, the twoway_merge() method\n> receives the following values:\n> \n> current: \"folder1/\" (tree OID)\n> oldtree: \"\" (NULL OID)\n> newtree: \"folder1\" (blob OID)\n> \n> Is this value for 'oldtree' correct? It seems strange to me,\n> so I'll look further into it.\n\nThis is correct. This 'oldtree' entry is actually the o->df_conflict\nplaceholder and is set to NULL inside the method.\n\n> Further, I expect it to be simpler to modify the behavior\n> here to match the full checkout case than to make the\n> sparse-index case match the normal sparse-checkout case.\n> The \"natural\" thing would be to keep the staged \"folder1/\"\n> directory, but that would present as adding all contained\n> content, not just the single staged entry.\nTaking a closer look at the full checkout case, I discovered that the\n'git checkout df-conflict' command succeeds in the full checkout case if I\napply it directly to the 'master' branch. In that situation, it completely\nremoves the staged change to folder1/edited-content! This seems like\nincorrect behavior, and has nothing to do with the sparse-checkout feature.\n\nIt just happens that a sparse-checkout will have a _different_ kind of\nincorrect behavior!\n\nHowever, when adding the test on top of the ds/status-with-sparse-index\nbranch, the full checkout case matches the sparse-checkout! I bisected\nthis to the additions of files adjacent to folder1/ (folder1. folder1-,\netc) in e669ffb (t1092: expand repository data shape, 2021-07-14). If I\nswitch the test to conflict on folder2, then I get the strange behavior\nthat I was noticing on 'master'.\n\nSome very subtle things are going on here, and they don't necessarily\ninvolve the sparse index. Adding the sparse index to the mix creates a\nthird incorrect behavior to this already-broken case.\n\nIf we agree that the correct thing to do here is to reject the merge and\nfail the command, then I can start working on making that change in\nisolation (because _none_ of the existing behaviors are correct).\n\nThat leaves a question as to whether we should hold up this series for\nthat reason, or if I should pursue a fix to this kind of conflict as a\nforward fix on top of it. What do you think, Elijah and Junio?\n\nThanks,\n-Stolee\n"},{"id":"430420","messageId":"CABPp-BGCnV-xHH-+S58pqFFhPbPj_0Rt=_QUf_ShAbTCyW9deA@mail.gmail.com","threadId":"56023","inReplyTo":"c127ceed-10fd-9ad0-e858-db79bec0cf8d@gmail.com","subject":"Re: [PATCH 0/5] Sparse index: integrate with commit and checkout","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2021-07-17T15:37:07Z","receivedAt":"2021-07-17T15:37:22Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Fri, Jul 16, 2021 at 6:59 AM Derrick Stolee wrote:\n>\n> On 7/12/2021 2:46 PM, Derrick Stolee wrote:\n> > On 7/9/2021 5:26 PM, Elijah Newren wrote:\n> >> On Mon, Jun 28, 2021 at 7:13 PM Derrick Stolee via GitGitGadget\n> >> wrote:\n> >>>\n> ...\n> > Further, I expect it to be simpler to modify the behavior\n> > here to match the full checkout case than to make the\n> > sparse-index case match the normal sparse-checkout case.\n> > The \"natural\" thing would be to keep the staged \"folder1/\"\n> > directory, but that would present as adding all contained\n> > content, not just the single staged entry.\n> Taking a closer look at the full checkout case, I discovered that the\n> 'git checkout df-conflict' command succeeds in the full checkout case if I\n> apply it directly to the 'master' branch. In that situation, it completely\n> removes the staged change to folder1/edited-content! This seems like\n> incorrect behavior, and has nothing to do with the sparse-checkout feature.\n\nI was not able to reproduce. Do you have other modifications to git,\nor is there some other special setup required to trigger the bug that\nI am missing in reading the paragraph above? Here's what I see:\n\n>folder1/edited-content\n$ git add -u\n$ git checkout df-conflict\nerror: Your local changes to the following files would be overwritten\nby checkout:\nfolder1/edited-content\nPlease commit your changes or stash them before you switch branches.\nAborting\n\nThis looks like the expected behavior to me, and is what I'd also\nexpect from the sparse-checkout and sparse-index cases.\n\n> It just happens that a sparse-checkout will have a _different_ kind of\n> incorrect behavior!\n>\n> However, when adding the test on top of the ds/status-with-sparse-index\n> branch, the full checkout case matches the sparse-checkout! I bisected\n> this to the additions of files adjacent to folder1/ (folder1. folder1-,\n> etc) in e669ffb (t1092: expand repository data shape, 2021-07-14). If I\n> switch the test to conflict on folder2, then I get the strange behavior\n> that I was noticing on 'master'.\n>\n> Some very subtle things are going on here, and they don't necessarily\n> involve the sparse index. Adding the sparse index to the mix creates a\n> third incorrect behavior to this already-broken case.\n>\n> If we agree that the correct thing to do here is to reject the merge and\n> fail the command, then I can start working on making that change in\n> isolation (because _none_ of the existing behaviors are correct).\n\nYes, rejecting the merge is the correct behavior. This is implied by\nthe existing documentation for both the --merge and --force options to\ncheckout.\n\n> That leaves a question as to whether we should hold up this series for\n> that reason, or if I should pursue a fix to this kind of conflict as a\n> forward fix on top of it. What do you think, Elijah and Junio?\n\nI only dug in and found the sparse-checkout/sparse-index bugs because\nthe D/F changes you made to twoway_merge() looked clearly wrong to me\nand I was trying to find a case that would demonstrate it and make it\neasier for you to fix up. I still think the patch is wrong and that\nit adds a bug. If you can drop that patch, and still get correct\nbehavior in your tests, then I think we can ignore other bugs in this\narea, but I'm not happy with that particular patch. If you need that\npatch, then it needs to be corrected, which probably means figuring\nout all these bugs.\n"},{"id":"430503","messageId":"91b81577-3ff8-3845-75d1-c47e8a42406e@gmail.com","threadId":"56023","inReplyTo":"CABPp-BGCnV-xHH-+S58pqFFhPbPj_0Rt=_QUf_ShAbTCyW9deA@mail.gmail.com","subject":"Re: [PATCH 0/5] Sparse index: integrate with commit and checkout","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2021-07-19T14:05:44Z","receivedAt":"2021-07-19T14:05:50Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 7/17/2021 11:37 AM, Elijah Newren wrote:\n> On Fri, Jul 16, 2021 at 6:59 AM Derrick Stolee wrote:\n>>\n>> On 7/12/2021 2:46 PM, Derrick Stolee wrote:\n>>> On 7/9/2021 5:26 PM, Elijah Newren wrote:\n>>>> On Mon, Jun 28, 2021 at 7:13 PM Derrick Stolee via GitGitGadget\n>>>> wrote:\n>>>>>\n>> ...\n>>> Further, I expect it to be simpler to modify the behavior\n>>> here to match the full checkout case than to make the\n>>> sparse-index case match the normal sparse-checkout case.\n>>> The \"natural\" thing would be to keep the staged \"folder1/\"\n>>> directory, but that would present as adding all contained\n>>> content, not just the single staged entry.\n>> Taking a closer look at the full checkout case, I discovered that the\n>> 'git checkout df-conflict' command succeeds in the full checkout case if I\n>> apply it directly to the 'master' branch. In that situation, it completely\n>> removes the staged change to folder1/edited-content! This seems like\n>> incorrect behavior, and has nothing to do with the sparse-checkout feature.\n> \n> I was not able to reproduce. Do you have other modifications to git,\n> or is there some other special setup required to trigger the bug that\n> I am missing in reading the paragraph above? Here's what I see:\n> \n> directory/file conflicts' test, run until first failure, then:\n> \n> $ cd trash directory.t1092-sparse-checkout-compatibility/full-checkout\n> $ git reset --hard\n> $ git checkout rename-in-to-out\n> $ echo more stuff >>folder1/edited-content\n> $ git add -u\n> $ git checkout df-conflict\n> error: Your local changes to the following files would be overwritten\n> by checkout:\n> folder1/edited-content\n> Please commit your changes or stash them before you switch branches.\n> Aborting\n> \n> This looks like the expected behavior to me, and is what I'd also\n> expect from the sparse-checkout and sparse-index cases.\n\nIt is fragile to the data shape in my test, so I'll be sure to\ninclude one in the next series version that demonstrates the change.\n\n>> It just happens that a sparse-checkout will have a _different_ kind of\n>> incorrect behavior!\n>>\n>> However, when adding the test on top of the ds/status-with-sparse-index\n>> branch, the full checkout case matches the sparse-checkout! I bisected\n>> this to the additions of files adjacent to folder1/ (folder1. folder1-,\n>> etc) in e669ffb (t1092: expand repository data shape, 2021-07-14). If I\n>> switch the test to conflict on folder2, then I get the strange behavior\n>> that I was noticing on 'master'.\n>>\n>> Some very subtle things are going on here, and they don't necessarily\n>> involve the sparse index. Adding the sparse index to the mix creates a\n>> third incorrect behavior to this already-broken case.\n>>\n>> If we agree that the correct thing to do here is to reject the merge and\n>> fail the command, then I can start working on making that change in\n>> isolation (because _none_ of the existing behaviors are correct).\n> \n> Yes, rejecting the merge is the correct behavior. This is implied by\n> the existing documentation for both the --merge and --force options to\n> checkout.\n> \n>> That leaves a question as to whether we should hold up this series for\n>> that reason, or if I should pursue a fix to this kind of conflict as a\n>> forward fix on top of it. What do you think, Elijah and Junio?\n> \n> I only dug in and found the sparse-checkout/sparse-index bugs because\n> the D/F changes you made to twoway_merge() looked clearly wrong to me\n> and I was trying to find a case that would demonstrate it and make it\n> easier for you to fix up. I still think the patch is wrong and that\n> it adds a bug. If you can drop that patch, and still get correct\n> behavior in your tests, then I think we can ignore other bugs in this\n> area, but I'm not happy with that particular patch. If you need that\n> patch, then it needs to be corrected, which probably means figuring\n> out all these bugs.\n\nThat's a good point. I reverted the patch and re-ran the test and\nfound that actually the patch is necessary in order to match the\n_incorrect_ behavior. Without the patch, the sparse-index case\n(correctly) refuses to complete the checkout.\n\nI'll replace this patch with a test change that demonstrates these\nsubtleties and marks them as NEEDSWORK.\n\nThanks,\n-Stolee\n\n"},{"id":"430684","messageId":"3e1d03c41be9701c2cd518afc6ebc2797bca4398.1626812081.git.gitgitgadget@gmail.com","threadId":"56023","inReplyTo":"pull.973.v2.git.1626812081.gitgitgadget@gmail.com","subject":"[PATCH v2 2/7] p2000: compress repo names","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-07-20T20:14:36Z","receivedAt":"2021-07-20T20:40:01Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nBy using shorter names for the test repos, we will get a slightly more\ncompressed performance summary without comprimising clarity.\n\nSigned-off-by: Derrick Stolee \n---\n t/perf/p2000-sparse-operations.sh | 20 ++++++++++----------\n 1 file changed, 10 insertions(+), 10 deletions(-)\n\ndiff --git a/t/perf/p2000-sparse-operations.sh b/t/perf/p2000-sparse-operations.sh\nindex f7f8c012103..597626276fb 100755\n--- a/t/perf/p2000-sparse-operations.sh\n+++ b/t/perf/p2000-sparse-operations.sh\n@@ -52,36 +52,36 @@ test_expect_success 'setup repo and indexes' '\n \t\tgit commit -a -m \"edit $SPARSE_CONE/$l2/a\" || return 1\n \tdone &&\n \n-\tgit -c core.sparseCheckoutCone=true clone --branch=wide --sparse . full-index-v3 &&\n+\tgit -c core.sparseCheckoutCone=true clone --branch=wide --sparse . full-v3 &&\n \t(\n-\t\tcd full-index-v3 &&\n+\t\tcd full-v3 &&\n \t\tgit sparse-checkout init --cone &&\n \t\tgit sparse-checkout set $SPARSE_CONE &&\n \t\tgit config index.version 3 &&\n \t\tgit update-index --index-version=3 &&\n \t\tgit checkout HEAD~4\n \t) &&\n-\tgit -c core.sparseCheckoutCone=true clone --branch=wide --sparse . full-index-v4 &&\n+\tgit -c core.sparseCheckoutCone=true clone --branch=wide --sparse . full-v4 &&\n \t(\n-\t\tcd full-index-v4 &&\n+\t\tcd full-v4 &&\n \t\tgit sparse-checkout init --cone &&\n \t\tgit sparse-checkout set $SPARSE_CONE &&\n \t\tgit config index.version 4 &&\n \t\tgit update-index --index-version=4 &&\n \t\tgit checkout HEAD~4\n \t) &&\n-\tgit -c core.sparseCheckoutCone=true clone --branch=wide --sparse . sparse-index-v3 &&\n+\tgit -c core.sparseCheckoutCone=true clone --branch=wide --sparse . sparse-v3 &&\n \t(\n-\t\tcd sparse-index-v3 &&\n+\t\tcd sparse-v3 &&\n \t\tgit sparse-checkout init --cone --sparse-index &&\n \t\tgit sparse-checkout set $SPARSE_CONE &&\n \t\tgit config index.version 3 &&\n \t\tgit update-index --index-version=3 &&\n \t\tgit checkout HEAD~4\n \t) &&\n-\tgit -c core.sparseCheckoutCone=true clone --branch=wide --sparse . sparse-index-v4 &&\n+\tgit -c core.sparseCheckoutCone=true clone --branch=wide --sparse . sparse-v4 &&\n \t(\n-\t\tcd sparse-index-v4 &&\n+\t\tcd sparse-v4 &&\n \t\tgit sparse-checkout init --cone --sparse-index &&\n \t\tgit sparse-checkout set $SPARSE_CONE &&\n \t\tgit config index.version 4 &&\n@@ -92,8 +92,8 @@ test_expect_success 'setup repo and indexes' '\n \n test_perf_on_all () {\n \tcommand=\"$@\"\n-\tfor repo in full-index-v3 full-index-v4 \\\n-\t\t sparse-index-v3 sparse-index-v4\n+\tfor repo in full-v3 full-v4 \\\n+\t\t sparse-v3 sparse-v4\n \tdo\n \t\ttest_perf \"$command ($repo)\" \"\n \t\t\t(\n-- \ngitgitgadget\n\n"},{"id":"430685","messageId":"pull.973.v2.git.1626812081.gitgitgadget@gmail.com","threadId":"56023","inReplyTo":"pull.973.git.1624932786.gitgitgadget@gmail.com","subject":"[PATCH v2 0/7] Sparse index: integrate with commit and checkout","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-07-20T20:14:34Z","receivedAt":"2021-07-20T20:40:52Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"This series extends our integration of sparse-index to 'git commit' and 'git\ncheckout'.\n\nThis is based on ds/status-with-sparse-index (v7) and v2.32.0. The hard work\nwas already done in that topic, so these changes are simple.\n\nRecall that we have delayed our integration with 'git add' until we can work\nout the concerns about how to deal with pathspecs outside of the\nsparse-checkout definition. Those concerns might have some overlap with how\n'git commit' takes a pathspec, but this seems like a rare enough case to\nhandle here and we can be more careful with the behavior change in the next\nseries which will integrate with git add.\n\nIn addition to the tests that already exist in t1092, I have integrated\nthese changes in microsoft/git and tested them against the Scalar functional\ntests, which go through quite a few complicated scenarios, verifying that\nthings work the same across the full index and sparse-index cases.\n\n\nUpdate in V2\n============\n\n * There is no change to the code, but it is presented in a slightly\n different order.\n * We've been discussing some complicated directory/file conflict cases, in\n particular with a staged change inside the directory. These tests are\n added and described as documenting incorrect behavior that should be\n changed.\n * After those tests are in place, we can motivate the change to\n twoway_merge() as necessary for a more-common situation (still rare) but\n still incorrect in an already-broken situation. Hopefully that balance is\n sufficient for now, until we can do the bigger work of fixing the bad\n behavior.\n\nThanks, -Stolee\n\nDerrick Stolee (7):\n p2000: add 'git checkout -' test and decrease depth\n p2000: compress repo names\n commit: integrate with sparse-index\n sparse-index: recompute cache-tree\n checkout: stop expanding sparse indexes\n t1092: document bad 'git checkout' behavior\n unpack-trees: resolve sparse-directory/file conflicts\n\n builtin/checkout.c | 8 +-\n builtin/commit.c | 3 +\n cache-tree.c | 2 -\n sparse-index.c | 2 +\n t/perf/p2000-sparse-operations.sh | 47 ++++--\n t/t1092-sparse-checkout-compatibility.sh | 197 ++++++++++++++++++++++-\n unpack-trees.c | 11 ++\n 7 files changed, 240 insertions(+), 30 deletions(-)\n\n\nbase-commit: e5ca291076a8a936283bb2c57433c4393d3f80c2\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-973%2Fderrickstolee%2Fsparse-index%2Fcommit-and-checkout-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-973/derrickstolee/sparse-index/commit-and-checkout-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/973\n\nRange-diff vs v1:\n\n 1: bb3dd1fdd48 = 1: 6e74958f590 p2000: add 'git checkout -' test and decrease depth\n 2: eb15bf37685 = 2: 3e1d03c41be p2000: compress repo names\n 3: 413babe6e77 ! 3: cd94f820052 commit: integrate with sparse-index\n @@ t/t1092-sparse-checkout-compatibility.sh: test_expect_success 'sparse-index is e\n +\tensure_not_expanded commit --include deep/deeper1/a -m deeper\n '\n \n - test_expect_success 'reset mixed and checkout orphan' '\n + # NEEDSWORK: a sparse-checkout behaves differently from a full checkout\n 4: ffe8473caab = 4: 65e79b8037c sparse-index: recompute cache-tree\n 5: 8710fee36b7 ! 5: e9a9981477e checkout: stop expanding sparse indexes\n @@ t/t1092-sparse-checkout-compatibility.sh: test_expect_success 'sparse-index is n\n +\tensure_not_expanded restore -s rename-out-to-out -- deep/deeper1\n '\n \n - test_expect_success 'reset mixed and checkout orphan' '\n + # NEEDSWORK: a sparse-checkout behaves differently from a full checkout\n -: ----------- > 6: 4b801c854fb t1092: document bad 'git checkout' behavior\n -: ----------- > 7: 71e301501c8 unpack-trees: resolve sparse-directory/file conflicts\n\n-- \ngitgitgadget\n"},{"id":"430686","messageId":"6e74958f5900855cc0c3b6484b2b2a7732293e6c.1626812081.git.gitgitgadget@gmail.com","threadId":"56023","inReplyTo":"pull.973.v2.git.1626812081.gitgitgadget@gmail.com","subject":"[PATCH v2 1/7] p2000: add 'git checkout -' test and decrease depth","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-07-20T20:14:35Z","receivedAt":"2021-07-20T20:41:19Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nAs we increase our list of commands to test in\np2000-sparse-operations.sh, we will want to have a slightly smaller test\nrepository. Reduce the size by a factor of four by reducing the depth of\nthe step that creates a big index around a moderately-sized repository.\n\nAlso add a step to run 'git checkout -' on repeat. This requires having\na previous location in the reflog, so add that to the initialization\nsteps.\n\nSigned-off-by: Derrick Stolee \n---\n t/perf/p2000-sparse-operations.sh | 27 ++++++++++++++++++++-------\n 1 file changed, 20 insertions(+), 7 deletions(-)\n\ndiff --git a/t/perf/p2000-sparse-operations.sh b/t/perf/p2000-sparse-operations.sh\nindex 94513c97748..f7f8c012103 100755\n--- a/t/perf/p2000-sparse-operations.sh\n+++ b/t/perf/p2000-sparse-operations.sh\n@@ -6,7 +6,7 @@ test_description=\"test performance of Git operations using the index\"\n \n test_perf_default_repo\n \n-SPARSE_CONE=f2/f4/f1\n+SPARSE_CONE=f2/f4\n \n test_expect_success 'setup repo and indexes' '\n \tgit reset --hard HEAD &&\n@@ -27,7 +27,7 @@ test_expect_success 'setup repo and indexes' '\n \tOLD_COMMIT=$(git rev-parse HEAD) &&\n \tOLD_TREE=$(git rev-parse HEAD^{tree}) &&\n \n-\tfor i in $(test_seq 1 4)\n+\tfor i in $(test_seq 1 3)\n \tdo\n \t\tcat >in <<-EOF &&\n \t\t\t100755 blob $BLOB\ta\n@@ -43,14 +43,23 @@ test_expect_success 'setup repo and indexes' '\n \tdone &&\n \n \tgit sparse-checkout init --cone &&\n-\tgit branch -f wide $OLD_COMMIT &&\n+\tgit sparse-checkout set $SPARSE_CONE &&\n+\tgit checkout -b wide $OLD_COMMIT &&\n+\n+\tfor l2 in f1 f2 f3 f4\n+\tdo\n+\t\techo more bogus >>$SPARSE_CONE/$l2/a &&\n+\t\tgit commit -a -m \"edit $SPARSE_CONE/$l2/a\" || return 1\n+\tdone &&\n+\n \tgit -c core.sparseCheckoutCone=true clone --branch=wide --sparse . full-index-v3 &&\n \t(\n \t\tcd full-index-v3 &&\n \t\tgit sparse-checkout init --cone &&\n \t\tgit sparse-checkout set $SPARSE_CONE &&\n \t\tgit config index.version 3 &&\n-\t\tgit update-index --index-version=3\n+\t\tgit update-index --index-version=3 &&\n+\t\tgit checkout HEAD~4\n \t) &&\n \tgit -c core.sparseCheckoutCone=true clone --branch=wide --sparse . full-index-v4 &&\n \t(\n@@ -58,7 +67,8 @@ test_expect_success 'setup repo and indexes' '\n \t\tgit sparse-checkout init --cone &&\n \t\tgit sparse-checkout set $SPARSE_CONE &&\n \t\tgit config index.version 4 &&\n-\t\tgit update-index --index-version=4\n+\t\tgit update-index --index-version=4 &&\n+\t\tgit checkout HEAD~4\n \t) &&\n \tgit -c core.sparseCheckoutCone=true clone --branch=wide --sparse . sparse-index-v3 &&\n \t(\n@@ -66,7 +76,8 @@ test_expect_success 'setup repo and indexes' '\n \t\tgit sparse-checkout init --cone --sparse-index &&\n \t\tgit sparse-checkout set $SPARSE_CONE &&\n \t\tgit config index.version 3 &&\n-\t\tgit update-index --index-version=3\n+\t\tgit update-index --index-version=3 &&\n+\t\tgit checkout HEAD~4\n \t) &&\n \tgit -c core.sparseCheckoutCone=true clone --branch=wide --sparse . sparse-index-v4 &&\n \t(\n@@ -74,7 +85,8 @@ test_expect_success 'setup repo and indexes' '\n \t\tgit sparse-checkout init --cone --sparse-index &&\n \t\tgit sparse-checkout set $SPARSE_CONE &&\n \t\tgit config index.version 4 &&\n-\t\tgit update-index --index-version=4\n+\t\tgit update-index --index-version=4 &&\n+\t\tgit checkout HEAD~4\n \t)\n '\n \n@@ -97,5 +109,6 @@ test_perf_on_all git status\n test_perf_on_all git add -A\n test_perf_on_all git add .\n test_perf_on_all git commit -a -m A\n+test_perf_on_all git checkout -f -\n \n test_done\n-- \ngitgitgadget\n\n"},{"id":"430687","messageId":"e9a9981477e9d6ba3aef96e39db5736b334d7189.1626812081.git.gitgitgadget@gmail.com","threadId":"56023","inReplyTo":"pull.973.v2.git.1626812081.gitgitgadget@gmail.com","subject":"[PATCH v2 5/7] checkout: stop expanding sparse indexes","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-07-20T20:14:39Z","receivedAt":"2021-07-20T20:41:46Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nPrevious changes did the necessary improvements to unpack-trees.c and\ndiff-lib.c in order to modify a sparse index based on its comparision\nwith a tree. The only remaining work is to remove some\nensure_full_index() calls and add tests that verify that the index is\nnot expanded in our interesting cases. Include 'switch' and 'restore' in\nthese tests, as they share a base implementation with 'checkout'.\n\nHere are the relevant performance results from\np2000-sparse-operations.sh:\n\nTest HEAD~1 HEAD\n--------------------------------------------------------------------------------\n2000.18: git checkout -f - (full-v3) 0.49(0.43+0.03) 0.47(0.39+0.05) -4.1%\n2000.19: git checkout -f - (full-v4) 0.45(0.37+0.06) 0.42(0.37+0.05) -6.7%\n2000.20: git checkout -f - (sparse-v3) 0.76(0.71+0.07) 0.04(0.03+0.04) -94.7%\n2000.21: git checkout -f - (sparse-v4) 0.75(0.72+0.04) 0.05(0.06+0.04) -93.3%\n\nIt is important to compare the full index case to the sparse index case,\nas the previous results for the sparse index were inflated by the index\nexpansion. For index v4, this is an 88% improvement.\n\nOn an internal repository with over two million paths at HEAD and a\nsparse-checkout definition containing ~60,000 of those paths, 'git\ncheckout' went from 3.5s to 297ms with this change. The theoretical\noptimum where only those ~60,000 paths exist was 275ms, so the extra\nsparse directory entries contribute a 22ms overhead.\n\nSigned-off-by: Derrick Stolee \n---\n builtin/checkout.c | 8 +++-----\n t/t1092-sparse-checkout-compatibility.sh | 10 +++++++++-\n 2 files changed, 12 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex f4cd7747d35..b5d477919a7 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -378,9 +378,6 @@ static int checkout_worktree(const struct checkout_opts *opts,\n \tif (pc_workers > 1)\n \t\tinit_parallel_checkout();\n \n-\t/* TODO: audit for interaction with sparse-index. */\n-\tensure_full_index(&the_index);\n-\n \tfor (pos = 0; pos < active_nr; pos++) {\n \t\tstruct cache_entry *ce = active_cache[pos];\n \t\tif (ce->ce_flags & CE_MATCHED) {\n@@ -530,8 +527,6 @@ static int checkout_paths(const struct checkout_opts *opts,\n \t * Make sure all pathspecs participated in locating the paths\n \t * to be checked out.\n \t */\n-\t/* TODO: audit for interaction with sparse-index. */\n-\tensure_full_index(&the_index);\n \tfor (pos = 0; pos < active_nr; pos++)\n \t\tif (opts->overlay_mode)\n \t\t\tmark_ce_for_checkout_overlay(active_cache[pos],\n@@ -1593,6 +1588,9 @@ static int checkout_main(int argc, const char **argv, const char *prefix,\n \n \tgit_config(git_checkout_config, opts);\n \n+\tprepare_repo_settings(the_repository);\n+\tthe_repository->settings.command_requires_full_index = 0;\n+\n \topts->track = BRANCH_TRACK_UNSPECIFIED;\n \n \tif (!opts->accept_pathspec && !opts->accept_ref)\ndiff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh\nindex d3e34d0acac..fde3b41aba8 100755\n--- a/t/t1092-sparse-checkout-compatibility.sh\n+++ b/t/t1092-sparse-checkout-compatibility.sh\n@@ -560,7 +560,15 @@ test_expect_success 'sparse-index is not expanded' '\n \techo >>sparse-index/a &&\n \tensure_not_expanded commit --include a -m a &&\n \techo >>sparse-index/deep/deeper1/a &&\n-\tensure_not_expanded commit --include deep/deeper1/a -m deeper\n+\tensure_not_expanded commit --include deep/deeper1/a -m deeper &&\n+\tensure_not_expanded checkout rename-out-to-out &&\n+\tensure_not_expanded checkout - &&\n+\tensure_not_expanded switch rename-out-to-out &&\n+\tensure_not_expanded switch - &&\n+\tgit -C sparse-index reset --hard &&\n+\tensure_not_expanded checkout rename-out-to-out -- deep/deeper1 &&\n+\tgit -C sparse-index reset --hard &&\n+\tensure_not_expanded restore -s rename-out-to-out -- deep/deeper1\n '\n \n # NEEDSWORK: a sparse-checkout behaves differently from a full checkout\n-- \ngitgitgadget\n\n"},{"id":"430688","messageId":"4b801c854fb7891a6530121281cff3f67451b283.1626812081.git.gitgitgadget@gmail.com","threadId":"56023","inReplyTo":"pull.973.v2.git.1626812081.gitgitgadget@gmail.com","subject":"[PATCH v2 6/7] t1092: document bad 'git checkout' behavior","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-07-20T20:14:40Z","receivedAt":"2021-07-20T20:42:01Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nAdd new branches to the test repo that demonstrate directory/file\nconflicts in different ways. Since the directory 'folder1/' has\nadjacent files 'folder1-', 'folder1.txt', and 'folder10' it causes\nsearches for 'folder1/' to land in a different place in the index than a\nsearch for 'folder1'. This causes a change in behavior when working with\nthe df-conflict-1 and df-conflict-2 branches, whose only difference is\nthat the first uses 'folder1' as the conflict and the other uses\n'folder2' which does not have these adjacent files.\n\nWe can extend two tests that compare the behavior across different 'git\ncheckout' commands, and we see already that the behavior will be\ndifferent in some cases and not in others. The difference between the\ntwo test loops is that one uses 'git reset --hard' between iterations.\n\nFurther, we isolate the behavior of creating a staged change within a\ndirectory and then checking out a branch where that directory is\nreplaced with a file. A full checkout behaves differently across these\ntwo cases, while a sparse-checkout cone behaves consistently. In both\ncases, the behavior is wrong. In one case, the staged change is dropped\nentirely. The other case the staged change is kept, replacing the file\nat that location, but none of the other files in the directory are kept.\n\nLikely, the correct behavior in this case is to reject the checkout and\nreport the conflict, leaving HEAD in its previous location. None of the\ncases behave this way currently. Use comments to demonstrate that the\ntested behavior is only a documentation of the current, incorrect\nbehavior to ensure we do not _accidentally_ change it. Instead, we would\nprefer to change it on purpose with a future change.\n\nAt this point, the sparse-index does not handle these 'git checkout'\ncommands correctly. Or rather, it _does_ reject the 'git checkout' when\nwe have the staged change, but for the wrong reason. It also rejects the\n'git checkout' commands when there is no staged change and we want to\nreplace a directory with a file. A fix for that unstaged case will\nfollow in the next change, but that will make the sparse-index agree\nwith the full checkout case in these documented incorrect behaviors.\n\nHelped-by: Elijah Newren \nSigned-off-by: Derrick Stolee \n---\n t/t1092-sparse-checkout-compatibility.sh | 142 ++++++++++++++++++++++-\n 1 file changed, 140 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh\nindex fde3b41aba8..79b4a8ce199 100755\n--- a/t/t1092-sparse-checkout-compatibility.sh\n+++ b/t/t1092-sparse-checkout-compatibility.sh\n@@ -95,6 +95,25 @@ test_expect_success 'setup' '\n \t\tgit add . &&\n \t\tgit commit -m \"rename deep/deeper1/... to folder1/...\" &&\n \n+\t\tgit checkout -b df-conflict-1 base &&\n+\t\trm -rf folder1 &&\n+\t\techo content >folder1 &&\n+\t\tgit add . &&\n+\t\tgit commit -m \"dir to file\" &&\n+\n+\t\tgit checkout -b df-conflict-2 base &&\n+\t\trm -rf folder2 &&\n+\t\techo content >folder2 &&\n+\t\tgit add . &&\n+\t\tgit commit -m \"dir to file\" &&\n+\n+\t\tgit checkout -b fd-conflict base &&\n+\t\trm a &&\n+\t\tmkdir a &&\n+\t\techo content >a/a &&\n+\t\tgit add . &&\n+\t\tgit commit -m \"file to dir\" &&\n+\n \t\tgit checkout -b deepest base &&\n \t\techo \"updated deepest\" >deep/deeper1/deepest/a &&\n \t\tgit commit -a -m \"update deepest\" &&\n@@ -358,10 +377,16 @@ test_expect_success 'diff --staged' '\n \ttest_all_match git diff --staged\n '\n \n+# NEEDSWORK: sparse-checkout behaves differently from full-checkout when\n+# running this test with 'df-conflict-2' after 'df-conflict-1'.\n test_expect_success 'diff with renames and conflicts' '\n \tinit_repos &&\n \n-\tfor branch in rename-out-to-out rename-out-to-in rename-in-to-out\n+\tfor branch in rename-out-to-out \\\n+\t\t rename-out-to-in \\\n+\t\t rename-in-to-out \\\n+\t\t df-conflict-1 \\\n+\t\t fd-conflict\n \tdo\n \t\ttest_all_match git checkout rename-base &&\n \t\ttest_all_match git checkout $branch -- . &&\n@@ -371,10 +396,15 @@ test_expect_success 'diff with renames and conflicts' '\n \tdone\n '\n \n+# NEEDSWORK: the sparse-index fails to move HEAD across a directory/file\n+# conflict such as when checking out df-conflict-1 and df-conflict2.\n test_expect_success 'diff with directory/file conflicts' '\n \tinit_repos &&\n \n-\tfor branch in rename-out-to-out rename-out-to-in rename-in-to-out\n+\tfor branch in rename-out-to-out \\\n+\t\t rename-out-to-in \\\n+\t\t rename-in-to-out \\\n+\t\t fd-conflict\n \tdo\n \t\tgit -C full-checkout reset --hard &&\n \t\ttest_sparse_match git reset --hard &&\n@@ -606,4 +636,112 @@ test_expect_success 'add everything with deep new file' '\n \ttest_all_match git status --porcelain=v2\n '\n \n+# NEEDSWORK: 'git checkout' behaves incorrectly in the case of\n+# directory/file conflicts, even without sparse-checkout. Use this\n+# test only as a documentation of the incorrect behavior, not a\n+# measure of how it _should_ behave.\n+test_expect_success 'checkout behaves oddly with df-conflict-1' '\n+\tinit_repos &&\n+\n+\ttest_sparse_match git sparse-checkout disable &&\n+\n+\twrite_script edit-content <<-\\EOF &&\n+\techo content >>folder1/larger-content\n+\tgit add folder1\n+\tEOF\n+\n+\trun_on_all ../edit-content &&\n+\ttest_all_match git status --porcelain=v2 &&\n+\n+\tgit -C sparse-checkout sparse-checkout init --cone &&\n+\tgit -C sparse-index sparse-checkout init --cone --sparse-index &&\n+\n+\ttest_all_match git status --porcelain=v2 &&\n+\n+\t# This checkout command should fail, because we have a staged\n+\t# change to folder1/larger-content, but the destination changes\n+\t# folder1 to a file.\n+\tgit -C full-checkout checkout df-conflict-1 \\\n+\t\t1>full-checkout-out \\\n+\t\t2>full-checkout-err &&\n+\tgit -C sparse-checkout checkout df-conflict-1 \\\n+\t\t1>sparse-checkout-out \\\n+\t\t2>sparse-checkout-err &&\n+\n+\t# NEEDSWORK: the sparse-index case refuses to change HEAD here,\n+\t# but for the wrong reason.\n+\ttest_must_fail git -C sparse-index checkout df-conflict-1 \\\n+\t\t1>sparse-index-out \\\n+\t\t2>sparse-index-err &&\n+\n+\t# Instead, the checkout deletes the folder1 file and adds the\n+\t# folder1/larger-content file, leaving all other paths that were\n+\t# in folder1/ as deleted (without any warning).\n+\tcat >expect <<-EOF &&\n+\tD\tfolder1\n+\tA\tfolder1/larger-content\n+\tEOF\n+\ttest_cmp expect full-checkout-out &&\n+\ttest_cmp expect sparse-checkout-out &&\n+\n+\t# stderr: Switched to branch df-conflict-1\n+\ttest_cmp full-checkout-err sparse-checkout-err\n+'\n+\n+# NEEDSWORK: 'git checkout' behaves incorrectly in the case of\n+# directory/file conflicts, even without sparse-checkout. Use this\n+# test only as a documentation of the incorrect behavior, not a\n+# measure of how it _should_ behave.\n+test_expect_success 'checkout behaves oddly with df-conflict-2' '\n+\tinit_repos &&\n+\n+\ttest_sparse_match git sparse-checkout disable &&\n+\n+\twrite_script edit-content <<-\\EOF &&\n+\techo content >>folder2/larger-content\n+\tgit add folder2\n+\tEOF\n+\n+\trun_on_all ../edit-content &&\n+\ttest_all_match git status --porcelain=v2 &&\n+\n+\tgit -C sparse-checkout sparse-checkout init --cone &&\n+\tgit -C sparse-index sparse-checkout init --cone --sparse-index &&\n+\n+\ttest_all_match git status --porcelain=v2 &&\n+\n+\t# This checkout command should fail, because we have a staged\n+\t# change to folder1/larger-content, but the destination changes\n+\t# folder1 to a file.\n+\tgit -C full-checkout checkout df-conflict-2 \\\n+\t\t1>full-checkout-out \\\n+\t\t2>full-checkout-err &&\n+\tgit -C sparse-checkout checkout df-conflict-2 \\\n+\t\t1>sparse-checkout-out \\\n+\t\t2>sparse-checkout-err &&\n+\n+\t# NEEDSWORK: the sparse-index case refuses to change HEAD\n+\t# here, but for the wrong reason.\n+\ttest_must_fail git -C sparse-index checkout df-conflict-2 \\\n+\t\t1>sparse-index-out \\\n+\t\t2>sparse-index-err &&\n+\n+\t# The full checkout deviates from the df-conflict-1 case here!\n+\t# It drops the change to folder1/larger-content and leaves the\n+\t# folder1 path as-is on disk.\n+\ttest_must_be_empty full-checkout-out &&\n+\n+\t# In the sparse-checkout case, the checkout deletes the folder1\n+\t# file and adds the folder1/larger-content file, leaving all other\n+\t# paths that were in folder1/ as deleted (without any warning).\n+\tcat >expect <<-EOF &&\n+\tD\tfolder2\n+\tA\tfolder2/larger-content\n+\tEOF\n+\ttest_cmp expect sparse-checkout-out &&\n+\n+\t# Switched to branch df-conflict-1\n+\ttest_cmp full-checkout-err sparse-checkout-err\n+'\n+\n test_done\n-- \ngitgitgadget\n\n"},{"id":"430689","messageId":"71e301501c88399711a1bf8515d1747e92cfbb9b.1626812081.git.gitgitgadget@gmail.com","threadId":"56023","inReplyTo":"pull.973.v2.git.1626812081.gitgitgadget@gmail.com","subject":"[PATCH v2 7/7] unpack-trees: resolve sparse-directory/file conflicts","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-07-20T20:14:41Z","receivedAt":"2021-07-20T20:42:15Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nWhen running unpack_trees() with a sparse index, we attempt to operate\non the index without expanding the sparse directory entries. Thus, we\noperate by manipulating entire directories and passing them to the\nunpack function. In the case of the 'git checkout' command, this is the\ntwoway_merge() function.\n\nThere are several cases in twoway_merge() that handle different\nsituations. One new one to add is the case of a directory/file conflict\nwhere the directory is sparse. Before the sparse index, such a conflict\nwould appear as a list of file additions and deletions. Now,\ntwoway_merge() initializes 'current', 'oldtree', and 'newtree' from\nsrc[0], src[1], and src[2], then sets 'oldtree' to NULL because it is\nequal to the df_conflict_entry. The way to determine that we have a\ndirectory/file conflict is to test that 'current' and 'newtree' disagree\non being sparse directory entries.\n\nWhen we are in this case, we want to resolve the situation by calling\nmerged_entry(). This allows replacing the 'current' entry with the\n'newtree' entry. This is important for cases where we want to run 'git\ncheckout' across the conflict and have the new HEAD represent the new\nfile type at that path. The first NEEDSWORK comment dropped in t1092\ndemonstrates this necessary behavior.\n\nHowever, we still are in a confusing state when 'current' corresponds to\na staged change within a sparse directory that is not present at HEAD.\nThis should be atypical, because it requires adding a change outside of\nthe sparse-checkout cone, but it is possible. Since we are unable to\ndetermine that this is a staged change within twoway_merge(), we cannot\nadd a case to reject the merge at this point. I believe this is due to\nthe use of df_conflict_entry in the place of 'oldtree' instead of using\nthe valud at HEAD, which would provide some perspective to this\ndecision. Any change that would allow this differentiation for staged\nentries would need to involve information further up in unpack_trees().\n\nThat work should be done, sometime, because we are further confusing the\nbehavior of a directory/file conflict when staging a change in the\ndirectory. The two cases 'checkout behaves oddly with df-conflict-?' in\nt1092 demonstrate that even without a sparse-checkout, Git is not\nconsistent in its behavior. Neither of the two options seems correct,\neither. This change makes the sparse-index behave differently than the\ntypcial sparse-checkout case, but it does match the full checkout\nbehavior in the df-conflict-2 case.\n\nSigned-off-by: Derrick Stolee \n---\n t/t1092-sparse-checkout-compatibility.sh | 24 ++++++++++++------------\n unpack-trees.c | 11 +++++++++++\n 2 files changed, 23 insertions(+), 12 deletions(-)\n\ndiff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh\nindex 79b4a8ce199..91e30d6ec22 100755\n--- a/t/t1092-sparse-checkout-compatibility.sh\n+++ b/t/t1092-sparse-checkout-compatibility.sh\n@@ -396,14 +396,14 @@ test_expect_success 'diff with renames and conflicts' '\n \tdone\n '\n \n-# NEEDSWORK: the sparse-index fails to move HEAD across a directory/file\n-# conflict such as when checking out df-conflict-1 and df-conflict2.\n test_expect_success 'diff with directory/file conflicts' '\n \tinit_repos &&\n \n \tfor branch in rename-out-to-out \\\n \t\t rename-out-to-in \\\n \t\t rename-in-to-out \\\n+\t\t df-conflict-1 \\\n+\t\t df-conflict-2 \\\n \t\t fd-conflict\n \tdo\n \t\tgit -C full-checkout reset --hard &&\n@@ -667,10 +667,7 @@ test_expect_success 'checkout behaves oddly with df-conflict-1' '\n \tgit -C sparse-checkout checkout df-conflict-1 \\\n \t\t1>sparse-checkout-out \\\n \t\t2>sparse-checkout-err &&\n-\n-\t# NEEDSWORK: the sparse-index case refuses to change HEAD here,\n-\t# but for the wrong reason.\n-\ttest_must_fail git -C sparse-index checkout df-conflict-1 \\\n+\tgit -C sparse-index checkout df-conflict-1 \\\n \t\t1>sparse-index-out \\\n \t\t2>sparse-index-err &&\n \n@@ -684,7 +681,11 @@ test_expect_success 'checkout behaves oddly with df-conflict-1' '\n \ttest_cmp expect full-checkout-out &&\n \ttest_cmp expect sparse-checkout-out &&\n \n+\t# The sparse-index reports no output\n+\ttest_must_be_empty sparse-index-out &&\n+\n \t# stderr: Switched to branch df-conflict-1\n+\ttest_cmp full-checkout-err sparse-checkout-err &&\n \ttest_cmp full-checkout-err sparse-checkout-err\n '\n \n@@ -719,17 +720,15 @@ test_expect_success 'checkout behaves oddly with df-conflict-2' '\n \tgit -C sparse-checkout checkout df-conflict-2 \\\n \t\t1>sparse-checkout-out \\\n \t\t2>sparse-checkout-err &&\n-\n-\t# NEEDSWORK: the sparse-index case refuses to change HEAD\n-\t# here, but for the wrong reason.\n-\ttest_must_fail git -C sparse-index checkout df-conflict-2 \\\n+\tgit -C sparse-index checkout df-conflict-2 \\\n \t\t1>sparse-index-out \\\n \t\t2>sparse-index-err &&\n \n \t# The full checkout deviates from the df-conflict-1 case here!\n \t# It drops the change to folder1/larger-content and leaves the\n-\t# folder1 path as-is on disk.\n+\t# folder1 path as-is on disk. The sparse-index behaves the same.\n \ttest_must_be_empty full-checkout-out &&\n+\ttest_must_be_empty sparse-index-out &&\n \n \t# In the sparse-checkout case, the checkout deletes the folder1\n \t# file and adds the folder1/larger-content file, leaving all other\n@@ -741,7 +740,8 @@ test_expect_success 'checkout behaves oddly with df-conflict-2' '\n \ttest_cmp expect sparse-checkout-out &&\n \n \t# Switched to branch df-conflict-1\n-\ttest_cmp full-checkout-err sparse-checkout-err\n+\ttest_cmp full-checkout-err sparse-checkout-err &&\n+\ttest_cmp full-checkout-err sparse-index-err\n '\n \n test_done\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex 0a5135ab397..309c1352f5d 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -2619,6 +2619,17 @@ int twoway_merge(const struct cache_entry * const *src,\n \t\t\t same(current, oldtree) && !same(current, newtree)) {\n \t\t\t/* 20 or 21 */\n \t\t\treturn merged_entry(newtree, current, o);\n+\t\t} else if (current && !oldtree && newtree &&\n+\t\t\t S_ISSPARSEDIR(current->ce_mode) != S_ISSPARSEDIR(newtree->ce_mode) &&\n+\t\t\t ce_stage(current) == 0) {\n+\t\t\t/*\n+\t\t\t * This case is a directory/file conflict across the sparse-index\n+\t\t\t * boundary. When we are changing from one path to another via\n+\t\t\t * 'git checkout', then we want to replace one entry with another\n+\t\t\t * via merged_entry(). If there are staged changes, then we should\n+\t\t\t * reject the merge instead.\n+\t\t\t */\n+\t\t\treturn merged_entry(newtree, current, o);\n \t\t} else\n \t\t\treturn reject_merge(current, o);\n \t}\n-- \ngitgitgadget\n"},{"id":"430690","messageId":"cd94f820052c948e522f947a05797ebaf1907121.1626812081.git.gitgitgadget@gmail.com","threadId":"56023","inReplyTo":"pull.973.v2.git.1626812081.gitgitgadget@gmail.com","subject":"[PATCH v2 3/7] commit: integrate with sparse-index","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-07-20T20:14:37Z","receivedAt":"2021-07-20T20:42:15Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nUpdate 'git commit' to allow using the sparse-index in memory without\nexpanding to a full one. The only place that had an ensure_full_index()\ncall was in cache_tree_update(). The recursive algorithm for\nupdate_one() was already updated in 2de37c536 (cache-tree: integrate\nwith sparse directory entries, 2021-03-03) to handle sparse directory\nentries in the index.\n\nMost of this change involves testing different command-line options that\nallow specifying which on-disk changes should be included in the commit.\nThis includes no options (only take currently-staged changes), -a (take\nall tracked changes), and --include (take a list of specific changes).\nTo simplify testing that these options do not expand the index, update\nthe test that previously verified that 'git status' does not expand the\nindex with a helper method, ensure_not_expanded().\n\nThis allows 'git commit' to operate much faster when the sparse-checkout\ncone is much smaller than the full list of files at HEAD.\n\nHere are the relevant lines from p2000-sparse-operations.sh:\n\nTest HEAD~1 HEAD\n----------------------------------------------------------------------------------\n2000.14: git commit -a -m A (full-v3) 0.35(0.26+0.06) 0.36(0.28+0.07) +2.9%\n2000.15: git commit -a -m A (full-v4) 0.32(0.26+0.05) 0.34(0.28+0.06) +6.3%\n2000.16: git commit -a -m A (sparse-v3) 0.63(0.59+0.06) 0.04(0.05+0.05) -93.7%\n2000.17: git commit -a -m A (sparse-v4) 0.64(0.59+0.08) 0.04(0.04+0.04) -93.8%\n\nIt is important to compare the full-index case to the sparse-index case,\nso the improvement for index version v4 is actually an 88% improvement in\nthis synthetic example.\n\nIn a real repository with over two million files at HEAD and 60,000\nfiles in the sparse-checkout definition, the time for 'git commit -a'\nwent from 2.61 seconds to 134ms. I compared this to the result if the\nindex only contained the paths in the sparse-checkout definition and\nfound the theoretical optimum to be 120ms, so the out-of-cone paths only\nadd a 12% overhead.\n\nSigned-off-by: Derrick Stolee \n---\n builtin/commit.c | 3 ++\n cache-tree.c | 2 -\n t/t1092-sparse-checkout-compatibility.sh | 47 ++++++++++++++++++++++--\n 3 files changed, 46 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 12f51db158a..0bc64892505 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -1682,6 +1682,9 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \tif (argc == 2 && !strcmp(argv[1], \"-h\"))\n \t\tusage_with_options(builtin_commit_usage, builtin_commit_options);\n \n+\tprepare_repo_settings(the_repository);\n+\tthe_repository->settings.command_requires_full_index = 0;\n+\n \tstatus_init_config(&s, git_commit_config);\n \ts.commit_template = 1;\n \tstatus_format = STATUS_FORMAT_NONE; /* Ignore status.short */\ndiff --git a/cache-tree.c b/cache-tree.c\nindex 45e58666afc..577b18d8811 100644\n--- a/cache-tree.c\n+++ b/cache-tree.c\n@@ -461,8 +461,6 @@ int cache_tree_update(struct index_state *istate, int flags)\n \tif (i)\n \t\treturn i;\n \n-\tensure_full_index(istate);\n-\n \tif (!istate->cache_tree)\n \t\tistate->cache_tree = cache_tree();\n \ndiff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh\nindex cabbd42e339..d3e34d0acac 100755\n--- a/t/t1092-sparse-checkout-compatibility.sh\n+++ b/t/t1092-sparse-checkout-compatibility.sh\n@@ -262,6 +262,34 @@ test_expect_success 'add, commit, checkout' '\n \ttest_all_match git checkout -\n '\n \n+test_expect_success 'commit including unstaged changes' '\n+\tinit_repos &&\n+\n+\twrite_script edit-file <<-\\EOF &&\n+\techo $1 >$2\n+\tEOF\n+\n+\trun_on_all ../edit-file 1 a &&\n+\trun_on_all ../edit-file 1 deep/a &&\n+\n+\ttest_all_match git commit -m \"-a\" -a &&\n+\ttest_all_match git status --porcelain=v2 &&\n+\n+\trun_on_all ../edit-file 2 a &&\n+\trun_on_all ../edit-file 2 deep/a &&\n+\n+\ttest_all_match git commit -m \"--include\" --include deep/a &&\n+\ttest_all_match git status --porcelain=v2 &&\n+\ttest_all_match git commit -m \"--include\" --include a &&\n+\ttest_all_match git status --porcelain=v2 &&\n+\n+\trun_on_all ../edit-file 3 a &&\n+\trun_on_all ../edit-file 3 deep/a &&\n+\n+\ttest_all_match git commit -m \"--amend\" -a --amend &&\n+\ttest_all_match git status --porcelain=v2\n+'\n+\n test_expect_success 'status/add: outside sparse cone' '\n \tinit_repos &&\n \n@@ -514,14 +542,25 @@ test_expect_success 'sparse-index is expanded and converted back' '\n \ttest_region index ensure_full_index trace2.txt\n '\n \n-test_expect_success 'sparse-index is not expanded' '\n-\tinit_repos &&\n-\n+ensure_not_expanded () {\n \trm -f trace2.txt &&\n \techo >>sparse-index/untracked.txt &&\n \tGIT_TRACE2_EVENT=\"$(pwd)/trace2.txt\" GIT_TRACE2_EVENT_NESTING=10 \\\n-\t\tgit -C sparse-index status &&\n+\t\tgit -C sparse-index \"$@\" &&\n \ttest_region ! index ensure_full_index trace2.txt\n+}\n+\n+test_expect_success 'sparse-index is not expanded' '\n+\tinit_repos &&\n+\n+\tensure_not_expanded status &&\n+\tensure_not_expanded commit --allow-empty -m empty &&\n+\techo >>sparse-index/a &&\n+\tensure_not_expanded commit -a -m a &&\n+\techo >>sparse-index/a &&\n+\tensure_not_expanded commit --include a -m a &&\n+\techo >>sparse-index/deep/deeper1/a &&\n+\tensure_not_expanded commit --include deep/deeper1/a -m deeper\n '\n \n # NEEDSWORK: a sparse-checkout behaves differently from a full checkout\n-- \ngitgitgadget\n\n"},{"id":"430691","messageId":"65e79b8037ca0268363db9ff967c1342a76485c5.1626812081.git.gitgitgadget@gmail.com","threadId":"56023","inReplyTo":"pull.973.v2.git.1626812081.gitgitgadget@gmail.com","subject":"[PATCH v2 4/7] sparse-index: recompute cache-tree","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-07-20T20:14:38Z","receivedAt":"2021-07-20T20:42:42Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nWhen some commands run with command_requires_full_index=1, then the\nindex can get in a state where the in-memory cache tree is actually\nequal to the sparse index's cache tree instead of the full one.\n\nThis results in incorrect entry_count values. By clearing the cache\ntree before converting to sparse, we avoid this issue.\n\nSigned-off-by: Derrick Stolee \n---\n sparse-index.c | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/sparse-index.c b/sparse-index.c\nindex 53c8f711ccc..c6b4feec413 100644\n--- a/sparse-index.c\n+++ b/sparse-index.c\n@@ -170,6 +170,8 @@ int convert_to_sparse(struct index_state *istate)\n \tif (index_has_unmerged_entries(istate))\n \t\treturn 0;\n \n+\t/* Clear and recompute the cache-tree */\n+\tcache_tree_free(&istate->cache_tree);\n \tif (cache_tree_update(istate, 0)) {\n \t\twarning(_(\"unable to update cache-tree, staying full\"));\n \t\treturn -1;\n-- \ngitgitgadget\n\n"},{"id":"430867","messageId":"CABPp-BE3T_uLKZMwX7UJPK4vbGZ81OuEKU5H9WBqeUtcd5kgKg@mail.gmail.com","threadId":"56023","inReplyTo":"71e301501c88399711a1bf8515d1747e92cfbb9b.1626812081.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 7/7] unpack-trees: resolve sparse-directory/file conflicts","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2021-07-22T04:19:05Z","receivedAt":"2021-07-22T04:19:21Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Tue, Jul 20, 2021 at 1:14 PM Derrick Stolee via GitGitGadget\n wrote:\n>\n> From: Derrick Stolee \n>\n> When running unpack_trees() with a sparse index, we attempt to operate\n> on the index without expanding the sparse directory entries. Thus, we\n> operate by manipulating entire directories and passing them to the\n> unpack function. In the case of the 'git checkout' command, this is the\n> twoway_merge() function.\n>\n> There are several cases in twoway_merge() that handle different\n> situations. One new one to add is the case of a directory/file conflict\n> where the directory is sparse. Before the sparse index, such a conflict\n> would appear as a list of file additions and deletions. Now,\n> twoway_merge() initializes 'current', 'oldtree', and 'newtree' from\n> src[0], src[1], and src[2], then sets 'oldtree' to NULL because it is\n> equal to the df_conflict_entry. The way to determine that we have a\n> directory/file conflict is to test that 'current' and 'newtree' disagree\n> on being sparse directory entries.\n>\n> When we are in this case, we want to resolve the situation by calling\n> merged_entry(). This allows replacing the 'current' entry with the\n> 'newtree' entry. This is important for cases where we want to run 'git\n> checkout' across the conflict and have the new HEAD represent the new\n> file type at that path. The first NEEDSWORK comment dropped in t1092\n> demonstrates this necessary behavior.\n>\n> However, we still are in a confusing state when 'current' corresponds to\n> a staged change within a sparse directory that is not present at HEAD.\n> This should be atypical, because it requires adding a change outside of\n> the sparse-checkout cone, but it is possible. Since we are unable to\n> determine that this is a staged change within twoway_merge(), we cannot\n> add a case to reject the merge at this point. I believe this is due to\n> the use of df_conflict_entry in the place of 'oldtree' instead of using\n> the valud at HEAD, which would provide some perspective to this\n> decision. Any change that would allow this differentiation for staged\n> entries would need to involve information further up in unpack_trees().\n>\n> That work should be done, sometime, because we are further confusing the\n> behavior of a directory/file conflict when staging a change in the\n> directory. The two cases 'checkout behaves oddly with df-conflict-?' in\n> t1092 demonstrate that even without a sparse-checkout, Git is not\n> consistent in its behavior. Neither of the two options seems correct,\n> either. This change makes the sparse-index behave differently than the\n> typcial sparse-checkout case, but it does match the full checkout\n> behavior in the df-conflict-2 case.\n>\n> Signed-off-by: Derrick Stolee \n> ---\n> t/t1092-sparse-checkout-compatibility.sh | 24 ++++++++++++------------\n> unpack-trees.c | 11 +++++++++++\n> 2 files changed, 23 insertions(+), 12 deletions(-)\n>\n> diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh\n> index 79b4a8ce199..91e30d6ec22 100755\n> --- a/t/t1092-sparse-checkout-compatibility.sh\n> +++ b/t/t1092-sparse-checkout-compatibility.sh\n> @@ -396,14 +396,14 @@ test_expect_success 'diff with renames and conflicts' '\n> done\n> '\n>\n> -# NEEDSWORK: the sparse-index fails to move HEAD across a directory/file\n> -# conflict such as when checking out df-conflict-1 and df-conflict2.\n> test_expect_success 'diff with directory/file conflicts' '\n> init_repos &&\n>\n> for branch in rename-out-to-out \\\n> rename-out-to-in \\\n> rename-in-to-out \\\n> + df-conflict-1 \\\n> + df-conflict-2 \\\n> fd-conflict\n> do\n> git -C full-checkout reset --hard &&\n> @@ -667,10 +667,7 @@ test_expect_success 'checkout behaves oddly with df-conflict-1' '\n> git -C sparse-checkout checkout df-conflict-1 \\\n> 1>sparse-checkout-out \\\n> 2>sparse-checkout-err &&\n> -\n> - # NEEDSWORK: the sparse-index case refuses to change HEAD here,\n> - # but for the wrong reason.\n> - test_must_fail git -C sparse-index checkout df-conflict-1 \\\n> + git -C sparse-index checkout df-conflict-1 \\\n> 1>sparse-index-out \\\n> 2>sparse-index-err &&\n>\n> @@ -684,7 +681,11 @@ test_expect_success 'checkout behaves oddly with df-conflict-1' '\n> test_cmp expect full-checkout-out &&\n> test_cmp expect sparse-checkout-out &&\n>\n> + # The sparse-index reports no output\n> + test_must_be_empty sparse-index-out &&\n> +\n> # stderr: Switched to branch df-conflict-1\n> + test_cmp full-checkout-err sparse-checkout-err &&\n> test_cmp full-checkout-err sparse-checkout-err\n> '\n>\n> @@ -719,17 +720,15 @@ test_expect_success 'checkout behaves oddly with df-conflict-2' '\n> git -C sparse-checkout checkout df-conflict-2 \\\n> 1>sparse-checkout-out \\\n> 2>sparse-checkout-err &&\n> -\n> - # NEEDSWORK: the sparse-index case refuses to change HEAD\n> - # here, but for the wrong reason.\n> - test_must_fail git -C sparse-index checkout df-conflict-2 \\\n> + git -C sparse-index checkout df-conflict-2 \\\n> 1>sparse-index-out \\\n> 2>sparse-index-err &&\n>\n> # The full checkout deviates from the df-conflict-1 case here!\n> # It drops the change to folder1/larger-content and leaves the\n> - # folder1 path as-is on disk.\n> + # folder1 path as-is on disk. The sparse-index behaves the same.\n> test_must_be_empty full-checkout-out &&\n> + test_must_be_empty sparse-index-out &&\n>\n> # In the sparse-checkout case, the checkout deletes the folder1\n> # file and adds the folder1/larger-content file, leaving all other\n> @@ -741,7 +740,8 @@ test_expect_success 'checkout behaves oddly with df-conflict-2' '\n> test_cmp expect sparse-checkout-out &&\n>\n> # Switched to branch df-conflict-1\n> - test_cmp full-checkout-err sparse-checkout-err\n> + test_cmp full-checkout-err sparse-checkout-err &&\n> + test_cmp full-checkout-err sparse-index-err\n> '\n>\n> test_done\n> diff --git a/unpack-trees.c b/unpack-trees.c\n> index 0a5135ab397..309c1352f5d 100644\n> --- a/unpack-trees.c\n> +++ b/unpack-trees.c\n> @@ -2619,6 +2619,17 @@ int twoway_merge(const struct cache_entry * const *src,\n> same(current, oldtree) && !same(current, newtree)) {\n> /* 20 or 21 */\n> return merged_entry(newtree, current, o);\n> + } else if (current && !oldtree && newtree &&\n> + S_ISSPARSEDIR(current->ce_mode) != S_ISSPARSEDIR(newtree->ce_mode) &&\n> + ce_stage(current) == 0) {\n> + /*\n> + * This case is a directory/file conflict across the sparse-index\n> + * boundary. When we are changing from one path to another via\n> + * 'git checkout', then we want to replace one entry with another\n> + * via merged_entry(). If there are staged changes, then we should\n> + * reject the merge instead.\n> + */\n> + return merged_entry(newtree, current, o);\n> } else\n> return reject_merge(current, o);\n> }\n> --\n\nI'm still a bit unhappy with the unpack-trees.c change (I wonder if\nhaving \"path/\" vs \"path\" is going to make D/F conflicts hard to handle\nand whether we need to make unpack_trees do something special to make\nboth paths be considered at the same time with one call to\ntwoway_merge() instead of two in order to fix this), BUT I think\nyou've done a really good job of documenting it and pointing out that\nunpack_trees() messes up even without sparse checkouts on D/F\nconflicts (though in a different way). I think you've documented it\nwell enough, and argued about the likelihood of issues well enough,\nthat it makes sense to proceed and circle back and fix this up later.\n"},{"id":"430868","messageId":"CABPp-BGbM_+bznuCCLfctt-XqVdCuROa9+s_jpeQXmTHEyZmjg@mail.gmail.com","threadId":"56023","inReplyTo":"pull.973.v2.git.1626812081.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 0/7] Sparse index: integrate with commit and checkout","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2021-07-22T04:22:37Z","receivedAt":"2021-07-22T04:22:51Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Tue, Jul 20, 2021 at 1:14 PM Derrick Stolee via GitGitGadget\n wrote:\n>\n> This series extends our integration of sparse-index to 'git commit' and 'git\n> checkout'.\n>\n> This is based on ds/status-with-sparse-index (v7) and v2.32.0. The hard work\n> was already done in that topic, so these changes are simple.\n>\n> Recall that we have delayed our integration with 'git add' until we can work\n> out the concerns about how to deal with pathspecs outside of the\n> sparse-checkout definition. Those concerns might have some overlap with how\n> 'git commit' takes a pathspec, but this seems like a rare enough case to\n> handle here and we can be more careful with the behavior change in the next\n> series which will integrate with git add.\n>\n> In addition to the tests that already exist in t1092, I have integrated\n> these changes in microsoft/git and tested them against the Scalar functional\n> tests, which go through quite a few complicated scenarios, verifying that\n> things work the same across the full index and sparse-index cases.\n>\n>\n> Update in V2\n> ============\n>\n> * There is no change to the code, but it is presented in a slightly\n> different order.\n> * We've been discussing some complicated directory/file conflict cases, in\n> particular with a staged change inside the directory. These tests are\n> added and described as documenting incorrect behavior that should be\n> changed.\n> * After those tests are in place, we can motivate the change to\n> twoway_merge() as necessary for a more-common situation (still rare) but\n> still incorrect in an already-broken situation. Hopefully that balance is\n> sufficient for now, until we can do the bigger work of fixing the bad\n> behavior.\n\nI read the first five patches previously. The tiny changes there in\nthe range-diff still look good to me.\n\nI very much appreciate the new patch 6.\n\nAs noted in 7/7, I'm a little unhappy with the patch to\ntwoway_merge(), BUT you've clearly documented the shortcomings in very\ngood detail and pointed out how git has (likely for decades) messed up\nin related ways for non-sparse checkouts with D/F conflicts. You've\ndocumented it well enough and argued well enough about the relative\nmerits, that I have to agree with you that this is a good step\nforward. I do hope we circle back and tie up the loose ends at some\npoint.\n\nSo, the whole series is:\n\nReviewed-by: Elijah Newren \n\n>\n> Thanks, -Stolee\n>\n> Derrick Stolee (7):\n> p2000: add 'git checkout -' test and decrease depth\n> p2000: compress repo names\n> commit: integrate with sparse-index\n> sparse-index: recompute cache-tree\n> checkout: stop expanding sparse indexes\n> t1092: document bad 'git checkout' behavior\n> unpack-trees: resolve sparse-directory/file conflicts\n>\n> builtin/checkout.c | 8 +-\n> builtin/commit.c | 3 +\n> cache-tree.c | 2 -\n> sparse-index.c | 2 +\n> t/perf/p2000-sparse-operations.sh | 47 ++++--\n> t/t1092-sparse-checkout-compatibility.sh | 197 ++++++++++++++++++++++-\n> unpack-trees.c | 11 ++\n> 7 files changed, 240 insertions(+), 30 deletions(-)\n>\n>\n> base-commit: e5ca291076a8a936283bb2c57433c4393d3f80c2\n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-973%2Fderrickstolee%2Fsparse-index%2Fcommit-and-checkout-v2\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-973/derrickstolee/sparse-index/commit-and-checkout-v2\n> Pull-Request: https://github.com/gitgitgadget/git/pull/973\n>\n> Range-diff vs v1:\n>\n> 1: bb3dd1fdd48 = 1: 6e74958f590 p2000: add 'git checkout -' test and decrease depth\n> 2: eb15bf37685 = 2: 3e1d03c41be p2000: compress repo names\n> 3: 413babe6e77 ! 3: cd94f820052 commit: integrate with sparse-index\n> @@ t/t1092-sparse-checkout-compatibility.sh: test_expect_success 'sparse-index is e\n> + ensure_not_expanded commit --include deep/deeper1/a -m deeper\n> '\n>\n> - test_expect_success 'reset mixed and checkout orphan' '\n> + # NEEDSWORK: a sparse-checkout behaves differently from a full checkout\n> 4: ffe8473caab = 4: 65e79b8037c sparse-index: recompute cache-tree\n> 5: 8710fee36b7 ! 5: e9a9981477e checkout: stop expanding sparse indexes\n> @@ t/t1092-sparse-checkout-compatibility.sh: test_expect_success 'sparse-index is n\n> + ensure_not_expanded restore -s rename-out-to-out -- deep/deeper1\n> '\n>\n> - test_expect_success 'reset mixed and checkout orphan' '\n> + # NEEDSWORK: a sparse-checkout behaves differently from a full checkout\n> -: ----------- > 6: 4b801c854fb t1092: document bad 'git checkout' behavior\n> -: ----------- > 7: 71e301501c8 unpack-trees: resolve sparse-directory/file conflicts\n>\n> --\n> gitgitgadget\n"}]}