Rev 5065: (parthm) 'bzr ignore' fails on bad pattern. bad pattern error messages now in file:///home/pqm/archives/thelove/bzr/2.2/
Canonical.com Patch Queue Manager
pqm at pqm.ubuntu.com
Thu Jul 22 04:28:58 BST 2010
At file:///home/pqm/archives/thelove/bzr/2.2/
------------------------------------------------------------
revno: 5065 [merge]
revision-id: pqm at pqm.ubuntu.com-20100722032854-gfj3a3pp8tapk0rf
parent: pqm at pqm.ubuntu.com-20100721161324-olpb0anw2ss7t5x2
parent: parth.malwankar at gmail.com-20100721101711-6tf78ktaam0eejvm
committer: Canonical.com Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: 2.2
timestamp: Thu 2010-07-22 04:28:54 +0100
message:
(parthm) 'bzr ignore' fails on bad pattern. bad pattern error messages now
shows the faulting pattern. (Parth Malwankar)
modified:
NEWS NEWS-20050323055033-4e00b5db738777ff
bzrlib/builtins.py builtins.py-20050830033751-fc01482b9ca23183
bzrlib/globbing.py glob.py-20061113075651-q63o2v35fm2ydk9x-1
bzrlib/tests/blackbox/test_ignore.py test_ignore.py-20060703063225-4tm8dc2pa7wwg2t3-1
bzrlib/tests/per_workingtree/test_is_ignored.py test_is_ignored.py-20060518083307-a5b383dd4d070083
bzrlib/tests/test_globbing.py test_glob.py-20061113075651-q63o2v35fm2ydk9x-2
=== modified file 'NEWS'
--- a/NEWS 2010-07-21 09:58:42 +0000
+++ b/NEWS 2010-07-22 03:28:54 +0000
@@ -31,6 +31,11 @@
Bug Fixes
*********
+* ``bzr ignore PATTERNS`` exits with error if a bad pattern is supplied.
+ ``InvalidPattern`` exception error message now shows faulting
+ regular expression.
+ (Parth Malwankar #300062)
+
* Configuration files in ``${BZR_HOME}`` are now written in an atomic
way which should help avoid problems with concurrent writers.
(Vincent Ladeuil, #525571)
=== modified file 'bzrlib/builtins.py'
--- a/bzrlib/builtins.py 2010-07-21 09:58:42 +0000
+++ b/bzrlib/builtins.py 2010-07-22 03:28:54 +0000
@@ -2712,6 +2712,14 @@
"NAME_PATTERN or --default-rules.")
name_pattern_list = [globbing.normalize_pattern(p)
for p in name_pattern_list]
+ bad_patterns = ''
+ for p in name_pattern_list:
+ if not globbing.Globster.is_pattern_valid(p):
+ bad_patterns += ('\n %s' % p)
+ if bad_patterns:
+ msg = ('Invalid ignore pattern(s) found. %s' % bad_patterns)
+ ui.ui_factory.show_error(msg)
+ raise errors.InvalidPattern('')
for name_pattern in name_pattern_list:
if (name_pattern[0] == '/' or
(len(name_pattern) > 1 and name_pattern[1] == ':')):
=== modified file 'bzrlib/globbing.py'
--- a/bzrlib/globbing.py 2010-07-09 16:16:11 +0000
+++ b/bzrlib/globbing.py 2010-07-21 10:17:11 +0000
@@ -179,24 +179,41 @@
so are matched first, then the basename patterns, then the fullpath
patterns.
"""
+ # We want to _add_patterns in a specific order (as per type_list below)
+ # starting with the shortest and going to the longest.
+ # As some Python version don't support ordered dicts the list below is
+ # used to select inputs for _add_pattern in a specific order.
+ pattern_types = [ "extension", "basename", "fullpath" ]
+
+ pattern_info = {
+ "extension" : {
+ "translator" : _sub_extension,
+ "prefix" : r'(?:.*/)?(?!.*/)(?:.*\.)'
+ },
+ "basename" : {
+ "translator" : _sub_basename,
+ "prefix" : r'(?:.*/)?(?!.*/)'
+ },
+ "fullpath" : {
+ "translator" : _sub_fullpath,
+ "prefix" : r''
+ },
+ }
+
def __init__(self, patterns):
self._regex_patterns = []
- path_patterns = []
- base_patterns = []
- ext_patterns = []
+ pattern_lists = {
+ "extension" : [],
+ "basename" : [],
+ "fullpath" : [],
+ }
for pat in patterns:
pat = normalize_pattern(pat)
- if pat.startswith(u'RE:') or u'/' in pat:
- path_patterns.append(pat)
- elif pat.startswith(u'*.'):
- ext_patterns.append(pat)
- else:
- base_patterns.append(pat)
- self._add_patterns(ext_patterns,_sub_extension,
- prefix=r'(?:.*/)?(?!.*/)(?:.*\.)')
- self._add_patterns(base_patterns,_sub_basename,
- prefix=r'(?:.*/)?(?!.*/)')
- self._add_patterns(path_patterns,_sub_fullpath)
+ pattern_lists[Globster.identify(pat)].append(pat)
+ pi = Globster.pattern_info
+ for t in Globster.pattern_types:
+ self._add_patterns(pattern_lists[t], pi[t]["translator"],
+ pi[t]["prefix"])
def _add_patterns(self, patterns, translator, prefix=''):
while patterns:
@@ -221,10 +238,50 @@
# 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.msg)
- e.msg = "File ~/.bazaar/ignore or .bzrignore contains errors."
+ e.msg = "File ~/.bazaar/ignore or .bzrignore contains error(s)."
+ bad_patterns = ''
+ for _, patterns in self._regex_patterns:
+ for p in patterns:
+ if not Globster.is_pattern_valid(p):
+ bad_patterns += ('\n %s' % p)
+ e.msg += bad_patterns
raise e
return None
+ @staticmethod
+ def identify(pattern):
+ """Returns pattern category.
+
+ :param pattern: normalized pattern.
+ Identify if a pattern is fullpath, basename or extension
+ and returns the appropriate type.
+ """
+ if pattern.startswith(u'RE:') or u'/' in pattern:
+ return "fullpath"
+ elif pattern.startswith(u'*.'):
+ return "extension"
+ else:
+ return "basename"
+
+ @staticmethod
+ def is_pattern_valid(pattern):
+ """Returns True if pattern is valid.
+
+ :param pattern: Normalized pattern.
+ is_pattern_valid() assumes pattern to be normalized.
+ see: globbing.normalize_pattern
+ """
+ result = True
+ translator = Globster.pattern_info[Globster.identify(pattern)]["translator"]
+ tpattern = '(%s)' % translator(pattern)
+ try:
+ re_obj = re.compile(tpattern, re.UNICODE)
+ re_obj.search("") # force compile
+ except errors.InvalidPattern, e:
+ result = False
+ return result
+
+
class ExceptionGlobster(object):
"""A Globster that supports exception patterns.
@@ -272,14 +329,9 @@
self._regex_patterns = []
for pat in patterns:
pat = normalize_pattern(pat)
- if pat.startswith(u'RE:') or u'/' in pat:
- self._add_patterns([pat], _sub_fullpath)
- elif pat.startswith(u'*.'):
- self._add_patterns([pat], _sub_extension,
- prefix=r'(?:.*/)?(?!.*/)(?:.*\.)')
- else:
- self._add_patterns([pat], _sub_basename,
- prefix=r'(?:.*/)?(?!.*/)')
+ t = Globster.identify(pat)
+ self._add_patterns([pat], Globster.pattern_info[t]["translator"],
+ Globster.pattern_info[t]["prefix"])
_slashes = re.compile(r'[\\/]+')
=== modified file 'bzrlib/tests/blackbox/test_ignore.py'
--- a/bzrlib/tests/blackbox/test_ignore.py 2010-06-11 07:32:12 +0000
+++ b/bzrlib/tests/blackbox/test_ignore.py 2010-07-08 12:36:29 +0000
@@ -162,3 +162,19 @@
tree = self.make_branch_and_tree('a')
self.run_bzr(['ignore', '--directory=a', 'README'])
self.check_file_contents('a/.bzrignore', 'README\n')
+
+ def test_ignored_invalid_pattern(self):
+ """Ensure graceful handling for invalid ignore pattern.
+
+ Test case for #300062.
+ Invalid pattern should show clear error message.
+ Invalid pattern should not be added to .bzrignore file.
+ """
+ tree = self.make_branch_and_tree('.')
+ out, err = self.run_bzr(['ignore', 'RE:*.cpp', 'foo', 'RE:['], 3)
+ self.assertEqual(out, '')
+ self.assertContainsRe(err,
+ 'Invalid ignore pattern.*RE:\*\.cpp.*RE:\[', re.DOTALL)
+ self.assertNotContainsRe(err, 'foo', re.DOTALL)
+ self.assertFalse(os.path.isfile('.bzrignore'))
+
=== modified file 'bzrlib/tests/per_workingtree/test_is_ignored.py'
--- a/bzrlib/tests/per_workingtree/test_is_ignored.py 2010-02-17 17:11:16 +0000
+++ b/bzrlib/tests/per_workingtree/test_is_ignored.py 2010-07-08 13:00:19 +0000
@@ -22,6 +22,16 @@
class TestIsIgnored(TestCaseWithWorkingTree):
+ def _set_user_ignore_content(self, ignores):
+ """Create user ignore file and set its content to ignores."""
+ config.ensure_config_dir_exists()
+ user_ignore_file = config.user_ignore_config_filename()
+ f = open(user_ignore_file, 'wb')
+ try:
+ f.write(ignores)
+ finally:
+ f.close()
+
def test_is_ignored(self):
tree = self.make_branch_and_tree('.')
# this will break if a tree changes the ignored format. That is fine
@@ -46,6 +56,11 @@
'#comment\n'
' xx \n' # whitespace
)])
+ # We set user ignore file to contain '' to avoid patterns from
+ # user ignore being used instead of bzrignore. For .e.g. If we
+ # don't do this 'foo.~1~' will match '*~' default user ignore
+ # pattern instead of '*.~*' from bzr ignore as we expect below.
+ self._set_user_ignore_content('')
# is_ignored returns the matching ignore regex when a path is ignored.
# we check some expected matches for each rule, and one or more
# relevant not-matches that look plausible as cases for bugs.
@@ -119,19 +134,16 @@
config.ensure_config_dir_exists()
user_ignore_file = config.user_ignore_config_filename()
- f = open(user_ignore_file, 'wb')
- try:
- f.write('*.py[co]\n'
- './.shelf\n'
- '# comment line\n'
- '\n' #Blank line
- '\r\n' #Blank dos line
- ' * \n' #Trailing and suffix spaces
- 'crlf\r\n' # dos style line
- '*\xc3\xa5*\n' # u'\xe5'.encode('utf8')
- )
- finally:
- f.close()
+ self._set_user_ignore_content(
+ '*.py[co]\n'
+ './.shelf\n'
+ '# comment line\n'
+ '\n' #Blank line
+ '\r\n' #Blank dos line
+ ' * \n' #Trailing and suffix spaces
+ 'crlf\r\n' # dos style line
+ '*\xc3\xa5*\n' # u'\xe5'.encode('utf8')
+ )
# Rooted
self.assertEqual('./.shelf', tree.is_ignored('.shelf'))
=== modified file 'bzrlib/tests/test_globbing.py'
--- a/bzrlib/tests/test_globbing.py 2010-07-09 16:16:11 +0000
+++ b/bzrlib/tests/test_globbing.py 2010-07-19 14:25:37 +0000
@@ -15,6 +15,8 @@
# along with this program; if not, write to the Free Software
# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+import re
+
from bzrlib import errors
from bzrlib.globbing import (
Globster,
@@ -311,10 +313,11 @@
def test_bad_pattern(self):
"""Ensure that globster handles bad patterns cleanly."""
- patterns = [u'RE:[']
+ patterns = [u'RE:[', u'/home/foo', u'RE:*.cpp']
g = Globster(patterns)
- e = self.assertRaises(errors.InvalidPattern, g.match, 'foo')
- self.assertContainsRe(e.msg, "File.*ignore.*contains errors")
+ e = self.assertRaises(errors.InvalidPattern, g.match, 'filename')
+ self.assertContainsRe(e.msg,
+ "File.*ignore.*contains error.*RE:\[.*RE:\*\.cpp", flags=re.DOTALL)
class TestExceptionGlobster(TestCase):
More information about the bazaar-commits
mailing list