[PATCH 1 of 2 STABLE] check-code: do not abort on an unreadable file, only report this

Simon Heimberg simohe at besonet.ch
Tue Jul 23 02:28:42 CDT 2013


I would prefer to not abort (and print a traceback) on EACCES, EISDIR 
and maybe some more exceptions.

When we only continue on ENOENT, using str(e) is unnecessary. It always 
tells the file does not exist. So it could be replaced by a constant.

All the mentioned exceptions (ENOENT, EACCES, EISDIR) have the filename 
after the colon, so stripping works as expected. When an exception 
without a colonwould be raised, split returns the entire message.
 >>> 'some message'.split(':', 1)[0]
     'some message'



ON 2013-07-22 12:40, Martin Geisler wrote:
> Simon Heimberg <simohe at besonet.ch> writes:
>
>> # HG changeset patch
>> # User Simon Heimberg <simohe at besonet.ch>
>> # Date 1374480285 -7200
>> # Node ID e6de3183429487a11ef1c28bc1562aa85617732b
>> # Parent  004f965630d907a3417a93e87d056ad4c2dab541
>> check-code: do not abort on an unreadable file, only report this
>>
>> diff -r 004f965630d9 -r e6de31834294 contrib/check-code.py
>> --- a/contrib/check-code.py	Fre Jul 12 11:14:42 2013 +0900
>> +++ b/contrib/check-code.py	Mon Jul 22 10:04:45 2013 +0200
>> @@ -407,7 +407,11 @@
>>                   print "Skipping %s for %s it doesn't match %s" % (
>>                          name, match, f)
>>               continue
>> -        fp = open(f)
>> +        try:
>> +            fp = open(f)
>> +        except IOError, e:
> I think you should include our normal
>
>      if e.errno != errno.ENOENT:
>          raise
>
> idiom here. That would also make the split below seem more robust.
>
>> +            print "Skipping %s, %s" % (f, str(e).split(':', 1)[0])
>> +            continue
>>           pre = post = fp.read()
>>           fp.close()
>>           if "no-" "check-code" in pre:
>> diff -r 004f965630d9 -r e6de31834294 tests/test-check-code.t
>> --- a/tests/test-check-code.t	Fre Jul 12 11:14:42 2013 +0900
>> +++ b/tests/test-check-code.t	Mon Jul 22 10:04:45 2013 +0200
>> @@ -187,8 +187,10 @@
>>     > # this next line is okay
>>     > raise SomeException(arg1, arg2)
>>     > EOF
>> -  $ "$check_code" raise-format.py
>> +  $ "$check_code" not-existing.py raise-format.py
>> +  Skipping*not-existing.py* (glob)
>>     raise-format.py:1:
>>      > raise SomeException, message
>>      don't use old-style two-argument raise, use Exception(message)
>>     [1]
>> +
>> _______________________________________________
>> Mercurial-devel mailing list
>> Mercurial-devel at selenic.com
>> http://selenic.com/mailman/listinfo/mercurial-devel



More information about the Mercurial-devel mailing list