Check for diverged branches.

Check for fast-forward pushes during push. Fixes #494
Check for fast-forwarding during pull. Fixes #666

NEWS

@@ -7,8 +7,15 @@
    already have those objects. Thanks to Martijn Pieters for
    the detailed bug report. (Jelmer Vernooij, #781)
 
+ * Add LCA implementation. (Kevin Hendricks)
+
  * Add functionality for finding the merge base. (Kevin Hendricks)
 
+ * Check for diverged branches during push.
+   (Jelmer Vernooij, #494)
+
+ * Check for fast-forward during pull. (Jelmer Vernooij, #666)
+
 0.20.3	2020-06-14
 
  * Add support for remembering remote refs after push/pull.

dulwich/graph.py

@@ -131,3 +131,22 @@ def find_octopus_base(object_store, commit_ids):
             next_lcas.extend(res)
         lcas = next_lcas[:]
     return lcas
+
+
+def can_fast_forward(object_store, c1, c2):
+    """Is it possible to fast-forward from c1 to c2?
+
+    Args:
+      object_store: Store to retrieve objects from
+      c1: Commit id for first commit
+      c2: Commit id for second commit
+    """
+    if c1 == c2:
+        return True
+
+    def lookup_parents(commit_id):
+        return object_store[commit_id].parents
+
+    # Algorithm: Find the common ancestor
+    lcas = _find_lcas(lookup_parents, c1, [c2])
+    return lcas == [c1]

dulwich/porcelain.py

@@ -98,6 +98,9 @@ from dulwich.errors import (
     SendPackError,
     UpdateRefsError,
     )
+from dulwich.graph import (
+    can_fast_forward,
+    )
 from dulwich.ignore import IgnoreFilterManager
 from dulwich.index import (
     blob_from_path_and_stat,
@@ -230,9 +233,12 @@ def check_diverged(store, current_sha, new_sha):
       current_sha: Current head sha
       new_sha: New head sha
     """
-    return
-    # TODO(jelmer): check for diverged branches. See bug #666, #494
-    raise DivergedBranches(current_sha, new_sha)
+    try:
+        can = can_fast_forward(store, current_sha, new_sha)
+    except KeyError:
+        can = False
+    if not can:
+        raise DivergedBranches(current_sha, new_sha)
 
 
 def archive(repo, committish=None, outstream=default_bytes_out_stream,
@@ -996,9 +1002,6 @@ def pull(repo, remote_location=None, refspecs=None,
       outstream: A stream file to write to output
       errstream: A stream file to write to errors
     """
-    if not fast_forward:
-        raise NotImplementedError('no-ff pull is not yet supported')
-
     # Open the repo
     with open_repo_closing(repo) as r:
         (remote_name, remote_location) = get_remote_repo(r, remote_location)
@@ -1018,9 +1021,14 @@ def pull(repo, remote_location=None, refspecs=None,
         fetch_result = client.fetch(
             path, r, progress=errstream.write, determine_wants=determine_wants)
         for (lh, rh, force_ref) in selected_refs:
-            if fast_forward:
+            try:
                 check_diverged(
                     r.object_store, r.refs[rh], fetch_result.refs[lh])
+            except DivergedBranches:
+                if fast_forward:
+                    raise
+                else:
+                    raise NotImplementedError('merge is not yet supported')
             r.refs[rh] = fetch_result.refs[lh]
         if selected_refs:
             r[b'HEAD'] = fetch_result.refs[selected_refs[0][1]]

dulwich/tests/test_graph.py

@@ -22,8 +22,10 @@
 """Tests for dulwich.graph."""
 
 from dulwich.tests import TestCase
+from dulwich.tests.utils import make_commit
+from dulwich.object_store import MemoryObjectStore
 
-from dulwich.graph import _find_lcas
+from dulwich.graph import _find_lcas, can_fast_forward
 
 
 class FindMergeBaseTests(TestCase):
@@ -154,3 +156,29 @@ class FindMergeBaseTests(TestCase):
                 next_lcas.extend(res)
             lcas = next_lcas[:]
         self.assertEqual(set(lcas), set(['2']))
+
+
+class CanFastForwardTests(TestCase):
+
+    def test_ff(self):
+        store = MemoryObjectStore()
+        base = make_commit()
+        c1 = make_commit(parents=[base.id])
+        c2 = make_commit(parents=[c1.id])
+        store.add_objects([(base, None), (c1, None), (c2, None)])
+        self.assertTrue(can_fast_forward(store, c1.id, c1.id))
+        self.assertTrue(can_fast_forward(store, base.id, c1.id))
+        self.assertTrue(can_fast_forward(store, c1.id, c2.id))
+        self.assertFalse(can_fast_forward(store, c2.id, c1.id))
+
+    def test_diverged(self):
+        store = MemoryObjectStore()
+        base = make_commit()
+        c1 = make_commit(parents=[base.id])
+        c2a = make_commit(parents=[c1.id], message=b'2a')
+        c2b = make_commit(parents=[c1.id], message=b'2b')
+        store.add_objects([(base, None), (c1, None), (c2a, None), (c2b, None)])
+        self.assertTrue(can_fast_forward(store, c1.id, c2a.id))
+        self.assertTrue(can_fast_forward(store, c1.id, c2b.id))
+        self.assertFalse(can_fast_forward(store, c2a.id, c2b.id))
+        self.assertFalse(can_fast_forward(store, c2b.id, c2a.id))

dulwich/tests/test_porcelain.py

@@ -955,6 +955,52 @@ class PushTests(PorcelainTestCase):
             b'refs/heads/master': new_id,
             }, self.repo.get_refs())
 
+    def test_diverged(self):
+        outstream = BytesIO()
+        errstream = BytesIO()
+
+        porcelain.commit(repo=self.repo.path, message=b'init',
+                         author=b'author <email>',
+                         committer=b'committer <email>')
+
+        # Setup target repo cloned from temp test repo
+        clone_path = tempfile.mkdtemp()
+        self.addCleanup(shutil.rmtree, clone_path)
+        target_repo = porcelain.clone(self.repo.path, target=clone_path,
+                                      errstream=errstream)
+        target_repo.close()
+
+        remote_id = porcelain.commit(
+            repo=self.repo.path, message=b'remote change',
+            author=b'author <email>',
+            committer=b'committer <email>')
+
+        local_id = porcelain.commit(
+            repo=clone_path, message=b'local change',
+            author=b'author <email>',
+            committer=b'committer <email>')
+
+        # Push to the remote
+        self.assertRaises(
+            porcelain.DivergedBranches,
+            porcelain.push, clone_path, self.repo.path, b'refs/heads/master',
+            outstream=outstream, errstream=errstream)
+
+        self.assertEqual({
+            b'HEAD': remote_id,
+            b'refs/heads/master': remote_id,
+            }, self.repo.get_refs())
+
+        # Push to the remote with --force
+        porcelain.push(
+            clone_path, self.repo.path, b'refs/heads/master',
+            outstream=outstream, errstream=errstream, force=True)
+
+        self.assertEqual({
+            b'HEAD': local_id,
+            b'refs/heads/master': local_id,
+            }, self.repo.get_refs())
+
 
 class PullTests(PorcelainTestCase):
 
@@ -983,8 +1029,8 @@ class PullTests(PorcelainTestCase):
                          author=b'test2 <email>',
                          committer=b'test2 <email>')
 
-        self.assertTrue(b'refs/heads/master' in self.repo.refs)
-        self.assertTrue(b'refs/heads/master' in target_repo.refs)
+        self.assertIn(b'refs/heads/master', self.repo.refs)
+        self.assertIn(b'refs/heads/master', target_repo.refs)
 
     def test_simple(self):
         outstream = BytesIO()
@@ -998,6 +1044,40 @@ class PullTests(PorcelainTestCase):
         with Repo(self.target_path) as r:
             self.assertEqual(r[b'HEAD'].id, self.repo[b'HEAD'].id)
 
+    def test_diverged(self):
+        outstream = BytesIO()
+        errstream = BytesIO()
+
+        c3a = porcelain.commit(
+            repo=self.target_path, message=b'test3a',
+            author=b'test2 <email>',
+            committer=b'test2 <email>')
+
+        porcelain.commit(
+            repo=self.repo.path, message=b'test3b',
+            author=b'test2 <email>',
+            committer=b'test2 <email>')
+
+        # Pull changes into the cloned repo
+        self.assertRaises(
+            porcelain.DivergedBranches, porcelain.pull, self.target_path,
+            self.repo.path, b'refs/heads/master', outstream=outstream,
+            errstream=errstream)
+
+        # Check the target repo for pushed changes
+        with Repo(self.target_path) as r:
+            self.assertEqual(r[b'refs/heads/master'].id, c3a)
+
+        self.assertRaises(
+            NotImplementedError, porcelain.pull,
+            self.target_path, self.repo.path,
+            b'refs/heads/master', outstream=outstream, errstream=errstream,
+            fast_forward=False)
+
+        # Check the target repo for pushed changes
+        with Repo(self.target_path) as r:
+            self.assertEqual(r[b'refs/heads/master'].id, c3a)
+
     def test_no_refspec(self):
         outstream = BytesIO()
         errstream = BytesIO()