merge-ort: accept rename settings given without a value
PATCH 1/1 merge-ort: accept rename settings given without a value
From: Thomas Bachem <[email protected]>
"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.
---
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(-)
merge-ort.c +8 -12
@@ -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)) {
t/t6423-merge-rename-directories.sh +17 -0
@@ -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
t/t6434-merge-recursive-rename-options.sh +12 -0
@@ -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
Signature · 1 line
gitgitgadget
Quoted 8 lines · Thomas Bachem via GitGitGadget
From: Thomas Bachem <[email protected]>
"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.
Quoted 7 lines
merge-ort.c +6
-0+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) {
merge-ort.c void merge_incore_recursive(struct merge_options *opt, · line 5477
Quoted 10 lines
merge-ort.c +1
-2opt->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.