Patchwork util: also catch IndexError

login
register
mail settings
Submitter Sean Farley
Date Oct. 13, 2015, 11:08 p.m.
Message ID <ef250ea05243630e486d.1444777706@laptop.office.atlassian.com>
Download mbox | patch
Permalink /patch/11038/
State Accepted
Commit 6331a0c310db8ad85cdd4c95709b41fc03bd79ad
Headers show

Comments

Sean Farley - Oct. 13, 2015, 11:08 p.m.
# HG changeset patch
# User Sean Farley <sean@farley.io>
# Date 1444777530 25200
#      Tue Oct 13 16:05:30 2015 -0700
# Node ID ef250ea05243630e486d04fa98d6481b25d32135
# Parent  a38924f7680c6b7d95e14ade999c35748c9dcafd
util: also catch IndexError

This makes life so, so much easier for hgwatchman, which provides a named tuple
but throws an IndexError instead of a TypeError.
Yuya Nishihara - Oct. 14, 2015, 2:02 p.m.
On Tue, 13 Oct 2015 16:08:26 -0700, Sean Farley wrote:
> # HG changeset patch
> # User Sean Farley <sean@farley.io>
> # Date 1444777530 25200
> #      Tue Oct 13 16:05:30 2015 -0700
> # Node ID ef250ea05243630e486d04fa98d6481b25d32135
> # Parent  a38924f7680c6b7d95e14ade999c35748c9dcafd
> util: also catch IndexError
> 
> This makes life so, so much easier for hgwatchman, which provides a named tuple
> but throws an IndexError instead of a TypeError.
> 
> diff --git a/mercurial/util.py b/mercurial/util.py
> --- a/mercurial/util.py
> +++ b/mercurial/util.py
> @@ -961,11 +961,11 @@ def statmtimesec(st):
>      to represent nanoseconds. If 'nsec' is close to 1 sec, 'int(st.st_mtime)'
>      can be 'sec + 1'. (issue4836)
>      """
>      try:
>          return st[stat.ST_MTIME]
> -    except TypeError:
> +    except (TypeError, IndexError):
>          # osutil.stat doesn't allow index access and its st_mtime is int
>          return st.st_mtime

I see. The watchmanstat has only 3 elements, so st[8] can raise IndexError
instead of returning wrong field.
Augie Fackler - Oct. 14, 2015, 2:29 p.m.
On Tue, Oct 13, 2015 at 04:08:26PM -0700, Sean Farley wrote:
> # HG changeset patch
> # User Sean Farley <sean@farley.io>
> # Date 1444777530 25200
> #      Tue Oct 13 16:05:30 2015 -0700
> # Node ID ef250ea05243630e486d04fa98d6481b25d32135
> # Parent  a38924f7680c6b7d95e14ade999c35748c9dcafd
> util: also catch IndexError
>
> This makes life so, so much easier for hgwatchman, which provides a named tuple
> but throws an IndexError instead of a TypeError.

Can we fix hgwatchman behavior to actually match what stat is returning?

>
> diff --git a/mercurial/util.py b/mercurial/util.py
> --- a/mercurial/util.py
> +++ b/mercurial/util.py
> @@ -961,11 +961,11 @@ def statmtimesec(st):
>      to represent nanoseconds. If 'nsec' is close to 1 sec, 'int(st.st_mtime)'
>      can be 'sec + 1'. (issue4836)
>      """
>      try:
>          return st[stat.ST_MTIME]
> -    except TypeError:
> +    except (TypeError, IndexError):
>          # osutil.stat doesn't allow index access and its st_mtime is int
>          return st.st_mtime
>
>  # File system features
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> https://selenic.com/mailman/listinfo/mercurial-devel
Augie Fackler - Oct. 14, 2015, 3:12 p.m.
On Wed, Oct 14, 2015 at 11:02:57PM +0900, Yuya Nishihara wrote:
> On Tue, 13 Oct 2015 16:08:26 -0700, Sean Farley wrote:
> > # HG changeset patch
> > # User Sean Farley <sean@farley.io>
> > # Date 1444777530 25200
> > #      Tue Oct 13 16:05:30 2015 -0700
> > # Node ID ef250ea05243630e486d04fa98d6481b25d32135
> > # Parent  a38924f7680c6b7d95e14ade999c35748c9dcafd
> > util: also catch IndexError
> >
> > This makes life so, so much easier for hgwatchman, which provides a named tuple
> > but throws an IndexError instead of a TypeError.
> >
> > diff --git a/mercurial/util.py b/mercurial/util.py
> > --- a/mercurial/util.py
> > +++ b/mercurial/util.py
> > @@ -961,11 +961,11 @@ def statmtimesec(st):
> >      to represent nanoseconds. If 'nsec' is close to 1 sec, 'int(st.st_mtime)'
> >      can be 'sec + 1'. (issue4836)
> >      """
> >      try:
> >          return st[stat.ST_MTIME]
> > -    except TypeError:
> > +    except (TypeError, IndexError):
> >          # osutil.stat doesn't allow index access and its st_mtime is int
> >          return st.st_mtime
>
> I see. The watchmanstat has only 3 elements, so st[8] can raise IndexError
> instead of returning wrong field.

Sigh. Okay, I'll go ahead and queue this then. Thanks!

> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> https://selenic.com/mailman/listinfo/mercurial-devel
Yuya Nishihara - Oct. 14, 2015, 3:33 p.m.
On Wed, 14 Oct 2015 11:12:42 -0400, Augie Fackler wrote:
> On Wed, Oct 14, 2015 at 11:02:57PM +0900, Yuya Nishihara wrote:
> > On Tue, 13 Oct 2015 16:08:26 -0700, Sean Farley wrote:
> > > # HG changeset patch
> > > # User Sean Farley <sean@farley.io>
> > > # Date 1444777530 25200
> > > #      Tue Oct 13 16:05:30 2015 -0700
> > > # Node ID ef250ea05243630e486d04fa98d6481b25d32135
> > > # Parent  a38924f7680c6b7d95e14ade999c35748c9dcafd
> > > util: also catch IndexError
> > >
> > > This makes life so, so much easier for hgwatchman, which provides a named tuple
> > > but throws an IndexError instead of a TypeError.
> > >
> > > diff --git a/mercurial/util.py b/mercurial/util.py
> > > --- a/mercurial/util.py
> > > +++ b/mercurial/util.py
> > > @@ -961,11 +961,11 @@ def statmtimesec(st):
> > >      to represent nanoseconds. If 'nsec' is close to 1 sec, 'int(st.st_mtime)'
> > >      can be 'sec + 1'. (issue4836)
> > >      """
> > >      try:
> > >          return st[stat.ST_MTIME]
> > > -    except TypeError:
> > > +    except (TypeError, IndexError):
> > >          # osutil.stat doesn't allow index access and its st_mtime is int
> > >          return st.st_mtime
> >
> > I see. The watchmanstat has only 3 elements, so st[8] can raise IndexError
> > instead of returning wrong field.
> 
> Sigh. Okay, I'll go ahead and queue this then. Thanks!

Oops, I just start to fix watchmanstat according your comment.
Augie Fackler - Oct. 14, 2015, 3:48 p.m.
> On Oct 14, 2015, at 11:33, Yuya Nishihara <yuya@tcha.org> wrote:
> 
> On Wed, 14 Oct 2015 11:12:42 -0400, Augie Fackler wrote:
>> On Wed, Oct 14, 2015 at 11:02:57PM +0900, Yuya Nishihara wrote:
>>> On Tue, 13 Oct 2015 16:08:26 -0700, Sean Farley wrote:
>>>> # HG changeset patch
>>>> # User Sean Farley <sean@farley.io>
>>>> # Date 1444777530 25200
>>>> #      Tue Oct 13 16:05:30 2015 -0700
>>>> # Node ID ef250ea05243630e486d04fa98d6481b25d32135
>>>> # Parent  a38924f7680c6b7d95e14ade999c35748c9dcafd
>>>> util: also catch IndexError
>>>> 
>>>> This makes life so, so much easier for hgwatchman, which provides a named tuple
>>>> but throws an IndexError instead of a TypeError.
>>>> 
>>>> diff --git a/mercurial/util.py b/mercurial/util.py
>>>> --- a/mercurial/util.py
>>>> +++ b/mercurial/util.py
>>>> @@ -961,11 +961,11 @@ def statmtimesec(st):
>>>>     to represent nanoseconds. If 'nsec' is close to 1 sec, 'int(st.st_mtime)'
>>>>     can be 'sec + 1'. (issue4836)
>>>>     """
>>>>     try:
>>>>         return st[stat.ST_MTIME]
>>>> -    except TypeError:
>>>> +    except (TypeError, IndexError):
>>>>         # osutil.stat doesn't allow index access and its st_mtime is int
>>>>         return st.st_mtime
>>> 
>>> I see. The watchmanstat has only 3 elements, so st[8] can raise IndexError
>>> instead of returning wrong field.
>> 
>> Sigh. Okay, I'll go ahead and queue this then. Thanks!
> 
> Oops, I just start to fix watchmanstat according your comment.

I'm happy to drop this patch if watchman can actually emulate stat results correctly. Should I drop the patch?
Yuya Nishihara - Oct. 15, 2015, 2:17 p.m.
On Wed, 14 Oct 2015 11:48:21 -0400, Augie Fackler wrote:
> > On Oct 14, 2015, at 11:33, Yuya Nishihara <yuya@tcha.org> wrote:
> > On Wed, 14 Oct 2015 11:12:42 -0400, Augie Fackler wrote:
> >> On Wed, Oct 14, 2015 at 11:02:57PM +0900, Yuya Nishihara wrote:
> >>> On Tue, 13 Oct 2015 16:08:26 -0700, Sean Farley wrote:
> >>>> # HG changeset patch
> >>>> # User Sean Farley <sean@farley.io>
> >>>> # Date 1444777530 25200
> >>>> #      Tue Oct 13 16:05:30 2015 -0700
> >>>> # Node ID ef250ea05243630e486d04fa98d6481b25d32135
> >>>> # Parent  a38924f7680c6b7d95e14ade999c35748c9dcafd
> >>>> util: also catch IndexError
> >>>> 
> >>>> This makes life so, so much easier for hgwatchman, which provides a named tuple
> >>>> but throws an IndexError instead of a TypeError.
> >>>> 
> >>>> diff --git a/mercurial/util.py b/mercurial/util.py
> >>>> --- a/mercurial/util.py
> >>>> +++ b/mercurial/util.py
> >>>> @@ -961,11 +961,11 @@ def statmtimesec(st):
> >>>>     to represent nanoseconds. If 'nsec' is close to 1 sec, 'int(st.st_mtime)'
> >>>>     can be 'sec + 1'. (issue4836)
> >>>>     """
> >>>>     try:
> >>>>         return st[stat.ST_MTIME]
> >>>> -    except TypeError:
> >>>> +    except (TypeError, IndexError):
> >>>>         # osutil.stat doesn't allow index access and its st_mtime is int
> >>>>         return st.st_mtime
> >>> 
> >>> I see. The watchmanstat has only 3 elements, so st[8] can raise IndexError
> >>> instead of returning wrong field.
> >> 
> >> Sigh. Okay, I'll go ahead and queue this then. Thanks!
> > 
> > Oops, I just start to fix watchmanstat according your comment.
> 
> I'm happy to drop this patch if watchman can actually emulate stat results
> correctly. Should I drop the patch?

