Skip to contentSkip to conversation

merge-ort: initialize repo in index state

PATCH Superseded John Cai via GitGitGadget 4 participants 8 messages Oct 5, 2023 – Oct 10, 2023

lore
Share & tools
lore

John Cai via GitGitGadget

author
#0 lore raw

PATCH 1/1 merge-ort: initialize repo in index state

From: John Cai <[email protected]>

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 <[email protected]>
Helped-by Christian Couder <[email protected]>

---
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.

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(+)
merge-ort.c +1 -0
index 7857ce9fbd1..172dc7d497d 100644
@@ -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;
 	attr_index->initialized = 1;
 
 	if (!opt->renormalize)
t/t4300-merge-tree.sh +20 -0
index 57c4f26e461..254453fff9c 100755
@@ -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

Signature · 1 line
gitgitgadget
#1 lore raw
Quoted 9 lines · John Cai via GitGitGadget

From: John Cai <[email protected]>

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.

Quoted 5 lines

Fix this by initializing the repository in the index state.

Signed-off-by John Cai <[email protected]>
Helped-by Christian Couder <[email protected]>

---

Nice spotting.

t/t4300-merge-tree.sh EXPECTED · line 97

Quoted 9 lines
t/t4300-merge-tree.sh +9 -0
+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 &&

We need a space before but not after ">>".

t/t4300-merge-tree.sh EXPECTED · line 100

Quoted 3 lines
t/t4300-merge-tree.sh +3 -0
+		git commit -am "adding bar" &&
+		source=$(git rev-parse HEAD) &&
+		echo baz>>file1 &&

Ditto.

t/t4300-merge-tree.sh EXPECTED · line 102

Quoted 2 lines
t/t4300-merge-tree.sh +2 -0
+		git commit -am "adding baz" &&
+		merge=$(git rev-parse HEAD) &&

Sorry, but I got lost. We have the $base commit on the default initial branch, from which forked branch-A which we created two commits to add lines "bar" and "baz" to file1. We are calling the tip of this branch-A $merge, and the parent of $merge is called $source.

t/t4300-merge-tree.sh EXPECTED · line 104

Quoted 2 lines
t/t4300-merge-tree.sh +2 -0
+		test_must_fail git --attr-source=HEAD merge-tree -z --write-tree \
+		--merge-base "$base" --end-of-options "$source" "$merge" >out &&

So, is this asking "merge-tree" to merge "$source" and "$merge", one of which is the direct parent of the other one? Aren't we missing a "checkout @{-1}" before we add "baz" or something?

If the attitude taken by this test is "we do not really care if the attempted merge is meaningless and would never happen in the real world, as the only thing we care is to see "git" not to segfault", then we probably shouldn't even check ...

t/t4300-merge-tree.sh EXPECTED · line 105

Quoted 1 line
t/t4300-merge-tree.sh +1 -0
+		grep "Merge conflict in file1" out

... if we failed due to conflict with a "grep" like this. As "out" is a binary file (thanks to the use of "-z" on the command line of "merge-tree" invocation), I am not sure if you can rely on "grep" to find this error message in 'out', depending on your implementation of "grep".

On the other hand, the new test can do a more realistic merge between two commits, where having an attribute in some tree object (which preferrably is *not* HEAD and .gitattribute does not exist in the working tree) given to the --attr-source option does make a difference, and verify the contents of the "file1" recorded in the resulting tree. That way, the test can verify that the attributes are read from the right place without segfaulting.

Quoted 8 lines
t/t4300-merge-tree.sh +3 -0
+	)
+'
+
 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

Thanks.

#2 lore raw
In reply to John Cai via GitGitGadget
Quoted 17 lines · John Cai via GitGitGadget

From: John Cai <[email protected]>

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 <[email protected]>
Helped-by Christian Couder <[email protected]>

---
merge-ort: initialize repo in index state

Quoted 8 lines

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/.

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 13 lines

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(+)
merge-ort.c +0 -0
index 7857ce9fbd1..172dc7d497d 100644

merge-ort.c static void initialize_attr_index(struct merge_options *opt) · line 1905

Quoted 4 lines
merge-ort.c +1 -0
@@ -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 38 lines
merge-ort.c +0 -0
        attr_index->initialized = 1;
 
        if (!opt->renormalize)
t/t4300-merge-tree.sh +20 -0
index 57c4f26e461..254453fff9c 100755
@@ -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

#3 lore raw

