On Thu, Oct 5, 2023 at 9:14 AM John Cai via GitGitGadget <gitgitgadget@xxxxxxxxx> wrote: > > From: John Cai <johncai86@xxxxxxxxx> > > 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 <johncai86@xxxxxxxxx> > Helped-by: Christian Couder <christian.couder@xxxxxxxxx> > --- > 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@xxxxxxxxxxxxxx/. 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... > 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? > 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
