Support removing refs from porcelain.push(). Fixes #437.

NEWS

@@ -5,6 +5,9 @@
   * Fix ShaFile.id after modification of a copied ShaFile.
     (Félix Mattrat, Jelmer Vernooij)
 
+  * Support removing refs from porcelain.push.
+    (Jelmer Vernooij, #437)
+
  IMPROVEMENTS
 
   * Add `dulwich.config.parse_submodules` function.

dulwich/client.py

@@ -784,20 +784,21 @@ class LocalGitClient(GitClient):
 
             have = [sha1 for sha1 in old_refs.values() if sha1 != ZERO_SHA]
             want = []
-            all_refs = set(new_refs.keys()).union(set(old_refs.keys()))
-            for refname in all_refs:
-                old_sha1 = old_refs.get(refname, ZERO_SHA)
-                new_sha1 = new_refs.get(refname, ZERO_SHA)
-                if new_sha1 not in have and new_sha1 != ZERO_SHA:
+            for refname, new_sha1 in new_refs.items():
+                if new_sha1 not in have and not new_sha1 in want and new_sha1 != ZERO_SHA:
                     want.append(new_sha1)
 
-            if not want and old_refs == new_refs:
+            if not want and set(old_refs.items()).issubset(set(new_refs.items())):
                 return new_refs
 
             target.object_store.add_objects(generate_pack_contents(have, want))
 
-            for name, sha in new_refs.items():
-                target.refs[name] = sha
+            for refname, new_sha1 in new_refs.items():
+                old_sha1 = old_refs.get(refname, ZERO_SHA)
+                if new_sha1 != ZERO_SHA:
+                    target.refs.set_if_equals(refname, old_sha1, new_sha1)
+                else:
+                    target.refs.remove_if_equals(refname, old_sha1)
 
         return new_refs
 

dulwich/porcelain.py

@@ -580,13 +580,14 @@ def push(repo, remote_location, refspecs=None,
 
         def update_refs(refs):
             selected_refs.extend(parse_reftuples(r.refs, refs, refspecs))
+            new_refs = {}
             # TODO: Handle selected_refs == {None: None}
             for (lh, rh, force) in selected_refs:
                 if lh is None:
-                    refs[rh] = ZERO_SHA
+                    new_refs[rh] = ZERO_SHA
                 else:
-                    refs[rh] = r.refs[lh]
-            return refs
+                    new_refs[rh] = r.refs[lh]
+            return new_refs
 
         err_encoding = getattr(errstream, 'encoding', None) or 'utf-8'
         remote_location_bytes = remote_location.encode(err_encoding)

dulwich/tests/test_client.py

@@ -45,7 +45,6 @@ from dulwich.tests import (
 from dulwich.protocol import (
     TCP_GIT_PORT,
     Protocol,
-    ZERO_SHA,
     )
 from dulwich.pack import (
     write_pack_objects,

dulwich/tests/test_porcelain.py

@@ -36,11 +36,11 @@ from dulwich.objects import (
     Blob,
     Tag,
     Tree,
+    ZERO_SHA,
     )
 from dulwich.repo import Repo
 from dulwich.tests import (
     TestCase,
-    expectedFailure,
     )
 from dulwich.tests.utils import (
     build_commit_graph,
@@ -459,7 +459,10 @@ class PushTests(PorcelainTestCase):
         self.addCleanup(shutil.rmtree, clone_path)
         target_repo = porcelain.clone(self.repo.path, target=clone_path,
             errstream=errstream)
-        target_repo.close()
+        try:
+            self.assertEqual(target_repo[b'HEAD'], self.repo[b'HEAD'])
+        finally:
+            target_repo.close()
 
         # create a second file to be pushed back to origin
         handle, fullpath = tempfile.mkstemp(dir=clone_path)
@@ -470,7 +473,9 @@ class PushTests(PorcelainTestCase):
 
         # Setup a non-checked out branch in the remote
         refs_path = b"refs/heads/foo"
-        self.repo.refs[refs_path] = self.repo[b'HEAD'].id
+        new_id = self.repo[b'HEAD'].id
+        self.assertNotEqual(new_id, ZERO_SHA)
+        self.repo.refs[refs_path] = new_id
 
         # Push to the remote
         porcelain.push(clone_path, self.repo.path, b"HEAD:" + refs_path, outstream=outstream,
@@ -478,7 +483,12 @@ class PushTests(PorcelainTestCase):
 
         # Check that the target and source
         with closing(Repo(clone_path)) as r_clone:
-            self.assertEqual(r_clone[b'HEAD'].id, self.repo[refs_path].id)
+            self.assertEqual({
+                b'HEAD': new_id,
+                b'refs/heads/foo': r_clone[b'HEAD'].id,
+                b'refs/heads/master': new_id,
+                }, self.repo.get_refs())
+            self.assertEqual(r_clone[b'HEAD'].id, self.repo.refs[refs_path])
 
             # Get the change in the target repo corresponding to the add
             # this will be in the foo branch.
@@ -487,8 +497,6 @@ class PushTests(PorcelainTestCase):
             self.assertEqual(os.path.basename(fullpath),
                 change.new.path.decode('ascii'))
 
-    # Deletions don't work yet. #437
-    @expectedFailure
     def test_delete(self):
         """Basic test of porcelain push, removing a branch.
         """
@@ -507,7 +515,9 @@ class PushTests(PorcelainTestCase):
 
         # Setup a non-checked out branch in the remote
         refs_path = b"refs/heads/foo"
-        self.repo.refs[refs_path] = self.repo[b'HEAD'].id
+        new_id = self.repo[b'HEAD'].id
+        self.assertNotEqual(new_id, ZERO_SHA)
+        self.repo.refs[refs_path] = new_id
 
         # Push to the remote
         porcelain.push(clone_path, self.repo.path, b":" + refs_path, outstream=outstream,
@@ -515,8 +525,8 @@ class PushTests(PorcelainTestCase):
 
         with closing(Repo(clone_path)) as r_clone:
             self.assertEqual({
-                b'HEAD': self.repo[b'HEAD'].id,
-                b'refs/heads/master': self.repo[b'HEAD'].id
+                b'HEAD': new_id,
+                b'refs/heads/master': new_id,
                 }, self.repo.get_refs())