Implement set_symbolic_ref separately in Disk/DictRefsContainer.

When writing tests, it became clear that the old implementation was broken.

Change-Id: Id98d7af0c92568d1faf555784214d0c1eb298feb

dulwich/repo.py

@@ -127,7 +127,7 @@ class RefsContainer(object):
         :param name: Name of the ref to set
         :param other: Name of the ref to point at
         """
-        self[name] = SYMREF + other
+        raise NotImplementedError(self.set_symbolic_ref)
 
     def get_packed_refs(self):
         """Get contents of the packed-refs file.
@@ -351,13 +351,14 @@ class DictRefsContainer(RefsContainer):
     def get_packed_refs(self):
         return {}
 
-    def __getitem__(self, name):
-        return self._refs[name]
+    def set_symbolic_ref(self, name, other):
+        self._refs[name] = SYMREF + other
 
     def set_if_equals(self, name, old_ref, new_ref):
         if old_ref is not None and self._refs.get(name, None) != old_ref:
             return False
-        self._refs[name] = new_ref
+        realname, _ = self._follow(name)
+        self._refs[realname] = new_ref
         return True
 
     def add_if_new(self, name, ref):
@@ -525,6 +526,25 @@ class DiskRefsContainer(RefsContainer):
         finally:
             f.abort()
 
+    def set_symbolic_ref(self, name, other):
+        """Make a ref point at another ref.
+
+        :param name: Name of the ref to set
+        :param other: Name of the ref to point at
+        """
+        self._check_refname(name)
+        self._check_refname(other)
+        filename = self.refpath(name)
+        try:
+            f = GitFile(filename, 'wb')
+            try:
+                f.write(SYMREF + other + '\n')
+            except (IOError, OSError):
+                f.abort()
+                raise
+        finally:
+            f.close()
+
     def set_if_equals(self, name, old_ref, new_ref):
         """Set a refname to new_ref only if it currently equals old_ref.
 

dulwich/tests/test_repository.py

@@ -539,6 +539,24 @@ class RefsContainerTests(object):
         self.assertTrue(self._refs.add_if_new('refs/some/ref', nines))
         self.assertEqual(nines, self._refs['refs/some/ref'])
 
+    def test_set_symbolic_ref(self):
+        self._refs.set_symbolic_ref('refs/heads/symbolic', 'refs/heads/master')
+        self.assertEqual('ref: refs/heads/master',
+                         self._refs.read_loose_ref('refs/heads/symbolic'))
+        self.assertEqual('42d06bd4b77fed026b154d16493e5deab78f02ec',
+                         self._refs['refs/heads/symbolic'])
+
+    def test_set_symbolic_ref_overwrite(self):
+        nines = '9' * 40
+        self.assertFalse('refs/heads/symbolic' in self._refs)
+        self._refs['refs/heads/symbolic'] = nines
+        self.assertEqual(nines, self._refs.read_loose_ref('refs/heads/symbolic'))
+        self._refs.set_symbolic_ref('refs/heads/symbolic', 'refs/heads/master')
+        self.assertEqual('ref: refs/heads/master',
+                         self._refs.read_loose_ref('refs/heads/symbolic'))
+        self.assertEqual('42d06bd4b77fed026b154d16493e5deab78f02ec',
+                         self._refs['refs/heads/symbolic'])
+
     def test_check_refname(self):
         try:
             self._refs._check_refname('HEAD')