Re: [maintainer-tools PATCH RFC 3/3] dim: fix rr_cache_dir discovery
On Tue, Dec 18, 2018 at 8:15 AM Andrzej Hajda 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
> ---
> 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
___
dim-tools mailing list
dim-tools@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dim-tools
Re: [maintainer-tools PATCH RFC 3/3] dim: fix rr_cache_dir discovery
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
---
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
>>
___
dim-tools mailing list
dim-tools@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dim-tools
Re: [maintainer-tools PATCH RFC 3/3] dim: fix rr_cache_dir discovery
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
> >> ---
> >> 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
___
dim-tools mailing list
dim-tools@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dim-tools
Re: [maintainer-tools PATCH RFC 3/3] dim: fix rr_cache_dir discovery
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
>> ---
>> 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
___
dim-tools mailing list
dim-tools@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dim-tools
