From f43bc2eb6afdf5f4ca21cd9b5f914588693d518b Mon Sep 17 00:00:00 2001 From: Oleksandr Usov Date: Mon, 17 Oct 2011 11:25:55 +0100 Subject: Implement comments from patch review: - style fixes - add integration test for --rewrite-tag-names - removed rewrite_dict as we can't really guarantee uniqness of tag names. --- exporter.py | 18 ++++++----------- tests/test_commands.py | 19 ++++++++++++++++++ tests/test_exporter.py | 54 ++++++++++++++++++++------------------------------ 3 files changed, 47 insertions(+), 44 deletions(-) diff --git a/exporter.py b/exporter.py index fd1de00..cad3241 100644 --- a/exporter.py +++ b/exporter.py @@ -111,7 +111,8 @@ def check_ref_format(refname): return False return True -def sanitize_ref_name_for_git(name_dict, refname): + +def sanitize_ref_name_for_git(refname): """Rewrite refname so that it will be accepted by git-fast-import. For the detailed rules see check_ref_format. @@ -119,11 +120,10 @@ def sanitize_ref_name_for_git(name_dict, refname): so we have to manually verify that resulting ref names are unique. - :param name_dict: additional dictionary used to enforce uniqueness of resulting refname's :param refname: refname to rewrite :return: new refname """ - newRefname = re.sub( + new_refname = re.sub( # '/.' in refname or startswith '.' r"/\.|^\." # '..' in refname @@ -141,12 +141,7 @@ def sanitize_ref_name_for_git(name_dict, refname): # "\\" in refname r"|\\", "_", refname) - idx = name_dict.get(newRefname, 1) - name_dict[newRefname] = idx + 1 - if idx != 1: - # append index to the resulting refname if it's not unique - newRefname += "_" + str(idx) - return newRefname + return new_refname class BzrFastExporter(object): @@ -174,7 +169,6 @@ class BzrFastExporter(object): self.excluded_revisions = set() self.plain_format = plain_format self.rewrite_tags = rewrite_tags - self.rewrite_dict = dict() self._multi_author_api_available = hasattr(bzrlib.revision.Revision, 'get_apparent_authors') self.properties_to_exclude = ['authors', 'author'] @@ -601,11 +595,11 @@ class BzrFastExporter(object): git_ref = 'refs/tags/%s' % tag.encode("utf-8") if self.plain_format and not check_ref_format(git_ref): if self.rewrite_tags: - new_ref = sanitize_ref_name_for_git(self.rewrite_dict, git_ref) + new_ref = sanitize_ref_name_for_git(git_ref) self.warning('tag %r is exported as %r to be valid in git.', git_ref, new_ref) git_ref = new_ref - else: + else: self.warning('not creating tag %r as its name would not be ' 'valid in git.', git_ref) continue diff --git a/tests/test_commands.py b/tests/test_commands.py index 282cfc3..5729660 100644 --- a/tests/test_commands.py +++ b/tests/test_commands.py @@ -80,6 +80,25 @@ class TestFastExport(ExternalBase): except AttributeError: # bzr < 2.4 self.failUnlessExists("br.fi") + def test_tag_rewriting(self): + tree = self.make_branch_and_tree("br") + tree.commit("pointless") + self.assertTrue(tree.branch.supports_tags()) + rev_id = tree.branch.dotted_revno_to_revision_id((1,)) + tree.branch.tags.set_tag("goodTag", rev_id) + tree.branch.tags.set_tag("bad Tag", rev_id) + + # first check --no-rewrite-tag-names + data = self.run_bzr("fast-export --plain --no-rewrite-tag-names br")[0] + self.assertNotEqual(-1, data.find("reset refs/tags/goodTag")) + self.assertEqual(data.find("reset refs/tags/"), data.rfind("reset refs/tags/")) + + # and now with --rewrite-tag-names + data = self.run_bzr("fast-export --plain --rewrite-tag-names br")[0] + self.assertNotEqual(-1, data.find("reset refs/tags/goodTag")) + # "bad Tag" should be exported as bad_Tag + self.assertNotEqual(-1, data.find("reset refs/tags/bad_Tag")) + simple_fast_import_stream = """commit refs/heads/master mark :1 diff --git a/tests/test_exporter.py b/tests/test_exporter.py index 957945f..f1c9530 100644 --- a/tests/test_exporter.py +++ b/tests/test_exporter.py @@ -95,40 +95,30 @@ class CheckRefFormatTests(tests.TestCase): self.assertFalse(check_ref_format('heads/foo\020bar')) self.assertFalse(check_ref_format('heads/foo\177bar')) + class CheckRefnameRewriting(tests.TestCase): """Tests for sanitize_ref_name_for_git function""" def test_passthrough_valid(self): - self.assertEqual(sanitize_ref_name_for_git(dict(), 'heads/foo'), 'heads/foo') - self.assertEqual(sanitize_ref_name_for_git(dict(), 'foo/bar/baz'), 'foo/bar/baz') - self.assertEqual(sanitize_ref_name_for_git(dict(), 'refs///heads/foo'), 'refs///heads/foo') - self.assertEqual(sanitize_ref_name_for_git(dict(), 'foo./bar'), 'foo./bar') - self.assertEqual(sanitize_ref_name_for_git(dict(), 'heads/foo@bar'), 'heads/foo@bar') - self.assertEqual(sanitize_ref_name_for_git(dict(), 'heads/fix.lock.error'), 'heads/fix.lock.error') - - def test_rewrite_to_unique_names(self): - self.assertEqual(sanitize_ref_name_for_git(dict(), 'heads/foo/'), 'heads/foo_') - # check that with persistent dictionary we generate unique names on each invocation - q = dict() - self.assertNotEqual( - sanitize_ref_name_for_git(q, 'heads/foo/'), - sanitize_ref_name_for_git(q, 'heads/foo/')) - self.assertNotEqual( - sanitize_ref_name_for_git(q, 'heads/foo/'), - sanitize_ref_name_for_git(q, 'heads/foo/')) - + self.assertEqual(sanitize_ref_name_for_git('heads/foo'), 'heads/foo') + self.assertEqual(sanitize_ref_name_for_git('foo/bar/baz'), 'foo/bar/baz') + self.assertEqual(sanitize_ref_name_for_git('refs///heads/foo'), 'refs///heads/foo') + self.assertEqual(sanitize_ref_name_for_git('foo./bar'), 'foo./bar') + self.assertEqual(sanitize_ref_name_for_git('heads/foo@bar'), 'heads/foo@bar') + self.assertEqual(sanitize_ref_name_for_git('heads/fix.lock.error'), 'heads/fix.lock.error') + def test_rewrite_invalid(self): - self.assertTrue(check_ref_format(sanitize_ref_name_for_git(dict(), 'foo./bar'))) - self.assertTrue(check_ref_format(sanitize_ref_name_for_git(dict(), 'heads/foo/'))) - self.assertTrue(check_ref_format(sanitize_ref_name_for_git(dict(), 'heads/foo.'))) - self.assertTrue(check_ref_format(sanitize_ref_name_for_git(dict(), './foo'))) - self.assertTrue(check_ref_format(sanitize_ref_name_for_git(dict(), '.refs/foo'))) - self.assertTrue(check_ref_format(sanitize_ref_name_for_git(dict(), 'heads/foo..bar'))) - self.assertTrue(check_ref_format(sanitize_ref_name_for_git(dict(), 'heads/foo?bar'))) - self.assertTrue(check_ref_format(sanitize_ref_name_for_git(dict(), 'heads/foo.lock'))) - self.assertTrue(check_ref_format(sanitize_ref_name_for_git(dict(), 'heads/v@{ation'))) - self.assertTrue(check_ref_format(sanitize_ref_name_for_git(dict(), 'heads/foo\bar'))) - self.assertTrue(check_ref_format(sanitize_ref_name_for_git(dict(), 'heads/foo\\bar'))) - self.assertTrue(check_ref_format(sanitize_ref_name_for_git(dict(), 'heads/foo bar'))) - self.assertTrue(check_ref_format(sanitize_ref_name_for_git(dict(), 'heads/foo\020bar'))) - self.assertTrue(check_ref_format(sanitize_ref_name_for_git(dict(), 'heads/foo\177bar'))) + self.assertTrue(check_ref_format(sanitize_ref_name_for_git('foo./bar'))) + self.assertTrue(check_ref_format(sanitize_ref_name_for_git('heads/foo/'))) + self.assertTrue(check_ref_format(sanitize_ref_name_for_git('heads/foo.'))) + self.assertTrue(check_ref_format(sanitize_ref_name_for_git('./foo'))) + self.assertTrue(check_ref_format(sanitize_ref_name_for_git('.refs/foo'))) + self.assertTrue(check_ref_format(sanitize_ref_name_for_git('heads/foo..bar'))) + self.assertTrue(check_ref_format(sanitize_ref_name_for_git('heads/foo?bar'))) + self.assertTrue(check_ref_format(sanitize_ref_name_for_git('heads/foo.lock'))) + self.assertTrue(check_ref_format(sanitize_ref_name_for_git('heads/v@{ation'))) + self.assertTrue(check_ref_format(sanitize_ref_name_for_git('heads/foo\bar'))) + self.assertTrue(check_ref_format(sanitize_ref_name_for_git('heads/foo\\bar'))) + self.assertTrue(check_ref_format(sanitize_ref_name_for_git('heads/foo bar'))) + self.assertTrue(check_ref_format(sanitize_ref_name_for_git('heads/foo\020bar'))) + self.assertTrue(check_ref_format(sanitize_ref_name_for_git('heads/foo\177bar'))) -- cgit v1.2.1