Merge lp:~jameinel/bzr/pyrex_099_bug_582656 into lp:bzr/2.0

Proposed by John A Meinel
Status: Merged
Approved by: Robert Collins
Approved revision: no longer in the source branch.
Merged at revision: 4749
Proposed branch: lp:~jameinel/bzr/pyrex_099_bug_582656
Merge into: lp:bzr/2.0
Diff against target: 72 lines (+8/-8)
1 file modified
bzrlib/_dirstate_helpers_pyx.pyx (+8/-8)
To merge this branch: bzr merge lp:~jameinel/bzr/pyrex_099_bug_582656
Reviewer Review Type Date Requested Status
Robert Collins (community) Approve
Review via email: mp+25625@code.launchpad.net

Commit message

Work around a bug (#582656) in how Pyrex handles catching exceptions.

Description of the change

This is a workaround for pyrex bug #582656. Basically, new versions of pyrex broke code that catches an exception but doesn't assign it to an object. Stuff like:

...
 except Exception:
  do_stuff

It seems to be leaving the trapped exception on the stack (PyErr_Occurred() == True). Which means that if 'do_stuff()' calls a function with indeterminate exception state, it can cause the exception to get raised.

In the bug, we were doing:

 except KeyError:
   ...
   if x == y:
   ...

And PyObject_Cmp() seems to have a side-effect exception check.

Anyway, the workaround is to just change all of those exception catches to assigning it to a variable, such as:

 except Exception, _:

_dirstate_helpers_pyx.pyx seems to be the only extension that suffers from this (as checked by:
 grep -rnI "^\s.*except[^,]:").

I've also sent a bug report to the pyrex mailing list (since I don't think they have a real bug tracker.)

I'm proposing this for 2.0 and expecting it to propagate up.

To post a comment you must log in.
Revision history for this message
Robert Collins (lifeless) wrote :

Doit.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'bzrlib/_dirstate_helpers_pyx.pyx'
2--- bzrlib/_dirstate_helpers_pyx.pyx 2010-01-05 04:59:57 +0000
3+++ bzrlib/_dirstate_helpers_pyx.pyx 2010-05-19 17:12:31 +0000
4@@ -1,4 +1,4 @@
5-# Copyright (C) 2007, 2008 Canonical Ltd
6+# Copyright (C) 2007-2010 Canonical Ltd
7 #
8 # This program is free software; you can redistribute it and/or modify
9 # it under the terms of the GNU General Public License as published by
10@@ -1219,7 +1219,7 @@
11 else:
12 try:
13 source_parent_id = self.old_dirname_to_file_id[old_dirname]
14- except KeyError:
15+ except KeyError, _:
16 source_parent_entry = self.state._get_entry(self.source_index,
17 path_utf8=old_dirname)
18 source_parent_id = source_parent_entry[0][2]
19@@ -1236,7 +1236,7 @@
20 else:
21 try:
22 target_parent_id = self.new_dirname_to_file_id[new_dirname]
23- except KeyError:
24+ except KeyError, _:
25 # TODO: We don't always need to do the lookup, because the
26 # parent entry will be the same as the source entry.
27 target_parent_entry = self.state._get_entry(self.target_index,
28@@ -1478,7 +1478,7 @@
29 # interface doesn't require it.
30 try:
31 self.current_root = self.search_specific_files.pop()
32- except KeyError:
33+ except KeyError, _:
34 raise StopIteration()
35 self.searched_specific_files.add(self.current_root)
36 # process the entries for this containing directory: the rest will be
37@@ -1567,7 +1567,7 @@
38 # and e.winerror == ERROR_DIRECTORY
39 try:
40 e_winerror = e.winerror
41- except AttributeError:
42+ except AttributeError, _:
43 e_winerror = None
44 win_errors = (ERROR_DIRECTORY, ERROR_PATH_NOT_FOUND)
45 if (e.errno in win_errors or e_winerror in win_errors):
46@@ -1656,7 +1656,7 @@
47 try:
48 self.current_dir_info = self.dir_iterator.next()
49 self.current_dir_list = self.current_dir_info[1]
50- except StopIteration:
51+ except StopIteration, _:
52 self.current_dir_info = None
53 else: #(dircmp > 0)
54 # We have a dirblock entry for this location, but there
55@@ -1803,7 +1803,7 @@
56 and stat.S_IEXEC & current_path_info[3].st_mode)
57 try:
58 relpath_unicode = self.utf8_decode(current_path_info[0])[0]
59- except UnicodeDecodeError:
60+ except UnicodeDecodeError, _:
61 raise errors.BadFilenameEncoding(
62 current_path_info[0], osutils._fs_enc)
63 if changed is not None:
64@@ -1851,7 +1851,7 @@
65 try:
66 self.current_dir_info = self.dir_iterator.next()
67 self.current_dir_list = self.current_dir_info[1]
68- except StopIteration:
69+ except StopIteration, _:
70 self.current_dir_info = None
71
72 cdef object _next_consistent_entries(self):

Subscribers

People subscribed via source and target branches