Patchwork [STABLE] context: translate FilteredIndex/LookupError at repo[changeid] (API)

login
register
mail settings
Submitter Yuya Nishihara
Date April 19, 2018, 11:25 a.m.
Message ID <8ba1c6dab49f5665dc68.1524137134@mimosa>
Download mbox | patch
Permalink /patch/31199/
State Accepted
Headers show

Comments

Yuya Nishihara - April 19, 2018, 11:25 a.m.
# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1524135351 -32400
#      Thu Apr 19 19:55:51 2018 +0900
# Branch stable
# Node ID 8ba1c6dab49f5665dc687000f149eb530a1079cf
# Parent  8cde3d58cdc88707dd61a24fdccf229e2ac83610
context: translate FilteredIndex/LookupError at repo[changeid] (API)

This partially backs out ecd3f6909184. It seems layering violation for
repo[changeid] to raise storage-level exceptions transparently. Otherwise,
we would have to rewrite callers to catch all of them.

  try:
      repo[rev_or_node]
  except (error.RepoLookupError, error.FilteredIndexError,
          error.FilteredLookupError):
      pass

This would also fix filectx._changectx(), which catches FilteredRepoLookupError
to fall back to the unfiltered path.
via Mercurial-devel - April 19, 2018, 4:36 p.m.
On Thu, Apr 19, 2018 at 4:25 AM Yuya Nishihara <yuya@tcha.org> wrote:

> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1524135351 -32400
> #      Thu Apr 19 19:55:51 2018 +0900
> # Branch stable
> # Node ID 8ba1c6dab49f5665dc687000f149eb530a1079cf
> # Parent  8cde3d58cdc88707dd61a24fdccf229e2ac83610
> context: translate FilteredIndex/LookupError at repo[changeid] (API)
>
> This partially backs out ecd3f6909184. It seems layering violation for
> repo[changeid] to raise storage-level exceptions transparently. Otherwise,
> we would have to rewrite callers to catch all of them.
>

Seems reasonable and it fixes that bug in _changectx() (thanks for noticing
that), so I'll queue this.

Btw, we should also fix the "abort: 00changelog.i@abc: ambiguous
identifier!" message. That's clearly not meant for the user.


>
>   try:
>       repo[rev_or_node]
>   except (error.RepoLookupError, error.FilteredIndexError,
>           error.FilteredLookupError):
>       pass
>

Perhaps we should introduce a FilteredAccessError that
FilteredIndexError, FilteredLookupError,
and FilteredRepoLookupError can extend from. It's freeze now, so I guess
I'll send patches for that in a few weeks.


