Re: [PATCH] merge-ort: initialize repo in index state
From: Elijah Newren <hidden>
Date: 2023-10-06 05:15:02
On Thu, Oct 5, 2023 at 9:14 AM John Cai via GitGitGadget [off-list ref] wrote:
From: John Cai <redacted>
initialize_attr_index() does not initialize the repo member of
attr_index. Starting in 44451a2e5e (attr: teach "--attr-source=<tree>"
global option to "git", 2023-05-06), this became a problem because
istate->repo gets passed down the call chain starting in
git_check_attr(). This gets passed all the way down to
replace_refs_enabled(), which segfaults when accessing r->gitdir.
Fix this by initializing the repository in the index state.
Signed-off-by: John Cai <redacted>
Helped-by: Christian Couder [off-list ref]
---
merge-ort: initialize repo in index state
initialize_attr_index() does not initialize the repo member of
attr_index. Starting in 44451a2e5e (attr: teach "--attr-source=" global
option to "git", 2023-05-06), this became a problem because istate->repo
gets passed down the call chain starting in git_check_attr(). This gets
passed all the way down to replace_refs_enabled(), which segfaults when
accessing r->gitdir.
Fix this by initializing the repository in the index state.Out of curiosity, are the changes in 44451a2e5e and its predecessors sufficient to allow us to gut this nasty initialize_attr_index() hack from merge-ort? See the comment at the top of the function and this old mailing list thread: https://lore.kernel.org/git/CABPp-BE1TvFJ1eOa8Ci5JTMET+dzZh3m3NxppqqWPyEp1UeAVg@mail.gmail.com/ (local). I never wanted to generate an index, Stolee didn't like it either, but the attribute code seemed hardcoded to require an index and I had gone down enough rabbit holes trying to get merge-ort into shape. But I still absolutely hate this awful hack. If we do have to live with it still, then...
quoted hunk ↗ jump to hunk
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1583%2Fjohn-cai%2Fjc%2Fpopulate-repo-when-init-attr-index-v1 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1583/john-cai/jc/populate-repo-when-init-attr-index-v1 Pull-Request: https://github.com/git/git/pull/1583 merge-ort.c | 1 + t/t4300-merge-tree.sh | 20 ++++++++++++++++++++ 2 files changed, 21 insertions(+)diff --git a/merge-ort.c b/merge-ort.c index 7857ce9fbd1..172dc7d497d 100644 --- a/merge-ort.c +++ b/merge-ort.c@@ -1902,6 +1902,7 @@ static void initialize_attr_index(struct merge_options *opt) struct index_state *attr_index = &opt->priv->attr_index; struct cache_entry *ce; + attr_index->repo = the_repository;
Can we use opt->repo instead and reduce the number of places hardcoding the_repository?
quoted hunk ↗ jump to hunk
attr_index->initialized = 1; if (!opt->renormalize)diff --git a/t/t4300-merge-tree.sh b/t/t4300-merge-tree.sh index 57c4f26e461..254453fff9c 100755 --- a/t/t4300-merge-tree.sh +++ b/t/t4300-merge-tree.sh@@ -86,6 +86,26 @@ EXPECTED test_cmp expected actual ' +test_expect_success '3-way merge with --attr-source' ' + test_when_finished rm -rf 3-way && + git init 3-way && + ( + cd 3-way && + test_commit initial file1 foo && + base=$(git rev-parse HEAD) && + git checkout -b brancha && + echo bar>>file1 && + git commit -am "adding bar" && + source=$(git rev-parse HEAD) && + echo baz>>file1 && + git commit -am "adding baz" && + merge=$(git rev-parse HEAD) && + test_must_fail git --attr-source=HEAD merge-tree -z --write-tree \ + --merge-base "$base" --end-of-options "$source" "$merge" >out && + grep "Merge conflict in file1" out + ) +' + test_expect_success 'file change A, B (same)' ' git reset --hard initial && test_commit "change-a-b-same-A" "initial-file" "AAA" &&base-commit: 493f4622739e9b64f24b465b21aa85870dd9dc09 -- gitgitgadget