[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 at 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 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
> }

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