On Tue, Dec 18, 2018 at 8:15 AM Andrzej Hajda <a.hajda@xxxxxxxxxxx> 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@xxxxxxxxxxx> > >>>> --- > >>>> 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@xxxxxxxxxxxxxxxxxxxxx > >>>> 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@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel