[PATCH] merge-ort: initialize repo in index state

[PATCH] merge-ort: initialize repo in index state

[John Cai via GitGitGadget]

From: John Cai <johncai86@gmail.com>

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@gmail.com>
Helped-by: Christian Couder <christian.couder@gmail.com>
---
    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(+)

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

Re: [PATCH] merge-ort: initialize repo in index state

[Junio C Hamano]

"John Cai via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: John Cai <johncai86@gmail.com>
>
> 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@gmail.com>
> Helped-by: Christian Couder <christian.couder@gmail.com>
> ---

Nice spotting.

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

> +		git commit -am "adding bar" &&
> +		source=$(git rev-parse HEAD) &&
> +		echo baz>>file1 &&

Ditto.

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

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

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

> +	)
> +'
> +
>  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.

Re: [PATCH] merge-ort: initialize repo in index state

[Elijah Newren]

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

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

Re: [PATCH] merge-ort: initialize repo in index state

[John Cai]

Hi Elijah,

On 6 Oct 2023, at 1:14, Elijah Newren wrote:

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

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.

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

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

sounds good!

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

thanks
John

[PATCH v2] merge-ort: initialize repo in index state

[John Cai via GitGitGadget]

From: John Cai <johncai86@gmail.com>

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@gmail.com>
Helped-by: Christian Couder <christian.couder@gmail.com>
---
    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(+)

diff --git a/merge-ort.c b/merge-ort.c
index 7857ce9fbd1..36537256613 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 = opt->repo;
 	attr_index->initialized = 1;
 
 	if (!opt->renormalize)
diff --git a/t/t4300-merge-tree.sh b/t/t4300-merge-tree.sh
index 57c4f26e461..2929ce3bf16 100755
--- a/t/t4300-merge-tree.sh
+++ b/t/t4300-merge-tree.sh
@@ -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
-- 
gitgitgadget

[PATCH v3] merge-ort: initialize repo in index state

[John Cai via GitGitGadget]

From: John Cai <johncai86@gmail.com>

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@gmail.com>
Helped-by: Christian Couder <christian.couder@gmail.com>
---
    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(+)

diff --git a/merge-ort.c b/merge-ort.c
index 7857ce9fbd1..36537256613 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 = opt->repo;
 	attr_index->initialized = 1;
 
 	if (!opt->renormalize)
diff --git a/t/t4300-merge-tree.sh b/t/t4300-merge-tree.sh
index 57c4f26e461..c3a03e54187 100755
--- a/t/t4300-merge-tree.sh
+++ b/t/t4300-merge-tree.sh
@@ -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
-- 
gitgitgadget

Re: [PATCH v3] merge-ort: initialize repo in index state

[Junio C Hamano]

"John Cai via GitGitGadget" <gitgitgadget@gmail.com> writes:

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

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

>       +		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(+)
>
> diff --git a/merge-ort.c b/merge-ort.c
> index 7857ce9fbd1..36537256613 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 = opt->repo;
>  	attr_index->initialized = 1;
>  
>  	if (!opt->renormalize)
> diff --git a/t/t4300-merge-tree.sh b/t/t4300-merge-tree.sh
> index 57c4f26e461..c3a03e54187 100755
> --- a/t/t4300-merge-tree.sh
> +++ b/t/t4300-merge-tree.sh
> @@ -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" &&

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

> +		git checkout @{-1} &&

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

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

> +		printf "foo\nbar\nbaz\n" >expect &&

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

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

Re: [PATCH v3] merge-ort: initialize repo in index state

[John Cai]

Hi Junio

On 9 Oct 2023, at 17:41, Junio C Hamano wrote:

> "John Cai via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
>>     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.
>
>>      -+		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.

wasn't aware of test_write_lines, thanks.

>
>>       +		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(+)
>>
>> diff --git a/merge-ort.c b/merge-ort.c
>> index 7857ce9fbd1..36537256613 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 = opt->repo;
>>  	attr_index->initialized = 1;
>>
>>  	if (!opt->renormalize)
>> diff --git a/t/t4300-merge-tree.sh b/t/t4300-merge-tree.sh
>> index 57c4f26e461..c3a03e54187 100755
>> --- a/t/t4300-merge-tree.sh
>> +++ b/t/t4300-merge-tree.sh
>> @@ -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" &&
>
> OK, the branch "gitattributes" will be used to drive merge of file1
> using the union merge to avoid conflicting.
>
>> +		git checkout @{-1} &&
>
> But such attribute will only be available in that branch, not in the
> checked out working tree.  And then
>
>> +		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.
>
>> +		printf "foo\nbar\nbaz\n" >expect &&
>
> I'll squash in the "test_write_lines" change while queuing.

thank you!
John

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