Hi Elijah,

Quoted 26 lines · Elijah Newren
Quoted 24 lines · John Cai via GitGitGadget

From: John Cai <[email protected]>

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 <[email protected]>
Helped-by Christian Couder <[email protected]>

---
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.

Quoted 7 lines
Quoted 1 line

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/.

Honestly, I'm not familiar enough with the attr code to know if the index can be taken out of the call chain. From first glance, it looks like that would take some additional work, which I agree would be nice to do.

Quoted 5 lines

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.

Perhaps a follow up patch series could address this--will keep it in mind.

Quoted 19 lines

If we do have to live with it still, then...

Quoted 16 lines

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(+)
merge-ort.c +0 -0
index 7857ce9fbd1..172dc7d497d 100644
@@ -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;
Quoted 4 lines

merge-ort.c static void initialize_attr_index(struct merge_options *opt) · line 1905

Quoted 1 line
merge-ort.c +1 -0
+       attr_index->repo = the_repository;

Can we use opt->repo instead and reduce the number of places hardcoding the_repository?

sounds good!

Quoted 37 lines
Quoted 37 lines
merge-ort.c +0 -0
        attr_index->initialized = 1;
 
        if (!opt->renormalize)
t/t4300-merge-tree.sh +20 -0
index 57c4f26e461..254453fff9c 100755
@@ -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" &&
Quoted 3 lines
Quoted 3 lines

base-commit: 493f4622739e9b64f24b465b21aa85870dd9dc09 --
gitgitgadget

thanks
John

author
#4 lore raw

PATCH v2 1/1 merge-ort: initialize repo in index state

In reply to John Cai via GitGitGadget

From: John Cai <[email protected]>

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 <[email protected]>
Helped-by Christian Couder <[email protected]>

---
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.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1583%2Fjohn-cai%2Fjc%2Fpopulate-repo-when-init-attr-index-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1583/john-cai/jc/populate-repo-when-init-attr-index-v2
Pull-Request: https://github.com/git/git/pull/1583

Range-diff vs v1:

1: 80a18252b30 ! 1: e178236064a merge-ort: initialize repo in index state

     @@ merge-ort.c: 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;
     ++	attr_index->repo = opt->repo;
       	attr_index->initialized = 1;
       
       	if (!opt->renormalize)
     @@ t/t4300-merge-tree.sh: EXPECTED
      +		test_commit initial file1 foo &&
      +		base=$(git rev-parse HEAD) &&
      +		git checkout -b brancha &&
     -+		echo bar>>file1 &&
     ++		echo bar >>file1 &&
      +		git commit -am "adding bar" &&
      +		source=$(git rev-parse HEAD) &&
     -+		echo baz>>file1 &&
     ++		git checkout @{-1} &&
     ++		git checkout -b branchb &&
     ++		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
     ++		git checkout -b gitattributes &&
     ++		test_commit "gitattributes" .gitattributes "file1 merge=union" &&
     ++		git checkout @{-1} &&
     ++		tree=$(git --attr-source=gitattributes merge-tree --write-tree \
     ++		--merge-base "$base" --end-of-options "$source" "$merge") &&
     ++		echo "foo\nbar\nbaz" >expect &&
     ++		git cat-file -p "$tree:file1" >actual &&
     ++		test_cmp expect actual
      +	)
      +'
      +
 merge-ort.c           |  1 +
 t/t4300-merge-tree.sh | 27 +++++++++++++++++++++++++++
 2 files changed, 28 insertions(+)
merge-ort.c +1 -0
index 7857ce9fbd1..36537256613 100644
@@ -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 = opt->repo;
 	attr_index->initialized = 1;
 
 	if (!opt->renormalize)
t/t4300-merge-tree.sh +27 -0
index 57c4f26e461..2929ce3bf16 100755
@@ -86,6 +86,33 @@ 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) &&
+		git checkout @{-1} &&
+		git checkout -b branchb &&
+		echo baz >>file1 &&
+		git commit -am "adding baz" &&
+		merge=$(git rev-parse HEAD) &&
+		git checkout -b gitattributes &&
+		test_commit "gitattributes" .gitattributes "file1 merge=union" &&
+		git checkout @{-1} &&
+		tree=$(git --attr-source=gitattributes merge-tree --write-tree \
+		--merge-base "$base" --end-of-options "$source" "$merge") &&
+		echo "foo\nbar\nbaz" >expect &&
+		git cat-file -p "$tree:file1" >actual &&
+		test_cmp expect actual
+	)
+'
+
 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

