[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 at 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 at lists.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/dim-tools
>>