> This would also fix filectx._changectx(), which catches
> FilteredRepoLookupError
> to fall back to the unfiltered path.
>
> diff --git a/mercurial/context.py b/mercurial/context.py
> --- a/mercurial/context.py
> +++ b/mercurial/context.py
> @@ -497,8 +497,10 @@ class changectx(basectx):
>                      changeid = hex(changeid)
>              except TypeError:
>                  pass
> -        except (error.FilteredIndexError, error.FilteredLookupError,
> -                error.FilteredRepoLookupError):
> +        except (error.FilteredIndexError, error.FilteredLookupError):
> +            raise error.FilteredRepoLookupError(_("filtered revision
> '%s'")
> +                                                % changeid)
> +        except error.FilteredRepoLookupError:
>              raise
>          except IndexError:
>              pass
> diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
> --- a/mercurial/localrepo.py
> +++ b/mercurial/localrepo.py
> @@ -850,8 +850,7 @@ class localrepository(object):
>          try:
>              self[changeid]
>              return True
> -        except (error.RepoLookupError, error.FilteredIndexError,
> -                error.FilteredLookupError):
> +        except error.RepoLookupError:
>              return False
>
>      def __nonzero__(self):
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
via Mercurial-devel - April 19, 2018, 4:37 p.m.
-8ba1c6dab49f@mercurial-scm.org

On Thu, Apr 19, 2018 at 9:36 AM Martin von Zweigbergk <martinvonz@google.com>
wrote:

>
>
> On Thu, Apr 19, 2018 at 4:25 AM Yuya Nishihara <yuya@tcha.org> wrote:
>
>> # HG changeset patch
>> # User Yuya Nishihara <yuya@tcha.org>
>> # Date 1524135351 -32400
>> #      Thu Apr 19 19:55:51 2018 +0900
>> # Branch stable
>> # Node ID 8ba1c6dab49f5665dc687000f149eb530a1079cf
>> # Parent  8cde3d58cdc88707dd61a24fdccf229e2ac83610
>> context: translate FilteredIndex/LookupError at repo[changeid] (API)
>>
>> This partially backs out ecd3f6909184. It seems layering violation for
>> repo[changeid] to raise storage-level exceptions transparently. Otherwise,
>> we would have to rewrite callers to catch all of them.
>>
>
> Seems reasonable and it fixes that bug in _changectx() (thanks for
> noticing that), so I'll queue this.
>
> Btw, we should also fix the "abort: 00changelog.i@abc: ambiguous
> identifier!" message. That's clearly not meant for the user.
>
>
>>
>>   try:
>>       repo[rev_or_node]
>>   except (error.RepoLookupError, error.FilteredIndexError,
>>           error.FilteredLookupError):
>>       pass
>>
>
> Perhaps we should introduce a FilteredAccessError that
> FilteredIndexError, FilteredLookupError, and FilteredRepoLookupError can
> extend from. It's freeze now, so I guess I'll send patches for that in a
> few weeks.
>
>
>> This would also fix filectx._changectx(), which catches
>> FilteredRepoLookupError
>> to fall back to the unfiltered path.
>>
>> diff --git a/mercurial/context.py b/mercurial/context.py
>> --- a/mercurial/context.py
>> +++ b/mercurial/context.py
>> @@ -497,8 +497,10 @@ class changectx(basectx):
>>                      changeid = hex(changeid)
>>              except TypeError:
>>                  pass
>> -        except (error.FilteredIndexError, error.FilteredLookupError,
>> -                error.FilteredRepoLookupError):
>> +        except (error.FilteredIndexError, error.FilteredLookupError):
>> +            raise error.FilteredRepoLookupError(_("filtered revision
>> '%s'")
>> +                                                % changeid)
>> +        except error.FilteredRepoLookupError:
>>              raise
>>          except IndexError:
>>              pass
>> diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
>> --- a/mercurial/localrepo.py
>> +++ b/mercurial/localrepo.py
>> @@ -850,8 +850,7 @@ class localrepository(object):
>>          try:
>>              self[changeid]
>>              return True
>> -        except (error.RepoLookupError, error.FilteredIndexError,
>> -                error.FilteredLookupError):
>> +        except error.RepoLookupError:
>>              return False
>>
>>      def __nonzero__(self):
>> _______________________________________________
>> Mercurial-devel mailing list
>> Mercurial-devel@mercurial-scm.org
>> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>>
>
Augie Fackler - April 19, 2018, 7:06 p.m.
On Thu, Apr 19, 2018 at 04:37:34PM +0000, Martin von Zweigbergk via Mercurial-devel wrote:
> -8ba1c6dab49f@mercurial-scm.org
>
> On Thu, Apr 19, 2018 at 9:36 AM Martin von Zweigbergk <martinvonz@google.com>
> wrote:
>
> >
> >
> > On Thu, Apr 19, 2018 at 4:25 AM Yuya Nishihara <yuya@tcha.org> wrote:
> >
> >> # HG changeset patch
> >> # User Yuya Nishihara <yuya@tcha.org>
> >> # Date 1524135351 -32400
> >> #      Thu Apr 19 19:55:51 2018 +0900
> >> # Branch stable
> >> # Node ID 8ba1c6dab49f5665dc687000f149eb530a1079cf
> >> # Parent  8cde3d58cdc88707dd61a24fdccf229e2ac83610
> >> context: translate FilteredIndex/LookupError at repo[changeid] (API)
> >>
> >> This partially backs out ecd3f6909184. It seems layering violation for
> >> repo[changeid] to raise storage-level exceptions transparently. Otherwise,
> >> we would have to rewrite callers to catch all of them.
> >>
> >
> > Seems reasonable and it fixes that bug in _changectx() (thanks for
> > noticing that), so I'll queue this.
> >
> > Btw, we should also fix the "abort: 00changelog.i@abc: ambiguous
> > identifier!" message. That's clearly not meant for the user.

(martin queued these, for those following along at home)
Yuya Nishihara - April 20, 2018, 12:30 p.m.
On Thu, 19 Apr 2018 16:36:21 +0000, Martin von Zweigbergk wrote:
> >   try:
> >       repo[rev_or_node]
> >   except (error.RepoLookupError, error.FilteredIndexError,
> >           error.FilteredLookupError):
> >       pass
> >
> 
> Perhaps we should introduce a FilteredAccessError that
> FilteredIndexError, FilteredLookupError,
> and FilteredRepoLookupError can extend from.

That could be, but we'll still need to translate them to a subclass of
RepoLookupError because a filtered revision should behave as if it weren't
there.

> -8ba1c6dab49f@mercurial-scm.org

Oops, copy-paste error.

Patch

diff --git a/mercurial/context.py b/mercurial/context.py
--- a/mercurial/context.py
+++ b/mercurial/context.py
@@ -497,8 +497,10 @@  class changectx(basectx):
                     changeid = hex(changeid)
             except TypeError:
                 pass
-        except (error.FilteredIndexError, error.FilteredLookupError,
-                error.FilteredRepoLookupError):
+        except (error.FilteredIndexError, error.FilteredLookupError):
+            raise error.FilteredRepoLookupError(_("filtered revision '%s'")
+                                                % changeid)
+        except error.FilteredRepoLookupError:
             raise
         except IndexError:
             pass
diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
--- a/mercurial/localrepo.py
+++ b/mercurial/localrepo.py
@@ -850,8 +850,7 @@  class localrepository(object):
         try:
             self[changeid]
             return True
-        except (error.RepoLookupError, error.FilteredIndexError,
-                error.FilteredLookupError):
+        except error.RepoLookupError:
             return False
 
     def __nonzero__(self):