Rev 4933: Merge the 2.0 branch, resolve one conflict. in http://bazaar.launchpad.net/~jameinel/bzr/2.1.0rc1-pyrex-propagation
John Arbash Meinel
john at arbash-meinel.com
Tue Jan 5 05:10:08 GMT 2010
At http://bazaar.launchpad.net/~jameinel/bzr/2.1.0rc1-pyrex-propagation
------------------------------------------------------------
revno: 4933 [merge]
revision-id: john at arbash-meinel.com-20100105050945-itxiesuol1op6usy
parent: pqm at pqm.ubuntu.com-20100105004911-i582o8sqfqt7t7bn
parent: john at arbash-meinel.com-20100105050731-wrvytmz3b2aavylr
committer: John Arbash Meinel <john at arbash-meinel.com>
branch nick: 2.1.0rc1-pyrex-propagation
timestamp: Mon 2010-01-04 23:09:45 -0600
message:
Merge the 2.0 branch, resolve one conflict.
modified:
NEWS NEWS-20050323055033-4e00b5db738777ff
bzrlib/_annotator_pyx.pyx _annotator_pyx.pyx-20090623201937-ic33ic1aqvd9mjje-1
bzrlib/_btree_serializer_pyx.pyx _parse_btree_c.pyx-20080703034413-3q25bklkenti3p8p-2
bzrlib/_chk_map_pyx.pyx _chk_map_pyx.pyx-20090309111231-peyz7p2azr0dzdrb-1
bzrlib/_dirstate_helpers_pyx.pyx dirstate_helpers.pyx-20070503201057-u425eni465q4idwn-3
bzrlib/_groupcompress_pyx.pyx _groupcompress_c.pyx-20080724041824-yelg6ii7c7zxt4z0-1
bzrlib/_knit_load_data_pyx.pyx knit_c.pyx-20070509143944-u42gy8w387a10m0j-1
bzrlib/_known_graph_pyx.pyx _known_graph_pyx.pyx-20090610194911-yjk73td9hpjilas0-1
bzrlib/_rio_pyx.pyx _rio_pyx.pyx-20090514104636-8203jcqvfny56yrd-1
bzrlib/_walkdirs_win32.pyx _walkdirs_win32.pyx-20080716220454-kweh3tgxez5dvw2l-2
bzrlib/tests/test__dirstate_helpers.py test_dirstate_helper-20070504035751-jsbn00xodv0y1eve-2
bzrlib/tests/test_source.py test_source.py-20051207061333-a58dea6abecc030d
-------------- next part --------------
=== modified file 'NEWS'
--- a/NEWS 2010-01-04 05:40:01 +0000
+++ b/NEWS 2010-01-05 05:09:45 +0000
@@ -160,6 +160,10 @@
changed underneath it (like another autopack). Now concurrent
autopackers will properly succeed. (John Arbash Meinel, #495000)
+* ``_update_current_block`` no longer suppresses exceptions, so ^C at just
+ the right time will get propagated, rather than silently failing to move
+ the block pointer. (John Arbash Meinel, Gareth White, #495023)
+
Improvements
************
@@ -175,6 +179,11 @@
Testing
*******
+* We have a new ``test_source`` that ensures all pyrex ``cdef`` functions
+ handle exceptions somehow. (Possibly by setting ``# cannot_raise``
+ rather than an ``except ?:`` clause.) This should help prevent bugs like
+ bug #495023. (John Arbash Meinel)
+
bzr 2.1.0b4
###########
=== modified file 'bzrlib/_annotator_pyx.pyx'
--- a/bzrlib/_annotator_pyx.pyx 2009-07-08 17:09:03 +0000
+++ b/bzrlib/_annotator_pyx.pyx 2010-01-05 04:59:57 +0000
@@ -83,7 +83,14 @@
return 0
-cdef PyObject *_next_tuple_entry(object tpl, Py_ssize_t *pos):
+cdef PyObject *_next_tuple_entry(object tpl, Py_ssize_t *pos): # cannot_raise
+ """Return the next entry from this tuple.
+
+ :param tpl: The tuple we are investigating, *must* be a PyTuple
+ :param pos: The last item we found. Will be updated to the new position.
+
+ This cannot raise an exception, as it does no error checking.
+ """
pos[0] = pos[0] + 1
if pos[0] >= PyTuple_GET_SIZE(tpl):
return NULL
=== modified file 'bzrlib/_btree_serializer_pyx.pyx'
--- a/bzrlib/_btree_serializer_pyx.pyx 2009-10-12 20:07:09 +0000
+++ b/bzrlib/_btree_serializer_pyx.pyx 2010-01-05 05:09:45 +0000
@@ -65,7 +65,7 @@
# TODO: Find some way to import this from _dirstate_helpers
-cdef void* _my_memrchr(void *s, int c, size_t n):
+cdef void* _my_memrchr(void *s, int c, size_t n): # cannot_raise
# memrchr seems to be a GNU extension, so we have to implement it ourselves
# It is not present in any win32 standard library
cdef char *pos
=== modified file 'bzrlib/_chk_map_pyx.pyx'
--- a/bzrlib/_chk_map_pyx.pyx 2009-10-21 21:27:19 +0000
+++ b/bzrlib/_chk_map_pyx.pyx 2010-01-05 05:09:45 +0000
@@ -82,7 +82,7 @@
_unknown = None
# We shouldn't just copy this from _dirstate_helpers_pyx
-cdef void* _my_memrchr(void *s, int c, size_t n):
+cdef void* _my_memrchr(void *s, int c, size_t n): # cannot_raise
# memrchr seems to be a GNU extension, so we have to implement it ourselves
cdef char *pos
cdef char *start
=== modified file 'bzrlib/_dirstate_helpers_pyx.pyx'
--- a/bzrlib/_dirstate_helpers_pyx.pyx 2009-10-02 05:43:41 +0000
+++ b/bzrlib/_dirstate_helpers_pyx.pyx 2010-01-05 05:09:45 +0000
@@ -119,7 +119,7 @@
# void *memrchr(void *s, int c, size_t len)
-cdef void* _my_memrchr(void *s, int c, size_t n):
+cdef void* _my_memrchr(void *s, int c, size_t n): # cannot_raise
# memrchr seems to be a GNU extension, so we have to implement it ourselves
cdef char *pos
cdef char *start
@@ -165,7 +165,7 @@
return PyString_FromStringAndSize(s, size)
-cdef int _is_aligned(void *ptr):
+cdef int _is_aligned(void *ptr): # cannot_raise
"""Is this pointer aligned to an integer size offset?
:return: 1 if this pointer is aligned, 0 otherwise.
@@ -173,7 +173,7 @@
return ((<intptr_t>ptr) & ((sizeof(int))-1)) == 0
-cdef int _cmp_by_dirs(char *path1, int size1, char *path2, int size2):
+cdef int _cmp_by_dirs(char *path1, int size1, char *path2, int size2): # cannot_raise
cdef unsigned char *cur1
cdef unsigned char *cur2
cdef unsigned char *end1
@@ -295,7 +295,7 @@
cdef int _cmp_path_by_dirblock_intern(char *path1, int path1_len,
- char *path2, int path2_len):
+ char *path2, int path2_len): # cannot_raise
"""Compare two paths by what directory they are in.
see ``_cmp_path_by_dirblock`` for details.
@@ -768,7 +768,7 @@
state._dirblock_state = DirState.IN_MEMORY_UNMODIFIED
-cdef int minikind_from_mode(int mode):
+cdef int minikind_from_mode(int mode): # cannot_raise
# in order of frequency:
if S_ISREG(mode):
return c"f"
@@ -915,7 +915,8 @@
return link_or_sha1
-cdef char _minikind_from_string(object string):
+# TODO: Do we want to worry about exceptions here?
+cdef char _minikind_from_string(object string) except? -1:
"""Convert a python string to a char."""
return PyString_AsString(string)[0]
@@ -953,7 +954,7 @@
raise KeyError(PyString_FromStringAndSize(_minikind, 1))
-cdef int _versioned_minikind(char minikind):
+cdef int _versioned_minikind(char minikind): # cannot_raise
"""Return non-zero if minikind is in fltd"""
return (minikind == c'f' or
minikind == c'd' or
@@ -1373,7 +1374,7 @@
def iter_changes(self):
return self
- cdef void _gather_result_for_consistency(self, result):
+ cdef int _gather_result_for_consistency(self, result) except -1:
"""Check a result we will yield to make sure we are consistent later.
This gathers result's parents into a set to output later.
@@ -1381,7 +1382,7 @@
:param result: A result tuple.
"""
if not self.partial or not result[0]:
- return
+ return 0
self.seen_ids.add(result[0])
new_path = result[1][1]
if new_path:
@@ -1391,8 +1392,9 @@
# Add the root directory which parent_directories does not
# provide.
self.search_specific_file_parents.add('')
+ return 0
- cdef void _update_current_block(self):
+ cdef int _update_current_block(self) except -1:
if (self.block_index < len(self.state._dirblocks) and
osutils.is_inside(self.current_root, self.state._dirblocks[self.block_index][0])):
self.current_block = self.state._dirblocks[self.block_index]
@@ -1401,6 +1403,7 @@
else:
self.current_block = None
self.current_block_list = None
+ return 0
def __next__(self):
# Simple thunk to allow tail recursion without pyrex confusion
=== modified file 'bzrlib/_groupcompress_pyx.pyx'
--- a/bzrlib/_groupcompress_pyx.pyx 2009-11-06 18:05:03 +0000
+++ b/bzrlib/_groupcompress_pyx.pyx 2010-01-05 05:09:45 +0000
@@ -279,7 +279,8 @@
cdef unsigned char *_decode_copy_instruction(unsigned char *bytes,
- unsigned char cmd, unsigned int *offset, unsigned int *length) nogil:
+ unsigned char cmd, unsigned int *offset,
+ unsigned int *length) nogil: # cannot_raise
"""Decode a copy instruction from the next few bytes.
A copy instruction is a variable number of bytes, so we will parse the
=== modified file 'bzrlib/_knit_load_data_pyx.pyx'
--- a/bzrlib/_knit_load_data_pyx.pyx 2009-06-22 12:52:39 +0000
+++ b/bzrlib/_knit_load_data_pyx.pyx 2010-01-04 23:13:20 +0000
@@ -97,11 +97,12 @@
self.end_str = NULL
self.history_len = 0
- cdef void validate(self):
+ cdef int validate(self) except -1:
if not PyDict_CheckExact(self.cache):
raise TypeError('kndx._cache must be a python dict')
if not PyList_CheckExact(self.history):
raise TypeError('kndx._history must be a python list')
+ return 0
cdef object process_options(self, char *option_str, char *end):
"""Process the options string into a list."""
=== modified file 'bzrlib/_known_graph_pyx.pyx'
--- a/bzrlib/_known_graph_pyx.pyx 2009-11-30 17:25:22 +0000
+++ b/bzrlib/_known_graph_pyx.pyx 2010-01-05 05:09:45 +0000
@@ -703,7 +703,7 @@
self._revno_first, self._revno_second, self._revno_last,
self.is_first_child, self.seen_by_child)
- cdef int has_pending_parents(self):
+ cdef int has_pending_parents(self): # cannot_raise
if self.left_pending_parent is not None or self.pending_parents:
return 1
return 0
=== modified file 'bzrlib/_rio_pyx.pyx'
--- a/bzrlib/_rio_pyx.pyx 2009-05-15 02:36:03 +0000
+++ b/bzrlib/_rio_pyx.pyx 2010-01-05 04:59:57 +0000
@@ -49,7 +49,7 @@
from bzrlib.rio import Stanza
-cdef int _valid_tag_char(char c):
+cdef int _valid_tag_char(char c): # cannot_raise
return (c == c'_' or c == c'-' or
(c >= c'a' and c <= c'z') or
(c >= c'A' and c <= c'Z') or
=== modified file 'bzrlib/_walkdirs_win32.pyx'
--- a/bzrlib/_walkdirs_win32.pyx 2009-03-23 14:59:43 +0000
+++ b/bzrlib/_walkdirs_win32.pyx 2010-01-05 04:59:57 +0000
@@ -109,7 +109,7 @@
wcslen(data.cFileName))
-cdef int _get_mode_bits(WIN32_FIND_DATAW *data):
+cdef int _get_mode_bits(WIN32_FIND_DATAW *data): # cannot_raise
cdef int mode_bits
mode_bits = 0100666 # writeable file, the most common
@@ -121,13 +121,13 @@
return mode_bits
-cdef __int64 _get_size(WIN32_FIND_DATAW *data):
+cdef __int64 _get_size(WIN32_FIND_DATAW *data): # cannot_raise
# Pyrex casts a DWORD into a PyLong anyway, so it is safe to do << 32
# on a DWORD
return ((<__int64>data.nFileSizeHigh) << 32) + data.nFileSizeLow
-cdef double _ftime_to_timestamp(FILETIME *ft):
+cdef double _ftime_to_timestamp(FILETIME *ft): # cannot_raise
"""Convert from a FILETIME struct into a floating point timestamp.
The fields of a FILETIME structure are the hi and lo part
@@ -147,7 +147,7 @@
return (val * 1.0e-7) - 11644473600.0
-cdef int _should_skip(WIN32_FIND_DATAW *data):
+cdef int _should_skip(WIN32_FIND_DATAW *data): # cannot_raise
"""Is this '.' or '..' so we should skip it?"""
if (data.cFileName[0] != c'.'):
return 0
=== modified file 'bzrlib/tests/test__dirstate_helpers.py'
--- a/bzrlib/tests/test__dirstate_helpers.py 2009-12-22 15:50:40 +0000
+++ b/bzrlib/tests/test__dirstate_helpers.py 2010-01-05 05:09:45 +0000
@@ -1,4 +1,4 @@
-# Copyright (C) 2007, 2008 Canonical Ltd
+# Copyright (C) 2007, 2008, 2009 Canonical Ltd
#
# This program is free software; you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
@@ -1293,6 +1293,25 @@
tree.unlock()
self.assertEqual(sorted(expected), sorted(file_ids))
+ def test_exceptions_raised(self):
+ # This is a direct test of bug #495023, it relies on osutils.is_inside
+ # getting called in an inner function. Which makes it a bit brittle,
+ # but at least it does reproduce the bug.
+ def is_inside_raises(*args, **kwargs):
+ raise RuntimeError('stop this')
+ tree = self.make_branch_and_tree('tree')
+ self.build_tree(['tree/file', 'tree/dir/', 'tree/dir/sub',
+ 'tree/dir2/', 'tree/dir2/sub2'])
+ tree.add(['file', 'dir', 'dir/sub', 'dir2', 'dir2/sub2'])
+ tree.commit('first commit')
+ tree.lock_read()
+ self.addCleanup(tree.unlock)
+ basis_tree = tree.basis_tree()
+ orig = osutils.is_inside
+ self.addCleanup(setattr, osutils, 'is_inside', orig)
+ osutils.is_inside = is_inside_raises
+ self.assertListRaises(RuntimeError, tree.iter_changes, basis_tree)
+
def test_simple_changes(self):
tree = self.make_branch_and_tree('tree')
self.build_tree(['tree/file'])
=== modified file 'bzrlib/tests/test_source.py'
--- a/bzrlib/tests/test_source.py 2009-09-17 19:16:58 +0000
+++ b/bzrlib/tests/test_source.py 2010-01-05 05:09:45 +0000
@@ -372,3 +372,48 @@
self.fail(
"these files contain an assert statement and should not:\n%s"
% '\n'.join(badfiles))
+
+ def test_extension_exceptions(self):
+ """Extension functions should propagate exceptions.
+
+ Either they should return an object, have an 'except' clause, or have a
+ "# cannot_raise" to indicate that we've audited them and defined them as not
+ raising exceptions.
+ """
+ both_exc_and_no_exc = []
+ missing_except = []
+ class_re = re.compile(r'^(cdef\s+)?class (\w+).*:', re.MULTILINE)
+ except_re = re.compile(r'cdef\s*' # start with cdef
+ r'([\w *]*?)\s*' # this is the return signature
+ r'(\w+)\s*\(' # the function name
+ r'[^)]*\)\s*' # parameters
+ r'(.*)\s*:' # the except clause
+ r'\s*(#\s*cannot[- _]raise)?' # cannot raise comment
+ )
+ for fname, text in self.get_source_file_contents(
+ extensions=('.pyx',)):
+ known_classes = set([m[1] for m in class_re.findall(text)])
+ cdefs = except_re.findall(text)
+ for sig, func, exc_clause, no_exc_comment in cdefs:
+ if not sig or sig in known_classes:
+ sig = 'object'
+ if exc_clause and no_exc_comment:
+ both_exc_and_no_exc.append((fname, func))
+ if sig != 'object' and not (exc_clause or no_exc_comment):
+ missing_except.append((fname, func))
+ error_msg = []
+ if both_exc_and_no_exc:
+ error_msg.append('The following functions had "cannot raise" comments'
+ ' but did have an except clause set:')
+ for fname, func in both_exc_and_no_exc:
+ error_msg.append('%s:%s' % (fname, func))
+ error_msg.extend(('', ''))
+ if missing_except:
+ error_msg.append('The following functions have fixed return types,'
+ ' but no except clause.')
+ error_msg.append('Either add an except or append "# cannot_raise".')
+ for fname, func in missing_except:
+ error_msg.append('%s:%s' % (fname, func))
+ error_msg.extend(('', ''))
+ if error_msg:
+ self.fail('\n'.join(error_msg))
More information about the bazaar-commits
mailing list