Derrick Stolee wrote:
> On 5/10/2023 4:30 PM, Felipe Contreras wrote:
> > Derrick Stolee via GitGitGadget wrote:
> >> From: Derrick Stolee <derrickstolee@xxxxxxxxxx>
> >>
> >> The 'git merge-tree' command handles creating root trees for merges
> >> without using the worktree. This is a critical operation in many Git
> >> hosts, as they typically store bare repositories.
> >>
> >> This builtin does not load the default Git config, which can have
> >> several important ramifications.
> >
> > For the record, I had already sent a better version of this patch almost 2
> > years ago [1], not just for `git merge-tree`, but other commands as well.
> >
> > The obvious fix was completely ignored by the maintainer.
> >
> > The reason why it should be git_xmerge_config and not git_default_config, is
> > that merge.conflictstyle would not be parsed if you call git_default_config.
>
> As mentioned by Elijah in a different thread, the merge machinery loads
> the merge config as needed. I confirmed by creating this test, which I
> may submit as an independent patch:
I wrote my patches before Elijah wrote the real merge implementation, and in
his function he does `init_merge_options()`, which eventually calls
`git_config(git_xmerge_config, NULL)`.
But if `git_config()` is already called, you shouldn't need to add yet another
`git_config()` call.
The problem is that he added `init_merge_options()` *after* the
`get_merge_parent()` calls, that's why the configuration is ignored.
If we move `init_merge_options()` to the right place, the problem is fixed:
--- a/builtin/merge-tree.c
+++ b/builtin/merge-tree.c
@@ -424,6 +424,8 @@ static int real_merge(struct merge_tree_options *o,
struct merge_result result = { 0 };
int show_messages = o->show_messages;
+ init_merge_options(&opt, the_repository);
+
parent1 = get_merge_parent(branch1);
if (!parent1)
help_unknown_ref(branch1, "merge-tree",
@@ -434,8 +436,6 @@ static int real_merge(struct merge_tree_options *o,
help_unknown_ref(branch2, "merge-tree",
_("not something we can merge"));
- init_merge_options(&opt, the_repository);
-
opt.show_rename_progress = 0;
opt.branch1 = branch1;
I ran your test case, and it passes.
I sent a patch for that here:
https://lore.kernel.org/git/20230511215608.1297686-1-felipe.contreras@xxxxxxxxx/
This is a more proper fix because a) it doesn't add any new line of code, b) it
doesn't add a new include, and c) it doesn't call `git_config()` twice.
Cheers.
--
Felipe Contreras
