threads / patch / 66724

patchmerge-ort: accept rename settings given without a value

Subject: [PATCH] merge-ort: accept rename settings given without a value

## tl;dr

"git -c merge.renames merge" currently fails even though the documentation says a setting with no value means true. Read the story.

replies: 1people: 2as markdown or json

Thomas Bachem via GitGitGadget· Oct 31, 2026, 10:06 UTC · lore
From: Thomas Bachem <mail@thomasbachem.com>

"git -c merge.renames merge" fails with "missing value for 'merge.renames'", although git(1) says that "git -c foo.bar" sets foo.bar to true. The same happens with diff.renames and merge.directoryRenames, and with a setting written without "= <value>" in a config file, which git-config(1) also documents as true. Every command that merges through merge-ort fails this way.

merge-ort reads these settings with repo_config_get_string(), which rejects a missing value. Read them with repo_config_get_value() instead. git_config_rename() and git_parse_maybe_bool() already take a missing value as true.

Assisted-by: Claude Fable 5.1
Signed-off-by: Thomas Bachem <mail@thomasbachem.com>
---
    merge-ort: accept rename settings given without a value
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2284%2Fthomasbachem%2Fmerge-ort-valueless-renames-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2284/thomasbachem/merge-ort-valueless-renames-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/2284
 merge-ort.c                               | 20 ++++++++------------
 t/t6423-merge-rename-directories.sh       | 17 +++++++++++++++++
 t/t6434-merge-recursive-rename-options.sh | 12 ++++++++++++
 3 files changed, 37 insertions(+), 12 deletions(-)
Show changes to 3 files +37 −12

merge-ort.c, t/t6423-merge-rename-directories.sh, t/t6434-merge-recursive-rename-options.sh