The fix of watchmanstat is trivial. I'll send a backout of this patch if it
gets merged to hgwatchman.
Augie Fackler - Oct. 15, 2015, 2:35 p.m.
On Thu, Oct 15, 2015 at 10:17 AM, Yuya Nishihara <yuya@tcha.org> wrote:
> On Wed, 14 Oct 2015 11:48:21 -0400, Augie Fackler wrote:
>> > On Oct 14, 2015, at 11:33, Yuya Nishihara <yuya@tcha.org> wrote:
>> > On Wed, 14 Oct 2015 11:12:42 -0400, Augie Fackler wrote:
>> >> On Wed, Oct 14, 2015 at 11:02:57PM +0900, Yuya Nishihara wrote:
>> >>> On Tue, 13 Oct 2015 16:08:26 -0700, Sean Farley wrote:
>> >>>> # HG changeset patch
>> >>>> # User Sean Farley <sean@farley.io>
>> >>>> # Date 1444777530 25200
>> >>>> #      Tue Oct 13 16:05:30 2015 -0700
>> >>>> # Node ID ef250ea05243630e486d04fa98d6481b25d32135
>> >>>> # Parent  a38924f7680c6b7d95e14ade999c35748c9dcafd
>> >>>> util: also catch IndexError
>> >>>>
>> >>>> This makes life so, so much easier for hgwatchman, which provides a named tuple
>> >>>> but throws an IndexError instead of a TypeError.
>> >>>>
>> >>>> diff --git a/mercurial/util.py b/mercurial/util.py
>> >>>> --- a/mercurial/util.py
>> >>>> +++ b/mercurial/util.py
>> >>>> @@ -961,11 +961,11 @@ def statmtimesec(st):
>> >>>>     to represent nanoseconds. If 'nsec' is close to 1 sec, 'int(st.st_mtime)'
>> >>>>     can be 'sec + 1'. (issue4836)
>> >>>>     """
>> >>>>     try:
>> >>>>         return st[stat.ST_MTIME]
>> >>>> -    except TypeError:
>> >>>> +    except (TypeError, IndexError):
>> >>>>         # osutil.stat doesn't allow index access and its st_mtime is int
>> >>>>         return st.st_mtime
>> >>>
>> >>> I see. The watchmanstat has only 3 elements, so st[8] can raise IndexError
>> >>> instead of returning wrong field.
>> >>
>> >> Sigh. Okay, I'll go ahead and queue this then. Thanks!
>> >
>> > Oops, I just start to fix watchmanstat according your comment.
>>
>> I'm happy to drop this patch if watchman can actually emulate stat results
>> correctly. Should I drop the patch?
>
> The fix of watchmanstat is trivial. I'll send a backout of this patch if it
> gets merged to hgwatchman.

Awesome, thanks!

Patch

diff --git a/mercurial/util.py b/mercurial/util.py
--- a/mercurial/util.py
+++ b/mercurial/util.py
@@ -961,11 +961,11 @@  def statmtimesec(st):
     to represent nanoseconds. If 'nsec' is close to 1 sec, 'int(st.st_mtime)'
     can be 'sec + 1'. (issue4836)
     """
     try:
         return st[stat.ST_MTIME]
-    except TypeError:
+    except (TypeError, IndexError):
         # osutil.stat doesn't allow index access and its st_mtime is int
         return st.st_mtime
 
 # File system features