Signature · 1 line
gitgitgadget
author
#5 lore raw

PATCH v3 1/1 merge-ort: initialize repo in index state

From: John Cai <[email protected]>

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 <[email protected]>
Helped-by Christian Couder <[email protected]>

---
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.

Changes since V2:

* fixed test by using printf instead of echo

Changes since v1:

* using opt->repo to avoid hardcoding another the_repository
* clarified test

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1583%2Fjohn-cai%2Fjc%2Fpopulate-repo-when-init-attr-index-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1583/john-cai/jc/populate-repo-when-init-attr-index-v3
Pull-Request: https://github.com/git/git/pull/1583

Range-diff vs v2:

1: e178236064a ! 1: 792b01fa616 merge-ort: initialize repo in index state

     @@ t/t4300-merge-tree.sh: EXPECTED
      +		git checkout @{-1} &&
      +		tree=$(git --attr-source=gitattributes merge-tree --write-tree \
      +		--merge-base "$base" --end-of-options "$source" "$merge") &&
     -+		echo "foo\nbar\nbaz" >expect &&
     ++		printf "foo\nbar\nbaz\n" >expect &&
      +		git cat-file -p "$tree:file1" >actual &&
      +		test_cmp expect actual
      +	)
 merge-ort.c           |  1 +
 t/t4300-merge-tree.sh | 27 +++++++++++++++++++++++++++
 2 files changed, 28 insertions(+)
merge-ort.c +1 -0
index 7857ce9fbd1..36537256613 100644
@@ -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 = opt->repo;
 	attr_index->initialized = 1;
 
 	if (!opt->renormalize)
t/t4300-merge-tree.sh +27 -0
index 57c4f26e461..c3a03e54187 100755
@@ -86,6 +86,33 @@ 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) &&
+		git checkout @{-1} &&
+		git checkout -b branchb &&
+		echo baz >>file1 &&
+		git commit -am "adding baz" &&
+		merge=$(git rev-parse HEAD) &&
+		git checkout -b gitattributes &&
+		test_commit "gitattributes" .gitattributes "file1 merge=union" &&
+		git checkout @{-1} &&
+		tree=$(git --attr-source=gitattributes merge-tree --write-tree \
+		--merge-base "$base" --end-of-options "$source" "$merge") &&
+		printf "foo\nbar\nbaz\n" >expect &&
+		git cat-file -p "$tree:file1" >actual &&
+		test_cmp expect actual
+	)
+'
+
 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

Signature · 1 line
gitgitgadget
#6 lore raw
Quoted 5 lines · John Cai via GitGitGadget

Fix this by initializing the repository in the index state.

Changes since V2:

* fixed test by using printf instead of echo

Much better than using unportable \n with echo.

Quoted 2 lines
     -+		echo "foo\nbar\nbaz" >expect &&
     ++		printf "foo\nbar\nbaz\n" >expect &&

But if we are using printf, it would be easier to read lines separately, which would look more like

printf "%s\n" foo bar baz >expect

And we have

test_write_lines foo bar baz >expect

to make it even more discoverable.

Quoted 37 lines
      +		git cat-file -p "$tree:file1" >actual &&
      +		test_cmp expect actual
      +	)
 merge-ort.c           |  1 +
 t/t4300-merge-tree.sh | 27 +++++++++++++++++++++++++++
 2 files changed, 28 insertions(+)
merge-ort.c +1 -0
index 7857ce9fbd1..36537256613 100644
@@ -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 = opt->repo;
 	attr_index->initialized = 1;
 
 	if (!opt->renormalize)
t/t4300-merge-tree.sh +8 -0
index 57c4f26e461..c3a03e54187 100755
@@ -86,6 +86,33 @@ 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 &&

t/t4300-merge-tree.sh EXPECTED · line 106

Quoted 10 lines
t/t4300-merge-tree.sh +10 -0
+		echo bar >>file1 &&
+		git commit -am "adding bar" &&
+		source=$(git rev-parse HEAD) &&
+		git checkout @{-1} &&
+		git checkout -b branchb &&
+		echo baz >>file1 &&
+		git commit -am "adding baz" &&
+		merge=$(git rev-parse HEAD) &&
+		git checkout -b gitattributes &&
+		test_commit "gitattributes" .gitattributes "file1 merge=union" &&

OK, the branch "gitattributes" will be used to drive merge of file1 using the union merge to avoid conflicting.

t/t4300-merge-tree.sh EXPECTED · line 107

Quoted 1 line
t/t4300-merge-tree.sh +1 -0
+		git checkout @{-1} &&

But such attribute will only be available in that branch, not in the checked out working tree. And then

t/t4300-merge-tree.sh EXPECTED · line 109

Quoted 2 lines
t/t4300-merge-tree.sh +2 -0
+		tree=$(git --attr-source=gitattributes merge-tree --write-tree \
+		--merge-base "$base" --end-of-options "$source" "$merge") &&

we use the gitattributes branch as the tree-ish to take the attribute information from. Makes sense.

t/t4300-merge-tree.sh EXPECTED · line 110

Quoted 1 line
t/t4300-merge-tree.sh +1 -0
+		printf "foo\nbar\nbaz\n" >expect &&

I'll squash in the "test_write_lines" change while queuing.

Quoted 10 lines
t/t4300-merge-tree.sh +5 -0
+		git cat-file -p "$tree:file1" >actual &&
+		test_cmp expect actual
+	)
+'
+
 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

Thanks. Looking good.

#7 lore raw

Hi Junio

Quoted 16 lines · Junio C Hamano
Quoted 5 lines · John Cai via GitGitGadget

Fix this by initializing the repository in the index state.

Changes since V2:

* fixed test by using printf instead of echo

Much better than using unportable \n with echo.

Quoted 2 lines
     -+		echo "foo\nbar\nbaz" >expect &&
     ++		printf "foo\nbar\nbaz\n" >expect &&

But if we are using printf, it would be easier to read lines separately, which would look more like

Quoted 7 lines

printf "%s\n" foo bar baz >expect

And we have

test_write_lines foo bar baz >expect

to make it even more discoverable.

wasn't aware of test_write_lines, thanks.

Quoted 63 lines
Quoted 38 lines
      +		git cat-file -p "$tree:file1" >actual &&
      +		test_cmp expect actual
      +	)
 merge-ort.c           |  1 +
 t/t4300-merge-tree.sh | 27 +++++++++++++++++++++++++++
 2 files changed, 28 insertions(+)
merge-ort.c +1 -0
index 7857ce9fbd1..36537256613 100644
@@ -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 = opt->repo;
 	attr_index->initialized = 1;
 
 	if (!opt->renormalize)
t/t4300-merge-tree.sh +8 -0
index 57c4f26e461..c3a03e54187 100755
@@ -86,6 +86,33 @@ 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 &&

t/t4300-merge-tree.sh EXPECTED · line 106

Quoted 10 lines
t/t4300-merge-tree.sh +10 -0
+		echo bar >>file1 &&
+		git commit -am "adding bar" &&
+		source=$(git rev-parse HEAD) &&
+		git checkout @{-1} &&
+		git checkout -b branchb &&
+		echo baz >>file1 &&
+		git commit -am "adding baz" &&
+		merge=$(git rev-parse HEAD) &&
+		git checkout -b gitattributes &&
+		test_commit "gitattributes" .gitattributes "file1 merge=union" &&

OK, the branch "gitattributes" will be used to drive merge of file1 using the union merge to avoid conflicting.

t/t4300-merge-tree.sh EXPECTED · line 107

Quoted 1 line
t/t4300-merge-tree.sh +1 -0
+		git checkout @{-1} &&

But such attribute will only be available in that branch, not in the checked out working tree. And then

t/t4300-merge-tree.sh EXPECTED · line 109

Quoted 2 lines
t/t4300-merge-tree.sh +2 -0
+		tree=$(git --attr-source=gitattributes merge-tree --write-tree \
+		--merge-base "$base" --end-of-options "$source" "$merge") &&

we use the gitattributes branch as the tree-ish to take the attribute information from. Makes sense.

Quoted 3 lines

t/t4300-merge-tree.sh EXPECTED · line 110

Quoted 1 line
t/t4300-merge-tree.sh +1 -0
+		printf "foo\nbar\nbaz\n" >expect &&

I'll squash in the "test_write_lines" change while queuing.

thank you!
John

Quoted 13 lines
Quoted 11 lines
t/t4300-merge-tree.sh +5 -0
+		git cat-file -p "$tree:file1" >actual &&
+		test_cmp expect actual
+	)
+'
+
 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

Thanks. Looking good.