diff --git a/merge-ort.c b/merge-ort.c
index c410a5d353..24ddd97696 100644
--- a/merge-ort.c
+++ b/merge-ort.c
@@ -5453,31 +5453,27 @@ void merge_incore_recursive(struct merge_options *opt,
 static void merge_recursive_config(struct merge_options *opt, int ui)
 {
 	char *value = NULL;
+	const char *v;
 	int renormalize = 0;
 	repo_config_get_int(opt->repo, "merge.verbosity", &opt->verbosity);
 	repo_config_get_int(opt->repo, "diff.renamelimit", &opt->rename_limit);
 	repo_config_get_int(opt->repo, "merge.renamelimit", &opt->rename_limit);
 	repo_config_get_bool(opt->repo, "merge.renormalize", &renormalize);
 	opt->renormalize = renormalize;
-	if (!repo_config_get_string(opt->repo, "diff.renames", &value)) {
-		opt->detect_renames = git_config_rename("diff.renames", value);
-		free(value);
-	}
-	if (!repo_config_get_string(opt->repo, "merge.renames", &value)) {
-		opt->detect_renames = git_config_rename("merge.renames", value);
-		free(value);
-	}
-	if (!repo_config_get_string(opt->repo, "merge.directoryrenames", &value)) {
-		int boolval = git_parse_maybe_bool(value);
+	if (!repo_config_get_value(opt->repo, "diff.renames", &v))
+		opt->detect_renames = git_config_rename("diff.renames", v);
+	if (!repo_config_get_value(opt->repo, "merge.renames", &v))
+		opt->detect_renames = git_config_rename("merge.renames", v);
+	if (!repo_config_get_value(opt->repo, "merge.directoryrenames", &v)) {
+		int boolval = git_parse_maybe_bool(v);
 		if (0 <= boolval) {
 			opt->detect_directory_renames = boolval ?
 				MERGE_DIRECTORY_RENAMES_TRUE :
 				MERGE_DIRECTORY_RENAMES_NONE;
-		} else if (!strcasecmp(value, "conflict")) {
+		} else if (!strcasecmp(v, "conflict")) {
 			opt->detect_directory_renames =
 				MERGE_DIRECTORY_RENAMES_CONFLICT;
 		} /* avoid erroring on values from future versions of git */
-		free(value);
 	}
 	if (ui) {
 		if (!repo_config_get_string(opt->repo, "diff.algorithm", &value)) {
diff --git a/t/t6423-merge-rename-directories.sh b/t/t6423-merge-rename-directories.sh
index 24d4c5447b..da145c150c 100755
--- a/t/t6423-merge-rename-directories.sh
+++ b/t/t6423-merge-rename-directories.sh
@@ -97,6 +97,23 @@ test_expect_success '1a: Simple directory rename detection' '
 	)
 '
 
+test_expect_success '1a (valueless): merge.directoryRenames without a value means true' '
+	mkdir valueless &&
+	(
+		cd valueless &&
+		test_setup_1a &&
+		cd 1a &&
+
+		git checkout A^0 &&
+
+		git -c merge.directoryRenames merge -s recursive B^0 &&
+
+		git rev-parse >actual HEAD:y/d HEAD:y/e/f &&
+		git rev-parse >expect B:z/d B:z/e/f &&
+		test_cmp expect actual
+	)
+'
+
 # Testcase 1b, Merge a directory with another
 #   Commit O: z/{b,c},   y/d
 #   Commit A: z/{b,c,e}, y/d
diff --git a/t/t6434-merge-recursive-rename-options.sh b/t/t6434-merge-recursive-rename-options.sh
index 5a6f74839c..427de2ba95 100755
--- a/t/t6434-merge-recursive-rename-options.sh
+++ b/t/t6434-merge-recursive-rename-options.sh
@@ -332,4 +332,16 @@ test_expect_success 'merge.renames overrides diff.renames' '
 	$check_50
 '
 
+test_expect_success 'diff.renames without a value means true' '
+	git read-tree --reset -u HEAD &&
+	test_must_fail git -c diff.renames merge-recursive $tail &&
+	$check_50
+'
+
+test_expect_success 'merge.renames without a value means true' '
+	git read-tree --reset -u HEAD &&
+	test_must_fail git -c diff.renames=false -c merge.renames merge-recursive $tail &&
+	$check_50
+'
+
 test_done

base-commit: 6de20f6092dcf9bdb1c8efe03db4b70c82b423dd

-- 
gitgitgadget
Junio C Hamano· Nov 1, 2026, 00:52 UTC · re: Thomas Bachem via GitGitGadget · lore

Re: [PATCH] merge-ort: accept rename settings given without a value

"Thomas Bachem via GitGitGadget" <gitgitgadget@gmail.com> writes:
Show 8 quoted lines
> From: Thomas Bachem <mail@thomasbachem.com>
>
> "git -c merge.renames merge" fails with "missing value for
> 'merge.renames'", although git(1) says that "git -c foo.bar" sets
> foo.bar to true. The same happens with diff.renames and
> merge.directoryRenames, and with a setting written without
> "= <value>" in a config file, which git-config(1) also documents as
> true. Every command that merges through merge-ort fails this way.

Searching the list archive for "valueless true" tells us that we had corrected a handful of instances of config_get_string() with a call to config_get_value() in the past half year.

And this patch seems to be in line with the other efforts to fix the same misuse.

Show 17 quoted lines
> +	if (!repo_config_get_value(opt->repo, "diff.renames", &v))
> +		opt->detect_renames = git_config_rename("diff.renames", v);
> +	if (!repo_config_get_value(opt->repo, "merge.renames", &v))
> +		opt->detect_renames = git_config_rename("merge.renames", v);
> +	if (!repo_config_get_value(opt->repo, "merge.directoryrenames", &v)) {
> +		int boolval = git_parse_maybe_bool(v);
>  		if (0 <= boolval) {
>  			opt->detect_directory_renames = boolval ?
>  				MERGE_DIRECTORY_RENAMES_TRUE :
>  				MERGE_DIRECTORY_RENAMES_NONE;
> -		} else if (!strcasecmp(value, "conflict")) {
> +		} else if (!strcasecmp(v, "conflict")) {
>  			opt->detect_directory_renames =
>  				MERGE_DIRECTORY_RENAMES_CONFLICT;
>  		} /* avoid erroring on values from future versions of git */
> -		free(value);
>  	}

OK. Since get_value(), unlike get_string(), lets us borrow the value as "const char *", once we used to drive git_config_rename() or compared with fixed string to turn it into an enum value, we can simply forget without calling free(), which is simpler.

Nicely done.

← back to recent threads