Rev 5339: (parthm) Better regex compile errors (Parth Malwankar) in file:///home/pqm/archives/thelove/bzr/%2Btrunk/
Canonical.com Patch Queue Manager
pqm at pqm.ubuntu.com
Thu Jul 8 11:34:15 BST 2010
At file:///home/pqm/archives/thelove/bzr/%2Btrunk/
------------------------------------------------------------
revno: 5339 [merge]
revision-id: pqm at pqm.ubuntu.com-20100708103412-bfv99ad1u9a63ly8
parent: pqm at pqm.ubuntu.com-20100707213006-lriphkkbzwwrl7ne
parent: parth.malwankar at gmail.com-20100708091144-1a1kqnah1fg1yrcx
committer: Canonical.com Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: +trunk
timestamp: Thu 2010-07-08 11:34:12 +0100
message:
(parthm) Better regex compile errors (Parth Malwankar)
modified:
NEWS NEWS-20050323055033-4e00b5db738777ff
bzrlib/errors.py errors.py-20050309040759-20512168c4e14fbd
bzrlib/globbing.py glob.py-20061113075651-q63o2v35fm2ydk9x-1
bzrlib/lazy_regex.py lazy_regex.py-20061009091222-fyettq6z5qomdl9e-1
bzrlib/log.py log.py-20050505065812-c40ce11702fe5fb1
bzrlib/osutils.py osutils.py-20050309040759-eeaff12fbf77ac86
bzrlib/tests/__init__.py selftest.py-20050531073622-8d0e3c8845c97a64
bzrlib/tests/blackbox/test_log.py test_log.py-20060112090212-78f6ea560c868e24
bzrlib/tests/test_globbing.py test_glob.py-20061113075651-q63o2v35fm2ydk9x-2
bzrlib/tests/test_lazy_regex.py test_lazy_regex.py-20061009091222-fyettq6z5qomdl9e-2
bzrlib/tests/test_osutils.py test_osutils.py-20051201224856-e48ee24c12182989
=== modified file 'NEWS'
--- a/NEWS 2010-07-04 06:22:11 +0000
+++ b/NEWS 2010-07-08 10:34:12 +0000
@@ -21,12 +21,17 @@
exiting the program, and it made sense to provide a full context
manager at the same time. (Robert Collins)
+* ``bzrlib.re_compile_checked`` is now deprecated. Caller should handle
+ ``bzrlib.errors.InvalidPattern`` exception thrown by ``re.match`` in
+ case the default error message not suitable for the use case.
+ (Parth Malwankar)
+
* The ``bzr`` front end now requires a ``bzrlib.ui.ui_factory`` which is a
context manager in the Python 2.5 and above sense. The bzrlib base class
is such a manager, but third party UI factories which do not derive from
``bzrlib.ui.UIFactory`` will be incompatible with the command line front
end.
-
+
* URLs like ``foo:bar/baz`` are now always parsed as a URL with scheme "foo"
and path "bar/baz", even if bzr does not recognize "foo" as a known URL
scheme. Previously these URLs would be treated as local paths.
@@ -87,6 +92,10 @@
test that all commands available to the test suite have help.
(Robert Collins, #177500)
+* Invalid patterns supplied to ``Globster`` or ``lazy_regex`` now raise
+ ``InvalidPattern`` exception showing clear error message to the user.
+ (Parth Malwankar #300062)
+
* Progress output is cleaned up when exiting. (Aaron Bentley)
* Raise ValueError instead of a string exception.
=== modified file 'bzrlib/errors.py'
--- a/bzrlib/errors.py 2010-06-17 11:52:13 +0000
+++ b/bzrlib/errors.py 2010-06-30 12:03:30 +0000
@@ -3155,4 +3155,10 @@
"Please, set your name with the 'whoami' command.\n"
'E.g. bzr whoami "Your Name <name at example.com>"')
+class InvalidPattern(BzrError):
+
+ _fmt = ('Invalid pattern(s) found. %(message)s')
+
+ def __init__(self, message):
+ self.message = message
=== modified file 'bzrlib/globbing.py'
--- a/bzrlib/globbing.py 2010-02-17 17:11:16 +0000
+++ b/bzrlib/globbing.py 2010-07-08 05:11:16 +0000
@@ -22,8 +22,10 @@
import re
+from bzrlib import errors
from bzrlib.trace import (
- warning
+ mutter,
+ warning,
)
@@ -209,10 +211,18 @@
:return A matching pattern or None if there is no matching pattern.
"""
- for regex, patterns in self._regex_patterns:
- match = regex.match(filename)
- if match:
- return patterns[match.lastindex -1]
+ try:
+ for regex, patterns in self._regex_patterns:
+ match = regex.match(filename)
+ if match:
+ return patterns[match.lastindex -1]
+ except errors.InvalidPattern, e:
+ # We can't show the default e.message to the user as thats for
+ # the combined pattern we sent to regex. Instead we indicate to
+ # the user that an ignore file needs fixing.
+ mutter('Invalid pattern found in regex: %s.', e.message)
+ e.message = "File ~/.bazaar/ignore or .bzrignore contains errors."
+ raise e
return None
class ExceptionGlobster(object):
=== modified file 'bzrlib/lazy_regex.py'
--- a/bzrlib/lazy_regex.py 2009-03-23 14:59:43 +0000
+++ b/bzrlib/lazy_regex.py 2010-06-30 12:03:30 +0000
@@ -22,6 +22,8 @@
import re
+from bzrlib import errors
+
class LazyRegex(object):
"""A proxy around a real regex, which won't be compiled until accessed."""
@@ -58,7 +60,12 @@
def _real_re_compile(self, *args, **kwargs):
"""Thunk over to the original re.compile"""
- return _real_re_compile(*args, **kwargs)
+ try:
+ return _real_re_compile(*args, **kwargs)
+ except re.error, e:
+ # raise InvalidPattern instead of re.error as this gives a
+ # cleaner message to the user.
+ raise errors.InvalidPattern('"' + args[0] + '" ' +str(e))
def __getattr__(self, attr):
"""Return a member from the proxied regex object.
=== modified file 'bzrlib/log.py'
--- a/bzrlib/log.py 2010-06-17 08:53:15 +0000
+++ b/bzrlib/log.py 2010-06-30 12:03:30 +0000
@@ -86,7 +86,6 @@
format_date,
format_date_with_offset_in_original_timezone,
get_terminal_encoding,
- re_compile_checked,
terminal_width,
)
from bzrlib.symbol_versioning import (
@@ -811,8 +810,7 @@
"""
if search is None:
return log_rev_iterator
- searchRE = re_compile_checked(search, re.IGNORECASE,
- 'log message filter')
+ searchRE = re.compile(search, re.IGNORECASE)
return _filter_message_re(searchRE, log_rev_iterator)
=== modified file 'bzrlib/osutils.py'
--- a/bzrlib/osutils.py 2010-07-04 07:48:25 +0000
+++ b/bzrlib/osutils.py 2010-07-08 10:34:12 +0000
@@ -2163,6 +2163,7 @@
raise
+ at deprecated_function(deprecated_in((2, 2, 0)))
def re_compile_checked(re_string, flags=0, where=""):
"""Return a compiled re, or raise a sensible error.
@@ -2178,12 +2179,12 @@
re_obj = re.compile(re_string, flags)
re_obj.search("")
return re_obj
- except re.error, e:
+ except errors.InvalidPattern, e:
if where:
where = ' in ' + where
# despite the name 'error' is a type
- raise errors.BzrCommandError('Invalid regular expression%s: %r: %s'
- % (where, re_string, e))
+ raise errors.BzrCommandError('Invalid regular expression%s: %s'
+ % (where, e.message))
if sys.platform == "win32":
=== modified file 'bzrlib/tests/__init__.py'
--- a/bzrlib/tests/__init__.py 2010-07-04 06:22:11 +0000
+++ b/bzrlib/tests/__init__.py 2010-07-08 10:34:12 +0000
@@ -2744,8 +2744,7 @@
:param pattern: A regular expression string.
:return: A callable that returns True if the re matches.
"""
- filter_re = osutils.re_compile_checked(pattern, 0,
- 'test filter')
+ filter_re = re.compile(pattern, 0)
def condition(test):
test_id = test.id()
return filter_re.search(test_id)
=== modified file 'bzrlib/tests/blackbox/test_log.py'
--- a/bzrlib/tests/blackbox/test_log.py 2010-06-25 08:01:24 +0000
+++ b/bzrlib/tests/blackbox/test_log.py 2010-06-30 12:03:30 +0000
@@ -381,16 +381,13 @@
def test_log_bad_message_re(self):
"""Bad --message argument gives a sensible message
-
+
See https://bugs.launchpad.net/bzr/+bug/251352
"""
self.make_minimal_branch()
out, err = self.run_bzr(['log', '-m', '*'], retcode=3)
- self.assertEqual("bzr: ERROR: Invalid regular expression"
- " in log message filter"
- ": '*'"
- ": nothing to repeat\n", err)
- self.assertEqual('', out)
+ self.assertContainsRe(err, "ERROR.*Invalid pattern.*nothing to repeat")
+ self.assertEqual(out, '')
def test_log_unsupported_timezone(self):
self.make_linear_branch()
=== modified file 'bzrlib/tests/test_globbing.py'
--- a/bzrlib/tests/test_globbing.py 2010-02-17 17:11:16 +0000
+++ b/bzrlib/tests/test_globbing.py 2010-06-30 13:49:02 +0000
@@ -15,6 +15,7 @@
# along with this program; if not, write to the Free Software
# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+from bzrlib import errors
from bzrlib.globbing import (
Globster,
ExceptionGlobster,
@@ -308,6 +309,14 @@
self.assertEqual(patterns[x],globster.match(filename))
self.assertEqual(None,globster.match('foobar.300'))
+ def test_bad_pattern(self):
+ """Ensure that globster handles bad patterns cleanly."""
+ patterns = [u'RE:[']
+ g = Globster(patterns)
+ e = self.assertRaises(errors.InvalidPattern, g.match, 'foo')
+ self.assertContainsRe(e.message, "File.*ignore.*contains errors")
+
+
class TestExceptionGlobster(TestCase):
def test_exclusion_patterns(self):
=== modified file 'bzrlib/tests/test_lazy_regex.py'
--- a/bzrlib/tests/test_lazy_regex.py 2009-03-23 14:59:43 +0000
+++ b/bzrlib/tests/test_lazy_regex.py 2010-06-30 13:49:02 +0000
@@ -18,6 +18,7 @@
import re
+from bzrlib import errors
from bzrlib import (
lazy_regex,
tests,
@@ -63,6 +64,15 @@
('_real_re_compile', ('foo',), {}),
], actions)
+ def test_bad_pattern(self):
+ """Ensure lazy regex handles bad patterns cleanly."""
+ p = lazy_regex.lazy_compile('RE:[')
+ # As p.match is lazy, we make it into a lambda so its handled
+ # by assertRaises correctly.
+ e = self.assertRaises(errors.InvalidPattern, lambda: p.match('foo'))
+ self.assertEqual(e.message,
+ '"RE:[" unexpected end of regular expression')
+
class TestLazyCompile(tests.TestCase):
=== modified file 'bzrlib/tests/test_osutils.py'
--- a/bzrlib/tests/test_osutils.py 2010-06-17 14:58:20 +0000
+++ b/bzrlib/tests/test_osutils.py 2010-07-08 09:11:44 +0000
@@ -27,7 +27,9 @@
from bzrlib import (
errors,
+ lazy_regex,
osutils,
+ symbol_versioning,
tests,
trace,
win32utils,
@@ -1705,19 +1707,27 @@
class TestReCompile(tests.TestCase):
+ def _deprecated_re_compile_checked(self, *args, **kwargs):
+ return self.applyDeprecated(symbol_versioning.deprecated_in((2, 2, 0)),
+ osutils.re_compile_checked, *args, **kwargs)
+
def test_re_compile_checked(self):
- r = osutils.re_compile_checked(r'A*', re.IGNORECASE)
+ r = self._deprecated_re_compile_checked(r'A*', re.IGNORECASE)
self.assertTrue(r.match('aaaa'))
self.assertTrue(r.match('aAaA'))
def test_re_compile_checked_error(self):
# like https://bugs.launchpad.net/bzr/+bug/251352
+
+ # Due to possible test isolation error, re.compile is not lazy at
+ # this point. We re-install lazy compile.
+ lazy_regex.install_lazy_compile()
err = self.assertRaises(
errors.BzrCommandError,
- osutils.re_compile_checked, '*', re.IGNORECASE, 'test case')
+ self._deprecated_re_compile_checked, '*', re.IGNORECASE, 'test case')
self.assertEqual(
- "Invalid regular expression in test case: '*': "
- "nothing to repeat",
+ 'Invalid regular expression in test case: '
+ '"*" nothing to repeat',
str(err))
More information about the bazaar-commits
mailing list