[maintainer-tools PATCH RFC 0/3] dim: fix git directory evaluation

[maintainer-tools PATCH RFC 0/3] dim: fix git directory evaluation

[Andrzej Hajda]

Hi all,

This small patchset fixes issues with dim used on git worktree's. It was not
widely tested - as I am little bit afraid to break drm infrastructure, and
I do not know if and how it interacts with other maintainer tools.
Especially in case of the last patch I am not sure what I am really touching :)

Regards
Andrzej


Andrzej Hajda (3):
  dim: allow git_dir to specify arbitrary work directory
  dim: fix git directory handling
  dim: fix rr_cache_dir discovery

 dim | 33 ++++++++++++++++-----------------
 1 file changed, 16 insertions(+), 17 deletions(-)

-- 
2.17.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[maintainer-tools PATCH RFC 1/3] dim: allow git_dir to specify arbitrary work directory

[Andrzej Hajda]

git_dir function returns git directory for current working directory.
Allowing specifying any directory allows to reuse it more widely.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 dim | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/dim b/dim
index 70939ff..df66c58 100755
--- a/dim
+++ b/dim
@@ -565,10 +565,12 @@ function rr_cache_dir
 
 function git_dir
 {
-	if [ -d $PWD/.git ] ; then
-		echo $PWD/.git
+	local dir=${1:-$PWD}
+
+	if [ -d $dir/.git ] ; then
+		echo $dir/.git
 	else
-		cut -d ' ' -f 2 < $PWD/.git
+		cut -d ' ' -f 2 < $dir/.git
 	fi
 }
 
-- 
2.17.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[maintainer-tools PATCH RFC 2/3] dim: fix git directory handling

[Andrzej Hajda]

Assumption that git directory is always located at REPO/.git is incorrect,
especially in case of git worktrees. There is already function to deal
with it correctly - git_dir, let's then use it.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 dim | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/dim b/dim
index df66c58..3afa8b6 100755
--- a/dim
+++ b/dim
@@ -1088,12 +1088,7 @@ function dim_backmerge
 
 	git merge --rerere-autoupdate --no-commit $upstream >& /dev/null || true
 
-	if [[ -d .git ]]; then
-		patch_file=".git"
-	else
-		patch_file=$(cut -d ' ' -f 2 .git)
-	fi
-	patch_file=$patch_file/MERGE_MSG
+	patch_file=$(git_dir)/MERGE_MSG
 
 
 	cat > $patch_file <<-HERE
@@ -1340,7 +1335,7 @@ dim_alias_mrr=magic-rebase-resolve
 function dim_magic_rebase_resolve
 {
 	git diff HEAD | patch -p1 -R
-	dim_magic_patch < .git/rebase-merge/patch
+	dim_magic_patch < $(git_dir)/rebase-merge/patch
 	make $DIM_MAKE_OPTIONS
 	git add -u
 	git rebase --continue
@@ -2102,7 +2097,7 @@ function setup_aux_checkout # name url directory
 			git clone --reference=$DIM_PREFIX/$DIM_REPO/.git $url $dir
 			cd $dir
 			git config remote.origin.url $url
-			echo "$DIM_PREFIX/$DIM_REPO/.git/objects" > .git/objects/info/alternates
+			echo "$(git_dir $DIM_PREFIX/$DIM_REPO)/objects" > $(git_dir)/objects/info/alternates
 			git repack -a -d -l
 			remote=origin
 		fi
@@ -2132,7 +2127,7 @@ function dim_setup
 	fi
 	cd $DIM_PREFIX
 
-	if [ ! -d $DIM_PREFIX/$DIM_REPO/.git ]; then
+	if [ ! -d $(git_dir $DIM_PREFIX/$DIM_REPO) ]; then
 		echoerr "No git checkout found in $DIM_PREFIX/$DIM_REPO."
 		echoerr "Please set up your maintainer linux repository at $DIM_PREFIX/$DIM_REPO with"
 		echoerr "    cd $DIM_PREFIX"
-- 
2.17.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[maintainer-tools PATCH RFC 3/3] dim: fix rr_cache_dir discovery

[Andrzej Hajda]

rr_cache_dir function cannot assume REPO/.git is a directory. On the other
side it should be backward compatible - if rr-cache directory/link already
exists it should be returned.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
Hi,

I am not sure of the purpose of rr-cache symbolic link, dim does not use
it (except its creation/removal). So this patch should be verified by
someone who knows better what is going on here.

Regards
Andrzej
---
 dim | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/dim b/dim
index 3afa8b6..b72ebfd 100755
--- a/dim
+++ b/dim
@@ -554,15 +554,6 @@ function check_conflicts # tree
 	true
 }
 
