threads / patch / 63143

patch, 5 partsMiscellaneous fixups preparing for the removal of merge-recursive

Subject: [PATCH 0/5] Miscellaneous fixups preparing for the removal of merge-recursive

## tl;dr

6 messages between Mar 16, 2025 and Mar 16, 2025. Diffs are folded; open one to read it.

replies: 5people: 1as markdown or json

Elijah Newren via GitGitGadget· Mar 16, 2025, 06:58 UTC · lore

While working on the removal of merge recursive, there were a handful of miscellaneous fixups, none of which changed any behavior. Each is a very small localized change. This does not depend on any other PRs, and in fact each commit in this series is orthogonal. I'm just grouping them because they are all tiny, miscellaneous fixups.

Elijah Newren (5):
  stash: remove merge-recursive.h include
  t6423: fix a comment that accidentally reversed two commits
  t7615: be more explicit about diff algorithm used
  merge-ort: fix accidental strset<->strintmap
  merge-ort: remove extraneous word in comment

 builtin/stash.c                            |  1 -
 merge-ort.c                                | 10 +++++-----
 t/t6423-merge-rename-directories.sh        |  6 +++---
 t/t7615-diff-algo-with-mergy-operations.sh |  4 ++--
 4 files changed, 10 insertions(+), 11 deletions(-)

base-commit: c9d3534de317f31915f37e9d9c0d52d4cf901482
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1882%2Fnewren%2Fendit-misc-fixups-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1882/newren/endit-misc-fixups-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1882

-- 
gitgitgadget

Elijah Newren via GitGitGadget· Mar 16, 2025, 06:58 UTC · re: Elijah Newren via GitGitGadget · lore

[PATCH 1/5] stash: remove merge-recursive.h include

From: Elijah Newren <newren@gmail.com>

stash was modified to use merge_ort_nonrecursive() instead of merge_recursive_generic() back in commit 874cf2a60444 (stash: apply stash using 'merge_ort_nonrecursive()', 2022-05-10). That makes the inclusion of merge-recursive.h unnecessary. In preparation for the removal of merge-recursive.h, remove the unnecessary include.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 builtin/stash.c | 1 -
 1 file changed, 1 deletion(-)

Show changes to builtin/stash.c +0 −1

diff --git a/builtin/stash.c b/builtin/stash.c
index dbaa999cf17..cfbd92852a6 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -13,7 +13,6 @@
 #include "lockfile.h"
 #include "cache-tree.h"
 #include "unpack-trees.h"
-#include "merge-recursive.h"
 #include "merge-ort-wrappers.h"
 #include "strvec.h"
 #include "run-command.h"

-- 
gitgitgadget

Elijah Newren via GitGitGadget· Mar 16, 2025, 06:58 UTC · re: Elijah Newren via GitGitGadget · lore

[PATCH 2/5] t6423: fix a comment that accidentally reversed two commits

From: Elijah Newren <newren@gmail.com>

The comment describing testcase 13b of t6423 somehow mixed up commits A and B in one paragraph. Fix the references.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 t/t6423-merge-rename-directories.sh | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Show changes to t/t6423-merge-rename-directories.sh +3 −3

diff --git a/t/t6423-merge-rename-directories.sh b/t/t6423-merge-rename-directories.sh
index 94080c65d12..ebf47cdd6b2 100755
--- a/t/t6423-merge-rename-directories.sh
+++ b/t/t6423-merge-rename-directories.sh
@@ -5549,9 +5549,9 @@ test_expect_success '13b(info): messages for transitive rename with conflicted c
 #   Commit A: y/{b,c,d}, x/e
 #   Commit B: z/{b,c,d}, x/e
 #   Expected: y/{b,c,d}, x/e, with info or conflict messages for d
-#             A: renamed x/d -> z/d; B: renamed z/ -> y/ AND renamed x/d to y/d
-#             One could argue A had partial knowledge of what was done with
-#             d and B had full knowledge, but that's a slippery slope as
+#             B: renamed x/d -> z/d; A: renamed z/ -> y/ AND renamed x/d to y/d
+#             One could argue B had partial knowledge of what was done with
+#             d and A had full knowledge, but that's a slippery slope as
 #             shown in testcase 13d.
 
 test_setup_13c () {

-- 
gitgitgadget

Elijah Newren via GitGitGadget· Mar 16, 2025, 06:58 UTC · re: Elijah Newren via GitGitGadget · lore

[PATCH 3/5] t7615: be more explicit about diff algorithm used

From: Elijah Newren <newren@gmail.com>

t7615 is entirely about testing the differences about different diff algorithms, but it doesn't specify any diff algorithm when it is testing myers. Given that we have discussed potentially switching defaults (https://lore.kernel.org/git/xmqqed873vgn.fsf@gitster.g/), it makes sense in tests that are about different diff algorithms to be explicitly about which one is intended to be used in each test. Add that specificity.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 t/t7615-diff-algo-with-mergy-operations.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Show changes to t/t7615-diff-algo-with-mergy-operations.sh +2 −2

diff --git a/t/t7615-diff-algo-with-mergy-operations.sh b/t/t7615-diff-algo-with-mergy-operations.sh
index 3b1aad0167a..ac5863e788c 100755
--- a/t/t7615-diff-algo-with-mergy-operations.sh
+++ b/t/t7615-diff-algo-with-mergy-operations.sh
@@ -26,7 +26,7 @@ GIT_TEST_MERGE_ALGORITHM=recursive
 
 test_expect_success 'merge c2 to c1 with recursive merge strategy fails with the current default myers diff algorithm' '
 	git reset --hard c1 &&
-	test_must_fail git merge -s recursive c2
+	test_must_fail git merge -s recursive -Xdiff-algorithm=myers c2
 '
 
 test_expect_success 'merge c2 to c1 with recursive merge strategy succeeds with -Xdiff-algorithm=histogram' '
@@ -42,7 +42,7 @@ test_expect_success 'merge c2 to c1 with recursive merge strategy succeeds with
 
 test_expect_success 'cherry-pick c2 to c1 with recursive merge strategy fails with the current default myers diff algorithm' '
 	git reset --hard c1 &&
-	test_must_fail git cherry-pick -s recursive c2
+	test_must_fail git cherry-pick -s recursive -Xdiff-algorithm=myers c2
 '
 
 test_expect_success 'cherry-pick c2 to c1 with recursive merge strategy succeeds with -Xdiff-algorithm=histogram' '

-- 
gitgitgadget

Elijah Newren via GitGitGadget· Mar 16, 2025, 06:58 UTC · re: Elijah Newren via GitGitGadget · lore

[PATCH 4/5] merge-ort: fix accidental strset<->strintmap

From: Elijah Newren <newren@gmail.com>

Both strset_for_each_entry and strintmap_for_each_entry are macros that evaluate to the same thing, so they are technically interchangeable. However, the intent is that we use the one matching the variable type we are passing. Unfortunately, I somehow mistakenly got one of these wrong in 7bee6c100431 (merge-ort: avoid recursing into directories when we don't need to, 2021-07-16) -- possibly related to the fact that relevant_sources was initially a strset and later refactored into a strintmap. Correct which macro we use.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 merge-ort.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Show changes to merge-ort.c +2 −2

diff --git a/merge-ort.c b/merge-ort.c
index 46e78c3ffa6..a12aa213b06 100644
--- a/merge-ort.c
+++ b/merge-ort.c
@@ -1517,8 +1517,8 @@ static int handle_deferred_entries(struct merge_options *opt,
 		struct strintmap copy;
 
 		/* Loop over the set of paths we need to know rename info for */
-		strset_for_each_entry(&renames->relevant_sources[side],
-				      &iter, entry) {
+		strintmap_for_each_entry(&renames->relevant_sources[side],
+					 &iter, entry) {
 			char *rename_target, *dir, *dir_marker;
 			struct strmap_entry *e;

-- 
gitgitgadget

Elijah Newren via GitGitGadget· Mar 16, 2025, 06:58 UTC · re: Elijah Newren via GitGitGadget · lore

[PATCH 5/5] merge-ort: remove extraneous word in comment

From: Elijah Newren <newren@gmail.com>

"is was" -> "was"

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 merge-ort.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Show changes to merge-ort.c +3 −3

diff --git a/merge-ort.c b/merge-ort.c
index a12aa213b06..9efc2285a94 100644
--- a/merge-ort.c
+++ b/merge-ort.c
@@ -3423,9 +3423,9 @@ static int collect_renames(struct merge_options *opt,
 
 		/*
 		 * p->score comes back from diffcore_rename_extended() with
-		 * the similarity of the renamed file.  The similarity is
-		 * was used to determine that the two files were related
-		 * and are a rename, which we have already used, but beyond
+		 * the similarity of the renamed file.  The similarity was
+		 * used to determine that the two files were related and
+		 * are a rename, which we have already used, but beyond
 		 * that we have no use for the similarity.  So p->score is
 		 * now irrelevant.  However, process_renames() will need to
 		 * know which side of the merge this rename was associated

-- 
gitgitgadget