Re: [PATCH v2 02/10] merge: check config before loading commits
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Apr 11, 2018, 02:12 UTC
- Message-ID
- <xmqqo9iqze1g.fsf@gitster-ct.c.googlers.com>
- In-Reply-To
- <20180409164131.37312-3-dstolee@microsoft.com>
Derrick Stolee <dstolee@microsoft.com> writes:
Show 18 quoted lines
> diff --git a/builtin/merge.c b/builtin/merge.c
> index ee050a47f3..20897f8223 100644
> --- a/builtin/merge.c
> +++ b/builtin/merge.c
> @@ -1183,13 +1183,14 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
> branch = branch_to_free = resolve_refdup("HEAD", 0, &head_oid, NULL);
> if (branch)
> skip_prefix(branch, "refs/heads/", &branch);
> + init_diff_ui_defaults();
> + git_config(git_merge_config, NULL);
> +
> if (!branch || is_null_oid(&head_oid))
> head_commit = NULL;
> else
> head_commit = lookup_commit_or_die(&head_oid, "HEAD");
>
> - init_diff_ui_defaults();
> - git_config(git_merge_config, NULL);Wow, that's tricky. git_merge_config() wants to know which "branch" we are on, and this place is as early as we can move the call to without breaking things. Is this to allow parse_object() called in lookup_commit_reference_gently() to know if we can rely on the data cached in the commit-graph data?
> Move the config load to be between the initialization of 'branch' > and the commit lookup. Also add a test to t5318-commit-graph.sh > that exercises this code path to prevent a regression.
It is not clear to me how a successful merge of commits/8 demonstrates that reading the config earlier than before is regression free.
Show 18 quoted lines
> diff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh > index a380419b65..77d85aefe7 100755 > --- a/t/t5318-commit-graph.sh > +++ b/t/t5318-commit-graph.sh > @@ -221,4 +221,13 @@ test_expect_success 'write graph in bare repo' ' > graph_git_behavior 'bare repo with graph, commit 8 vs merge 1' bare commits/8 merge/1 > graph_git_behavior 'bare repo with graph, commit 8 vs merge 2' bare commits/8 merge/2 > > +test_expect_success 'perform fast-forward merge in full repo' ' > + cd "$TRASH_DIRECTORY/full" && > + git checkout -b merge-5-to-8 commits/5 && > + git merge commits/8 && > + git show-ref -s merge-5-to-8 >output && > + git show-ref -s commits/8 >expect && > + test_cmp expect output > +' > + > test_done