{"thread":{"id":"55329","subject":"[PATCH 2/2] t6423: mark remaining expected failure under merge-ort as such","startedAt":"2021-03-16T04:06:28Z","lastAt":"2021-03-20T01:50:37Z","messageCount":40,"participants":["Elijah Newren via GitGitGadget","Derrick Stolee","Elijah Newren"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"419266","messageId":"0d41038fad91f8e61eaff87884dd45e5c7d9275a.1615867503.git.gitgitgadget@gmail.com","threadId":"55329","inReplyTo":"pull.905.git.1615867503.gitgitgadget@gmail.com","subject":"[PATCH 2/2] t6423: mark remaining expected failure under merge-ort as such","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-03-16T04:05:02Z","receivedAt":"2021-03-16T04:06:28Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nWhen we started on merge-ort, thousands of tests failed when run with\nthe GIT_TEST_MERGE_ALGORITHM=ort flag; with so many, it didn't make\nsense to flip all their test expectations. The ones in t6409, t6418,\nand the submodule tests are being handled by an independent in-flight\ntopic (\"Complete merge-ort implemenation...almost\"). The ones in\nt6423 were left out of the other series because other ongoing series\nthat this commit depends upon were addressing those. Now that we only\nhave one remaining test failure in t6423, let's mark it as such.\n\nThis remaining test will be fixed by a future optimization series, but\nsince merge-recursive doesn't pass this test either, passing it is not\nnecessary for declaring merge-ort ready for general use.\n\nSigned-off-by: Elijah Newren \n---\n t/t6423-merge-rename-directories.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t6423-merge-rename-directories.sh b/t/t6423-merge-rename-directories.sh\nindex 4c568050dd27..4c3d0b95dc5c 100755\n--- a/t/t6423-merge-rename-directories.sh\n+++ b/t/t6423-merge-rename-directories.sh\n@@ -4797,7 +4797,7 @@ test_setup_12f () {\n \t)\n }\n \n-test_expect_merge_algorithm failure success '12f: Trivial directory resolve, caching, all kinds of fun' '\n+test_expect_merge_algorithm failure failure '12f: Trivial directory resolve, caching, all kinds of fun' '\n \ttest_setup_12f &&\n \t(\n \t\tcd 12f &&\n-- \ngitgitgadget\n"},{"id":"419267","messageId":"pull.905.git.1615867503.gitgitgadget@gmail.com","threadId":"55329","inReplyTo":null,"subject":"[PATCH 0/2] Declare merge-ort ready for general usage","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-03-16T04:05:00Z","receivedAt":"2021-03-16T04:06:28Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"This tiny series depends on ort-perf-batch-10[1].\n\nIf the ort-remainder topic[2] is merged with this series, then the result is\na version of merge-ort ready for general usage. Users can select it by (a)\npassing -sort to either git merge or git rebase, or (b) by setting\npull.twohead=ort [3], or (c) by setting GIT_TEST_MERGE_ALGORITHM=ort.\n\nThere are still more optimizations that will be submitted for merge-ort, but\nfolks don't need to wait for all of them to start using it. The version of\nmerge-ort that comes from merging both these topics will run the testsuite\ncleanly under GIT_MERGE_TEST_ALGORITHM=ort, which includes passing every\ntest that merge-recursive does and passing dozens of tests that\nmerge-recursive doesn't. merge-ort is also already significantly faster than\nmerge-recursive when rename detection is involved.\n\nI'll send out a separate email later requesting feedback about what people\nwould like to see before switching the default from merge-recursive to\nmerge-ort.\n\n[1]\nhttps://lore.kernel.org/git/pull.853.git.1615674128.gitgitgadget@gmail.com/\n[2]\nhttps://lore.kernel.org/git/pull.973.v2.git.git.1615271086.gitgitgadget@gmail.com/\n[3] See commit 14c4586c2d (\"merge,rebase,revert: select ort or recursive by\nconfig or environment\", 2020-11-02)\n\nElijah Newren (2):\n Revert \"merge-ort: ignore the directory rename split conflict for now\"\n t6423: mark remaining expected failure under merge-ort as such\n\n merge-ort.c | 13 +------------\n t/t6423-merge-rename-directories.sh | 2 +-\n 2 files changed, 2 insertions(+), 13 deletions(-)\n\n\nbase-commit: ac0ba91ce275227f5df8f16fb986308ff88b198b\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-905%2Fgitgitgadget%2Fort-readiness-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-905/gitgitgadget/ort-readiness-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/905\n-- \ngitgitgadget\n"},{"id":"419268","messageId":"7a8e26638a16edcd2317a28a3cc106321ebda5eb.1615867503.git.gitgitgadget@gmail.com","threadId":"55329","inReplyTo":"pull.905.git.1615867503.gitgitgadget@gmail.com","subject":"[PATCH 1/2] Revert \"merge-ort: ignore the directory rename split conflict for now\"","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-03-16T04:05:01Z","receivedAt":"2021-03-16T04:06:28Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nThis reverts commit 5ced7c3da009090c5a926e3123a71314c7f28d42, which was\nput in place as a temporary measure to avoid optimizations unstably\nerroring out on no destination having a majority of the necessary\nrenames for directories that had no new files and thus no need for\ndirectory rename detection anyway. Now that optimizations are in place\nto prevent us from trying to compute directory rename count computations\nfor directories that do not need it, we can undo this temporary measure.\n\nSigned-off-by: Elijah Newren \n---\n merge-ort.c | 13 +------------\n 1 file changed, 1 insertion(+), 12 deletions(-)\n\ndiff --git a/merge-ort.c b/merge-ort.c\nindex 7f5750ce6ab0..60270feadc54 100644\n--- a/merge-ort.c\n+++ b/merge-ort.c\n@@ -1562,18 +1562,7 @@ static void get_provisional_directory_renames(struct merge_options *opt,\n \t\t\t\t \"no destination getting a majority of the \"\n \t\t\t\t \"files.\"),\n \t\t\t source_dir);\n-\t\t\t/*\n-\t\t\t * We should mark this as unclean IF something attempts\n-\t\t\t * to use this rename. We do not yet have the logic\n-\t\t\t * in place to detect if this directory rename is being\n-\t\t\t * used, and optimizations that reduce the number of\n-\t\t\t * renames cause this to falsely trigger. For now,\n-\t\t\t * just disable it, causing t6423 testcase 2a to break.\n-\t\t\t * We'll later fix the detection, and when we do we\n-\t\t\t * will re-enable setting *clean to 0 (and thereby fix\n-\t\t\t * t6423 testcase 2a).\n-\t\t\t */\n-\t\t\t/* *clean = 0; */\n+\t\t\t*clean = 0;\n \t\t} else {\n \t\t\tstrmap_put(&renames->dir_renames[side],\n \t\t\t\t source_dir, (void*)best);\n-- \ngitgitgadget\n\n"},{"id":"419396","messageId":"44487d2a-f591-7e0f-8532-41397ec805c9@gmail.com","threadId":"55329","inReplyTo":"pull.905.git.1615867503.gitgitgadget@gmail.com","subject":"Re: [PATCH 0/2] Declare merge-ort ready for general usage","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2021-03-16T17:01:50Z","receivedAt":"2021-03-16T17:02:38Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 3/16/2021 12:05 AM, Elijah Newren via GitGitGadget wrote:\n> This tiny series depends on ort-perf-batch-10[1].\n> \n> If the ort-remainder topic[2] is merged with this series, then the result is\n> a version of merge-ort ready for general usage. Users can select it by (a)\n> passing -sort to either git merge or git rebase, or (b) by setting\n> pull.twohead=ort [3], or (c) by setting GIT_TEST_MERGE_ALGORITHM=ort.\n\nDoes the other topic add GIT_TEST_MERGE_ALGORITHM=ort to the CI builds?\n\nSpecifically, the Linux builds have a second run with some optional\nGIT_TEST_* environment variables. This seems like a nice addition.\n Other than that extra request, this series was easy to review. LGTM.\n-Stolee\n"},{"id":"419397","messageId":"CABPp-BEP5dJpT3q9Lx_Kk5qqSq+zN2F=13nTuTg7UpLCeFKSWw@mail.gmail.com","threadId":"55329","inReplyTo":"44487d2a-f591-7e0f-8532-41397ec805c9@gmail.com","subject":"Re: [PATCH 0/2] Declare merge-ort ready for general usage","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2021-03-16T17:25:21Z","receivedAt":"2021-03-16T17:26:38Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Tue, Mar 16, 2021 at 10:01 AM Derrick Stolee wrote:\n>\n> On 3/16/2021 12:05 AM, Elijah Newren via GitGitGadget wrote:\n> > This tiny series depends on ort-perf-batch-10[1].\n> >\n> > If the ort-remainder topic[2] is merged with this series, then the result is\n> > a version of merge-ort ready for general usage. Users can select it by (a)\n> > passing -sort to either git merge or git rebase, or (b) by setting\n> > pull.twohead=ort [3], or (c) by setting GIT_TEST_MERGE_ALGORITHM=ort.\n>\n> Does the other topic add GIT_TEST_MERGE_ALGORITHM=ort to the CI builds?\n>\n> Specifically, the Linux builds have a second run with some optional\n> GIT_TEST_* environment variables. This seems like a nice addition.\n> Other than that extra request, this series was easy to review. LGTM.\n\nThe other topic left tests in t6423 failing. This topic leaves the\ntests in t6409 and t6418 failing; it's only the merge of the two that\nhas all passing tests.\n\nI guess since the ort-remainder topic still hasn't been picked up, I\ncould just combine it with this series (basing on ort-perf-batch-10).\nThen I could either add a patch at the end of the series that runs\ntests under GIT_TEST_MERGE_ALGORITHM=ort, or which changes the default\nmerge backend to ort.\n"},{"id":"419399","messageId":"0d71ae08-68e9-3dd9-b462-1ba888ea94b0@gmail.com","threadId":"55329","inReplyTo":"CABPp-BEP5dJpT3q9Lx_Kk5qqSq+zN2F=13nTuTg7UpLCeFKSWw@mail.gmail.com","subject":"Re: [PATCH 0/2] Declare merge-ort ready for general usage","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2021-03-16T17:33:06Z","receivedAt":"2021-03-16T17:33:52Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 3/16/2021 1:25 PM, Elijah Newren wrote:\n> On Tue, Mar 16, 2021 at 10:01 AM Derrick Stolee wrote:\n>>\n>> On 3/16/2021 12:05 AM, Elijah Newren via GitGitGadget wrote:\n>>> This tiny series depends on ort-perf-batch-10[1].\n>>>\n>>> If the ort-remainder topic[2] is merged with this series, then the result is\n>>> a version of merge-ort ready for general usage. Users can select it by (a)\n>>> passing -sort to either git merge or git rebase, or (b) by setting\n>>> pull.twohead=ort [3], or (c) by setting GIT_TEST_MERGE_ALGORITHM=ort.\n>>\n>> Does the other topic add GIT_TEST_MERGE_ALGORITHM=ort to the CI builds?\n>>\n>> Specifically, the Linux builds have a second run with some optional\n>> GIT_TEST_* environment variables. This seems like a nice addition.\n>> Other than that extra request, this series was easy to review. LGTM.\n> \n> The other topic left tests in t6423 failing. This topic leaves the\n> tests in t6409 and t6418 failing; it's only the merge of the two that\n> has all passing tests.\n\nCombining the series might be the right call, or say this one depends\non that one.\n \n> I guess since the ort-remainder topic still hasn't been picked up, I\n> could just combine it with this series (basing on ort-perf-batch-10).\n> Then I could either add a patch at the end of the series that runs\n> tests under GIT_TEST_MERGE_ALGORITHM=ort, or which changes the default\n> merge backend to ort. \n\nPerhaps change the backend to \"ort\" only when \"features.experimental\"\nis enabled, at least for one full release? I'll do my part to do some\ntesting with our repos in that time, too.\n\nThanks,\n-Stolee\n"},{"id":"419584","messageId":"6d34cc466bd53e074c690b58ef03391eac10e5d9.1616016485.git.gitgitgadget@gmail.com","threadId":"55329","inReplyTo":"pull.905.v2.git.1616016485.gitgitgadget@gmail.com","subject":"[PATCH v2 02/13] merge-ort: add a special minimal index just for renormalization","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-03-17T21:27:54Z","receivedAt":"2021-03-17T21:28:56Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nrenormalize_buffer() requires an index_state, which is something that\nmerge-ort does not operate with. However, all the renormalization code\nneeds is an index with a .gitattributes file...plus a little bit of\nsetup. Create such an index, along with the deallocation and\nattr_direction handling.\n\nA subsequent commit will add a function to finish the initialization\nof this index.\n\nSigned-off-by: Elijah Newren \n---\n merge-ort.c | 20 ++++++++++++++++++++\n 1 file changed, 20 insertions(+)\n\ndiff --git a/merge-ort.c b/merge-ort.c\nindex 34a91c435737..3c606fa7e4b3 100644\n--- a/merge-ort.c\n+++ b/merge-ort.c\n@@ -18,6 +18,7 @@\n #include \"merge-ort.h\"\n \n #include \"alloc.h\"\n+#include \"attr.h\"\n #include \"blob.h\"\n #include \"cache-tree.h\"\n #include \"commit.h\"\n@@ -220,6 +221,16 @@ struct merge_options_internal {\n \t */\n \tstruct rename_info renames;\n \n+\t/*\n+\t * attr_index: hacky minimal index used for renormalization\n+\t *\n+\t * renormalization code _requires_ an index, though it only needs to\n+\t * find a .gitattributes file within the index. So, when\n+\t * renormalization is important, we create a special index with just\n+\t * that one file.\n+\t */\n+\tstruct index_state attr_index;\n+\n \t/*\n \t * current_dir_name, toplevel_dir: temporary vars\n \t *\n@@ -399,6 +410,9 @@ static void clear_or_reinit_internal_opts(struct merge_options_internal *opti,\n \tstring_list_clear(&opti->paths_to_free, 0);\n \topti->paths_to_free.strdup_strings = 0;\n \n+\tif (opti->attr_index.cache_nr)\n+\t\tdiscard_index(&opti->attr_index);\n+\n \t/* Free memory used by various renames maps */\n \tfor (i = MERGE_SIDE1; i <= MERGE_SIDE2; ++i) {\n \t\tstrintmap_func(&renames->dirs_removed[i]);\n@@ -3407,6 +3421,8 @@ void merge_finalize(struct merge_options *opt,\n {\n \tstruct merge_options_internal *opti = result->priv;\n \n+\tif (opt->renormalize)\n+\t\tgit_attr_set_direction(GIT_ATTR_CHECKIN);\n \tassert(opt->priv == NULL);\n \n \tclear_or_reinit_internal_opts(opti, 0);\n@@ -3482,6 +3498,10 @@ static void merge_start(struct merge_options *opt, struct merge_result *result)\n \t/* Default to histogram diff. Actually, just hardcode it...for now. */\n \topt->xdl_opts = DIFF_WITH_ALG(opt, HISTOGRAM_DIFF);\n \n+\t/* Handle attr direction stuff for renormalization */\n+\tif (opt->renormalize)\n+\t\tgit_attr_set_direction(GIT_ATTR_CHECKOUT);\n+\n \t/* Initialization of opt->priv, our internal merge data */\n \ttrace2_region_enter(\"merge\", \"allocate/init\", opt->repo);\n \tif (opt->priv) {\n-- \ngitgitgadget\n\n"},{"id":"419585","messageId":"41fffcdd3b786c460c3a0652108212d8c361159f.1616016485.git.gitgitgadget@gmail.com","threadId":"55329","inReplyTo":"pull.905.v2.git.1616016485.gitgitgadget@gmail.com","subject":"[PATCH v2 05/13] merge-ort: support subtree shifting","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-03-17T21:27:57Z","receivedAt":"2021-03-17T21:28:57Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nmerge-recursive has some simple code to support subtree shifting; copy\nit over to merge-ort. This fixes t6409.12 under\nGIT_TEST_MERGE_ALGORITHM=ort.\n\nSigned-off-by: Elijah Newren \n---\n merge-ort.c | 24 ++++++++++++++++++++++++\n 1 file changed, 24 insertions(+)\n\ndiff --git a/merge-ort.c b/merge-ort.c\nindex c7083e3769aa..c4fe234d8972 100644\n--- a/merge-ort.c\n+++ b/merge-ort.c\n@@ -3551,6 +3551,23 @@ void merge_finalize(struct merge_options *opt,\n \n /*** Function Grouping: helper functions for merge_incore_*() ***/\n \n+static struct tree *shift_tree_object(struct repository *repo,\n+\t\t\t\t struct tree *one, struct tree *two,\n+\t\t\t\t const char *subtree_shift)\n+{\n+\tstruct object_id shifted;\n+\n+\tif (!*subtree_shift) {\n+\t\tshift_tree(repo, &one->object.oid, &two->object.oid, &shifted, 0);\n+\t} else {\n+\t\tshift_tree_by(repo, &one->object.oid, &two->object.oid, &shifted,\n+\t\t\t subtree_shift);\n+\t}\n+\tif (oideq(&two->object.oid, &shifted))\n+\t\treturn two;\n+\treturn lookup_tree(repo, &shifted);\n+}\n+\n static inline void set_commit_tree(struct commit *c, struct tree *t)\n {\n \tc->maybe_tree = t;\n@@ -3680,6 +3697,13 @@ static void merge_ort_nonrecursive_internal(struct merge_options *opt,\n {\n \tstruct object_id working_tree_oid;\n \n+\tif (opt->subtree_shift) {\n+\t\tside2 = shift_tree_object(opt->repo, side1, side2,\n+\t\t\t\t\t opt->subtree_shift);\n+\t\tmerge_base = shift_tree_object(opt->repo, side1, merge_base,\n+\t\t\t\t\t opt->subtree_shift);\n+\t}\n+\n \ttrace2_region_enter(\"merge\", \"collect_merge_info\", opt->repo);\n \tif (collect_merge_info(opt, merge_base, side1, side2) != 0) {\n \t\t/*\n-- \ngitgitgadget\n\n"},{"id":"419587","messageId":"e223f842748c797a3df967321de7650aba3a9801.1616016485.git.gitgitgadget@gmail.com","threadId":"55329","inReplyTo":"pull.905.v2.git.1616016485.gitgitgadget@gmail.com","subject":"[PATCH v2 01/13] merge-ort: use STABLE_QSORT instead of QSORT where required","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-03-17T21:27:53Z","receivedAt":"2021-03-17T21:28:57Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nrename/rename conflict handling depends on the fact that if both sides\nrenamed the same path, that the one on the MERGE_SIDE1 will appear first\nin the combined diff_queue_struct passed to process_renames(). Since we\nadd all pairs from MERGE_SIDE1 to combined first, and then all pairs\nfrom MERGE_SIDE2, and then sort based on filename, this will only be\ntrue if the sort used is stable. This was found due to the fact that\nMac, unlike Linux, apparently has a system-defined qsort that is not\nstable.\n\nWhile we are at it, review the other callers of QSORT and add comments\nabout why they can remain as calls to QSORT instead of being modified\nto call STABLE_QSORT.\n\nSigned-off-by: Elijah Newren \n---\n merge-ort.c | 8 +++++++-\n 1 file changed, 7 insertions(+), 1 deletion(-)\n\ndiff --git a/merge-ort.c b/merge-ort.c\nindex 7f5750ce6ab0..34a91c435737 100644\n--- a/merge-ort.c\n+++ b/merge-ort.c\n@@ -2405,7 +2405,7 @@ static int detect_and_process_renames(struct merge_options *opt,\n \tclean &= collect_renames(opt, &combined, MERGE_SIDE2,\n \t\t\t\t &renames->dir_renames[1],\n \t\t\t\t &renames->dir_renames[2]);\n-\tQSORT(combined.queue, combined.nr, compare_pairs);\n+\tSTABLE_QSORT(combined.queue, combined.nr, compare_pairs);\n \ttrace2_region_leave(\"merge\", \"directory renames\", opt->repo);\n \n \ttrace2_region_enter(\"merge\", \"process renames\", opt->repo);\n@@ -2550,6 +2550,7 @@ static void write_tree(struct object_id *result_oid,\n \t */\n \trelevant_entries.items = versions->items + offset;\n \trelevant_entries.nr = versions->nr - offset;\n+\t/* No need for STABLE_QSORT -- filenames must be unique */\n \tQSORT(relevant_entries.items, relevant_entries.nr, tree_entry_order);\n \n \t/* Pre-allocate some space in buf */\n@@ -3325,6 +3326,11 @@ static int record_conflicted_index_entries(struct merge_options *opt,\n \t * entries we added to the end into their right locations.\n \t */\n \tremove_marked_cache_entries(index, 1);\n+\t/*\n+\t * No need for STABLE_QSORT -- cmp_cache_name_compare sorts primarily\n+\t * on filename and secondarily on stage, and (name, stage #) are a\n+\t * unique tuple.\n+\t */\n \tQSORT(index->cache, index->cache_nr, cmp_cache_name_compare);\n \n \treturn errs;\n-- \ngitgitgadget\n\n"},{"id":"419589","messageId":"pull.905.v2.git.1616016485.gitgitgadget@gmail.com","threadId":"55329","inReplyTo":"pull.905.git.1615867503.gitgitgadget@gmail.com","subject":"[PATCH v2 00/13] Declare merge-ort ready for general usage","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-03-17T21:27:52Z","receivedAt":"2021-03-17T21:28:57Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"This series depends on ort-perf-batch-10[1], and obsoletes the ort-remainder\ntopic[2] (that hadn't been picked up yet, so hopefully this doesn't cause\nany confusion)\n\nWith this series, merge-ort is ready for general usage -- it passes all\ntests, passes dozens of tests that don't under merge-recursive, and\nmerge-ort is is already significantly faster than merge-recursive when\nrename detection is involved. Users can select merge-ort by (a) passing\n-sort to either git merge or git rebase, or (b) by setting pull.twohead=ort\n[3], or (c) by setting GIT_TEST_MERGE_ALGORITHM=ort.\n\nChanges since v1:\n\n * subsumed the ort-remainder topic (the first 10 patches), which has\n already been reviewed by both Ævar and Stolee[2].\n * the next two patches were the original v1, reviewed by Stolee\n * the final patch is new and adds testing.\n\n[1]\nhttps://lore.kernel.org/git/pull.853.git.1615674128.gitgitgadget@gmail.com/\n[2]\nhttps://lore.kernel.org/git/pull.973.v2.git.git.1615271086.gitgitgadget@gmail.com/\n[3] See commit 14c4586c2d (\"merge,rebase,revert: select ort or recursive by\nconfig or environment\", 2020-11-02)\n\nElijah Newren (13):\n merge-ort: use STABLE_QSORT instead of QSORT where required\n merge-ort: add a special minimal index just for renormalization\n merge-ort: have ll_merge() use a special attr_index for\n renormalization\n merge-ort: let renormalization change modify/delete into clean delete\n merge-ort: support subtree shifting\n t6428: new test for SKIP_WORKTREE handling and conflicts\n merge-ort: implement CE_SKIP_WORKTREE handling with conflicted entries\n t: mark several submodule merging tests as fixed under merge-ort\n merge-ort: write $GIT_DIR/AUTO_MERGE whenever we hit a conflict\n merge-recursive: add a bunch of FIXME comments documenting known bugs\n Revert \"merge-ort: ignore the directory rename split conflict for now\"\n t6423: mark remaining expected failure under merge-ort as such\n Add testing with merge-ort merge strategy\n\n .github/workflows/main.yml | 1 +\n branch.c | 1 +\n builtin/rebase.c | 1 +\n ci/lib.sh | 6 +\n merge-ort.c | 242 ++++++++++++++++--\n merge-recursive.c | 37 +++\n path.c | 1 +\n path.h | 2 +\n sequencer.c | 5 +\n t/t3512-cherry-pick-submodule.sh | 7 +-\n t/t3513-revert-submodule.sh | 5 +-\n t/t5572-pull-submodule.sh | 7 +-\n t/t6423-merge-rename-directories.sh | 2 +-\n t/t6428-merge-conflicts-sparse.sh | 158 ++++++++++++\n t/t6437-submodule-merge.sh | 5 +-\n t/t6438-submodule-directory-file-conflicts.sh | 7 +-\n 16 files changed, 449 insertions(+), 38 deletions(-)\n create mode 100755 t/t6428-merge-conflicts-sparse.sh\n\n\nbase-commit: ac0ba91ce275227f5df8f16fb986308ff88b198b\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-905%2Fgitgitgadget%2Fort-readiness-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-905/gitgitgadget/ort-readiness-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/905\n\nRange-diff vs v1:\n\n -: ------------ > 1: e223f842748c merge-ort: use STABLE_QSORT instead of QSORT where required\n -: ------------ > 2: 6d34cc466bd5 merge-ort: add a special minimal index just for renormalization\n -: ------------ > 3: 4ff23d2f52a0 merge-ort: have ll_merge() use a special attr_index for renormalization\n -: ------------ > 4: c1c9605c1932 merge-ort: let renormalization change modify/delete into clean delete\n -: ------------ > 5: 41fffcdd3b78 merge-ort: support subtree shifting\n -: ------------ > 6: 6aec1f499b80 t6428: new test for SKIP_WORKTREE handling and conflicts\n -: ------------ > 7: fe3baf696785 merge-ort: implement CE_SKIP_WORKTREE handling with conflicted entries\n -: ------------ > 8: f9325647a9fc t: mark several submodule merging tests as fixed under merge-ort\n -: ------------ > 9: 4a79e6134691 merge-ort: write $GIT_DIR/AUTO_MERGE whenever we hit a conflict\n -: ------------ > 10: a37979454069 merge-recursive: add a bunch of FIXME comments documenting known bugs\n 1: 7a8e26638a16 = 11: 6bda855f2980 Revert \"merge-ort: ignore the directory rename split conflict for now\"\n 2: 0d41038fad91 = 12: 1c6361c9b88a t6423: mark remaining expected failure under merge-ort as such\n -: ------------ > 13: d8536f56ab29 Add testing with merge-ort merge strategy\n\n-- \ngitgitgadget\n"},{"id":"419588","messageId":"4ff23d2f52a0abd41d81076f9dfa3a93319deca5.1616016485.git.gitgitgadget@gmail.com","threadId":"55329","inReplyTo":"pull.905.v2.git.1616016485.gitgitgadget@gmail.com","subject":"[PATCH v2 03/13] merge-ort: have ll_merge() use a special attr_index for renormalization","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-03-17T21:27:55Z","receivedAt":"2021-03-17T21:28:58Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nll_merge() needs an index when renormalization is requested. Create one\nspecifically for just this purpose with just the one needed entry. This\nfixes t6418.4 and t6418.5 under GIT_TEST_MERGE_ALGORITHM=ort.\n\nNOTE 1: Even if the user has a working copy or a real index (which is\nnot a given as merge-ort can be used in bare repositories), we\nexplicitly ignore any .gitattributes file from either of these\nlocations. merge-ort can be used to merge two branches that are\nunrelated to HEAD, so .gitattributes from the working copy and current\nindex should not be considered relevant.\n\nNOTE 2: Since we are in the middle of merging, there is a risk that\n.gitattributes itself is conflicted...leaving us with an ill-defined\nsituation about how to perform the rest of the merge. It could be that\nthe .gitattributes file does not even exist on one of the sides of the\nmerge, or that it has been modified on both sides. If it's been\nmodified on both sides, it's possible that it could itself be merged\ncleanly, though it's also possible that it only merges cleanly if you\nuse the right version of the .gitattributes file to drive the merge. It\ngets kind of complicated. The only test we ever had that attempted to\ntest behavior in this area was seemingly unaware of the undefined\nbehavior, but knew the test wouldn't work for lack of attribute handling\nsupport, marked it as test_expect_failure from the beginning, but\nmanaged to fail for several reasons unrelated to attribute handling.\nSee commit 6f6e7cfb52 (\"t6038: remove problematic test\", 2020-08-03) for\ndetails. So there are probably various ways to improve what\ninitialize_attr_index() picks in the case of a conflicted .gitattributes\nbut for now I just implemented something simple -- look for whatever\n.gitattributes file we can find in any of the higher order stages and\nuse it.\n\nSigned-off-by: Elijah Newren \n---\n merge-ort.c | 64 +++++++++++++++++++++++++++++++++++++++++++++++++++--\n 1 file changed, 62 insertions(+), 2 deletions(-)\n\ndiff --git a/merge-ort.c b/merge-ort.c\nindex 3c606fa7e4b3..cdc1e2fe7a24 100644\n--- a/merge-ort.c\n+++ b/merge-ort.c\n@@ -410,7 +410,7 @@ static void clear_or_reinit_internal_opts(struct merge_options_internal *opti,\n \tstring_list_clear(&opti->paths_to_free, 0);\n \topti->paths_to_free.strdup_strings = 0;\n \n-\tif (opti->attr_index.cache_nr)\n+\tif (opti->attr_index.cache_nr) /* true iff opt->renormalize */\n \t\tdiscard_index(&opti->attr_index);\n \n \t/* Free memory used by various renames maps */\n@@ -1201,6 +1201,63 @@ static int merge_submodule(struct merge_options *opt,\n \treturn 0;\n }\n \n+static void initialize_attr_index(struct merge_options *opt)\n+{\n+\t/*\n+\t * The renormalize_buffer() functions require attributes, and\n+\t * annoyingly those can only be read from the working tree or from\n+\t * an index_state. merge-ort doesn't have an index_state, so we\n+\t * generate a fake one containing only attribute information.\n+\t */\n+\tstruct merged_info *mi;\n+\tstruct index_state *attr_index = &opt->priv->attr_index;\n+\tstruct cache_entry *ce;\n+\n+\tattr_index->initialized = 1;\n+\n+\tif (!opt->renormalize)\n+\t\treturn;\n+\n+\tmi = strmap_get(&opt->priv->paths, GITATTRIBUTES_FILE);\n+\tif (!mi)\n+\t\treturn;\n+\n+\tif (mi->clean) {\n+\t\tint len = strlen(GITATTRIBUTES_FILE);\n+\t\tce = make_empty_cache_entry(attr_index, len);\n+\t\tce->ce_mode = create_ce_mode(mi->result.mode);\n+\t\tce->ce_flags = create_ce_flags(0);\n+\t\tce->ce_namelen = len;\n+\t\toidcpy(&ce->oid, &mi->result.oid);\n+\t\tmemcpy(ce->name, GITATTRIBUTES_FILE, len);\n+\t\tadd_index_entry(attr_index, ce,\n+\t\t\t\tADD_CACHE_OK_TO_ADD | ADD_CACHE_OK_TO_REPLACE);\n+\t\tget_stream_filter(attr_index, GITATTRIBUTES_FILE, &ce->oid);\n+\t} else {\n+\t\tint stage, len;\n+\t\tstruct conflict_info *ci;\n+\n+\t\tASSIGN_AND_VERIFY_CI(ci, mi);\n+\t\tfor (stage = 0; stage < 3; stage++) {\n+\t\t\tunsigned stage_mask = (1 << stage);\n+\n+\t\t\tif (!(ci->filemask & stage_mask))\n+\t\t\t\tcontinue;\n+\t\t\tlen = strlen(GITATTRIBUTES_FILE);\n+\t\t\tce = make_empty_cache_entry(attr_index, len);\n+\t\t\tce->ce_mode = create_ce_mode(ci->stages[stage].mode);\n+\t\t\tce->ce_flags = create_ce_flags(stage);\n+\t\t\tce->ce_namelen = len;\n+\t\t\toidcpy(&ce->oid, &ci->stages[stage].oid);\n+\t\t\tmemcpy(ce->name, GITATTRIBUTES_FILE, len);\n+\t\t\tadd_index_entry(attr_index, ce,\n+\t\t\t\t\tADD_CACHE_OK_TO_ADD | ADD_CACHE_OK_TO_REPLACE);\n+\t\t\tget_stream_filter(attr_index, GITATTRIBUTES_FILE,\n+\t\t\t\t\t &ce->oid);\n+\t\t}\n+\t}\n+}\n+\n static int merge_3way(struct merge_options *opt,\n \t\t const char *path,\n \t\t const struct object_id *o,\n@@ -1215,6 +1272,9 @@ static int merge_3way(struct merge_options *opt,\n \tchar *base, *name1, *name2;\n \tint merge_status;\n \n+\tif (!opt->priv->attr_index.initialized)\n+\t\tinitialize_attr_index(opt);\n+\n \tll_opts.renormalize = opt->renormalize;\n \tll_opts.extra_marker_size = extra_marker_size;\n \tll_opts.xdl_opts = opt->xdl_opts;\n@@ -1253,7 +1313,7 @@ static int merge_3way(struct merge_options *opt,\n \n \tmerge_status = ll_merge(result_buf, path, &orig, base,\n \t\t\t\t&src1, name1, &src2, name2,\n-\t\t\t\topt->repo->index, &ll_opts);\n+\t\t\t\t&opt->priv->attr_index, &ll_opts);\n \n \tfree(base);\n \tfree(name1);\n-- \ngitgitgadget\n\n"},{"id":"419590","messageId":"fe3baf696785c4b7bee8ae93e4567c2ee2a03492.1616016485.git.gitgitgadget@gmail.com","threadId":"55329","inReplyTo":"pull.905.v2.git.1616016485.gitgitgadget@gmail.com","subject":"[PATCH v2 07/13] merge-ort: implement CE_SKIP_WORKTREE handling with conflicted entries","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-03-17T21:27:59Z","receivedAt":"2021-03-17T21:28:58Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nWhen merge conflicts occur in paths removed by a sparse-checkout, we\nneed to unsparsify those paths (clear the SKIP_WORKTREE bit), and write\nout the conflicted file to the working copy. In the very unlikely case\nthat someone manually put a file into the working copy at the location\nof the SKIP_WORKTREE file, we need to avoid overwriting whatever edits\nthey have made and move that file to a different location first.\n\nSigned-off-by: Elijah Newren \n---\n merge-ort.c | 43 +++++++++++++++++++++----------\n t/t6428-merge-conflicts-sparse.sh | 4 +--\n 2 files changed, 32 insertions(+), 15 deletions(-)\n\ndiff --git a/merge-ort.c b/merge-ort.c\nindex c4fe234d8972..303e89414274 100644\n--- a/merge-ort.c\n+++ b/merge-ort.c\n@@ -3369,23 +3369,27 @@ static int checkout(struct merge_options *opt,\n \treturn ret;\n }\n \n-static int record_conflicted_index_entries(struct merge_options *opt,\n-\t\t\t\t\t struct index_state *index,\n-\t\t\t\t\t struct strmap *paths,\n-\t\t\t\t\t struct strmap *conflicted)\n+static int record_conflicted_index_entries(struct merge_options *opt)\n {\n \tstruct hashmap_iter iter;\n \tstruct strmap_entry *e;\n+\tstruct index_state *index = opt->repo->index;\n+\tstruct checkout state = CHECKOUT_INIT;\n \tint errs = 0;\n \tint original_cache_nr;\n \n-\tif (strmap_empty(conflicted))\n+\tif (strmap_empty(&opt->priv->conflicted))\n \t\treturn 0;\n \n+\t/* If any entries have skip_worktree set, we'll have to check 'em out */\n+\tstate.force = 1;\n+\tstate.quiet = 1;\n+\tstate.refresh_cache = 1;\n+\tstate.istate = index;\n \toriginal_cache_nr = index->cache_nr;\n \n \t/* Put every entry from paths into plist, then sort */\n-\tstrmap_for_each_entry(conflicted, &iter, e) {\n+\tstrmap_for_each_entry(&opt->priv->conflicted, &iter, e) {\n \t\tconst char *path = e->key;\n \t\tstruct conflict_info *ci = e->value;\n \t\tint pos;\n@@ -3426,9 +3430,23 @@ static int record_conflicted_index_entries(struct merge_options *opt,\n \t\t\t * the higher order stages. Thus, we need override\n \t\t\t * the CE_SKIP_WORKTREE bit and manually write those\n \t\t\t * files to the working disk here.\n-\t\t\t *\n-\t\t\t * TODO: Implement this CE_SKIP_WORKTREE fixup.\n \t\t\t */\n+\t\t\tif (ce_skip_worktree(ce)) {\n+\t\t\t\tstruct stat st;\n+\n+\t\t\t\tif (!lstat(path, &st)) {\n+\t\t\t\t\tchar *new_name = unique_path(&opt->priv->paths,\n+\t\t\t\t\t\t\t\t path,\n+\t\t\t\t\t\t\t\t \"cruft\");\n+\n+\t\t\t\t\tpath_msg(opt, path, 1,\n+\t\t\t\t\t\t _(\"Note: %s not up to date and in way of checking out conflicted version; old copy renamed to %s\"),\n+\t\t\t\t\t\t path, new_name);\n+\t\t\t\t\terrs |= rename(path, new_name);\n+\t\t\t\t\tfree(new_name);\n+\t\t\t\t}\n+\t\t\t\terrs |= checkout_entry(ce, &state, NULL, NULL);\n+\t\t\t}\n \n \t\t\t/*\n \t\t\t * Mark this cache entry for removal and instead add\n@@ -3478,8 +3496,6 @@ void merge_switch_to_result(struct merge_options *opt,\n {\n \tassert(opt->priv == NULL);\n \tif (result->clean >= 0 && update_worktree_and_index) {\n-\t\tstruct merge_options_internal *opti = result->priv;\n-\n \t\ttrace2_region_enter(\"merge\", \"checkout\", opt->repo);\n \t\tif (checkout(opt, head, result->tree)) {\n \t\t\t/* failure to function */\n@@ -3489,13 +3505,14 @@ void merge_switch_to_result(struct merge_options *opt,\n \t\ttrace2_region_leave(\"merge\", \"checkout\", opt->repo);\n \n \t\ttrace2_region_enter(\"merge\", \"record_conflicted\", opt->repo);\n-\t\tif (record_conflicted_index_entries(opt, opt->repo->index,\n-\t\t\t\t\t\t &opti->paths,\n-\t\t\t\t\t\t &opti->conflicted)) {\n+\t\topt->priv = result->priv;\n+\t\tif (record_conflicted_index_entries(opt)) {\n \t\t\t/* failure to function */\n+\t\t\topt->priv = NULL;\n \t\t\tresult->clean = -1;\n \t\t\treturn;\n \t\t}\n+\t\topt->priv = NULL;\n \t\ttrace2_region_leave(\"merge\", \"record_conflicted\", opt->repo);\n \t}\n \ndiff --git a/t/t6428-merge-conflicts-sparse.sh b/t/t6428-merge-conflicts-sparse.sh\nindex 1bb52ff6f38c..7e8bf497f821 100755\n--- a/t/t6428-merge-conflicts-sparse.sh\n+++ b/t/t6428-merge-conflicts-sparse.sh\n@@ -76,7 +76,7 @@ test_setup_numerals () {\n \t)\n }\n \n-test_expect_merge_algorithm success failure 'conflicting entries written to worktree even if sparse' '\n+test_expect_success 'conflicting entries written to worktree even if sparse' '\n \ttest_setup_numerals plain &&\n \t(\n \t\tcd numerals_plain &&\n@@ -112,7 +112,7 @@ test_expect_merge_algorithm success failure 'conflicting entries written to work\n \t)\n '\n \n-test_expect_merge_algorithm failure failure 'present-despite-SKIP_WORKTREE handled reasonably' '\n+test_expect_merge_algorithm failure success 'present-despite-SKIP_WORKTREE handled reasonably' '\n \ttest_setup_numerals in_the_way &&\n \t(\n \t\tcd numerals_in_the_way &&\n-- \ngitgitgadget\n\n"},{"id":"419591","messageId":"f9325647a9fca1471fead860c18db86aba34eb54.1616016485.git.gitgitgadget@gmail.com","threadId":"55329","inReplyTo":"pull.905.v2.git.1616016485.gitgitgadget@gmail.com","subject":"[PATCH v2 08/13] t: mark several submodule merging tests as fixed under merge-ort","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-03-17T21:28:00Z","receivedAt":"2021-03-17T21:28:58Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nmerge-ort handles submodules (and directory/file conflicts in general)\ndifferently than merge-recursive does; it basically puts all the special\nhandling for different filetypes into one place in the codebase instead\nof needing special handling for different filetypes in many different\ncode paths. This one code path in merge-ort could perhaps use some work\nstill (there are still test_expect_failure cases in the testsuite), but\nit passes all the tests that merge-recursive does as well as 12\nadditional ones that merge-recursive fails. Mark those 12 tests as\ntest_expect_success under merge-ort.\n\nSigned-off-by: Elijah Newren \n---\n t/t3512-cherry-pick-submodule.sh | 7 +++++--\n t/t3513-revert-submodule.sh | 5 ++++-\n t/t5572-pull-submodule.sh | 7 +++++--\n t/t6437-submodule-merge.sh | 5 +++--\n t/t6438-submodule-directory-file-conflicts.sh | 7 +++++--\n 5 files changed, 22 insertions(+), 9 deletions(-)\n\ndiff --git a/t/t3512-cherry-pick-submodule.sh b/t/t3512-cherry-pick-submodule.sh\nindex 822f2d4bfbd5..c657840db33b 100755\n--- a/t/t3512-cherry-pick-submodule.sh\n+++ b/t/t3512-cherry-pick-submodule.sh\n@@ -8,8 +8,11 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n . ./test-lib.sh\n . \"$TEST_DIRECTORY\"/lib-submodule-update.sh\n \n-KNOWN_FAILURE_NOFF_MERGE_DOESNT_CREATE_EMPTY_SUBMODULE_DIR=1\n-KNOWN_FAILURE_NOFF_MERGE_ATTEMPTS_TO_MERGE_REMOVED_SUBMODULE_FILES=1\n+if test \"$GIT_TEST_MERGE_ALGORITHM\" != ort\n+then\n+\tKNOWN_FAILURE_NOFF_MERGE_DOESNT_CREATE_EMPTY_SUBMODULE_DIR=1\n+\tKNOWN_FAILURE_NOFF_MERGE_ATTEMPTS_TO_MERGE_REMOVED_SUBMODULE_FILES=1\n+fi\n test_submodule_switch \"cherry-pick\"\n \n test_expect_success 'unrelated submodule/file conflict is ignored' '\ndiff --git a/t/t3513-revert-submodule.sh b/t/t3513-revert-submodule.sh\nindex a759f12cbb1d..74cd96e58223 100755\n--- a/t/t3513-revert-submodule.sh\n+++ b/t/t3513-revert-submodule.sh\n@@ -30,7 +30,10 @@ git_revert () {\n \tgit revert HEAD\n }\n \n-KNOWN_FAILURE_NOFF_MERGE_DOESNT_CREATE_EMPTY_SUBMODULE_DIR=1\n+if test \"$GIT_TEST_MERGE_ALGORITHM\" != ort\n+then\n+\tKNOWN_FAILURE_NOFF_MERGE_DOESNT_CREATE_EMPTY_SUBMODULE_DIR=1\n+fi\n test_submodule_switch_func \"git_revert\"\n \n test_done\ndiff --git a/t/t5572-pull-submodule.sh b/t/t5572-pull-submodule.sh\nindex 29537f4798ef..4f92a116e1f0 100755\n--- a/t/t5572-pull-submodule.sh\n+++ b/t/t5572-pull-submodule.sh\n@@ -42,8 +42,11 @@ git_pull_noff () {\n \t$2 git pull --no-ff\n }\n \n-KNOWN_FAILURE_NOFF_MERGE_DOESNT_CREATE_EMPTY_SUBMODULE_DIR=1\n-KNOWN_FAILURE_NOFF_MERGE_ATTEMPTS_TO_MERGE_REMOVED_SUBMODULE_FILES=1\n+if test \"$GIT_TEST_MERGE_ALGORITHM\" != ort\n+then\n+\tKNOWN_FAILURE_NOFF_MERGE_DOESNT_CREATE_EMPTY_SUBMODULE_DIR=1\n+\tKNOWN_FAILURE_NOFF_MERGE_ATTEMPTS_TO_MERGE_REMOVED_SUBMODULE_FILES=1\n+fi\n test_submodule_switch_func \"git_pull_noff\"\n \n test_expect_success 'pull --recurse-submodule setup' '\ndiff --git a/t/t6437-submodule-merge.sh b/t/t6437-submodule-merge.sh\nindex 0f92bcf326c8..e5e89c2045e7 100755\n--- a/t/t6437-submodule-merge.sh\n+++ b/t/t6437-submodule-merge.sh\n@@ -6,6 +6,7 @@ GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n \n . ./test-lib.sh\n+. \"$TEST_DIRECTORY\"/lib-merge.sh\n \n #\n # history\n@@ -328,7 +329,7 @@ test_expect_success 'setup file/submodule conflict' '\n \t)\n '\n \n-test_expect_failure 'file/submodule conflict' '\n+test_expect_merge_algorithm failure success 'file/submodule conflict' '\n \ttest_when_finished \"git -C file-submodule reset --hard\" &&\n \t(\n \t\tcd file-submodule &&\n@@ -437,7 +438,7 @@ test_expect_failure 'directory/submodule conflict; keep submodule clean' '\n \t)\n '\n \n-test_expect_failure !FAIL_PREREQS 'directory/submodule conflict; should not treat submodule files as untracked or in the way' '\n+test_expect_merge_algorithm failure success !FAIL_PREREQS 'directory/submodule conflict; should not treat submodule files as untracked or in the way' '\n \ttest_when_finished \"git -C directory-submodule/path reset --hard\" &&\n \ttest_when_finished \"git -C directory-submodule reset --hard\" &&\n \t(\ndiff --git a/t/t6438-submodule-directory-file-conflicts.sh b/t/t6438-submodule-directory-file-conflicts.sh\nindex 04bf4be7d792..8df67a0ef99d 100755\n--- a/t/t6438-submodule-directory-file-conflicts.sh\n+++ b/t/t6438-submodule-directory-file-conflicts.sh\n@@ -12,8 +12,11 @@ test_submodule_switch \"merge --ff\"\n \n test_submodule_switch \"merge --ff-only\"\n \n-KNOWN_FAILURE_NOFF_MERGE_DOESNT_CREATE_EMPTY_SUBMODULE_DIR=1\n-KNOWN_FAILURE_NOFF_MERGE_ATTEMPTS_TO_MERGE_REMOVED_SUBMODULE_FILES=1\n+if test \"$GIT_TEST_MERGE_ALGORITHM\" != ort\n+then\n+\tKNOWN_FAILURE_NOFF_MERGE_DOESNT_CREATE_EMPTY_SUBMODULE_DIR=1\n+\tKNOWN_FAILURE_NOFF_MERGE_ATTEMPTS_TO_MERGE_REMOVED_SUBMODULE_FILES=1\n+fi\n test_submodule_switch \"merge --no-ff\"\n \n test_done\n-- \ngitgitgadget\n\n"},{"id":"419592","messageId":"6aec1f499b8068329b81fb1221717c2ee336fd8d.1616016485.git.gitgitgadget@gmail.com","threadId":"55329","inReplyTo":"pull.905.v2.git.1616016485.gitgitgadget@gmail.com","subject":"[PATCH v2 06/13] t6428: new test for SKIP_WORKTREE handling and conflicts","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-03-17T21:27:58Z","receivedAt":"2021-03-17T21:28:58Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nIf there is a conflict during a merge for a SKIP_WORKTREE entry, we\nexpect that file to be written to the working copy and have the\nSKIP_WORKTREE bit cleared in the index. If the user had manually\ncreated a file in the working tree despite SKIP_WORKTREE being set, we\ndo not want to clobber their changes to that file, but want to move it\nout of the way. Add tests that check for these behaviors.\n\nSigned-off-by: Elijah Newren \n---\n t/t6428-merge-conflicts-sparse.sh | 158 ++++++++++++++++++++++++++++++\n 1 file changed, 158 insertions(+)\n create mode 100755 t/t6428-merge-conflicts-sparse.sh\n\ndiff --git a/t/t6428-merge-conflicts-sparse.sh b/t/t6428-merge-conflicts-sparse.sh\nnew file mode 100755\nindex 000000000000..1bb52ff6f38c\n--- /dev/null\n+++ b/t/t6428-merge-conflicts-sparse.sh\n@@ -0,0 +1,158 @@\n+#!/bin/sh\n+\n+test_description=\"merge cases\"\n+\n+# The setup for all of them, pictorially, is:\n+#\n+# A\n+# o\n+# / \\\n+# O o ?\n+# \\ /\n+# o\n+# B\n+#\n+# To help make it easier to follow the flow of tests, they have been\n+# divided into sections and each test will start with a quick explanation\n+# of what commits O, A, and B contain.\n+#\n+# Notation:\n+# z/{b,c} means files z/b and z/c both exist\n+# x/d_1 means file x/d exists with content d1. (Purpose of the\n+# underscore notation is to differentiate different\n+# files that might be renamed into each other's paths.)\n+\n+. ./test-lib.sh\n+. \"$TEST_DIRECTORY\"/lib-merge.sh\n+\n+\n+# Testcase basic, conflicting changes in 'numerals'\n+\n+test_setup_numerals () {\n+\ttest_create_repo numerals_$1 &&\n+\t(\n+\t\tcd numerals_$1 &&\n+\n+\t\t>README &&\n+\t\ttest_write_lines I II III >numerals &&\n+\t\tgit add README numerals &&\n+\t\ttest_tick &&\n+\t\tgit commit -m \"O\" &&\n+\n+\t\tgit branch O &&\n+\t\tgit branch A &&\n+\t\tgit branch B &&\n+\n+\t\tgit checkout A &&\n+\t\ttest_write_lines I II III IIII >numerals &&\n+\t\tgit add numerals &&\n+\t\ttest_tick &&\n+\t\tgit commit -m \"A\" &&\n+\n+\t\tgit checkout B &&\n+\t\ttest_write_lines I II III IV >numerals &&\n+\t\tgit add numerals &&\n+\t\ttest_tick &&\n+\t\tgit commit -m \"B\" &&\n+\n+\t\tcat <<-EOF >expected-index &&\n+\t\tH README\n+\t\tM numerals\n+\t\tM numerals\n+\t\tM numerals\n+\t\tEOF\n+\n+\t\tcat <<-EOF >expected-merge\n+\t\tI\n+\t\tII\n+\t\tIII\n+\t\t<<<<<<< HEAD\n+\t\tIIII\n+\t\t=======\n+\t\tIV\n+\t\t>>>>>>> B^0\n+\t\tEOF\n+\n+\t)\n+}\n+\n+test_expect_merge_algorithm success failure 'conflicting entries written to worktree even if sparse' '\n+\ttest_setup_numerals plain &&\n+\t(\n+\t\tcd numerals_plain &&\n+\n+\t\tgit checkout A^0 &&\n+\n+\t\ttest_path_is_file README &&\n+\t\ttest_path_is_file numerals &&\n+\n+\t\tgit sparse-checkout init &&\n+\t\tgit sparse-checkout set README &&\n+\n+\t\ttest_path_is_file README &&\n+\t\ttest_path_is_missing numerals &&\n+\n+\t\ttest_must_fail git merge -s recursive B^0 &&\n+\n+\t\tgit ls-files -t >index_files &&\n+\t\ttest_cmp expected-index index_files &&\n+\n+\t\ttest_path_is_file README &&\n+\t\ttest_path_is_file numerals &&\n+\n+\t\ttest_cmp expected-merge numerals &&\n+\n+\t\t# 4 other files:\n+\t\t# * expected-merge\n+\t\t# * expected-index\n+\t\t# * index_files\n+\t\t# * others\n+\t\tgit ls-files -o >others &&\n+\t\ttest_line_count = 4 others\n+\t)\n+'\n+\n+test_expect_merge_algorithm failure failure 'present-despite-SKIP_WORKTREE handled reasonably' '\n+\ttest_setup_numerals in_the_way &&\n+\t(\n+\t\tcd numerals_in_the_way &&\n+\n+\t\tgit checkout A^0 &&\n+\n+\t\ttest_path_is_file README &&\n+\t\ttest_path_is_file numerals &&\n+\n+\t\tgit sparse-checkout init &&\n+\t\tgit sparse-checkout set README &&\n+\n+\t\ttest_path_is_file README &&\n+\t\ttest_path_is_missing numerals &&\n+\n+\t\techo foobar >numerals &&\n+\n+\t\ttest_must_fail git merge -s recursive B^0 &&\n+\n+\t\tgit ls-files -t >index_files &&\n+\t\ttest_cmp expected-index index_files &&\n+\n+\t\ttest_path_is_file README &&\n+\t\ttest_path_is_file numerals &&\n+\n+\t\ttest_cmp expected-merge numerals &&\n+\n+\t\t# There should still be a file with \"foobar\" in it\n+\t\tgrep foobar * &&\n+\n+\t\t# 5 other files:\n+\t\t# * expected-merge\n+\t\t# * expected-index\n+\t\t# * index_files\n+\t\t# * others\n+\t\t# * whatever name was given to the numerals file that had\n+\t\t# \"foobar\" in it\n+\t\tgit ls-files -o >others &&\n+\t\ttest_line_count = 5 others\n+\t)\n+'\n+\n+test_done\n-- \ngitgitgadget\n\n"},{"id":"419593","messageId":"c1c9605c1932cab6191f47beb992a15c64ada237.1616016485.git.gitgitgadget@gmail.com","threadId":"55329","inReplyTo":"pull.905.v2.git.1616016485.gitgitgadget@gmail.com","subject":"[PATCH v2 04/13] merge-ort: let renormalization change modify/delete into clean delete","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-03-17T21:27:56Z","receivedAt":"2021-03-17T21:28:58Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nWhen we have a modify/delete conflict, but the only change to the\nmodification is e.g. change of line endings, then if renormalization is\nrequested then we should be able to recognize such a case as a\nnot-modified/delete and resolve the conflict automatically.\n\nThis fixes t6418.10 under GIT_TEST_MERGE_ALGORITHM=ort.\n\nSigned-off-by: Elijah Newren \n---\n merge-ort.c | 64 +++++++++++++++++++++++++++++++++++++++++++++++++++--\n 1 file changed, 62 insertions(+), 2 deletions(-)\n\ndiff --git a/merge-ort.c b/merge-ort.c\nindex cdc1e2fe7a24..c7083e3769aa 100644\n--- a/merge-ort.c\n+++ b/merge-ort.c\n@@ -2549,6 +2549,61 @@ static int string_list_df_name_compare(const char *one, const char *two)\n \treturn onelen - twolen;\n }\n \n+static int read_oid_strbuf(struct merge_options *opt,\n+\t\t\t const struct object_id *oid,\n+\t\t\t struct strbuf *dst)\n+{\n+\tvoid *buf;\n+\tenum object_type type;\n+\tunsigned long size;\n+\tbuf = read_object_file(oid, &type, &size);\n+\tif (!buf)\n+\t\treturn err(opt, _(\"cannot read object %s\"), oid_to_hex(oid));\n+\tif (type != OBJ_BLOB) {\n+\t\tfree(buf);\n+\t\treturn err(opt, _(\"object %s is not a blob\"), oid_to_hex(oid));\n+\t}\n+\tstrbuf_attach(dst, buf, size, size + 1);\n+\treturn 0;\n+}\n+\n+static int blob_unchanged(struct merge_options *opt,\n+\t\t\t const struct version_info *base,\n+\t\t\t const struct version_info *side,\n+\t\t\t const char *path)\n+{\n+\tstruct strbuf basebuf = STRBUF_INIT;\n+\tstruct strbuf sidebuf = STRBUF_INIT;\n+\tint ret = 0; /* assume changed for safety */\n+\tconst struct index_state *idx = &opt->priv->attr_index;\n+\n+\tif (!idx->initialized)\n+\t\tinitialize_attr_index(opt);\n+\n+\tif (base->mode != side->mode)\n+\t\treturn 0;\n+\tif (oideq(&base->oid, &side->oid))\n+\t\treturn 1;\n+\n+\tif (read_oid_strbuf(opt, &base->oid, &basebuf) ||\n+\t read_oid_strbuf(opt, &side->oid, &sidebuf))\n+\t\tgoto error_return;\n+\t/*\n+\t * Note: binary | is used so that both renormalizations are\n+\t * performed. Comparison can be skipped if both files are\n+\t * unchanged since their sha1s have already been compared.\n+\t */\n+\tif (renormalize_buffer(idx, path, basebuf.buf, basebuf.len, &basebuf) |\n+\t renormalize_buffer(idx, path, sidebuf.buf, sidebuf.len, &sidebuf))\n+\t\tret = (basebuf.len == sidebuf.len &&\n+\t\t !memcmp(basebuf.buf, sidebuf.buf, basebuf.len));\n+\n+error_return:\n+\tstrbuf_release(&basebuf);\n+\tstrbuf_release(&sidebuf);\n+\treturn ret;\n+}\n+\n struct directory_versions {\n \t/*\n \t * versions: list of (basename -> version_info)\n@@ -3136,8 +3191,13 @@ static void process_entry(struct merge_options *opt,\n \t\tmodify_branch = (side == 1) ? opt->branch1 : opt->branch2;\n \t\tdelete_branch = (side == 1) ? opt->branch2 : opt->branch1;\n \n-\t\tif (ci->path_conflict &&\n-\t\t oideq(&ci->stages[0].oid, &ci->stages[side].oid)) {\n+\t\tif (opt->renormalize &&\n+\t\t blob_unchanged(opt, &ci->stages[0], &ci->stages[side],\n+\t\t\t\t path)) {\n+\t\t\tci->merged.is_null = 1;\n+\t\t\tci->merged.clean = 1;\n+\t\t} else if (ci->path_conflict &&\n+\t\t\t oideq(&ci->stages[0].oid, &ci->stages[side].oid)) {\n \t\t\t/*\n \t\t\t * This came from a rename/delete; no action to take,\n \t\t\t * but avoid printing \"modify/delete\" conflict notice\n-- \ngitgitgadget\n\n"},{"id":"419594","messageId":"a37979454069eef830ff19e70a0e998522d30448.1616016485.git.gitgitgadget@gmail.com","threadId":"55329","inReplyTo":"pull.905.v2.git.1616016485.gitgitgadget@gmail.com","subject":"[PATCH v2 10/13] merge-recursive: add a bunch of FIXME comments documenting known bugs","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-03-17T21:28:02Z","receivedAt":"2021-03-17T21:28:58Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nThe plan is to just delete merge-recursive, but not until everyone is\ncomfortable with merge-ort as a replacement. Given that I haven't\nswitched all callers of merge-recursive over yet (e.g. git-am still uses\nmerge-recursive), maybe there's some value documenting known bugs in the\nalgorithm in case we end up keeping it or someone wants to dig it up in\nthe future.\n\nSigned-off-by: Elijah Newren \n---\n merge-recursive.c | 37 +++++++++++++++++++++++++++++++++++++\n 1 file changed, 37 insertions(+)\n\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex b052974f191c..99a197597db5 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -1075,6 +1075,11 @@ static int merge_3way(struct merge_options *opt,\n \tread_mmblob(&src1, &a->oid);\n \tread_mmblob(&src2, &b->oid);\n \n+\t/*\n+\t * FIXME: Using a->path for normalization rules in ll_merge could be\n+\t * wrong if we renamed from a->path to b->path. We should use the\n+\t * target path for where the file will be written.\n+\t */\n \tmerge_status = ll_merge(result_buf, a->path, &orig, base,\n \t\t\t\t&src1, name1, &src2, name2,\n \t\t\t\topt->repo->index, &ll_opts);\n@@ -1154,6 +1159,8 @@ static void print_commit(struct commit *commit)\n \tstruct strbuf sb = STRBUF_INIT;\n \tstruct pretty_print_context ctx = {0};\n \tctx.date_mode.type = DATE_NORMAL;\n+\t/* FIXME: Merge this with output_commit_title() */\n+\tassert(!merge_remote_util(commit));\n \tformat_commit_message(commit, \" %h: %m %s\", &sb, &ctx);\n \tfprintf(stderr, \"%s\\n\", sb.buf);\n \tstrbuf_release(&sb);\n@@ -1177,6 +1184,11 @@ static int merge_submodule(struct merge_options *opt,\n \tint search = !opt->priv->call_depth;\n \n \t/* store a in result in case we fail */\n+\t/* FIXME: This is the WRONG resolution for the recursive case when\n+\t * we need to be careful to avoid accidentally matching either side.\n+\t * Should probably use o instead there, much like we do for merging\n+\t * binaries.\n+\t */\n \toidcpy(result, a);\n \n \t/* we can not handle deletion conflicts */\n@@ -1301,6 +1313,13 @@ static int merge_mode_and_contents(struct merge_options *opt,\n \n \tif ((S_IFMT & a->mode) != (S_IFMT & b->mode)) {\n \t\tresult->clean = 0;\n+\t\t/*\n+\t\t * FIXME: This is a bad resolution for recursive case; for\n+\t\t * the recursive case we want something that is unlikely to\n+\t\t * accidentally match either side. Also, while it makes\n+\t\t * sense to prefer regular files over symlinks, it doesn't\n+\t\t * make sense to prefer regular files over submodules.\n+\t\t */\n \t\tif (S_ISREG(a->mode)) {\n \t\t\tresult->blob.mode = a->mode;\n \t\t\toidcpy(&result->blob.oid, &a->oid);\n@@ -1349,6 +1368,7 @@ static int merge_mode_and_contents(struct merge_options *opt,\n \t\t\tfree(result_buf.ptr);\n \t\t\tif (ret)\n \t\t\t\treturn ret;\n+\t\t\t/* FIXME: bug, what if modes didn't match? */\n \t\t\tresult->clean = (merge_status == 0);\n \t\t} else if (S_ISGITLINK(a->mode)) {\n \t\t\tresult->clean = merge_submodule(opt, &result->blob.oid,\n@@ -2664,6 +2684,14 @@ static int process_renames(struct merge_options *opt,\n \tstruct string_list b_by_dst = STRING_LIST_INIT_NODUP;\n \tconst struct rename *sre;\n \n+\t/*\n+\t * FIXME: As string-list.h notes, it's O(n^2) to build a sorted\n+\t * string_list one-by-one, but O(n log n) to build it unsorted and\n+\t * then sort it. Note that as we build the list, we do not need to\n+\t * check if the existing destination path is already in the list,\n+\t * because the structure of diffcore_rename guarantees we won't\n+\t * have duplicates.\n+\t */\n \tfor (i = 0; i < a_renames->nr; i++) {\n \t\tsre = a_renames->items[i].util;\n \t\tstring_list_insert(&a_by_dst, sre->pair->two->path)->util\n@@ -3602,6 +3630,15 @@ static int merge_recursive_internal(struct merge_options *opt,\n \t\t\treturn err(opt, _(\"merge returned no commit\"));\n \t}\n \n+\t/*\n+\t * FIXME: Since merge_recursive_internal() is only ever called by\n+\t * places that ensure the index is loaded first\n+\t * (e.g. builtin/merge.c, rebase/sequencer, etc.), in the common\n+\t * case where the merge base was unique that means when we get here\n+\t * we immediately discard the index and re-read it, which is a\n+\t * complete waste of time. We should only be discarding and\n+\t * re-reading if we were forced to recurse.\n+\t */\n \tdiscard_index(opt->repo->index);\n \tif (!opt->priv->call_depth)\n \t\trepo_read_index(opt->repo);\n-- \ngitgitgadget\n\n"},{"id":"419595","messageId":"4a79e61346919291f49885298da4b8c714ee00ff.1616016485.git.gitgitgadget@gmail.com","threadId":"55329","inReplyTo":"pull.905.v2.git.1616016485.gitgitgadget@gmail.com","subject":"[PATCH v2 09/13] merge-ort: write $GIT_DIR/AUTO_MERGE whenever we hit a conflict","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-03-17T21:28:01Z","receivedAt":"2021-03-17T21:28:58Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nThere are a variety of questions users might ask while resolving\nconflicts:\n * What changes have been made since the previous (first) parent?\n * What changes are staged?\n * What is still unstaged? (or what is still conflicted?)\n * What changes did I make to resolve conflicts so far?\nThe first three of these have simple answers:\n * git diff HEAD\n * git diff --cached\n * git diff\nThere was no way to answer the final question previously. Adding one\nis trivial in merge-ort, since it works by creating a tree representing\nwhat should be written to the working copy complete with conflict\nmarkers. Simply write that tree to .git/AUTO_MERGE, allowing users to\nanswer the fourth question with\n * git diff AUTO_MERGE\n\nI avoided using a name like \"MERGE_AUTO\", because that would be\nmerge-specific (much like MERGE_HEAD, REBASE_HEAD, REVERT_HEAD,\nCHERRY_PICK_HEAD) and I wanted a name that didn't change depending on\nwhich type of operation the merge was part of.\n\nEnsure that paths which clean out other temporary operation-specific\nfiles (e.g. CHERRY_PICK_HEAD, MERGE_MSG, rebase-merge/ state directory)\nalso clean out this AUTO_MERGE file.\n\nSigned-off-by: Elijah Newren \n---\n branch.c | 1 +\n builtin/rebase.c | 1 +\n merge-ort.c | 10 ++++++++++\n path.c | 1 +\n path.h | 2 ++\n sequencer.c | 5 +++++\n 6 files changed, 20 insertions(+)\n\ndiff --git a/branch.c b/branch.c\nindex 9c9dae1eae32..b71a2de29dbe 100644\n--- a/branch.c\n+++ b/branch.c\n@@ -344,6 +344,7 @@ void remove_merge_branch_state(struct repository *r)\n \tunlink(git_path_merge_rr(r));\n \tunlink(git_path_merge_msg(r));\n \tunlink(git_path_merge_mode(r));\n+\tunlink(git_path_auto_merge(r));\n \tsave_autostash(git_path_merge_autostash(r));\n }\n \ndiff --git a/builtin/rebase.c b/builtin/rebase.c\nindex 840dbd7eb777..6c252d62758c 100644\n--- a/builtin/rebase.c\n+++ b/builtin/rebase.c\n@@ -737,6 +737,7 @@ static int finish_rebase(struct rebase_options *opts)\n \tint ret = 0;\n \n \tdelete_ref(NULL, \"REBASE_HEAD\", NULL, REF_NO_DEREF);\n+\tunlink(git_path_auto_merge(the_repository));\n \tapply_autostash(state_dir_path(\"autostash\", opts));\n \tclose_object_store(the_repository->objects);\n \t/*\ndiff --git a/merge-ort.c b/merge-ort.c\nindex 303e89414274..e8f1a435f99a 100644\n--- a/merge-ort.c\n+++ b/merge-ort.c\n@@ -3496,6 +3496,9 @@ void merge_switch_to_result(struct merge_options *opt,\n {\n \tassert(opt->priv == NULL);\n \tif (result->clean >= 0 && update_worktree_and_index) {\n+\t\tconst char *filename;\n+\t\tFILE *fp;\n+\n \t\ttrace2_region_enter(\"merge\", \"checkout\", opt->repo);\n \t\tif (checkout(opt, head, result->tree)) {\n \t\t\t/* failure to function */\n@@ -3514,6 +3517,13 @@ void merge_switch_to_result(struct merge_options *opt,\n \t\t}\n \t\topt->priv = NULL;\n \t\ttrace2_region_leave(\"merge\", \"record_conflicted\", opt->repo);\n+\n+\t\ttrace2_region_enter(\"merge\", \"write_auto_merge\", opt->repo);\n+\t\tfilename = git_path_auto_merge(opt->repo);\n+\t\tfp = xfopen(filename, \"w\");\n+\t\tfprintf(fp, \"%s\\n\", oid_to_hex(&result->tree->object.oid));\n+\t\tfclose(fp);\n+\t\ttrace2_region_leave(\"merge\", \"write_auto_merge\", opt->repo);\n \t}\n \n \tif (display_update_msgs) {\ndiff --git a/path.c b/path.c\nindex 7b385e5eb282..9e883eb52446 100644\n--- a/path.c\n+++ b/path.c\n@@ -1534,5 +1534,6 @@ REPO_GIT_PATH_FUNC(merge_rr, \"MERGE_RR\")\n REPO_GIT_PATH_FUNC(merge_mode, \"MERGE_MODE\")\n REPO_GIT_PATH_FUNC(merge_head, \"MERGE_HEAD\")\n REPO_GIT_PATH_FUNC(merge_autostash, \"MERGE_AUTOSTASH\")\n+REPO_GIT_PATH_FUNC(auto_merge, \"AUTO_MERGE\")\n REPO_GIT_PATH_FUNC(fetch_head, \"FETCH_HEAD\")\n REPO_GIT_PATH_FUNC(shallow, \"shallow\")\ndiff --git a/path.h b/path.h\nindex e7e77da6aaa5..251c78d98000 100644\n--- a/path.h\n+++ b/path.h\n@@ -176,6 +176,7 @@ struct path_cache {\n \tconst char *merge_mode;\n \tconst char *merge_head;\n \tconst char *merge_autostash;\n+\tconst char *auto_merge;\n \tconst char *fetch_head;\n \tconst char *shallow;\n };\n@@ -191,6 +192,7 @@ const char *git_path_merge_rr(struct repository *r);\n const char *git_path_merge_mode(struct repository *r);\n const char *git_path_merge_head(struct repository *r);\n const char *git_path_merge_autostash(struct repository *r);\n+const char *git_path_auto_merge(struct repository *r);\n const char *git_path_fetch_head(struct repository *r);\n const char *git_path_shallow(struct repository *r);\n \ndiff --git a/sequencer.c b/sequencer.c\nindex d2332d3e1787..472cdd8c620d 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -2096,6 +2096,7 @@ static int do_pick_commit(struct repository *r,\n \t\trefs_delete_ref(get_main_ref_store(r), \"\", \"CHERRY_PICK_HEAD\",\n \t\t\t\tNULL, 0);\n \t\tunlink(git_path_merge_msg(r));\n+\t\tunlink(git_path_auto_merge(r));\n \t\tfprintf(stderr,\n \t\t\t_(\"dropping %s %s -- patch contents already upstream\\n\"),\n \t\t\toid_to_hex(&commit->object.oid), msg.subject);\n@@ -2451,6 +2452,8 @@ void sequencer_post_commit_cleanup(struct repository *r, int verbose)\n \t\tneed_cleanup = 1;\n \t}\n \n+\tunlink(git_path_auto_merge(r));\n+\n \tif (!need_cleanup)\n \t\treturn;\n \n@@ -4111,6 +4114,7 @@ static int pick_commits(struct repository *r,\n \t\t\tunlink(rebase_path_stopped_sha());\n \t\t\tunlink(rebase_path_amend());\n \t\t\tunlink(git_path_merge_head(r));\n+\t\t\tunlink(git_path_auto_merge(r));\n \t\t\tdelete_ref(NULL, \"REBASE_HEAD\", NULL, REF_NO_DEREF);\n \n \t\t\tif (item->command == TODO_BREAK) {\n@@ -4505,6 +4509,7 @@ static int commit_staged_changes(struct repository *r,\n \t\treturn error(_(\"could not commit staged changes.\"));\n \tunlink(rebase_path_amend());\n \tunlink(git_path_merge_head(r));\n+\tunlink(git_path_auto_merge(r));\n \tif (final_fixup) {\n \t\tunlink(rebase_path_fixup_msg());\n \t\tunlink(rebase_path_squash_msg());\n-- \ngitgitgadget\n\n"},{"id":"419596","messageId":"6bda855f29806475be14d133e185fec222c8b0be.1616016485.git.gitgitgadget@gmail.com","threadId":"55329","inReplyTo":"pull.905.v2.git.1616016485.gitgitgadget@gmail.com","subject":"[PATCH v2 11/13] Revert \"merge-ort: ignore the directory rename split conflict for now\"","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-03-17T21:28:03Z","receivedAt":"2021-03-17T21:28:59Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nThis reverts commit 5ced7c3da009090c5a926e3123a71314c7f28d42, which was\nput in place as a temporary measure to avoid optimizations unstably\nerroring out on no destination having a majority of the necessary\nrenames for directories that had no new files and thus no need for\ndirectory rename detection anyway. Now that optimizations are in place\nto prevent us from trying to compute directory rename count computations\nfor directories that do not need it, we can undo this temporary measure.\n\nSigned-off-by: Elijah Newren \n---\n merge-ort.c | 13 +------------\n 1 file changed, 1 insertion(+), 12 deletions(-)\n\ndiff --git a/merge-ort.c b/merge-ort.c\nindex e8f1a435f99a..8258d3fd621e 100644\n--- a/merge-ort.c\n+++ b/merge-ort.c\n@@ -1636,18 +1636,7 @@ static void get_provisional_directory_renames(struct merge_options *opt,\n \t\t\t\t \"no destination getting a majority of the \"\n \t\t\t\t \"files.\"),\n \t\t\t source_dir);\n-\t\t\t/*\n-\t\t\t * We should mark this as unclean IF something attempts\n-\t\t\t * to use this rename. We do not yet have the logic\n-\t\t\t * in place to detect if this directory rename is being\n-\t\t\t * used, and optimizations that reduce the number of\n-\t\t\t * renames cause this to falsely trigger. For now,\n-\t\t\t * just disable it, causing t6423 testcase 2a to break.\n-\t\t\t * We'll later fix the detection, and when we do we\n-\t\t\t * will re-enable setting *clean to 0 (and thereby fix\n-\t\t\t * t6423 testcase 2a).\n-\t\t\t */\n-\t\t\t/* *clean = 0; */\n+\t\t\t*clean = 0;\n \t\t} else {\n \t\t\tstrmap_put(&renames->dir_renames[side],\n \t\t\t\t source_dir, (void*)best);\n-- \ngitgitgadget\n\n"},{"id":"419597","messageId":"1c6361c9b88ac5f970054989bae0845ef3b6baee.1616016485.git.gitgitgadget@gmail.com","threadId":"55329","inReplyTo":"pull.905.v2.git.1616016485.gitgitgadget@gmail.com","subject":"[PATCH v2 12/13] t6423: mark remaining expected failure under merge-ort as such","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-03-17T21:28:04Z","receivedAt":"2021-03-17T21:28:59Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nWhen we started on merge-ort, thousands of tests failed when run with\nthe GIT_TEST_MERGE_ALGORITHM=ort flag; with so many, it didn't make\nsense to flip all their test expectations. The ones in t6409, t6418,\nand the submodule tests are being handled by an independent in-flight\ntopic (\"Complete merge-ort implemenation...almost\"). The ones in\nt6423 were left out of the other series because other ongoing series\nthat this commit depends upon were addressing those. Now that we only\nhave one remaining test failure in t6423, let's mark it as such.\n\nThis remaining test will be fixed by a future optimization series, but\nsince merge-recursive doesn't pass this test either, passing it is not\nnecessary for declaring merge-ort ready for general use.\n\nSigned-off-by: Elijah Newren \n---\n t/t6423-merge-rename-directories.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t6423-merge-rename-directories.sh b/t/t6423-merge-rename-directories.sh\nindex 4c568050dd27..4c3d0b95dc5c 100755\n--- a/t/t6423-merge-rename-directories.sh\n+++ b/t/t6423-merge-rename-directories.sh\n@@ -4797,7 +4797,7 @@ test_setup_12f () {\n \t)\n }\n \n-test_expect_merge_algorithm failure success '12f: Trivial directory resolve, caching, all kinds of fun' '\n+test_expect_merge_algorithm failure failure '12f: Trivial directory resolve, caching, all kinds of fun' '\n \ttest_setup_12f &&\n \t(\n \t\tcd 12f &&\n-- \ngitgitgadget\n\n"},{"id":"419598","messageId":"d8536f56ab296171c09e667e5c9299e95078388e.1616016485.git.gitgitgadget@gmail.com","threadId":"55329","inReplyTo":"pull.905.v2.git.1616016485.gitgitgadget@gmail.com","subject":"[PATCH v2 13/13] Add testing with merge-ort merge strategy","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-03-17T21:28:05Z","receivedAt":"2021-03-17T21:28:59Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nIn preparation for switching from merge-recursive to merge-ort as the\ndefault strategy, ensure that we are testing it.\n\nSigned-off-by: Elijah Newren \n---\n .github/workflows/main.yml | 1 +\n ci/lib.sh | 6 ++++++\n 2 files changed, 7 insertions(+)\n\ndiff --git a/.github/workflows/main.yml b/.github/workflows/main.yml\nindex 5f2f884b92f6..e1f59861a2ca 100644\n--- a/.github/workflows/main.yml\n+++ b/.github/workflows/main.yml\n@@ -252,6 +252,7 @@ jobs:\n MSYSTEM: MINGW64\n NO_SVN_TESTS: 1\n GIT_TEST_SKIP_REBASE_P: 1\n+ GIT_TEST_MERGE_ALGORITHM: ort\n run: |\n & .\\git-sdk-64-minimal\\usr\\bin\\bash.exe -lc @\"\n # Let Git ignore the SDK and the test-cache\ndiff --git a/ci/lib.sh b/ci/lib.sh\nindex d848c036c50f..2a869b598c7f 100755\n--- a/ci/lib.sh\n+++ b/ci/lib.sh\n@@ -182,6 +182,12 @@ export DEFAULT_TEST_TARGET=prove\n export GIT_TEST_CLONE_2GB=true\n export SKIP_DASHED_BUILT_INS=YesPlease\n \n+case \"$jobname\" in\n+*-gcc)\n+\texport GIT_TEST_MERGE_ALGORITHM=ort\n+\t;;\n+esac\n+\n case \"$jobname\" in\n linux-clang|linux-gcc)\n \tif [ \"$jobname\" = linux-gcc ]\n-- \ngitgitgadget\n"},{"id":"419701","messageId":"6b619bd0-c924-274e-d8bb-3bb9c67a2625@gmail.com","threadId":"55329","inReplyTo":"d8536f56ab296171c09e667e5c9299e95078388e.1616016485.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 13/13] Add testing with merge-ort merge strategy","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2021-03-19T13:05:06Z","receivedAt":"2021-03-19T13:05:57Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 3/17/2021 5:28 PM, Elijah Newren via GitGitGadget wrote:\n> From: Elijah Newren \n> \n> In preparation for switching from merge-recursive to merge-ort as the\n> default strategy, ensure that we are testing it.\n\nOK, so here we are testing it by default in CI, not just in that\nsecond round of optional environment variables. If that is what\nyou intended, then this is the diff I was expecting:\n\ndiff --git a/ci/run-build-and-tests.sh b/ci/run-build-and-tests.sh\nindex a66b5e8c75a..c013e9e646a 100755\n--- a/ci/run-build-and-tests.sh\n+++ b/ci/run-build-and-tests.sh\n@@ -25,6 +25,7 @@ linux-gcc)\n \texport GIT_TEST_ADD_I_USE_BUILTIN=1\n \texport GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=master\n \texport GIT_TEST_WRITE_REV_INDEX=1\n+\texport GIT_TEST_MERGE_STRATEGY=org\n \tmake test\n \t;;\n linux-clang)\n\nHowever, if we want to be aggressive in adopting the ort strategy\nvery soon, then your approach of testing more frequently is\nvaluable.\n\nWould it be worth setting GIT_TEST_MERGE_ALGORITHM=ort by default\nin test-lib.sh, too? That could help developers working on top of\nyour topic avoid creating test failures that only show up in CI.\n\nThanks,\n-Stolee\n"},{"id":"419702","messageId":"80cccc70-296a-05d2-94a8-0e005e4abe1d@gmail.com","threadId":"55329","inReplyTo":"pull.905.v2.git.1616016485.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 00/13] Declare merge-ort ready for general usage","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2021-03-19T13:09:08Z","receivedAt":"2021-03-19T13:10:15Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 3/17/2021 5:27 PM, Elijah Newren via GitGitGadget wrote:\n> This series depends on ort-perf-batch-10[1], and obsoletes the ort-remainder\n> topic[2] (that hadn't been picked up yet, so hopefully this doesn't cause\n> any confusion)\n> \n> With this series, merge-ort is ready for general usage -- it passes all\n> tests, passes dozens of tests that don't under merge-recursive, and\n> merge-ort is is already significantly faster than merge-recursive when\n> rename detection is involved. Users can select merge-ort by (a) passing\n> -sort to either git merge or git rebase, or (b) by setting pull.twohead=ort\n> [3], or (c) by setting GIT_TEST_MERGE_ALGORITHM=ort.\n> \n> Changes since v1:\n> \n> * subsumed the ort-remainder topic (the first 10 patches), which has\n> already been reviewed by both Ævar and Stolee[2].\n> * the next two patches were the original v1, reviewed by Stolee\n> * the final patch is new and adds testing.\n\nSorry for the delay in looking at this. I read the two series before\nthis, and found this to be a good union of them.\n\nMy only question on the final patch is a two parter:\n\n1. Did you mean to go this far?\n2. Did you want to go farther?\n\nMostly: how much do we want to prepare for ORT as the default\nstrategy, at the expense of reducing testing of the recursive\nstrategy?\n\nThanks,\n-Stolee\n"},{"id":"419712","messageId":"CABPp-BFiTCVZhyS551vUit=7bw4cnbU2zdBH0DZ7UKAoieDFFA@mail.gmail.com","threadId":"55329","inReplyTo":"6b619bd0-c924-274e-d8bb-3bb9c67a2625@gmail.com","subject":"Re: [PATCH v2 13/13] Add testing with merge-ort merge strategy","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2021-03-19T15:21:08Z","receivedAt":"2021-03-19T15:22:14Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Fri, Mar 19, 2021 at 6:05 AM Derrick Stolee wrote:\n>\n> On 3/17/2021 5:28 PM, Elijah Newren via GitGitGadget wrote:\n> > From: Elijah Newren \n> >\n> > In preparation for switching from merge-recursive to merge-ort as the\n> > default strategy, ensure that we are testing it.\n>\n> OK, so here we are testing it by default in CI, not just in that\n> second round of optional environment variables. If that is what\n\nNote quite; my patch adds testing of merge-ort with *-gcc variants on\neach of the major platforms, but still includes merge-recursive\ntesting in the *-clang variants.\n\n> you intended, then this is the diff I was expecting:\n>\n> diff --git a/ci/run-build-and-tests.sh b/ci/run-build-and-tests.sh\n> index a66b5e8c75a..c013e9e646a 100755\n> --- a/ci/run-build-and-tests.sh\n> +++ b/ci/run-build-and-tests.sh\n> @@ -25,6 +25,7 @@ linux-gcc)\n> export GIT_TEST_ADD_I_USE_BUILTIN=1\n> export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=master\n> export GIT_TEST_WRITE_REV_INDEX=1\n> + export GIT_TEST_MERGE_STRATEGY=org\n\n\"ort\", not \"org\". :-)\n\n> make test\n> ;;\n> linux-clang)\n>\n> However, if we want to be aggressive in adopting the ort strategy\n> very soon, then your approach of testing more frequently is\n> valuable.\n\nI don't think testing limited to linux-gcc is very valuable:\n\n* I've got opt-in volunteers (~60 of them) using it for the last half\nyear for their daily work (on Mac & Linux)\n* For these volunteers, `git log -p` _also_ remerges every 2-parent\nmerge automatically (there's a --remerge-diff capability)\n* We also had a bug where an internal script ('sparsify') looked for\nfiles changed in \"local-only\" commits via `git log --name-only HEAD\n--not --remotes=origin`.\n* At least one person renamed their default remote to something other\nthan \"origin\"\n\nThe upshot is we not only have many testers using it for daily work,\nwe had at least one guinea pig redoing every merge in his\nrepository...and found a platform specific bug in some commit from\nyears ago because of it.\n\nIn the last half year, I've only had three bug reports, none reported\nby more than one person:\n\n* Platform specific merge issue triggered only in a commit in a\ncertain repository from years ago (fixed by first patch of this\nseries, the STABLE_QSORT() one)\n* Mis-handling of present-despite-SKIP_WORKTREE file that *also* was\ninvolved in a merge conflict (see t6428 addition in this series; note\nthat merge-recursive still fails it)\n* A bug in the --remerge-diff results\n\nSo, at this point, beyond avoiding regressions the primary value I\nfind in testing is in expanding it. windows is my big hole so more\nthan anything I want ort testing there.\n\n> Would it be worth setting GIT_TEST_MERGE_ALGORITHM=ort by default\n> in test-lib.sh, too? That could help developers working on top of\n> your topic avoid creating test failures that only show up in CI.\n\nActually, that sounds like a good idea, and we could just pick one or\ntwo special CI jobs to test with GIT_TEST_MERGE_STRATEGY=recursive.\n"},{"id":"419740","messageId":"CABPp-BGm1R0OVywK50eMf9OWKv95nA+V2bzX6WLm2iFvfN_i9Q@mail.gmail.com","threadId":"55329","inReplyTo":"80cccc70-296a-05d2-94a8-0e005e4abe1d@gmail.com","subject":"Re: [PATCH v2 00/13] Declare merge-ort ready for general usage","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2021-03-19T23:21:44Z","receivedAt":"2021-03-19T23:22:51Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Fri, Mar 19, 2021 at 6:09 AM Derrick Stolee wrote:\n>\n> On 3/17/2021 5:27 PM, Elijah Newren via GitGitGadget wrote:\n> > This series depends on ort-perf-batch-10[1], and obsoletes the ort-remainder\n> > topic[2] (that hadn't been picked up yet, so hopefully this doesn't cause\n> > any confusion)\n> >\n> > With this series, merge-ort is ready for general usage -- it passes all\n> > tests, passes dozens of tests that don't under merge-recursive, and\n> > merge-ort is is already significantly faster than merge-recursive when\n> > rename detection is involved. Users can select merge-ort by (a) passing\n> > -sort to either git merge or git rebase, or (b) by setting pull.twohead=ort\n> > [3], or (c) by setting GIT_TEST_MERGE_ALGORITHM=ort.\n> >\n> > Changes since v1:\n> >\n> > * subsumed the ort-remainder topic (the first 10 patches), which has\n> > already been reviewed by both Ævar and Stolee[2].\n> > * the next two patches were the original v1, reviewed by Stolee\n> > * the final patch is new and adds testing.\n>\n> Sorry for the delay in looking at this. I read the two series before\n> this, and found this to be a good union of them.\n>\n> My only question on the final patch is a two parter:\n>\n> 1. Did you mean to go this far?\n\nAt least, yes.\n\n> 2. Did you want to go farther?\n\nI like your suggestion in the other email; I'll resubmit to take\nadvantage of it. :-)\n\n> Mostly: how much do we want to prepare for ORT as the default\n> strategy, at the expense of reducing testing of the recursive\n> strategy?\n\nWe definitely should prepare for merge-ort as the default. There's a\nquestion of how soon the switch should be, but no question in my mind\nthat we should move towards it.\n\nWhat do others think is needed before we switch the default?\nPersonally, I think there are three things:\n\n1) merge-ort must handle the same cases that merge-recursive does\n2) merge-ort must provide some benefit over merge-recursive\n3) folks on the mailing list need to be comfortable with the default switch.\n\nWhat would others add?\n\nThe first 2 conditions are already met, in spades. For all the code\nthat calls merge-ort, merge-ort handles all the same cases, is more\ncorrect, more performant, and more featureful than merge-recursive. I\nwas surprised by how smooth the roll-out was and has continued to be\nfor internal users at $DAYJOB.\n\nThe only question is item #3. If it weren't for that, I'd say we\nshould switch the default now, because AFAICT delaying the default\nswitch will just delay when expanded testing occurs, and I have run\nout of other ways to expand testing. But I realize I'm the only one\nwho knows that and is comfortable with that. So I'm not proposing a\ndefault switch yet; I want to hear feedback on what others want to see\ndone before we switch. (At some point in the future, say another year\nor two, I'll ask what needs to be done before we *delete*\nmerge-recursive.[ch]. But that's still off in the distant future.)\n"},{"id":"419741","messageId":"CABPp-BEFPCmr-_cn5bzoOefMN4LKARefKaoobcgJLjcRLBBoZg@mail.gmail.com","threadId":"55329","inReplyTo":"80cccc70-296a-05d2-94a8-0e005e4abe1d@gmail.com","subject":"Re: [PATCH v2 00/13] Declare merge-ort ready for general usage","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2021-03-19T23:35:39Z","receivedAt":"2021-03-19T23:37:06Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"One more thing...\n\nOn Fri, Mar 19, 2021 at 6:09 AM Derrick Stolee wrote:\n>\n> On 3/17/2021 5:27 PM, Elijah Newren via GitGitGadget wrote:\n\n^^^ Note: 18 hours, 42 minutes difference.\n\n> > This series depends on ort-perf-batch-10[1], and obsoletes the ort-remainder\n> > topic[2] (that hadn't been picked up yet, so hopefully this doesn't cause\n> > any confusion)\n> >\n> > With this series, merge-ort is ready for general usage -- it passes all\n> > tests, passes dozens of tests that don't under merge-recursive, and\n> > merge-ort is is already significantly faster than merge-recursive when\n> > rename detection is involved. Users can select merge-ort by (a) passing\n> > -sort to either git merge or git rebase, or (b) by setting pull.twohead=ort\n> > [3], or (c) by setting GIT_TEST_MERGE_ALGORITHM=ort.\n> >\n> > Changes since v1:\n> >\n> > * subsumed the ort-remainder topic (the first 10 patches), which has\n> > already been reviewed by both Ævar and Stolee[2].\n> > * the next two patches were the original v1, reviewed by Stolee\n> > * the final patch is new and adds testing.\n>\n> Sorry for the delay in looking at this.\n\nDelay?!? You only took a day and a half to respond! That's a fast\nturnaround, not a delay. Don't apologize for that, or you'll set\nunreasonably high expectations that'll scare the rest of us away. :-)\n\n\nThanks for all your review efforts; it's very much appreciated!\n"},{"id":"419743","messageId":"pull.905.v3.git.1616198636.gitgitgadget@gmail.com","threadId":"55329","inReplyTo":"pull.905.v2.git.1616016485.gitgitgadget@gmail.com","subject":"[PATCH v3 00/13] Declare merge-ort ready for general usage","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-03-20T00:03:43Z","receivedAt":"2021-03-20T00:04:37Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"This series depends on ort-perf-batch-10[1], and obsoletes the ort-remainder\ntopic[2] (that hadn't been picked up yet, so hopefully this doesn't cause\nany confusion)\n\nWith this series, merge-ort is ready for general usage -- it passes all\ntests, passes dozens of tests that don't under merge-recursive, and\nmerge-ort is is already significantly faster than merge-recursive when\nrename detection is involved. Users can select merge-ort by (a) passing\n-sort to either git merge or git rebase, or (b) by setting pull.twohead=ort\n[3], or (c) by setting GIT_TEST_MERGE_ALGORITHM=ort.\n\nChanges since v2:\n\n * changed the last patch to default testing to ort so people don't have to\n wait until CI for failures (but still keep linux-gcc testing\n merge-recursive so it retains coverage), as suggested by Stolee\n\n[1]\nhttps://lore.kernel.org/git/pull.853.git.1615674128.gitgitgadget@gmail.com/\n[2]\nhttps://lore.kernel.org/git/pull.973.v2.git.git.1615271086.gitgitgadget@gmail.com/\n[3] See commit 14c4586c2d (\"merge,rebase,revert: select ort or recursive by\nconfig or environment\", 2020-11-02)\n\nElijah Newren (13):\n merge-ort: use STABLE_QSORT instead of QSORT where required\n merge-ort: add a special minimal index just for renormalization\n merge-ort: have ll_merge() use a special attr_index for\n renormalization\n merge-ort: let renormalization change modify/delete into clean delete\n merge-ort: support subtree shifting\n t6428: new test for SKIP_WORKTREE handling and conflicts\n merge-ort: implement CE_SKIP_WORKTREE handling with conflicted entries\n t: mark several submodule merging tests as fixed under merge-ort\n merge-ort: write $GIT_DIR/AUTO_MERGE whenever we hit a conflict\n merge-recursive: add a bunch of FIXME comments documenting known bugs\n Revert \"merge-ort: ignore the directory rename split conflict for now\"\n t6423: mark remaining expected failure under merge-ort as such\n Add testing with merge-ort merge strategy\n\n branch.c | 1 +\n builtin/rebase.c | 1 +\n ci/run-build-and-tests.sh | 1 +\n merge-ort.c | 242 ++++++++++++++++--\n merge-recursive.c | 37 +++\n path.c | 1 +\n path.h | 2 +\n sequencer.c | 5 +\n t/t3512-cherry-pick-submodule.sh | 7 +-\n t/t3513-revert-submodule.sh | 5 +-\n t/t5572-pull-submodule.sh | 7 +-\n t/t6423-merge-rename-directories.sh | 2 +-\n t/t6428-merge-conflicts-sparse.sh | 158 ++++++++++++\n t/t6437-submodule-merge.sh | 5 +-\n t/t6438-submodule-directory-file-conflicts.sh | 7 +-\n t/test-lib.sh | 2 +\n 16 files changed, 445 insertions(+), 38 deletions(-)\n create mode 100755 t/t6428-merge-conflicts-sparse.sh\n\n\nbase-commit: ac0ba91ce275227f5df8f16fb986308ff88b198b\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-905%2Fgitgitgadget%2Fort-readiness-v3\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-905/gitgitgadget/ort-readiness-v3\nPull-Request: https://github.com/gitgitgadget/git/pull/905\n\nRange-diff vs v2:\n\n 1: e223f842748c = 1: e223f842748c merge-ort: use STABLE_QSORT instead of QSORT where required\n 2: 6d34cc466bd5 = 2: 6d34cc466bd5 merge-ort: add a special minimal index just for renormalization\n 3: 4ff23d2f52a0 = 3: 4ff23d2f52a0 merge-ort: have ll_merge() use a special attr_index for renormalization\n 4: c1c9605c1932 = 4: c1c9605c1932 merge-ort: let renormalization change modify/delete into clean delete\n 5: 41fffcdd3b78 = 5: 41fffcdd3b78 merge-ort: support subtree shifting\n 6: 6aec1f499b80 = 6: 6aec1f499b80 t6428: new test for SKIP_WORKTREE handling and conflicts\n 7: fe3baf696785 = 7: fe3baf696785 merge-ort: implement CE_SKIP_WORKTREE handling with conflicted entries\n 8: f9325647a9fc = 8: f9325647a9fc t: mark several submodule merging tests as fixed under merge-ort\n 9: 4a79e6134691 = 9: 4a79e6134691 merge-ort: write $GIT_DIR/AUTO_MERGE whenever we hit a conflict\n 10: a37979454069 = 10: a37979454069 merge-recursive: add a bunch of FIXME comments documenting known bugs\n 11: 6bda855f2980 = 11: 6bda855f2980 Revert \"merge-ort: ignore the directory rename split conflict for now\"\n 12: 1c6361c9b88a = 12: 1c6361c9b88a t6423: mark remaining expected failure under merge-ort as such\n 13: d8536f56ab29 < -: ------------ Add testing with merge-ort merge strategy\n -: ------------ > 13: c2d2a1ccaea7 Add testing with merge-ort merge strategy\n\n-- \ngitgitgadget\n"},{"id":"419744","messageId":"4ff23d2f52a0abd41d81076f9dfa3a93319deca5.1616198636.git.gitgitgadget@gmail.com","threadId":"55329","inReplyTo":"pull.905.v3.git.1616198636.gitgitgadget@gmail.com","subject":"[PATCH v3 03/13] merge-ort: have ll_merge() use a special attr_index for renormalization","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-03-20T00:03:46Z","receivedAt":"2021-03-20T00:05:09Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nll_merge() needs an index when renormalization is requested. Create one\nspecifically for just this purpose with just the one needed entry. This\nfixes t6418.4 and t6418.5 under GIT_TEST_MERGE_ALGORITHM=ort.\n\nNOTE 1: Even if the user has a working copy or a real index (which is\nnot a given as merge-ort can be used in bare repositories), we\nexplicitly ignore any .gitattributes file from either of these\nlocations. merge-ort can be used to merge two branches that are\nunrelated to HEAD, so .gitattributes from the working copy and current\nindex should not be considered relevant.\n\nNOTE 2: Since we are in the middle of merging, there is a risk that\n.gitattributes itself is conflicted...leaving us with an ill-defined\nsituation about how to perform the rest of the merge. It could be that\nthe .gitattributes file does not even exist on one of the sides of the\nmerge, or that it has been modified on both sides. If it's been\nmodified on both sides, it's possible that it could itself be merged\ncleanly, though it's also possible that it only merges cleanly if you\nuse the right version of the .gitattributes file to drive the merge. It\ngets kind of complicated. The only test we ever had that attempted to\ntest behavior in this area was seemingly unaware of the undefined\nbehavior, but knew the test wouldn't work for lack of attribute handling\nsupport, marked it as test_expect_failure from the beginning, but\nmanaged to fail for several reasons unrelated to attribute handling.\nSee commit 6f6e7cfb52 (\"t6038: remove problematic test\", 2020-08-03) for\ndetails. So there are probably various ways to improve what\ninitialize_attr_index() picks in the case of a conflicted .gitattributes\nbut for now I just implemented something simple -- look for whatever\n.gitattributes file we can find in any of the higher order stages and\nuse it.\n\nSigned-off-by: Elijah Newren \n---\n merge-ort.c | 64 +++++++++++++++++++++++++++++++++++++++++++++++++++--\n 1 file changed, 62 insertions(+), 2 deletions(-)\n\ndiff --git a/merge-ort.c b/merge-ort.c\nindex 3c606fa7e4b3..cdc1e2fe7a24 100644\n--- a/merge-ort.c\n+++ b/merge-ort.c\n@@ -410,7 +410,7 @@ static void clear_or_reinit_internal_opts(struct merge_options_internal *opti,\n \tstring_list_clear(&opti->paths_to_free, 0);\n \topti->paths_to_free.strdup_strings = 0;\n \n-\tif (opti->attr_index.cache_nr)\n+\tif (opti->attr_index.cache_nr) /* true iff opt->renormalize */\n \t\tdiscard_index(&opti->attr_index);\n \n \t/* Free memory used by various renames maps */\n@@ -1201,6 +1201,63 @@ static int merge_submodule(struct merge_options *opt,\n \treturn 0;\n }\n \n+static void initialize_attr_index(struct merge_options *opt)\n+{\n+\t/*\n+\t * The renormalize_buffer() functions require attributes, and\n+\t * annoyingly those can only be read from the working tree or from\n+\t * an index_state. merge-ort doesn't have an index_state, so we\n+\t * generate a fake one containing only attribute information.\n+\t */\n+\tstruct merged_info *mi;\n+\tstruct index_state *attr_index = &opt->priv->attr_index;\n+\tstruct cache_entry *ce;\n+\n+\tattr_index->initialized = 1;\n+\n+\tif (!opt->renormalize)\n+\t\treturn;\n+\n+\tmi = strmap_get(&opt->priv->paths, GITATTRIBUTES_FILE);\n+\tif (!mi)\n+\t\treturn;\n+\n+\tif (mi->clean) {\n+\t\tint len = strlen(GITATTRIBUTES_FILE);\n+\t\tce = make_empty_cache_entry(attr_index, len);\n+\t\tce->ce_mode = create_ce_mode(mi->result.mode);\n+\t\tce->ce_flags = create_ce_flags(0);\n+\t\tce->ce_namelen = len;\n+\t\toidcpy(&ce->oid, &mi->result.oid);\n+\t\tmemcpy(ce->name, GITATTRIBUTES_FILE, len);\n+\t\tadd_index_entry(attr_index, ce,\n+\t\t\t\tADD_CACHE_OK_TO_ADD | ADD_CACHE_OK_TO_REPLACE);\n+\t\tget_stream_filter(attr_index, GITATTRIBUTES_FILE, &ce->oid);\n+\t} else {\n+\t\tint stage, len;\n+\t\tstruct conflict_info *ci;\n+\n+\t\tASSIGN_AND_VERIFY_CI(ci, mi);\n+\t\tfor (stage = 0; stage < 3; stage++) {\n+\t\t\tunsigned stage_mask = (1 << stage);\n+\n+\t\t\tif (!(ci->filemask & stage_mask))\n+\t\t\t\tcontinue;\n+\t\t\tlen = strlen(GITATTRIBUTES_FILE);\n+\t\t\tce = make_empty_cache_entry(attr_index, len);\n+\t\t\tce->ce_mode = create_ce_mode(ci->stages[stage].mode);\n+\t\t\tce->ce_flags = create_ce_flags(stage);\n+\t\t\tce->ce_namelen = len;\n+\t\t\toidcpy(&ce->oid, &ci->stages[stage].oid);\n+\t\t\tmemcpy(ce->name, GITATTRIBUTES_FILE, len);\n+\t\t\tadd_index_entry(attr_index, ce,\n+\t\t\t\t\tADD_CACHE_OK_TO_ADD | ADD_CACHE_OK_TO_REPLACE);\n+\t\t\tget_stream_filter(attr_index, GITATTRIBUTES_FILE,\n+\t\t\t\t\t &ce->oid);\n+\t\t}\n+\t}\n+}\n+\n static int merge_3way(struct merge_options *opt,\n \t\t const char *path,\n \t\t const struct object_id *o,\n@@ -1215,6 +1272,9 @@ static int merge_3way(struct merge_options *opt,\n \tchar *base, *name1, *name2;\n \tint merge_status;\n \n+\tif (!opt->priv->attr_index.initialized)\n+\t\tinitialize_attr_index(opt);\n+\n \tll_opts.renormalize = opt->renormalize;\n \tll_opts.extra_marker_size = extra_marker_size;\n \tll_opts.xdl_opts = opt->xdl_opts;\n@@ -1253,7 +1313,7 @@ static int merge_3way(struct merge_options *opt,\n \n \tmerge_status = ll_merge(result_buf, path, &orig, base,\n \t\t\t\t&src1, name1, &src2, name2,\n-\t\t\t\topt->repo->index, &ll_opts);\n+\t\t\t\t&opt->priv->attr_index, &ll_opts);\n \n \tfree(base);\n \tfree(name1);\n-- \ngitgitgadget\n\n"},{"id":"419745","messageId":"e223f842748c797a3df967321de7650aba3a9801.1616198636.git.gitgitgadget@gmail.com","threadId":"55329","inReplyTo":"pull.905.v3.git.1616198636.gitgitgadget@gmail.com","subject":"[PATCH v3 01/13] merge-ort: use STABLE_QSORT instead of QSORT where required","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-03-20T00:03:44Z","receivedAt":"2021-03-20T00:05:10Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nrename/rename conflict handling depends on the fact that if both sides\nrenamed the same path, that the one on the MERGE_SIDE1 will appear first\nin the combined diff_queue_struct passed to process_renames(). Since we\nadd all pairs from MERGE_SIDE1 to combined first, and then all pairs\nfrom MERGE_SIDE2, and then sort based on filename, this will only be\ntrue if the sort used is stable. This was found due to the fact that\nMac, unlike Linux, apparently has a system-defined qsort that is not\nstable.\n\nWhile we are at it, review the other callers of QSORT and add comments\nabout why they can remain as calls to QSORT instead of being modified\nto call STABLE_QSORT.\n\nSigned-off-by: Elijah Newren \n---\n merge-ort.c | 8 +++++++-\n 1 file changed, 7 insertions(+), 1 deletion(-)\n\ndiff --git a/merge-ort.c b/merge-ort.c\nindex 7f5750ce6ab0..34a91c435737 100644\n--- a/merge-ort.c\n+++ b/merge-ort.c\n@@ -2405,7 +2405,7 @@ static int detect_and_process_renames(struct merge_options *opt,\n \tclean &= collect_renames(opt, &combined, MERGE_SIDE2,\n \t\t\t\t &renames->dir_renames[1],\n \t\t\t\t &renames->dir_renames[2]);\n-\tQSORT(combined.queue, combined.nr, compare_pairs);\n+\tSTABLE_QSORT(combined.queue, combined.nr, compare_pairs);\n \ttrace2_region_leave(\"merge\", \"directory renames\", opt->repo);\n \n \ttrace2_region_enter(\"merge\", \"process renames\", opt->repo);\n@@ -2550,6 +2550,7 @@ static void write_tree(struct object_id *result_oid,\n \t */\n \trelevant_entries.items = versions->items + offset;\n \trelevant_entries.nr = versions->nr - offset;\n+\t/* No need for STABLE_QSORT -- filenames must be unique */\n \tQSORT(relevant_entries.items, relevant_entries.nr, tree_entry_order);\n \n \t/* Pre-allocate some space in buf */\n@@ -3325,6 +3326,11 @@ static int record_conflicted_index_entries(struct merge_options *opt,\n \t * entries we added to the end into their right locations.\n \t */\n \tremove_marked_cache_entries(index, 1);\n+\t/*\n+\t * No need for STABLE_QSORT -- cmp_cache_name_compare sorts primarily\n+\t * on filename and secondarily on stage, and (name, stage #) are a\n+\t * unique tuple.\n+\t */\n \tQSORT(index->cache, index->cache_nr, cmp_cache_name_compare);\n \n \treturn errs;\n-- \ngitgitgadget\n\n"},{"id":"419746","messageId":"41fffcdd3b786c460c3a0652108212d8c361159f.1616198636.git.gitgitgadget@gmail.com","threadId":"55329","inReplyTo":"pull.905.v3.git.1616198636.gitgitgadget@gmail.com","subject":"[PATCH v3 05/13] merge-ort: support subtree shifting","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-03-20T00:03:48Z","receivedAt":"2021-03-20T00:05:10Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nmerge-recursive has some simple code to support subtree shifting; copy\nit over to merge-ort. This fixes t6409.12 under\nGIT_TEST_MERGE_ALGORITHM=ort.\n\nSigned-off-by: Elijah Newren \n---\n merge-ort.c | 24 ++++++++++++++++++++++++\n 1 file changed, 24 insertions(+)\n\ndiff --git a/merge-ort.c b/merge-ort.c\nindex c7083e3769aa..c4fe234d8972 100644\n--- a/merge-ort.c\n+++ b/merge-ort.c\n@@ -3551,6 +3551,23 @@ void merge_finalize(struct merge_options *opt,\n \n /*** Function Grouping: helper functions for merge_incore_*() ***/\n \n+static struct tree *shift_tree_object(struct repository *repo,\n+\t\t\t\t struct tree *one, struct tree *two,\n+\t\t\t\t const char *subtree_shift)\n+{\n+\tstruct object_id shifted;\n+\n+\tif (!*subtree_shift) {\n+\t\tshift_tree(repo, &one->object.oid, &two->object.oid, &shifted, 0);\n+\t} else {\n+\t\tshift_tree_by(repo, &one->object.oid, &two->object.oid, &shifted,\n+\t\t\t subtree_shift);\n+\t}\n+\tif (oideq(&two->object.oid, &shifted))\n+\t\treturn two;\n+\treturn lookup_tree(repo, &shifted);\n+}\n+\n static inline void set_commit_tree(struct commit *c, struct tree *t)\n {\n \tc->maybe_tree = t;\n@@ -3680,6 +3697,13 @@ static void merge_ort_nonrecursive_internal(struct merge_options *opt,\n {\n \tstruct object_id working_tree_oid;\n \n+\tif (opt->subtree_shift) {\n+\t\tside2 = shift_tree_object(opt->repo, side1, side2,\n+\t\t\t\t\t opt->subtree_shift);\n+\t\tmerge_base = shift_tree_object(opt->repo, side1, merge_base,\n+\t\t\t\t\t opt->subtree_shift);\n+\t}\n+\n \ttrace2_region_enter(\"merge\", \"collect_merge_info\", opt->repo);\n \tif (collect_merge_info(opt, merge_base, side1, side2) != 0) {\n \t\t/*\n-- \ngitgitgadget\n\n"},{"id":"419747","messageId":"6d34cc466bd53e074c690b58ef03391eac10e5d9.1616198636.git.gitgitgadget@gmail.com","threadId":"55329","inReplyTo":"pull.905.v3.git.1616198636.gitgitgadget@gmail.com","subject":"[PATCH v3 02/13] merge-ort: add a special minimal index just for renormalization","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-03-20T00:03:45Z","receivedAt":"2021-03-20T00:05:10Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nrenormalize_buffer() requires an index_state, which is something that\nmerge-ort does not operate with. However, all the renormalization code\nneeds is an index with a .gitattributes file...plus a little bit of\nsetup. Create such an index, along with the deallocation and\nattr_direction handling.\n\nA subsequent commit will add a function to finish the initialization\nof this index.\n\nSigned-off-by: Elijah Newren \n---\n merge-ort.c | 20 ++++++++++++++++++++\n 1 file changed, 20 insertions(+)\n\ndiff --git a/merge-ort.c b/merge-ort.c\nindex 34a91c435737..3c606fa7e4b3 100644\n--- a/merge-ort.c\n+++ b/merge-ort.c\n@@ -18,6 +18,7 @@\n #include \"merge-ort.h\"\n \n #include \"alloc.h\"\n+#include \"attr.h\"\n #include \"blob.h\"\n #include \"cache-tree.h\"\n #include \"commit.h\"\n@@ -220,6 +221,16 @@ struct merge_options_internal {\n \t */\n \tstruct rename_info renames;\n \n+\t/*\n+\t * attr_index: hacky minimal index used for renormalization\n+\t *\n+\t * renormalization code _requires_ an index, though it only needs to\n+\t * find a .gitattributes file within the index. So, when\n+\t * renormalization is important, we create a special index with just\n+\t * that one file.\n+\t */\n+\tstruct index_state attr_index;\n+\n \t/*\n \t * current_dir_name, toplevel_dir: temporary vars\n \t *\n@@ -399,6 +410,9 @@ static void clear_or_reinit_internal_opts(struct merge_options_internal *opti,\n \tstring_list_clear(&opti->paths_to_free, 0);\n \topti->paths_to_free.strdup_strings = 0;\n \n+\tif (opti->attr_index.cache_nr)\n+\t\tdiscard_index(&opti->attr_index);\n+\n \t/* Free memory used by various renames maps */\n \tfor (i = MERGE_SIDE1; i <= MERGE_SIDE2; ++i) {\n \t\tstrintmap_func(&renames->dirs_removed[i]);\n@@ -3407,6 +3421,8 @@ void merge_finalize(struct merge_options *opt,\n {\n \tstruct merge_options_internal *opti = result->priv;\n \n+\tif (opt->renormalize)\n+\t\tgit_attr_set_direction(GIT_ATTR_CHECKIN);\n \tassert(opt->priv == NULL);\n \n \tclear_or_reinit_internal_opts(opti, 0);\n@@ -3482,6 +3498,10 @@ static void merge_start(struct merge_options *opt, struct merge_result *result)\n \t/* Default to histogram diff. Actually, just hardcode it...for now. */\n \topt->xdl_opts = DIFF_WITH_ALG(opt, HISTOGRAM_DIFF);\n \n+\t/* Handle attr direction stuff for renormalization */\n+\tif (opt->renormalize)\n+\t\tgit_attr_set_direction(GIT_ATTR_CHECKOUT);\n+\n \t/* Initialization of opt->priv, our internal merge data */\n \ttrace2_region_enter(\"merge\", \"allocate/init\", opt->repo);\n \tif (opt->priv) {\n-- \ngitgitgadget\n\n"},{"id":"419748","messageId":"fe3baf696785c4b7bee8ae93e4567c2ee2a03492.1616198636.git.gitgitgadget@gmail.com","threadId":"55329","inReplyTo":"pull.905.v3.git.1616198636.gitgitgadget@gmail.com","subject":"[PATCH v3 07/13] merge-ort: implement CE_SKIP_WORKTREE handling with conflicted entries","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-03-20T00:03:50Z","receivedAt":"2021-03-20T00:05:10Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nWhen merge conflicts occur in paths removed by a sparse-checkout, we\nneed to unsparsify those paths (clear the SKIP_WORKTREE bit), and write\nout the conflicted file to the working copy. In the very unlikely case\nthat someone manually put a file into the working copy at the location\nof the SKIP_WORKTREE file, we need to avoid overwriting whatever edits\nthey have made and move that file to a different location first.\n\nSigned-off-by: Elijah Newren \n---\n merge-ort.c | 43 +++++++++++++++++++++----------\n t/t6428-merge-conflicts-sparse.sh | 4 +--\n 2 files changed, 32 insertions(+), 15 deletions(-)\n\ndiff --git a/merge-ort.c b/merge-ort.c\nindex c4fe234d8972..303e89414274 100644\n--- a/merge-ort.c\n+++ b/merge-ort.c\n@@ -3369,23 +3369,27 @@ static int checkout(struct merge_options *opt,\n \treturn ret;\n }\n \n-static int record_conflicted_index_entries(struct merge_options *opt,\n-\t\t\t\t\t struct index_state *index,\n-\t\t\t\t\t struct strmap *paths,\n-\t\t\t\t\t struct strmap *conflicted)\n+static int record_conflicted_index_entries(struct merge_options *opt)\n {\n \tstruct hashmap_iter iter;\n \tstruct strmap_entry *e;\n+\tstruct index_state *index = opt->repo->index;\n+\tstruct checkout state = CHECKOUT_INIT;\n \tint errs = 0;\n \tint original_cache_nr;\n \n-\tif (strmap_empty(conflicted))\n+\tif (strmap_empty(&opt->priv->conflicted))\n \t\treturn 0;\n \n+\t/* If any entries have skip_worktree set, we'll have to check 'em out */\n+\tstate.force = 1;\n+\tstate.quiet = 1;\n+\tstate.refresh_cache = 1;\n+\tstate.istate = index;\n \toriginal_cache_nr = index->cache_nr;\n \n \t/* Put every entry from paths into plist, then sort */\n-\tstrmap_for_each_entry(conflicted, &iter, e) {\n+\tstrmap_for_each_entry(&opt->priv->conflicted, &iter, e) {\n \t\tconst char *path = e->key;\n \t\tstruct conflict_info *ci = e->value;\n \t\tint pos;\n@@ -3426,9 +3430,23 @@ static int record_conflicted_index_entries(struct merge_options *opt,\n \t\t\t * the higher order stages. Thus, we need override\n \t\t\t * the CE_SKIP_WORKTREE bit and manually write those\n \t\t\t * files to the working disk here.\n-\t\t\t *\n-\t\t\t * TODO: Implement this CE_SKIP_WORKTREE fixup.\n \t\t\t */\n+\t\t\tif (ce_skip_worktree(ce)) {\n+\t\t\t\tstruct stat st;\n+\n+\t\t\t\tif (!lstat(path, &st)) {\n+\t\t\t\t\tchar *new_name = unique_path(&opt->priv->paths,\n+\t\t\t\t\t\t\t\t path,\n+\t\t\t\t\t\t\t\t \"cruft\");\n+\n+\t\t\t\t\tpath_msg(opt, path, 1,\n+\t\t\t\t\t\t _(\"Note: %s not up to date and in way of checking out conflicted version; old copy renamed to %s\"),\n+\t\t\t\t\t\t path, new_name);\n+\t\t\t\t\terrs |= rename(path, new_name);\n+\t\t\t\t\tfree(new_name);\n+\t\t\t\t}\n+\t\t\t\terrs |= checkout_entry(ce, &state, NULL, NULL);\n+\t\t\t}\n \n \t\t\t/*\n \t\t\t * Mark this cache entry for removal and instead add\n@@ -3478,8 +3496,6 @@ void merge_switch_to_result(struct merge_options *opt,\n {\n \tassert(opt->priv == NULL);\n \tif (result->clean >= 0 && update_worktree_and_index) {\n-\t\tstruct merge_options_internal *opti = result->priv;\n-\n \t\ttrace2_region_enter(\"merge\", \"checkout\", opt->repo);\n \t\tif (checkout(opt, head, result->tree)) {\n \t\t\t/* failure to function */\n@@ -3489,13 +3505,14 @@ void merge_switch_to_result(struct merge_options *opt,\n \t\ttrace2_region_leave(\"merge\", \"checkout\", opt->repo);\n \n \t\ttrace2_region_enter(\"merge\", \"record_conflicted\", opt->repo);\n-\t\tif (record_conflicted_index_entries(opt, opt->repo->index,\n-\t\t\t\t\t\t &opti->paths,\n-\t\t\t\t\t\t &opti->conflicted)) {\n+\t\topt->priv = result->priv;\n+\t\tif (record_conflicted_index_entries(opt)) {\n \t\t\t/* failure to function */\n+\t\t\topt->priv = NULL;\n \t\t\tresult->clean = -1;\n \t\t\treturn;\n \t\t}\n+\t\topt->priv = NULL;\n \t\ttrace2_region_leave(\"merge\", \"record_conflicted\", opt->repo);\n \t}\n \ndiff --git a/t/t6428-merge-conflicts-sparse.sh b/t/t6428-merge-conflicts-sparse.sh\nindex 1bb52ff6f38c..7e8bf497f821 100755\n--- a/t/t6428-merge-conflicts-sparse.sh\n+++ b/t/t6428-merge-conflicts-sparse.sh\n@@ -76,7 +76,7 @@ test_setup_numerals () {\n \t)\n }\n \n-test_expect_merge_algorithm success failure 'conflicting entries written to worktree even if sparse' '\n+test_expect_success 'conflicting entries written to worktree even if sparse' '\n \ttest_setup_numerals plain &&\n \t(\n \t\tcd numerals_plain &&\n@@ -112,7 +112,7 @@ test_expect_merge_algorithm success failure 'conflicting entries written to work\n \t)\n '\n \n-test_expect_merge_algorithm failure failure 'present-despite-SKIP_WORKTREE handled reasonably' '\n+test_expect_merge_algorithm failure success 'present-despite-SKIP_WORKTREE handled reasonably' '\n \ttest_setup_numerals in_the_way &&\n \t(\n \t\tcd numerals_in_the_way &&\n-- \ngitgitgadget\n\n"},{"id":"419749","messageId":"4a79e61346919291f49885298da4b8c714ee00ff.1616198636.git.gitgitgadget@gmail.com","threadId":"55329","inReplyTo":"pull.905.v3.git.1616198636.gitgitgadget@gmail.com","subject":"[PATCH v3 09/13] merge-ort: write $GIT_DIR/AUTO_MERGE whenever we hit a conflict","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-03-20T00:03:52Z","receivedAt":"2021-03-20T00:05:10Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nThere are a variety of questions users might ask while resolving\nconflicts:\n * What changes have been made since the previous (first) parent?\n * What changes are staged?\n * What is still unstaged? (or what is still conflicted?)\n * What changes did I make to resolve conflicts so far?\nThe first three of these have simple answers:\n * git diff HEAD\n * git diff --cached\n * git diff\nThere was no way to answer the final question previously. Adding one\nis trivial in merge-ort, since it works by creating a tree representing\nwhat should be written to the working copy complete with conflict\nmarkers. Simply write that tree to .git/AUTO_MERGE, allowing users to\nanswer the fourth question with\n * git diff AUTO_MERGE\n\nI avoided using a name like \"MERGE_AUTO\", because that would be\nmerge-specific (much like MERGE_HEAD, REBASE_HEAD, REVERT_HEAD,\nCHERRY_PICK_HEAD) and I wanted a name that didn't change depending on\nwhich type of operation the merge was part of.\n\nEnsure that paths which clean out other temporary operation-specific\nfiles (e.g. CHERRY_PICK_HEAD, MERGE_MSG, rebase-merge/ state directory)\nalso clean out this AUTO_MERGE file.\n\nSigned-off-by: Elijah Newren \n---\n branch.c | 1 +\n builtin/rebase.c | 1 +\n merge-ort.c | 10 ++++++++++\n path.c | 1 +\n path.h | 2 ++\n sequencer.c | 5 +++++\n 6 files changed, 20 insertions(+)\n\ndiff --git a/branch.c b/branch.c\nindex 9c9dae1eae32..b71a2de29dbe 100644\n--- a/branch.c\n+++ b/branch.c\n@@ -344,6 +344,7 @@ void remove_merge_branch_state(struct repository *r)\n \tunlink(git_path_merge_rr(r));\n \tunlink(git_path_merge_msg(r));\n \tunlink(git_path_merge_mode(r));\n+\tunlink(git_path_auto_merge(r));\n \tsave_autostash(git_path_merge_autostash(r));\n }\n \ndiff --git a/builtin/rebase.c b/builtin/rebase.c\nindex 840dbd7eb777..6c252d62758c 100644\n--- a/builtin/rebase.c\n+++ b/builtin/rebase.c\n@@ -737,6 +737,7 @@ static int finish_rebase(struct rebase_options *opts)\n \tint ret = 0;\n \n \tdelete_ref(NULL, \"REBASE_HEAD\", NULL, REF_NO_DEREF);\n+\tunlink(git_path_auto_merge(the_repository));\n \tapply_autostash(state_dir_path(\"autostash\", opts));\n \tclose_object_store(the_repository->objects);\n \t/*\ndiff --git a/merge-ort.c b/merge-ort.c\nindex 303e89414274..e8f1a435f99a 100644\n--- a/merge-ort.c\n+++ b/merge-ort.c\n@@ -3496,6 +3496,9 @@ void merge_switch_to_result(struct merge_options *opt,\n {\n \tassert(opt->priv == NULL);\n \tif (result->clean >= 0 && update_worktree_and_index) {\n+\t\tconst char *filename;\n+\t\tFILE *fp;\n+\n \t\ttrace2_region_enter(\"merge\", \"checkout\", opt->repo);\n \t\tif (checkout(opt, head, result->tree)) {\n \t\t\t/* failure to function */\n@@ -3514,6 +3517,13 @@ void merge_switch_to_result(struct merge_options *opt,\n \t\t}\n \t\topt->priv = NULL;\n \t\ttrace2_region_leave(\"merge\", \"record_conflicted\", opt->repo);\n+\n+\t\ttrace2_region_enter(\"merge\", \"write_auto_merge\", opt->repo);\n+\t\tfilename = git_path_auto_merge(opt->repo);\n+\t\tfp = xfopen(filename, \"w\");\n+\t\tfprintf(fp, \"%s\\n\", oid_to_hex(&result->tree->object.oid));\n+\t\tfclose(fp);\n+\t\ttrace2_region_leave(\"merge\", \"write_auto_merge\", opt->repo);\n \t}\n \n \tif (display_update_msgs) {\ndiff --git a/path.c b/path.c\nindex 7b385e5eb282..9e883eb52446 100644\n--- a/path.c\n+++ b/path.c\n@@ -1534,5 +1534,6 @@ REPO_GIT_PATH_FUNC(merge_rr, \"MERGE_RR\")\n REPO_GIT_PATH_FUNC(merge_mode, \"MERGE_MODE\")\n REPO_GIT_PATH_FUNC(merge_head, \"MERGE_HEAD\")\n REPO_GIT_PATH_FUNC(merge_autostash, \"MERGE_AUTOSTASH\")\n+REPO_GIT_PATH_FUNC(auto_merge, \"AUTO_MERGE\")\n REPO_GIT_PATH_FUNC(fetch_head, \"FETCH_HEAD\")\n REPO_GIT_PATH_FUNC(shallow, \"shallow\")\ndiff --git a/path.h b/path.h\nindex e7e77da6aaa5..251c78d98000 100644\n--- a/path.h\n+++ b/path.h\n@@ -176,6 +176,7 @@ struct path_cache {\n \tconst char *merge_mode;\n \tconst char *merge_head;\n \tconst char *merge_autostash;\n+\tconst char *auto_merge;\n \tconst char *fetch_head;\n \tconst char *shallow;\n };\n@@ -191,6 +192,7 @@ const char *git_path_merge_rr(struct repository *r);\n const char *git_path_merge_mode(struct repository *r);\n const char *git_path_merge_head(struct repository *r);\n const char *git_path_merge_autostash(struct repository *r);\n+const char *git_path_auto_merge(struct repository *r);\n const char *git_path_fetch_head(struct repository *r);\n const char *git_path_shallow(struct repository *r);\n \ndiff --git a/sequencer.c b/sequencer.c\nindex d2332d3e1787..472cdd8c620d 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -2096,6 +2096,7 @@ static int do_pick_commit(struct repository *r,\n \t\trefs_delete_ref(get_main_ref_store(r), \"\", \"CHERRY_PICK_HEAD\",\n \t\t\t\tNULL, 0);\n \t\tunlink(git_path_merge_msg(r));\n+\t\tunlink(git_path_auto_merge(r));\n \t\tfprintf(stderr,\n \t\t\t_(\"dropping %s %s -- patch contents already upstream\\n\"),\n \t\t\toid_to_hex(&commit->object.oid), msg.subject);\n@@ -2451,6 +2452,8 @@ void sequencer_post_commit_cleanup(struct repository *r, int verbose)\n \t\tneed_cleanup = 1;\n \t}\n \n+\tunlink(git_path_auto_merge(r));\n+\n \tif (!need_cleanup)\n \t\treturn;\n \n@@ -4111,6 +4114,7 @@ static int pick_commits(struct repository *r,\n \t\t\tunlink(rebase_path_stopped_sha());\n \t\t\tunlink(rebase_path_amend());\n \t\t\tunlink(git_path_merge_head(r));\n+\t\t\tunlink(git_path_auto_merge(r));\n \t\t\tdelete_ref(NULL, \"REBASE_HEAD\", NULL, REF_NO_DEREF);\n \n \t\t\tif (item->command == TODO_BREAK) {\n@@ -4505,6 +4509,7 @@ static int commit_staged_changes(struct repository *r,\n \t\treturn error(_(\"could not commit staged changes.\"));\n \tunlink(rebase_path_amend());\n \tunlink(git_path_merge_head(r));\n+\tunlink(git_path_auto_merge(r));\n \tif (final_fixup) {\n \t\tunlink(rebase_path_fixup_msg());\n \t\tunlink(rebase_path_squash_msg());\n-- \ngitgitgadget\n\n"},{"id":"419750","messageId":"c1c9605c1932cab6191f47beb992a15c64ada237.1616198636.git.gitgitgadget@gmail.com","threadId":"55329","inReplyTo":"pull.905.v3.git.1616198636.gitgitgadget@gmail.com","subject":"[PATCH v3 04/13] merge-ort: let renormalization change modify/delete into clean delete","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-03-20T00:03:47Z","receivedAt":"2021-03-20T00:05:10Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nWhen we have a modify/delete conflict, but the only change to the\nmodification is e.g. change of line endings, then if renormalization is\nrequested then we should be able to recognize such a case as a\nnot-modified/delete and resolve the conflict automatically.\n\nThis fixes t6418.10 under GIT_TEST_MERGE_ALGORITHM=ort.\n\nSigned-off-by: Elijah Newren \n---\n merge-ort.c | 64 +++++++++++++++++++++++++++++++++++++++++++++++++++--\n 1 file changed, 62 insertions(+), 2 deletions(-)\n\ndiff --git a/merge-ort.c b/merge-ort.c\nindex cdc1e2fe7a24..c7083e3769aa 100644\n--- a/merge-ort.c\n+++ b/merge-ort.c\n@@ -2549,6 +2549,61 @@ static int string_list_df_name_compare(const char *one, const char *two)\n \treturn onelen - twolen;\n }\n \n+static int read_oid_strbuf(struct merge_options *opt,\n+\t\t\t const struct object_id *oid,\n+\t\t\t struct strbuf *dst)\n+{\n+\tvoid *buf;\n+\tenum object_type type;\n+\tunsigned long size;\n+\tbuf = read_object_file(oid, &type, &size);\n+\tif (!buf)\n+\t\treturn err(opt, _(\"cannot read object %s\"), oid_to_hex(oid));\n+\tif (type != OBJ_BLOB) {\n+\t\tfree(buf);\n+\t\treturn err(opt, _(\"object %s is not a blob\"), oid_to_hex(oid));\n+\t}\n+\tstrbuf_attach(dst, buf, size, size + 1);\n+\treturn 0;\n+}\n+\n+static int blob_unchanged(struct merge_options *opt,\n+\t\t\t const struct version_info *base,\n+\t\t\t const struct version_info *side,\n+\t\t\t const char *path)\n+{\n+\tstruct strbuf basebuf = STRBUF_INIT;\n+\tstruct strbuf sidebuf = STRBUF_INIT;\n+\tint ret = 0; /* assume changed for safety */\n+\tconst struct index_state *idx = &opt->priv->attr_index;\n+\n+\tif (!idx->initialized)\n+\t\tinitialize_attr_index(opt);\n+\n+\tif (base->mode != side->mode)\n+\t\treturn 0;\n+\tif (oideq(&base->oid, &side->oid))\n+\t\treturn 1;\n+\n+\tif (read_oid_strbuf(opt, &base->oid, &basebuf) ||\n+\t read_oid_strbuf(opt, &side->oid, &sidebuf))\n+\t\tgoto error_return;\n+\t/*\n+\t * Note: binary | is used so that both renormalizations are\n+\t * performed. Comparison can be skipped if both files are\n+\t * unchanged since their sha1s have already been compared.\n+\t */\n+\tif (renormalize_buffer(idx, path, basebuf.buf, basebuf.len, &basebuf) |\n+\t renormalize_buffer(idx, path, sidebuf.buf, sidebuf.len, &sidebuf))\n+\t\tret = (basebuf.len == sidebuf.len &&\n+\t\t !memcmp(basebuf.buf, sidebuf.buf, basebuf.len));\n+\n+error_return:\n+\tstrbuf_release(&basebuf);\n+\tstrbuf_release(&sidebuf);\n+\treturn ret;\n+}\n+\n struct directory_versions {\n \t/*\n \t * versions: list of (basename -> version_info)\n@@ -3136,8 +3191,13 @@ static void process_entry(struct merge_options *opt,\n \t\tmodify_branch = (side == 1) ? opt->branch1 : opt->branch2;\n \t\tdelete_branch = (side == 1) ? opt->branch2 : opt->branch1;\n \n-\t\tif (ci->path_conflict &&\n-\t\t oideq(&ci->stages[0].oid, &ci->stages[side].oid)) {\n+\t\tif (opt->renormalize &&\n+\t\t blob_unchanged(opt, &ci->stages[0], &ci->stages[side],\n+\t\t\t\t path)) {\n+\t\t\tci->merged.is_null = 1;\n+\t\t\tci->merged.clean = 1;\n+\t\t} else if (ci->path_conflict &&\n+\t\t\t oideq(&ci->stages[0].oid, &ci->stages[side].oid)) {\n \t\t\t/*\n \t\t\t * This came from a rename/delete; no action to take,\n \t\t\t * but avoid printing \"modify/delete\" conflict notice\n-- \ngitgitgadget\n\n"},{"id":"419752","messageId":"6aec1f499b8068329b81fb1221717c2ee336fd8d.1616198636.git.gitgitgadget@gmail.com","threadId":"55329","inReplyTo":"pull.905.v3.git.1616198636.gitgitgadget@gmail.com","subject":"[PATCH v3 06/13] t6428: new test for SKIP_WORKTREE handling and conflicts","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-03-20T00:03:49Z","receivedAt":"2021-03-20T00:05:10Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nIf there is a conflict during a merge for a SKIP_WORKTREE entry, we\nexpect that file to be written to the working copy and have the\nSKIP_WORKTREE bit cleared in the index. If the user had manually\ncreated a file in the working tree despite SKIP_WORKTREE being set, we\ndo not want to clobber their changes to that file, but want to move it\nout of the way. Add tests that check for these behaviors.\n\nSigned-off-by: Elijah Newren \n---\n t/t6428-merge-conflicts-sparse.sh | 158 ++++++++++++++++++++++++++++++\n 1 file changed, 158 insertions(+)\n create mode 100755 t/t6428-merge-conflicts-sparse.sh\n\ndiff --git a/t/t6428-merge-conflicts-sparse.sh b/t/t6428-merge-conflicts-sparse.sh\nnew file mode 100755\nindex 000000000000..1bb52ff6f38c\n--- /dev/null\n+++ b/t/t6428-merge-conflicts-sparse.sh\n@@ -0,0 +1,158 @@\n+#!/bin/sh\n+\n+test_description=\"merge cases\"\n+\n+# The setup for all of them, pictorially, is:\n+#\n+# A\n+# o\n+# / \\\n+# O o ?\n+# \\ /\n+# o\n+# B\n+#\n+# To help make it easier to follow the flow of tests, they have been\n+# divided into sections and each test will start with a quick explanation\n+# of what commits O, A, and B contain.\n+#\n+# Notation:\n+# z/{b,c} means files z/b and z/c both exist\n+# x/d_1 means file x/d exists with content d1. (Purpose of the\n+# underscore notation is to differentiate different\n+# files that might be renamed into each other's paths.)\n+\n+. ./test-lib.sh\n+. \"$TEST_DIRECTORY\"/lib-merge.sh\n+\n+\n+# Testcase basic, conflicting changes in 'numerals'\n+\n+test_setup_numerals () {\n+\ttest_create_repo numerals_$1 &&\n+\t(\n+\t\tcd numerals_$1 &&\n+\n+\t\t>README &&\n+\t\ttest_write_lines I II III >numerals &&\n+\t\tgit add README numerals &&\n+\t\ttest_tick &&\n+\t\tgit commit -m \"O\" &&\n+\n+\t\tgit branch O &&\n+\t\tgit branch A &&\n+\t\tgit branch B &&\n+\n+\t\tgit checkout A &&\n+\t\ttest_write_lines I II III IIII >numerals &&\n+\t\tgit add numerals &&\n+\t\ttest_tick &&\n+\t\tgit commit -m \"A\" &&\n+\n+\t\tgit checkout B &&\n+\t\ttest_write_lines I II III IV >numerals &&\n+\t\tgit add numerals &&\n+\t\ttest_tick &&\n+\t\tgit commit -m \"B\" &&\n+\n+\t\tcat <<-EOF >expected-index &&\n+\t\tH README\n+\t\tM numerals\n+\t\tM numerals\n+\t\tM numerals\n+\t\tEOF\n+\n+\t\tcat <<-EOF >expected-merge\n+\t\tI\n+\t\tII\n+\t\tIII\n+\t\t<<<<<<< HEAD\n+\t\tIIII\n+\t\t=======\n+\t\tIV\n+\t\t>>>>>>> B^0\n+\t\tEOF\n+\n+\t)\n+}\n+\n+test_expect_merge_algorithm success failure 'conflicting entries written to worktree even if sparse' '\n+\ttest_setup_numerals plain &&\n+\t(\n+\t\tcd numerals_plain &&\n+\n+\t\tgit checkout A^0 &&\n+\n+\t\ttest_path_is_file README &&\n+\t\ttest_path_is_file numerals &&\n+\n+\t\tgit sparse-checkout init &&\n+\t\tgit sparse-checkout set README &&\n+\n+\t\ttest_path_is_file README &&\n+\t\ttest_path_is_missing numerals &&\n+\n+\t\ttest_must_fail git merge -s recursive B^0 &&\n+\n+\t\tgit ls-files -t >index_files &&\n+\t\ttest_cmp expected-index index_files &&\n+\n+\t\ttest_path_is_file README &&\n+\t\ttest_path_is_file numerals &&\n+\n+\t\ttest_cmp expected-merge numerals &&\n+\n+\t\t# 4 other files:\n+\t\t# * expected-merge\n+\t\t# * expected-index\n+\t\t# * index_files\n+\t\t# * others\n+\t\tgit ls-files -o >others &&\n+\t\ttest_line_count = 4 others\n+\t)\n+'\n+\n+test_expect_merge_algorithm failure failure 'present-despite-SKIP_WORKTREE handled reasonably' '\n+\ttest_setup_numerals in_the_way &&\n+\t(\n+\t\tcd numerals_in_the_way &&\n+\n+\t\tgit checkout A^0 &&\n+\n+\t\ttest_path_is_file README &&\n+\t\ttest_path_is_file numerals &&\n+\n+\t\tgit sparse-checkout init &&\n+\t\tgit sparse-checkout set README &&\n+\n+\t\ttest_path_is_file README &&\n+\t\ttest_path_is_missing numerals &&\n+\n+\t\techo foobar >numerals &&\n+\n+\t\ttest_must_fail git merge -s recursive B^0 &&\n+\n+\t\tgit ls-files -t >index_files &&\n+\t\ttest_cmp expected-index index_files &&\n+\n+\t\ttest_path_is_file README &&\n+\t\ttest_path_is_file numerals &&\n+\n+\t\ttest_cmp expected-merge numerals &&\n+\n+\t\t# There should still be a file with \"foobar\" in it\n+\t\tgrep foobar * &&\n+\n+\t\t# 5 other files:\n+\t\t# * expected-merge\n+\t\t# * expected-index\n+\t\t# * index_files\n+\t\t# * others\n+\t\t# * whatever name was given to the numerals file that had\n+\t\t# \"foobar\" in it\n+\t\tgit ls-files -o >others &&\n+\t\ttest_line_count = 5 others\n+\t)\n+'\n+\n+test_done\n-- \ngitgitgadget\n\n"},{"id":"419751","messageId":"6bda855f29806475be14d133e185fec222c8b0be.1616198636.git.gitgitgadget@gmail.com","threadId":"55329","inReplyTo":"pull.905.v3.git.1616198636.gitgitgadget@gmail.com","subject":"[PATCH v3 11/13] Revert \"merge-ort: ignore the directory rename split conflict for now\"","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-03-20T00:03:54Z","receivedAt":"2021-03-20T00:05:11Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nThis reverts commit 5ced7c3da009090c5a926e3123a71314c7f28d42, which was\nput in place as a temporary measure to avoid optimizations unstably\nerroring out on no destination having a majority of the necessary\nrenames for directories that had no new files and thus no need for\ndirectory rename detection anyway. Now that optimizations are in place\nto prevent us from trying to compute directory rename count computations\nfor directories that do not need it, we can undo this temporary measure.\n\nSigned-off-by: Elijah Newren \n---\n merge-ort.c | 13 +------------\n 1 file changed, 1 insertion(+), 12 deletions(-)\n\ndiff --git a/merge-ort.c b/merge-ort.c\nindex e8f1a435f99a..8258d3fd621e 100644\n--- a/merge-ort.c\n+++ b/merge-ort.c\n@@ -1636,18 +1636,7 @@ static void get_provisional_directory_renames(struct merge_options *opt,\n \t\t\t\t \"no destination getting a majority of the \"\n \t\t\t\t \"files.\"),\n \t\t\t source_dir);\n-\t\t\t/*\n-\t\t\t * We should mark this as unclean IF something attempts\n-\t\t\t * to use this rename. We do not yet have the logic\n-\t\t\t * in place to detect if this directory rename is being\n-\t\t\t * used, and optimizations that reduce the number of\n-\t\t\t * renames cause this to falsely trigger. For now,\n-\t\t\t * just disable it, causing t6423 testcase 2a to break.\n-\t\t\t * We'll later fix the detection, and when we do we\n-\t\t\t * will re-enable setting *clean to 0 (and thereby fix\n-\t\t\t * t6423 testcase 2a).\n-\t\t\t */\n-\t\t\t/* *clean = 0; */\n+\t\t\t*clean = 0;\n \t\t} else {\n \t\t\tstrmap_put(&renames->dir_renames[side],\n \t\t\t\t source_dir, (void*)best);\n-- \ngitgitgadget\n\n"},{"id":"419753","messageId":"f9325647a9fca1471fead860c18db86aba34eb54.1616198636.git.gitgitgadget@gmail.com","threadId":"55329","inReplyTo":"pull.905.v3.git.1616198636.gitgitgadget@gmail.com","subject":"[PATCH v3 08/13] t: mark several submodule merging tests as fixed under merge-ort","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-03-20T00:03:51Z","receivedAt":"2021-03-20T00:05:11Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nmerge-ort handles submodules (and directory/file conflicts in general)\ndifferently than merge-recursive does; it basically puts all the special\nhandling for different filetypes into one place in the codebase instead\nof needing special handling for different filetypes in many different\ncode paths. This one code path in merge-ort could perhaps use some work\nstill (there are still test_expect_failure cases in the testsuite), but\nit passes all the tests that merge-recursive does as well as 12\nadditional ones that merge-recursive fails. Mark those 12 tests as\ntest_expect_success under merge-ort.\n\nSigned-off-by: Elijah Newren \n---\n t/t3512-cherry-pick-submodule.sh | 7 +++++--\n t/t3513-revert-submodule.sh | 5 ++++-\n t/t5572-pull-submodule.sh | 7 +++++--\n t/t6437-submodule-merge.sh | 5 +++--\n t/t6438-submodule-directory-file-conflicts.sh | 7 +++++--\n 5 files changed, 22 insertions(+), 9 deletions(-)\n\ndiff --git a/t/t3512-cherry-pick-submodule.sh b/t/t3512-cherry-pick-submodule.sh\nindex 822f2d4bfbd5..c657840db33b 100755\n--- a/t/t3512-cherry-pick-submodule.sh\n+++ b/t/t3512-cherry-pick-submodule.sh\n@@ -8,8 +8,11 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n . ./test-lib.sh\n . \"$TEST_DIRECTORY\"/lib-submodule-update.sh\n \n-KNOWN_FAILURE_NOFF_MERGE_DOESNT_CREATE_EMPTY_SUBMODULE_DIR=1\n-KNOWN_FAILURE_NOFF_MERGE_ATTEMPTS_TO_MERGE_REMOVED_SUBMODULE_FILES=1\n+if test \"$GIT_TEST_MERGE_ALGORITHM\" != ort\n+then\n+\tKNOWN_FAILURE_NOFF_MERGE_DOESNT_CREATE_EMPTY_SUBMODULE_DIR=1\n+\tKNOWN_FAILURE_NOFF_MERGE_ATTEMPTS_TO_MERGE_REMOVED_SUBMODULE_FILES=1\n+fi\n test_submodule_switch \"cherry-pick\"\n \n test_expect_success 'unrelated submodule/file conflict is ignored' '\ndiff --git a/t/t3513-revert-submodule.sh b/t/t3513-revert-submodule.sh\nindex a759f12cbb1d..74cd96e58223 100755\n--- a/t/t3513-revert-submodule.sh\n+++ b/t/t3513-revert-submodule.sh\n@@ -30,7 +30,10 @@ git_revert () {\n \tgit revert HEAD\n }\n \n-KNOWN_FAILURE_NOFF_MERGE_DOESNT_CREATE_EMPTY_SUBMODULE_DIR=1\n+if test \"$GIT_TEST_MERGE_ALGORITHM\" != ort\n+then\n+\tKNOWN_FAILURE_NOFF_MERGE_DOESNT_CREATE_EMPTY_SUBMODULE_DIR=1\n+fi\n test_submodule_switch_func \"git_revert\"\n \n test_done\ndiff --git a/t/t5572-pull-submodule.sh b/t/t5572-pull-submodule.sh\nindex 29537f4798ef..4f92a116e1f0 100755\n--- a/t/t5572-pull-submodule.sh\n+++ b/t/t5572-pull-submodule.sh\n@@ -42,8 +42,11 @@ git_pull_noff () {\n \t$2 git pull --no-ff\n }\n \n-KNOWN_FAILURE_NOFF_MERGE_DOESNT_CREATE_EMPTY_SUBMODULE_DIR=1\n-KNOWN_FAILURE_NOFF_MERGE_ATTEMPTS_TO_MERGE_REMOVED_SUBMODULE_FILES=1\n+if test \"$GIT_TEST_MERGE_ALGORITHM\" != ort\n+then\n+\tKNOWN_FAILURE_NOFF_MERGE_DOESNT_CREATE_EMPTY_SUBMODULE_DIR=1\n+\tKNOWN_FAILURE_NOFF_MERGE_ATTEMPTS_TO_MERGE_REMOVED_SUBMODULE_FILES=1\n+fi\n test_submodule_switch_func \"git_pull_noff\"\n \n test_expect_success 'pull --recurse-submodule setup' '\ndiff --git a/t/t6437-submodule-merge.sh b/t/t6437-submodule-merge.sh\nindex 0f92bcf326c8..e5e89c2045e7 100755\n--- a/t/t6437-submodule-merge.sh\n+++ b/t/t6437-submodule-merge.sh\n@@ -6,6 +6,7 @@ GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n \n . ./test-lib.sh\n+. \"$TEST_DIRECTORY\"/lib-merge.sh\n \n #\n # history\n@@ -328,7 +329,7 @@ test_expect_success 'setup file/submodule conflict' '\n \t)\n '\n \n-test_expect_failure 'file/submodule conflict' '\n+test_expect_merge_algorithm failure success 'file/submodule conflict' '\n \ttest_when_finished \"git -C file-submodule reset --hard\" &&\n \t(\n \t\tcd file-submodule &&\n@@ -437,7 +438,7 @@ test_expect_failure 'directory/submodule conflict; keep submodule clean' '\n \t)\n '\n \n-test_expect_failure !FAIL_PREREQS 'directory/submodule conflict; should not treat submodule files as untracked or in the way' '\n+test_expect_merge_algorithm failure success !FAIL_PREREQS 'directory/submodule conflict; should not treat submodule files as untracked or in the way' '\n \ttest_when_finished \"git -C directory-submodule/path reset --hard\" &&\n \ttest_when_finished \"git -C directory-submodule reset --hard\" &&\n \t(\ndiff --git a/t/t6438-submodule-directory-file-conflicts.sh b/t/t6438-submodule-directory-file-conflicts.sh\nindex 04bf4be7d792..8df67a0ef99d 100755\n--- a/t/t6438-submodule-directory-file-conflicts.sh\n+++ b/t/t6438-submodule-directory-file-conflicts.sh\n@@ -12,8 +12,11 @@ test_submodule_switch \"merge --ff\"\n \n test_submodule_switch \"merge --ff-only\"\n \n-KNOWN_FAILURE_NOFF_MERGE_DOESNT_CREATE_EMPTY_SUBMODULE_DIR=1\n-KNOWN_FAILURE_NOFF_MERGE_ATTEMPTS_TO_MERGE_REMOVED_SUBMODULE_FILES=1\n+if test \"$GIT_TEST_MERGE_ALGORITHM\" != ort\n+then\n+\tKNOWN_FAILURE_NOFF_MERGE_DOESNT_CREATE_EMPTY_SUBMODULE_DIR=1\n+\tKNOWN_FAILURE_NOFF_MERGE_ATTEMPTS_TO_MERGE_REMOVED_SUBMODULE_FILES=1\n+fi\n test_submodule_switch \"merge --no-ff\"\n \n test_done\n-- \ngitgitgadget\n\n"},{"id":"419754","messageId":"1c6361c9b88ac5f970054989bae0845ef3b6baee.1616198636.git.gitgitgadget@gmail.com","threadId":"55329","inReplyTo":"pull.905.v3.git.1616198636.gitgitgadget@gmail.com","subject":"[PATCH v3 12/13] t6423: mark remaining expected failure under merge-ort as such","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-03-20T00:03:55Z","receivedAt":"2021-03-20T00:05:11Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nWhen we started on merge-ort, thousands of tests failed when run with\nthe GIT_TEST_MERGE_ALGORITHM=ort flag; with so many, it didn't make\nsense to flip all their test expectations. The ones in t6409, t6418,\nand the submodule tests are being handled by an independent in-flight\ntopic (\"Complete merge-ort implemenation...almost\"). The ones in\nt6423 were left out of the other series because other ongoing series\nthat this commit depends upon were addressing those. Now that we only\nhave one remaining test failure in t6423, let's mark it as such.\n\nThis remaining test will be fixed by a future optimization series, but\nsince merge-recursive doesn't pass this test either, passing it is not\nnecessary for declaring merge-ort ready for general use.\n\nSigned-off-by: Elijah Newren \n---\n t/t6423-merge-rename-directories.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t6423-merge-rename-directories.sh b/t/t6423-merge-rename-directories.sh\nindex 4c568050dd27..4c3d0b95dc5c 100755\n--- a/t/t6423-merge-rename-directories.sh\n+++ b/t/t6423-merge-rename-directories.sh\n@@ -4797,7 +4797,7 @@ test_setup_12f () {\n \t)\n }\n \n-test_expect_merge_algorithm failure success '12f: Trivial directory resolve, caching, all kinds of fun' '\n+test_expect_merge_algorithm failure failure '12f: Trivial directory resolve, caching, all kinds of fun' '\n \ttest_setup_12f &&\n \t(\n \t\tcd 12f &&\n-- \ngitgitgadget\n\n"},{"id":"419755","messageId":"c2d2a1ccaea70b7dc8c0539ba9d3a132f9687040.1616198636.git.gitgitgadget@gmail.com","threadId":"55329","inReplyTo":"pull.905.v3.git.1616198636.gitgitgadget@gmail.com","subject":"[PATCH v3 13/13] Add testing with merge-ort merge strategy","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-03-20T00:03:56Z","receivedAt":"2021-03-20T00:05:11Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nIn preparation for switching from merge-recursive to merge-ort as the\ndefault strategy, have the testsuite default to running with merge-ort.\nKeep coverage of the recursive backend by having the linux-gcc job run\nwith it.\n\nSigned-off-by: Elijah Newren \n---\n ci/run-build-and-tests.sh | 1 +\n t/test-lib.sh | 2 ++\n 2 files changed, 3 insertions(+)\n\ndiff --git a/ci/run-build-and-tests.sh b/ci/run-build-and-tests.sh\nindex 50e0b90073f1..2279ff70a735 100755\n--- a/ci/run-build-and-tests.sh\n+++ b/ci/run-build-and-tests.sh\n@@ -16,6 +16,7 @@ linux-gcc)\n \texport GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n \tmake test\n \texport GIT_TEST_SPLIT_INDEX=yes\n+\texport GIT_TEST_MERGE_ALGORITHM=recursive\n \texport GIT_TEST_FULL_IN_PACK_ARRAY=true\n \texport GIT_TEST_OE_SIZE=10\n \texport GIT_TEST_OE_DELTA_SIZE=5\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 431adba0fb3f..6f4185037183 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -448,6 +448,8 @@ export EDITOR\n \n GIT_DEFAULT_HASH=\"${GIT_TEST_DEFAULT_HASH:-sha1}\"\n export GIT_DEFAULT_HASH\n+GIT_TEST_MERGE_ALGORITHM=\"${GIT_TEST_MERGE_ALGORITHM:-ort}\"\n+export GIT_TEST_MERGE_ALGORITHM\n \n # Tests using GIT_TRACE typically don't want : output\n GIT_TRACE_BARE=1\n-- \ngitgitgadget\n"},{"id":"419756","messageId":"a37979454069eef830ff19e70a0e998522d30448.1616198636.git.gitgitgadget@gmail.com","threadId":"55329","inReplyTo":"pull.905.v3.git.1616198636.gitgitgadget@gmail.com","subject":"[PATCH v3 10/13] merge-recursive: add a bunch of FIXME comments documenting known bugs","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-03-20T00:03:53Z","receivedAt":"2021-03-20T00:05:11Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nThe plan is to just delete merge-recursive, but not until everyone is\ncomfortable with merge-ort as a replacement. Given that I haven't\nswitched all callers of merge-recursive over yet (e.g. git-am still uses\nmerge-recursive), maybe there's some value documenting known bugs in the\nalgorithm in case we end up keeping it or someone wants to dig it up in\nthe future.\n\nSigned-off-by: Elijah Newren \n---\n merge-recursive.c | 37 +++++++++++++++++++++++++++++++++++++\n 1 file changed, 37 insertions(+)\n\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex b052974f191c..99a197597db5 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -1075,6 +1075,11 @@ static int merge_3way(struct merge_options *opt,\n \tread_mmblob(&src1, &a->oid);\n \tread_mmblob(&src2, &b->oid);\n \n+\t/*\n+\t * FIXME: Using a->path for normalization rules in ll_merge could be\n+\t * wrong if we renamed from a->path to b->path. We should use the\n+\t * target path for where the file will be written.\n+\t */\n \tmerge_status = ll_merge(result_buf, a->path, &orig, base,\n \t\t\t\t&src1, name1, &src2, name2,\n \t\t\t\topt->repo->index, &ll_opts);\n@@ -1154,6 +1159,8 @@ static void print_commit(struct commit *commit)\n \tstruct strbuf sb = STRBUF_INIT;\n \tstruct pretty_print_context ctx = {0};\n \tctx.date_mode.type = DATE_NORMAL;\n+\t/* FIXME: Merge this with output_commit_title() */\n+\tassert(!merge_remote_util(commit));\n \tformat_commit_message(commit, \" %h: %m %s\", &sb, &ctx);\n \tfprintf(stderr, \"%s\\n\", sb.buf);\n \tstrbuf_release(&sb);\n@@ -1177,6 +1184,11 @@ static int merge_submodule(struct merge_options *opt,\n \tint search = !opt->priv->call_depth;\n \n \t/* store a in result in case we fail */\n+\t/* FIXME: This is the WRONG resolution for the recursive case when\n+\t * we need to be careful to avoid accidentally matching either side.\n+\t * Should probably use o instead there, much like we do for merging\n+\t * binaries.\n+\t */\n \toidcpy(result, a);\n \n \t/* we can not handle deletion conflicts */\n@@ -1301,6 +1313,13 @@ static int merge_mode_and_contents(struct merge_options *opt,\n \n \tif ((S_IFMT & a->mode) != (S_IFMT & b->mode)) {\n \t\tresult->clean = 0;\n+\t\t/*\n+\t\t * FIXME: This is a bad resolution for recursive case; for\n+\t\t * the recursive case we want something that is unlikely to\n+\t\t * accidentally match either side. Also, while it makes\n+\t\t * sense to prefer regular files over symlinks, it doesn't\n+\t\t * make sense to prefer regular files over submodules.\n+\t\t */\n \t\tif (S_ISREG(a->mode)) {\n \t\t\tresult->blob.mode = a->mode;\n \t\t\toidcpy(&result->blob.oid, &a->oid);\n@@ -1349,6 +1368,7 @@ static int merge_mode_and_contents(struct merge_options *opt,\n \t\t\tfree(result_buf.ptr);\n \t\t\tif (ret)\n \t\t\t\treturn ret;\n+\t\t\t/* FIXME: bug, what if modes didn't match? */\n \t\t\tresult->clean = (merge_status == 0);\n \t\t} else if (S_ISGITLINK(a->mode)) {\n \t\t\tresult->clean = merge_submodule(opt, &result->blob.oid,\n@@ -2664,6 +2684,14 @@ static int process_renames(struct merge_options *opt,\n \tstruct string_list b_by_dst = STRING_LIST_INIT_NODUP;\n \tconst struct rename *sre;\n \n+\t/*\n+\t * FIXME: As string-list.h notes, it's O(n^2) to build a sorted\n+\t * string_list one-by-one, but O(n log n) to build it unsorted and\n+\t * then sort it. Note that as we build the list, we do not need to\n+\t * check if the existing destination path is already in the list,\n+\t * because the structure of diffcore_rename guarantees we won't\n+\t * have duplicates.\n+\t */\n \tfor (i = 0; i < a_renames->nr; i++) {\n \t\tsre = a_renames->items[i].util;\n \t\tstring_list_insert(&a_by_dst, sre->pair->two->path)->util\n@@ -3602,6 +3630,15 @@ static int merge_recursive_internal(struct merge_options *opt,\n \t\t\treturn err(opt, _(\"merge returned no commit\"));\n \t}\n \n+\t/*\n+\t * FIXME: Since merge_recursive_internal() is only ever called by\n+\t * places that ensure the index is loaded first\n+\t * (e.g. builtin/merge.c, rebase/sequencer, etc.), in the common\n+\t * case where the merge base was unique that means when we get here\n+\t * we immediately discard the index and re-read it, which is a\n+\t * complete waste of time. We should only be discarding and\n+\t * re-reading if we were forced to recurse.\n+\t */\n \tdiscard_index(opt->repo->index);\n \tif (!opt->priv->call_depth)\n \t\trepo_read_index(opt->repo);\n-- \ngitgitgadget\n\n"},{"id":"419758","messageId":"4da61e15-a490-673a-ef15-800321ac9eea@gmail.com","threadId":"55329","inReplyTo":"pull.905.v3.git.1616198636.gitgitgadget@gmail.com","subject":"Re: [PATCH v3 00/13] Declare merge-ort ready for general usage","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2021-03-20T01:49:38Z","receivedAt":"2021-03-20T01:50:37Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 3/19/2021 8:03 PM, Elijah Newren via GitGitGadget wrote:\n> This series depends on ort-perf-batch-10[1], and obsoletes the ort-remainder\n> topic[2] (that hadn't been picked up yet, so hopefully this doesn't cause\n> any confusion)\n> \n> With this series, merge-ort is ready for general usage -- it passes all\n> tests, passes dozens of tests that don't under merge-recursive, and\n> merge-ort is is already significantly faster than merge-recursive when\n> rename detection is involved. Users can select merge-ort by (a) passing\n> -sort to either git merge or git rebase, or (b) by setting pull.twohead=ort\n> [3], or (c) by setting GIT_TEST_MERGE_ALGORITHM=ort.\n> \n> Changes since v2:\n> \n> * changed the last patch to default testing to ort so people don't have to\n> wait until CI for failures (but still keep linux-gcc testing\n> merge-recursive so it retains coverage), as suggested by Stolee\n\nWhat a great suggestion! This series gets\n\nReviewed-by: Derrick Stolee \n"}]}