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

[Daniel Vetter <daniel@ffwll.ch>]

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