-function rr_cache_dir
-{
-	if [ -d $DIM_PREFIX/drm-tip/.git/ ] ; then
-		echo $DIM_PREFIX/drm-tip/.git/rr-cache
-	else
-		echo $DIM_PREFIX/$DIM_REPO/.git/rr-cache
-	fi
-}
-
 function git_dir
 {
 	local dir=${1:-$PWD}
@@ -574,6 +565,17 @@ function git_dir
 	fi
 }
 
+function rr_cache_dir
+{
+	local dir=$(git_dir $DIM_PREFIX/$DIM_REPO)/rr-cache
+
+	if [ -d $dir ]; then
+		echo $dir
+	else
+		echo $(git_dir $DIM_PREFIX/drm-tip)/rr-cache
+	fi
+}
+
 function pull_rerere_cache
 {
 	cd $DIM_PREFIX/drm-rerere/
-- 
2.17.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [maintainer-tools PATCH RFC 2/3] dim: fix git directory handling

[Daniel Vetter]

On Fri, Dec 14, 2018 at 02:38:51PM +0100, Andrzej Hajda wrote:
> Assumption that git directory is always located at REPO/.git is incorrect,
> especially in case of git worktrees. There is already function to deal
> with it correctly - git_dir, let's then use it.
> 
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>

Patch 1&2 are Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> ---
>  dim | 13 ++++---------
>  1 file changed, 4 insertions(+), 9 deletions(-)
> 
> diff --git a/dim b/dim
> index df66c58..3afa8b6 100755
> --- a/dim
> +++ b/dim
> @@ -1088,12 +1088,7 @@ function dim_backmerge
>  
>  	git merge --rerere-autoupdate --no-commit $upstream >& /dev/null || true
>  
> -	if [[ -d .git ]]; then
> -		patch_file=".git"
> -	else
> -		patch_file=$(cut -d ' ' -f 2 .git)
> -	fi
> -	patch_file=$patch_file/MERGE_MSG
> +	patch_file=$(git_dir)/MERGE_MSG
>  
>  
>  	cat > $patch_file <<-HERE
> @@ -1340,7 +1335,7 @@ dim_alias_mrr=magic-rebase-resolve
>  function dim_magic_rebase_resolve
>  {
>  	git diff HEAD | patch -p1 -R
> -	dim_magic_patch < .git/rebase-merge/patch
> +	dim_magic_patch < $(git_dir)/rebase-merge/patch
>  	make $DIM_MAKE_OPTIONS
>  	git add -u
>  	git rebase --continue
> @@ -2102,7 +2097,7 @@ function setup_aux_checkout # name url directory
>  			git clone --reference=$DIM_PREFIX/$DIM_REPO/.git $url $dir
>  			cd $dir
>  			git config remote.origin.url $url
> -			echo "$DIM_PREFIX/$DIM_REPO/.git/objects" > .git/objects/info/alternates
> +			echo "$(git_dir $DIM_PREFIX/$DIM_REPO)/objects" > $(git_dir)/objects/info/alternates
>  			git repack -a -d -l
>  			remote=origin
>  		fi
> @@ -2132,7 +2127,7 @@ function dim_setup
>  	fi
>  	cd $DIM_PREFIX
>  
> -	if [ ! -d $DIM_PREFIX/$DIM_REPO/.git ]; then
> +	if [ ! -d $(git_dir $DIM_PREFIX/$DIM_REPO) ]; then
>  		echoerr "No git checkout found in $DIM_PREFIX/$DIM_REPO."
>  		echoerr "Please set up your maintainer linux repository at $DIM_PREFIX/$DIM_REPO with"
>  		echoerr "    cd $DIM_PREFIX"
> -- 
> 2.17.1
> 
> _______________________________________________
> dim-tools mailing list
> dim-tools@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dim-tools

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [maintainer-tools PATCH RFC 3/3] dim: fix rr_cache_dir discovery

[Daniel Vetter]

On Fri, Dec 14, 2018 at 02:38:52PM +0100, Andrzej Hajda wrote:
> rr_cache_dir function cannot assume REPO/.git is a directory. On the other
> side it should be backward compatible - if rr-cache directory/link already
> exists it should be returned.
> 
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
> Hi,
> 
> I am not sure of the purpose of rr-cache symbolic link, dim does not use
> it (except its creation/removal). So this patch should be verified by
> someone who knows better what is going on here.
> 
> Regards
> Andrzej
> ---
>  dim | 20 +++++++++++---------
>  1 file changed, 11 insertions(+), 9 deletions(-)
> 
> diff --git a/dim b/dim
> index 3afa8b6..b72ebfd 100755
> --- a/dim
> +++ b/dim
> @@ -554,15 +554,6 @@ function check_conflicts # tree
>  	true
>  }
>  
> -function rr_cache_dir
> -{
> -	if [ -d $DIM_PREFIX/drm-tip/.git/ ] ; then
> -		echo $DIM_PREFIX/drm-tip/.git/rr-cache
> -	else
> -		echo $DIM_PREFIX/$DIM_REPO/.git/rr-cache
> -	fi
> -}

I think this breaks it, rr-cache is shared among all worktrees (which is a
big reason for having them).

And yes dim only sets up the symlink, dim rebuild-tip uses the rr-cache
stuff automatically through git merge.
-Daniel

> -
>  function git_dir
>  {
>  	local dir=${1:-$PWD}
> @@ -574,6 +565,17 @@ function git_dir
>  	fi
>  }
>  
> +function rr_cache_dir
> +{
> +	local dir=$(git_dir $DIM_PREFIX/$DIM_REPO)/rr-cache
> +
> +	if [ -d $dir ]; then
> +		echo $dir
> +	else
> +		echo $(git_dir $DIM_PREFIX/drm-tip)/rr-cache
> +	fi
> +}
> +
>  function pull_rerere_cache
>  {
>  	cd $DIM_PREFIX/drm-rerere/
> -- 
> 2.17.1
> 
> _______________________________________________
> dim-tools mailing list
> dim-tools@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dim-tools

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [maintainer-tools PATCH RFC 3/3] dim: fix rr_cache_dir discovery

[Andrzej Hajda]

Hi Daniel,

Thanks for reviewing other two patches.


On 14.12.2018 17:29, Daniel Vetter wrote:
> On Fri, Dec 14, 2018 at 02:38:52PM +0100, Andrzej Hajda wrote:
>> rr_cache_dir function cannot assume REPO/.git is a directory. On the other
>> side it should be backward compatible - if rr-cache directory/link already
>> exists it should be returned.
>>
>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
>> ---
>> Hi,
>>
>> I am not sure of the purpose of rr-cache symbolic link, dim does not use
>> it (except its creation/removal). So this patch should be verified by
>> someone who knows better what is going on here.
>>
>> Regards
>> Andrzej
>> ---
>>  dim | 20 +++++++++++---------
>>  1 file changed, 11 insertions(+), 9 deletions(-)
>>
>> diff --git a/dim b/dim
>> index 3afa8b6..b72ebfd 100755
>> --- a/dim
>> +++ b/dim
>> @@ -554,15 +554,6 @@ function check_conflicts # tree
>>  	true
>>  }
>>  
>> -function rr_cache_dir
>> -{
>> -	if [ -d $DIM_PREFIX/drm-tip/.git/ ] ; then
>> -		echo $DIM_PREFIX/drm-tip/.git/rr-cache
>> -	else
>> -		echo $DIM_PREFIX/$DIM_REPO/.git/rr-cache
>> -	fi
>> -}
> I think this breaks it, rr-cache is shared among all worktrees (which is a
> big reason for having them).


If drm-tip and $DIM_REPO are worktrees (more exactly "linked working
tree" - ie .git is a file), then rr_cache_dir will return invalid path.

As a result update_rerere_cache will fail create symlink, with this kind
of error:

    ln: failed to create symbolic link '/lab/dim/drm-misc-next/.git':
File exists

> And yes dim only sets up the symlink, dim rebuild-tip uses the rr-cache
> stuff automatically through git merge.


I still do not see who and when is using (ie. reading) this link.
rebuild_tip seems to use only $DIM_PREFIX/drm_rerere/rr-cache. Is there
some magic inside git itself, or I am just blind?


Regards

Andrzej



> -Daniel
>
>> -
>>  function git_dir
>>  {
>>  	local dir=${1:-$PWD}
>> @@ -574,6 +565,17 @@ function git_dir
>>  	fi
>>  }
>>  
>> +function rr_cache_dir
>> +{
>> +	local dir=$(git_dir $DIM_PREFIX/$DIM_REPO)/rr-cache
>> +
>> +	if [ -d $dir ]; then
>> +		echo $dir
>> +	else
>> +		echo $(git_dir $DIM_PREFIX/drm-tip)/rr-cache
>> +	fi
>> +}
>> +
>>  function pull_rerere_cache
>>  {
>>  	cd $DIM_PREFIX/drm-rerere/
>> -- 
>> 2.17.1
>>
>> _______________________________________________
>> dim-tools mailing list
>> dim-tools@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dim-tools


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [maintainer-tools PATCH RFC 3/3] dim: fix rr_cache_dir discovery

[Daniel Vetter]

On Mon, Dec 17, 2018 at 10:54:48AM +0100, Andrzej Hajda wrote:
> Hi Daniel,
> 
> Thanks for reviewing other two patches.
> 
> 
> On 14.12.2018 17:29, Daniel Vetter wrote:
> > On Fri, Dec 14, 2018 at 02:38:52PM +0100, Andrzej Hajda wrote:
> >> rr_cache_dir function cannot assume REPO/.git is a directory. On the other
> >> side it should be backward compatible - if rr-cache directory/link already
> >> exists it should be returned.
> >>
> >> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> >> ---
> >> Hi,
> >>
> >> I am not sure of the purpose of rr-cache symbolic link, dim does not use
> >> it (except its creation/removal). So this patch should be verified by
> >> someone who knows better what is going on here.
> >>
> >> Regards
> >> Andrzej
> >> ---
> >>  dim | 20 +++++++++++---------
> >>  1 file changed, 11 insertions(+), 9 deletions(-)
> >>
> >> diff --git a/dim b/dim
> >> index 3afa8b6..b72ebfd 100755
> >> --- a/dim
> >> +++ b/dim
> >> @@ -554,15 +554,6 @@ function check_conflicts # tree
> >>  	true
> >>  }
> >>  
> >> -function rr_cache_dir
> >> -{
> >> -	if [ -d $DIM_PREFIX/drm-tip/.git/ ] ; then
> >> -		echo $DIM_PREFIX/drm-tip/.git/rr-cache
> >> -	else
> >> -		echo $DIM_PREFIX/$DIM_REPO/.git/rr-cache
> >> -	fi
> >> -}
> > I think this breaks it, rr-cache is shared among all worktrees (which is a
> > big reason for having them).
> 
> 
> If drm-tip and $DIM_REPO are worktrees (more exactly "linked working
> tree" - ie .git is a file), then rr_cache_dir will return invalid path.
> 
> As a result update_rerere_cache will fail create symlink, with this kind
> of error:
> 
>     ln: failed to create symbolic link '/lab/dim/drm-misc-next/.git':
> File exists

Ah, now I get it, your $DIM_REPO is also a worktree. Yes that's not
supported with the current code.

But your code doesn't fix it. We do _not_ want $(git_dir)/rr-cache here,
but really the .git/rr-cache of the master repo. The worktrees do not have
their own private rr-cache, but use the one of the master repo.

> > And yes dim only sets up the symlink, dim rebuild-tip uses the rr-cache
> > stuff automatically through git merge.
> 
> 
> I still do not see who and when is using (ie. reading) this link.
> rebuild_tip seems to use only $DIM_PREFIX/drm_rerere/rr-cache. Is there
> some magic inside git itself, or I am just blind?

git merge --rerere-autoupdate uses it internally. We want that drm-tip
uses the rr-cache we store in drm-rerere/rr-cache.

Maybe $(git_dir drm-tip)/../../rr-cache works? Again, the important part
is that we edit the .git/rr-cache in your master repo, not in any of the
worktrees (rr-cache doesn't exist there).
-Daniel

> 
> 
> Regards
> 
> Andrzej
> 
> 
> 
> > -Daniel
> >
> >> -
> >>  function git_dir
> >>  {
> >>  	local dir=${1:-$PWD}
> >> @@ -574,6 +565,17 @@ function git_dir
> >>  	fi
> >>  }
> >>  
> >> +function rr_cache_dir
> >> +{
> >> +	local dir=$(git_dir $DIM_PREFIX/$DIM_REPO)/rr-cache
> >> +
> >> +	if [ -d $dir ]; then
> >> +		echo $dir
> >> +	else
> >> +		echo $(git_dir $DIM_PREFIX/drm-tip)/rr-cache
> >> +	fi
> >> +}
> >> +
> >>  function pull_rerere_cache
> >>  {
> >>  	cd $DIM_PREFIX/drm-rerere/
> >> -- 
> >> 2.17.1
> >>
> >> _______________________________________________
> >> dim-tools mailing list
> >> dim-tools@lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/dim-tools
> 
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [maintainer-tools PATCH RFC 3/3] dim: fix rr_cache_dir discovery

[Andrzej Hajda]

On 17.12.2018 15:46, Daniel Vetter wrote:
> On Mon, Dec 17, 2018 at 10:54:48AM +0100, Andrzej Hajda wrote:
>> Hi Daniel,
>>
>> Thanks for reviewing other two patches.
>>
>>
>> On 14.12.2018 17:29, Daniel Vetter wrote:
>>> On Fri, Dec 14, 2018 at 02:38:52PM +0100, Andrzej Hajda wrote:
>>>> rr_cache_dir function cannot assume REPO/.git is a directory. On the other
>>>> side it should be backward compatible - if rr-cache directory/link already
>>>> exists it should be returned.
>>>>
>>>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
>>>> ---
>>>> Hi,
>>>>
>>>> I am not sure of the purpose of rr-cache symbolic link, dim does not use
>>>> it (except its creation/removal). So this patch should be verified by
>>>> someone who knows better what is going on here.
>>>>
>>>> Regards
>>>> Andrzej
>>>> ---
>>>>  dim | 20 +++++++++++---------
>>>>  1 file changed, 11 insertions(+), 9 deletions(-)
>>>>
>>>> diff --git a/dim b/dim
>>>> index 3afa8b6..b72ebfd 100755
>>>> --- a/dim
>>>> +++ b/dim
>>>> @@ -554,15 +554,6 @@ function check_conflicts # tree
>>>>  	true
>>>>  }
>>>>  
>>>> -function rr_cache_dir
>>>> -{
>>>> -	if [ -d $DIM_PREFIX/drm-tip/.git/ ] ; then
>>>> -		echo $DIM_PREFIX/drm-tip/.git/rr-cache
>>>> -	else
>>>> -		echo $DIM_PREFIX/$DIM_REPO/.git/rr-cache
>>>> -	fi
>>>> -}
>>> I think this breaks it, rr-cache is shared among all worktrees (which is a
>>> big reason for having them).
>>
>> If drm-tip and $DIM_REPO are worktrees (more exactly "linked working
>> tree" - ie .git is a file), then rr_cache_dir will return invalid path.
>>
>> As a result update_rerere_cache will fail create symlink, with this kind
>> of error:
>>
>>     ln: failed to create symbolic link '/lab/dim/drm-misc-next/.git':
>> File exists
> Ah, now I get it, your $DIM_REPO is also a worktree. Yes that's not
> supported with the current code.
>
> But your code doesn't fix it. We do _not_ want $(git_dir)/rr-cache here,
> but really the .git/rr-cache of the master repo. The worktrees do not have
> their own private rr-cache, but use the one of the master repo.
>
>>> And yes dim only sets up the symlink, dim rebuild-tip uses the rr-cache
>>> stuff automatically through git merge.
>>
>> I still do not see who and when is using (ie. reading) this link.
>> rebuild_tip seems to use only $DIM_PREFIX/drm_rerere/rr-cache. Is there
>> some magic inside git itself, or I am just blind?
> git merge --rerere-autoupdate uses it internally. We want that drm-tip
> uses the rr-cache we store in drm-rerere/rr-cache.
>
> Maybe $(git_dir drm-tip)/../../rr-cache works? Again, the important part
> is that we edit the .git/rr-cache in your master repo, not in any of the
> worktrees (rr-cache doesn't exist there).


I think the proper way of finding rr-cache would be:


function rr_cache_dir
{
	echo $(git rev-parse --git-common-dir)/rr-cache
}


If you are OK with it I can prepare patch. In fact since the function is
used only in update_rerere_cache, it could be squashed there:

function update_rerere_cache
{
        echo -n "Updating rerere cache... "
        pull_rerere_cache

        local rr_cache_dir=$(git rev-parse --git-common-dir)/rr-cache

        ...

}


Is this approach OK?


Regards

Andrzej



> -Daniel
>
>>
>> Regards
>>
>> Andrzej
>>
>>
>>
>>> -Daniel
>>>
>>>> -
>>>>  function git_dir
>>>>  {
>>>>  	local dir=${1:-$PWD}
>>>> @@ -574,6 +565,17 @@ function git_dir
>>>>  	fi
>>>>  }
>>>>  
>>>> +function rr_cache_dir
>>>> +{
>>>> +	local dir=$(git_dir $DIM_PREFIX/$DIM_REPO)/rr-cache
>>>> +
>>>> +	if [ -d $dir ]; then
>>>> +		echo $dir
>>>> +	else
>>>> +		echo $(git_dir $DIM_PREFIX/drm-tip)/rr-cache
>>>> +	fi
>>>> +}
>>>> +
>>>>  function pull_rerere_cache
>>>>  {
>>>>  	cd $DIM_PREFIX/drm-rerere/
>>>> -- 
>>>> 2.17.1
>>>>
>>>> _______________________________________________
>>>> dim-tools mailing list
>>>> dim-tools@lists.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/dim-tools
>>

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [maintainer-tools PATCH RFC 3/3] dim: fix rr_cache_dir discovery

[Daniel Vetter]

On Tue, Dec 18, 2018 at 8:15 AM Andrzej Hajda <a.hajda@samsung.com> wrote:
>
> On 17.12.2018 15:46, Daniel Vetter wrote:
> > On Mon, Dec 17, 2018 at 10:54:48AM +0100, Andrzej Hajda wrote:
> >> Hi Daniel,
> >>
> >> Thanks for reviewing other two patches.
> >>
> >>
> >> On 14.12.2018 17:29, Daniel Vetter wrote:
> >>> On Fri, Dec 14, 2018 at 02:38:52PM +0100, Andrzej Hajda wrote:
> >>>> rr_cache_dir function cannot assume REPO/.git is a directory. On the other
> >>>> side it should be backward compatible - if rr-cache directory/link already
> >>>> exists it should be returned.
> >>>>
> >>>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> >>>> ---
> >>>> Hi,
> >>>>
> >>>> I am not sure of the purpose of rr-cache symbolic link, dim does not use
> >>>> it (except its creation/removal). So this patch should be verified by
> >>>> someone who knows better what is going on here.
> >>>>
> >>>> Regards
> >>>> Andrzej
> >>>> ---
> >>>>  dim | 20 +++++++++++---------
> >>>>  1 file changed, 11 insertions(+), 9 deletions(-)
> >>>>
> >>>> diff --git a/dim b/dim
> >>>> index 3afa8b6..b72ebfd 100755
> >>>> --- a/dim
> >>>> +++ b/dim
> >>>> @@ -554,15 +554,6 @@ function check_conflicts # tree
> >>>>    true
> >>>>  }
> >>>>
> >>>> -function rr_cache_dir
> >>>> -{
> >>>> -  if [ -d $DIM_PREFIX/drm-tip/.git/ ] ; then
> >>>> -          echo $DIM_PREFIX/drm-tip/.git/rr-cache
> >>>> -  else
> >>>> -          echo $DIM_PREFIX/$DIM_REPO/.git/rr-cache
> >>>> -  fi
> >>>> -}
> >>> I think this breaks it, rr-cache is shared among all worktrees (which is a
> >>> big reason for having them).
> >>
> >> If drm-tip and $DIM_REPO are worktrees (more exactly "linked working
> >> tree" - ie .git is a file), then rr_cache_dir will return invalid path.
> >>
> >> As a result update_rerere_cache will fail create symlink, with this kind
> >> of error:
> >>
> >>     ln: failed to create symbolic link '/lab/dim/drm-misc-next/.git':
> >> File exists
> > Ah, now I get it, your $DIM_REPO is also a worktree. Yes that's not
> > supported with the current code.
> >
> > But your code doesn't fix it. We do _not_ want $(git_dir)/rr-cache here,
> > but really the .git/rr-cache of the master repo. The worktrees do not have
> > their own private rr-cache, but use the one of the master repo.
> >
> >>> And yes dim only sets up the symlink, dim rebuild-tip uses the rr-cache
> >>> stuff automatically through git merge.
> >>
> >> I still do not see who and when is using (ie. reading) this link.
> >> rebuild_tip seems to use only $DIM_PREFIX/drm_rerere/rr-cache. Is there
> >> some magic inside git itself, or I am just blind?
> > git merge --rerere-autoupdate uses it internally. We want that drm-tip
> > uses the rr-cache we store in drm-rerere/rr-cache.
> >
> > Maybe $(git_dir drm-tip)/../../rr-cache works? Again, the important part
> > is that we edit the .git/rr-cache in your master repo, not in any of the
> > worktrees (rr-cache doesn't exist there).
>
>
> I think the proper way of finding rr-cache would be:
>
>
> function rr_cache_dir
> {
>         echo $(git rev-parse --git-common-dir)/rr-cache
> }

Indeed. Also just noticed that rev-parse also has a --git-dir option.
Care to also change git_dir() to use that?

> If you are OK with it I can prepare patch. In fact since the function is
> used only in update_rerere_cache, it could be squashed there:
>
> function update_rerere_cache
> {
>         echo -n "Updating rerere cache... "
>         pull_rerere_cache
>
>         local rr_cache_dir=$(git rev-parse --git-common-dir)/rr-cache
>
>         ...
>
> }

Yeah makes sense.
-Daniel

>
>
> Is this approach OK?
>
>
> Regards
>
> Andrzej
>
>
>
> > -Daniel
> >
> >>
> >> Regards
> >>
> >> Andrzej
> >>
> >>
> >>
> >>> -Daniel
> >>>
> >>>> -
> >>>>  function git_dir
> >>>>  {
> >>>>    local dir=${1:-$PWD}
> >>>> @@ -574,6 +565,17 @@ function git_dir
> >>>>    fi
> >>>>  }
> >>>>
> >>>> +function rr_cache_dir
> >>>> +{
> >>>> +  local dir=$(git_dir $DIM_PREFIX/$DIM_REPO)/rr-cache
> >>>> +
> >>>> +  if [ -d $dir ]; then
> >>>> +          echo $dir
> >>>> +  else
> >>>> +          echo $(git_dir $DIM_PREFIX/drm-tip)/rr-cache
> >>>> +  fi
> >>>> +}
> >>>> +
> >>>>  function pull_rerere_cache
> >>>>  {
> >>>>    cd $DIM_PREFIX/drm-rerere/
> >>>> --
> >>>> 2.17.1
> >>>>
> >>>> _______________________________________________
> >>>> dim-tools mailing list
> >>>> dim-tools@lists.freedesktop.org
> >>>> https://lists.freedesktop.org/mailman/listinfo/dim-tools
> >>
>


-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel