Merge lp:~jelmer/brz/memorytags into lp:brz

[Jelmer Vernooij]

Add a MemoryTags object that provides the Tags implementation but stores tags in a dict.

Add a MemoryTags object that provides the Tags implementation but stores tags in a dict.

[Martin Packman (gz)]

No content in this branch.

[Martin Packman (gz)]

Thanks, couple of small comments inline.

review: Approve

[The Breezy Bot (the-breezy-bot)]

Running landing tests failed
https://ci.breezy-vcs.org/job/brz/job/brz-land/212/

=== modified file 'breezy/tag.py'
--- breezy/tag.py	2018-11-12 01:41:38 +0000
+++ breezy/tag.py	2019-02-02 21:58:58 +0000
@@ -24,6 +24,8 @@
 
 from __future__ import absolute_import
 
+from collections import defaultdict
+
 # NOTE: I was going to call this tags.py, but vim seems to think all files
 # called tags* are ctags files... mbp 20070220.
 
@@ -44,6 +46,32 @@
 """)
 
 
+def _reconcile_tags(source_dict, dest_dict, overwrite):
+    """Do a two-way merge of two tag dictionaries.
+
+    * only in source => source value
+    * only in destination => destination value
+    * same definitions => that
+    * different definitions => if overwrite is False, keep destination
+      value and add to conflict list, otherwise use the source value
+
+    :returns: (result_dict, updates,
+        [(conflicting_tag, source_target, dest_target)])
+    """
+    conflicts = []
+    updates = {}
+    result = dict(dest_dict)  # copy
+    for name, target in source_dict.items():
+        if result.get(name) == target:
+            pass
+        elif name not in result or overwrite:
+            updates[name] = target
+            result[name] = target
+        else:
+            conflicts.append((name, target, result[name]))
+    return result, updates, conflicts
+
+
 class _Tags(object):
 
     def __init__(self, branch):
@@ -148,16 +176,13 @@
         Behaviour if the tag is already present is not defined (yet).
         """
         # all done with a write lock held, so this looks atomic
-        self.branch.lock_write()
-        try:
+        with self.branch.lock_write():
             master = self.branch.get_master_branch()
             if master is not None:
                 master.tags.set_tag(tag_name, tag_target)
             td = self.get_tag_dict()
             td[tag_name] = tag_target
             self._set_tag_dict(td)
-        finally:
-            self.branch.unlock()
 
     def lookup_tag(self, tag_name):
         """Return the referent string of a tag"""
@@ -184,19 +209,15 @@
         """Returns a dict with revisions as keys
            and a list of tags for that revision as value"""
         d = self.get_tag_dict()
-        rev = {}
+        rev = defaultdict(set)
         for key in d:
-            try:
-                rev[d[key]].append(key)
-            except KeyError:
-                rev[d[key]] = [key]
+            rev[d[key]].add(key)
         return rev
 
     def delete_tag(self, tag_name):
         """Delete a tag definition.
         """
-        self.branch.lock_write()
-        try:
+        with self.branch.lock_write():
             d = self.get_tag_dict()
             try:
                 del d[tag_name]
@@ -209,8 +230,6 @@
                 except errors.NoSuchTag:
                     pass
             self._set_tag_dict(d)
-        finally:
-            self.branch.unlock()
 
     def _set_tag_dict(self, new_dict):
         """Replace all tag definitions
@@ -310,8 +329,8 @@
 
     def _merge_to(self, to_tags, source_dict, overwrite):
         dest_dict = to_tags.get_tag_dict()
-        result, updates, conflicts = self._reconcile_tags(source_dict,
-                                                          dest_dict, overwrite)
+        result, updates, conflicts = _reconcile_tags(
+            source_dict, dest_dict, overwrite)
         if result != dest_dict:
             to_tags._set_tag_dict(result)
         return updates, conflicts
@@ -327,30 +346,57 @@
                 for name in names:
                     self.set_tag(name, rename_map[revid])
 
-    def _reconcile_tags(self, source_dict, dest_dict, overwrite):
-        """Do a two-way merge of two tag dictionaries.
-
-        * only in source => source value
-        * only in destination => destination value
-        * same definitions => that
-        * different definitions => if overwrite is False, keep destination
-          value and give a warning, otherwise use the source value
-
-        :returns: (result_dict, updates,
-            [(conflicting_tag, source_target, dest_target)])
-        """
-        conflicts = []
-        updates = {}
-        result = dict(dest_dict)  # copy
-        for name, target in source_dict.items():
-            if result.get(name) == target:
-                pass
-            elif name not in result or overwrite:
-                updates[name] = target
-                result[name] = target
-            else:
-                conflicts.append((name, target, result[name]))
-        return result, updates, conflicts
+
+class MemoryTags(_Tags):
+
+    def __init__(self, tag_dict):
+        self._tag_dict = tag_dict
+
+    def get_tag_dict(self):
+        return self._tag_dict
+
+    def lookup_tag(self, tag_name):
+        """Return the referent string of a tag"""
+        td = self.get_tag_dict()
+        try:
+            return td[tag_name]
+        except KeyError:
+            raise errors.NoSuchTag(tag_name)
+
+    def get_reverse_tag_dict(self):
+        """Returns a dict with revisions as keys
+           and a list of tags for that revision as value"""
+        d = self.get_tag_dict()
+        rev = defaultdict(set)
+        for key in d:
+            rev[d[key]].add(key)
+        return rev
+
+    def set_tag(self, name, revid):
+        self._tag_dict[name] = revid
+
+    def delete_tag(self, name):
+        try:
+            del self._tag_dict[name]
+        except KeyError:
+            raise errors.NoSuchTag(name)
+
+    def rename_revisions(self, revid_map):
+        self._tag_dict = {
+            name: revid_map.get(revid, revid)
+            for name, revid in self._tag_dict.items()}
+
+    def _set_tag_dict(self, result):
+        self._tag_dict = dict(result.items())
+
+    def merge_to(self, to_tags, overwrite=False, ignore_master=False):
+        source_dict = self.get_tag_dict()
+        dest_dict = to_tags.get_tag_dict()
+        result, updates, conflicts = _reconcile_tags(
+            source_dict, dest_dict, overwrite)
+        if result != dest_dict:
+            to_tags._set_tag_dict(result)
+        return updates, conflicts
 
 
 def sort_natural(branch, tags):

=== modified file 'breezy/tests/per_branch/test_tags.py'
--- breezy/tests/per_branch/test_tags.py	2018-11-11 04:08:32 +0000
+++ breezy/tests/per_branch/test_tags.py	2019-02-02 21:58:58 +0000
@@ -91,8 +91,8 @@
         b = branch.Branch.open('b')
         self.assertEqual(
             dict(b.tags.get_reverse_tag_dict()),
-            {target_revid1: ['tag-name'],
-             target_revid2: ['other-name'],
+            {target_revid1: set(['tag-name']),
+             target_revid2: set(['other-name']),
              })
 
     def test_ghost_tag(self):

=== modified file 'breezy/tests/test_tag.py'
--- breezy/tests/test_tag.py	2018-11-11 04:08:32 +0000
+++ breezy/tests/test_tag.py	2019-02-02 21:58:58 +0000
@@ -24,6 +24,7 @@
 from breezy.tag import (
     BasicTags,
     DisabledTags,
+    MemoryTags,
     )
 from breezy.tests import (
     TestCase,
@@ -187,3 +188,66 @@
 
     def test_get_reverse_tag_dict(self):
         self.assertEqual(self.tags.get_reverse_tag_dict(), {})
+
+
+class MemoryTagsTests(TestCase):
+
+    def setUp(self):
+        super(MemoryTagsTests, self).setUp()
+        self.tags = MemoryTags({})
+
+    def test_set_tag(self):
+        self.tags.set_tag('foo', b'revid1')
+        self.assertEqual({'foo': b'revid1'}, self.tags.get_tag_dict())
+
+    def test_reverse_tag_dict(self):
+        self.tags.set_tag('foo', b'revid1')
+        self.tags.set_tag('bar', b'revid2')
+        self.tags.set_tag('blah', b'revid1')
+        self.assertEqual({
+            b'revid1': set(['foo', 'blah']),
+            b'revid2': set(['bar'])},
+            self.tags.get_reverse_tag_dict())
+
+    def test_lookup_tag(self):
+        self.tags.set_tag('foo', b'revid1')
+        self.assertEqual(b'revid1', self.tags.lookup_tag('foo'))
+        self.assertRaises(errors.NoSuchTag, self.tags.lookup_tag, 'bar')
+
+    def test_delete_tag(self):
+        self.tags.set_tag('foo', b'revid1')
+        self.assertEqual(b'revid1', self.tags.lookup_tag('foo'))
+        self.tags.delete_tag('foo')
+        self.assertRaises(errors.NoSuchTag, self.tags.lookup_tag, 'foo')
+        self.assertRaises(errors.NoSuchTag, self.tags.delete_tag, 'foo')
+
+    def test_has_tag(self):
+        self.tags.set_tag('foo', b'revid1')
+        self.assertTrue(self.tags.has_tag('foo'))
+        self.assertFalse(self.tags.has_tag('bar'))
+
+    def test_rename_revisions(self):
+        self.tags.set_tag('foo', b'revid1')
+        self.assertEqual({'foo': b'revid1'}, self.tags.get_tag_dict())
+        self.tags.rename_revisions({b'revid1': b'revid2'})
+        self.assertEqual({'foo': b'revid2'}, self.tags.get_tag_dict())
+
+    def test_merge_to(self):
+        other_tags = MemoryTags({})
+        other_tags.set_tag('tag-1', b'x')
+        self.tags.set_tag('tag-2', b'y')
+        other_tags.merge_to(self.tags)
+        self.assertEqual(b'x', self.tags.lookup_tag('tag-1'))
+        self.assertEqual(b'y', self.tags.lookup_tag('tag-2'))
+        self.assertRaises(errors.NoSuchTag, other_tags.lookup_tag, 'tag-2')
+        # conflicting merge
+        other_tags.set_tag('tag-2', b'z')
+        updates, conflicts = other_tags.merge_to(self.tags)
+        self.assertEqual({}, updates)
+        self.assertEqual(list(conflicts), [('tag-2', b'z', b'y')])
+        self.assertEqual(b'y', self.tags.lookup_tag('tag-2'))
+        # overwrite conflicts
+        updates, conflicts = other_tags.merge_to(self.tags, overwrite=True)
+        self.assertEqual(list(conflicts), [])
+        self.assertEqual({u'tag-2': b'z'}, updates)
+        self.assertEqual(b'z', self.tags.lookup_tag('tag-2'))