Bazaar

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

  1. pyrex_099_bug_582656
  2. Merge into 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
=== modified file 'bzrlib/_dirstate_helpers_pyx.pyx'
--- bzrlib/_dirstate_helpers_pyx.pyx 2010-01-05 04:59:57 +0000
+++ bzrlib/_dirstate_helpers_pyx.pyx 2010-05-19 17:12:31 +0000
@@ -1,4 +1,4 @@
1# Copyright (C) 2007, 2008 Canonical Ltd1# Copyright (C) 2007-2010 Canonical Ltd
2#2#
3# This program is free software; you can redistribute it and/or modify3# This program is free software; you can redistribute it and/or modify
4# it under the terms of the GNU General Public License as published by4# it under the terms of the GNU General Public License as published by
@@ -1219,7 +1219,7 @@
1219 else:1219 else:
1220 try:1220 try:
1221 source_parent_id = self.old_dirname_to_file_id[old_dirname]1221 source_parent_id = self.old_dirname_to_file_id[old_dirname]
1222 except KeyError:1222 except KeyError, _:
1223 source_parent_entry = self.state._get_entry(self.source_index,1223 source_parent_entry = self.state._get_entry(self.source_index,
1224 path_utf8=old_dirname)1224 path_utf8=old_dirname)
1225 source_parent_id = source_parent_entry[0][2]1225 source_parent_id = source_parent_entry[0][2]
@@ -1236,7 +1236,7 @@
1236 else:1236 else:
1237 try:1237 try:
1238 target_parent_id = self.new_dirname_to_file_id[new_dirname]1238 target_parent_id = self.new_dirname_to_file_id[new_dirname]
1239 except KeyError:1239 except KeyError, _:
1240 # TODO: We don't always need to do the lookup, because the1240 # TODO: We don't always need to do the lookup, because the
1241 # parent entry will be the same as the source entry.1241 # parent entry will be the same as the source entry.
1242 target_parent_entry = self.state._get_entry(self.target_index,1242 target_parent_entry = self.state._get_entry(self.target_index,
@@ -1478,7 +1478,7 @@
1478 # interface doesn't require it.1478 # interface doesn't require it.
1479 try:1479 try:
1480 self.current_root = self.search_specific_files.pop()1480 self.current_root = self.search_specific_files.pop()
1481 except KeyError:1481 except KeyError, _:
1482 raise StopIteration()1482 raise StopIteration()
1483 self.searched_specific_files.add(self.current_root)1483 self.searched_specific_files.add(self.current_root)
1484 # process the entries for this containing directory: the rest will be1484 # process the entries for this containing directory: the rest will be
@@ -1567,7 +1567,7 @@
1567 # and e.winerror == ERROR_DIRECTORY1567 # and e.winerror == ERROR_DIRECTORY
1568 try:1568 try:
1569 e_winerror = e.winerror1569 e_winerror = e.winerror
1570 except AttributeError:1570 except AttributeError, _:
1571 e_winerror = None1571 e_winerror = None
1572 win_errors = (ERROR_DIRECTORY, ERROR_PATH_NOT_FOUND)1572 win_errors = (ERROR_DIRECTORY, ERROR_PATH_NOT_FOUND)
1573 if (e.errno in win_errors or e_winerror in win_errors):1573 if (e.errno in win_errors or e_winerror in win_errors):
@@ -1656,7 +1656,7 @@
1656 try:1656 try:
1657 self.current_dir_info = self.dir_iterator.next()1657 self.current_dir_info = self.dir_iterator.next()
1658 self.current_dir_list = self.current_dir_info[1]1658 self.current_dir_list = self.current_dir_info[1]
1659 except StopIteration:1659 except StopIteration, _:
1660 self.current_dir_info = None1660 self.current_dir_info = None
1661 else: #(dircmp > 0)1661 else: #(dircmp > 0)
1662 # We have a dirblock entry for this location, but there1662 # We have a dirblock entry for this location, but there
@@ -1803,7 +1803,7 @@
1803 and stat.S_IEXEC & current_path_info[3].st_mode)1803 and stat.S_IEXEC & current_path_info[3].st_mode)
1804 try:1804 try:
1805 relpath_unicode = self.utf8_decode(current_path_info[0])[0]1805 relpath_unicode = self.utf8_decode(current_path_info[0])[0]
1806 except UnicodeDecodeError:1806 except UnicodeDecodeError, _:
1807 raise errors.BadFilenameEncoding(1807 raise errors.BadFilenameEncoding(
1808 current_path_info[0], osutils._fs_enc)1808 current_path_info[0], osutils._fs_enc)
1809 if changed is not None:1809 if changed is not None:
@@ -1851,7 +1851,7 @@
1851 try:1851 try:
1852 self.current_dir_info = self.dir_iterator.next()1852 self.current_dir_info = self.dir_iterator.next()
1853 self.current_dir_list = self.current_dir_info[1]1853 self.current_dir_list = self.current_dir_info[1]
1854 except StopIteration:1854 except StopIteration, _:
1855 self.current_dir_info = None1855 self.current_dir_info = None
18561856
1857 cdef object _next_consistent_entries(self):1857 cdef object _next_consistent_entries(self):

Subscribers

People subscribed via source and target branches