Simplify the API a bit and fix backwards compatibility, add NEWS item.

NEWS

@@ -3,6 +3,9 @@
  * Don't break when encountering block devices.
    (Jelmer Vernooij)
 
+ * Fix pushing from a shallow clone.
+   (Brecht Machiels, #705)
+
 0.19.16	2020-04-17
 
  * Don't send "deepen None" to server if graph walker

dulwich/client.py

@@ -30,10 +30,10 @@ The Dulwich client supports the following capabilities:
  * quiet
  * report-status
  * delete-refs
+ * shallow
 
 Known capabilities that are not supported:
 
- * shallow
  * no-progress
  * include-tag
 """
@@ -361,7 +361,7 @@ class GitClient(object):
         """
         raise NotImplementedError(cls.from_parsedurl)
 
-    def send_pack(self, path, update_refs, shallow, generate_pack_data,
+    def send_pack(self, path, update_refs, generate_pack_data,
                   progress=None):
         """Upload a pack to a remote repository.
 
@@ -370,7 +370,6 @@ class GitClient(object):
           update_refs: Function to determine changes to remote refs. Receive
             dict with existing remote refs, returns dict with
             changed refs (name -> sha, where sha=ZERO_SHA for deletions)
-          shallow: Set of shallow commits generate_pack_data should skip
           generate_pack_data: Function that can return a tuple
             with number of objects and list of pack data to include
           progress: Optional progress function
@@ -773,7 +772,7 @@ class TraditionalGitClient(GitClient):
         """
         raise NotImplementedError()
 
-    def send_pack(self, path, update_refs, shallow, generate_pack_data,
+    def send_pack(self, path, update_refs, generate_pack_data,
                   progress=None):
         """Upload a pack to a remote repository.
 
@@ -782,7 +781,6 @@ class TraditionalGitClient(GitClient):
           update_refs: Function to determine changes to remote refs.
         Receive dict with existing remote refs, returns dict with
         changed refs (name -> sha, where sha=ZERO_SHA for deletions)
-          shallow: Set of shallow commits generate_pack_data should skip
           generate_pack_data: Function that can return a tuple with
         number of objects and pack data to upload.
           progress: Optional callback called with progress updates
@@ -844,7 +842,7 @@ class TraditionalGitClient(GitClient):
                     set(new_refs.items()).issubset(set(old_refs.items()))):
                 return new_refs
             pack_data_count, pack_data = generate_pack_data(
-                have, want, shallow,
+                have, want,
                 ofs_delta=(CAPABILITY_OFS_DELTA in negotiated_capabilities))
 
             dowrite = bool(pack_data_count)
@@ -1118,7 +1116,7 @@ class LocalGitClient(GitClient):
             path = path.decode(sys.getfilesystemencoding())
         return closing(Repo(path))
 
-    def send_pack(self, path, update_refs, shallow, generate_pack_data,
+    def send_pack(self, path, update_refs, generate_pack_data,
                   progress=None):
         """Upload a pack to a remote repository.
 
@@ -1127,8 +1125,6 @@ class LocalGitClient(GitClient):
           update_refs: Function to determine changes to remote refs.
         Receive dict with existing remote refs, returns dict with
         changed refs (name -> sha, where sha=ZERO_SHA for deletions)
-          shallow: Set of shallow commits generate_pack_data should skip
-          generate_pack_data: Function that can return a tuple
         with number of items and pack data to upload.
           progress: Optional progress function
 
@@ -1163,7 +1159,7 @@ class LocalGitClient(GitClient):
                 return new_refs
 
             target.object_store.add_pack_data(
-                *generate_pack_data(have, want, shallow, ofs_delta=True))
+                *generate_pack_data(have, want, ofs_delta=True))
 
             for refname, new_sha1 in new_refs.items():
                 old_sha1 = old_refs.get(refname, ZERO_SHA)
@@ -1673,7 +1669,7 @@ class HttpGitClient(GitClient):
                                    % resp.content_type)
         return resp, read
 
-    def send_pack(self, path, update_refs, shallow, generate_pack_data,
+    def send_pack(self, path, update_refs, generate_pack_data,
                   progress=None):
         """Upload a pack to a remote repository.
 
@@ -1682,7 +1678,6 @@ class HttpGitClient(GitClient):
           update_refs: Function to determine changes to remote refs.
         Receives dict with existing remote refs, returns dict with
         changed refs (name -> sha, where sha=ZERO_SHA for deletions)
-          shallow: Set of shallow commits generate_pack_data should skip
           generate_pack_data: Function that can return a tuple
         with number of elements and pack data to upload.
           progress: Optional progress function
@@ -1720,7 +1715,7 @@ class HttpGitClient(GitClient):
         if not want and set(new_refs.items()).issubset(set(old_refs.items())):
             return new_refs
         pack_data_count, pack_data = generate_pack_data(
-                have, want, shallow,
+                have, want,
                 ofs_delta=(CAPABILITY_OFS_DELTA in negotiated_capabilities))
         if pack_data_count:
             write_pack_data(req_proto.write_file(), pack_data_count, pack_data)

dulwich/contrib/test_swift_smoke.py

@@ -141,8 +141,7 @@ class SwiftRepoSmokeTest(unittest.TestCase):
         tcp_client = client.TCPGitClient(self.server_address,
                                          port=self.port)
         tcp_client.send_pack(self.fakerepo, determine_wants,
-                             local_repo.get_shallow(),
-                             local_repo.object_store.generate_pack_data)
+                             local_repo.generate_pack_data)
         swift_repo = swift.SwiftRepo("fakerepo", self.conf)
         remote_sha = swift_repo.refs.read_loose_ref('refs/heads/master')
         self.assertEqual(sha, remote_sha)
@@ -161,8 +160,7 @@ class SwiftRepoSmokeTest(unittest.TestCase):
         tcp_client = client.TCPGitClient(self.server_address,
                                          port=self.port)
         tcp_client.send_pack("/fakerepo", determine_wants,
-                             local_repo.get_shallow(),
-                             local_repo.object_store.generate_pack_data)
+                             local_repo.generate_pack_data)
         swift_repo = swift.SwiftRepo(self.fakerepo, self.conf)
         remote_sha = swift_repo.refs.read_loose_ref('refs/heads/mybranch')
         self.assertEqual(sha, remote_sha)
@@ -188,8 +186,7 @@ class SwiftRepoSmokeTest(unittest.TestCase):
         tcp_client = client.TCPGitClient(self.server_address,
                                          port=self.port)
         tcp_client.send_pack(self.fakerepo, determine_wants,
-                             local_repo.get_shallow(),
-                             local_repo.object_store.generate_pack_data)
+                             local_repo.generate_pack_data)
         swift_repo = swift.SwiftRepo("fakerepo", self.conf)
         for branch in ('master', 'mybranch', 'pullr-108'):
             remote_shas[branch] = swift_repo.refs.read_loose_ref(
@@ -213,8 +210,7 @@ class SwiftRepoSmokeTest(unittest.TestCase):
         tcp_client = client.TCPGitClient(self.server_address,
                                          port=self.port)
         tcp_client.send_pack(self.fakerepo, determine_wants,
-                             local_repo.get_shallow(),
-                             local_repo.object_store.generate_pack_data)
+                             local_repo.generate_pack_data)
         swift_repo = swift.SwiftRepo("fakerepo", self.conf)
         commit_sha = swift_repo.refs.read_loose_ref('refs/heads/master')
         otype, data = swift_repo.object_store.get_raw(commit_sha)
@@ -260,8 +256,7 @@ class SwiftRepoSmokeTest(unittest.TestCase):
         local_repo.do_commit('Test commit', 'fbo@localhost',
                              ref='refs/heads/master')
         tcp_client.send_pack("/fakerepo", determine_wants,
-                             local_repo.get_shallow(),
-                             local_repo.object_store.generate_pack_data)
+                             local_repo.generate_pack_data)
 
     def test_push_remove_branch(self):
         def determine_wants(*args):
@@ -276,8 +271,7 @@ class SwiftRepoSmokeTest(unittest.TestCase):
         tcp_client = client.TCPGitClient(self.server_address,
                                          port=self.port)
         tcp_client.send_pack(self.fakerepo, determine_wants,
-                             local_repo.get_shallow(),
-                             local_repo.object_store.generate_pack_data)
+                             local_repo.generate_pack_data)
         swift_repo = swift.SwiftRepo("fakerepo", self.conf)
         self.assertNotIn('refs/heads/pullr-108', swift_repo.refs.allkeys())
 
@@ -303,8 +297,7 @@ class SwiftRepoSmokeTest(unittest.TestCase):
         tcp_client = client.TCPGitClient(self.server_address,
                                          port=self.port)
         tcp_client.send_pack(self.fakerepo, determine_wants,
-                             local_repo.get_shallow(),
-                             local_repo.object_store.generate_pack_data)
+                             local_repo.generate_pack_data)
         swift_repo = swift.SwiftRepo(self.fakerepo, self.conf)
         tag_sha = swift_repo.refs.read_loose_ref('refs/tags/v1.0')
         otype, data = swift_repo.object_store.get_raw(tag_sha)

dulwich/object_store.py

@@ -201,7 +201,7 @@ class BaseObjectStore(object):
                  not stat.S_ISDIR(entry.mode)) or include_trees):
                 yield entry
 
-    def find_missing_objects(self, haves, wants, shallow, progress=None,
+    def find_missing_objects(self, haves, wants, shallow=None, progress=None,
                              get_tagged=None,
                              get_parents=lambda commit: commit.parents,
                              depth=None):
@@ -239,7 +239,7 @@ class BaseObjectStore(object):
             sha = next(graphwalker)
         return haves
 
-    def generate_pack_contents(self, have, want, shallow, progress=None):
+    def generate_pack_contents(self, have, want, shallow=None, progress=None):
         """Iterate over the contents of a pack file.
 
         Args:
@@ -251,7 +251,7 @@ class BaseObjectStore(object):
         missing = self.find_missing_objects(have, want, shallow, progress)
         return self.iter_shas(missing)
 
-    def generate_pack_data(self, have, want, shallow, progress=None,
+    def generate_pack_data(self, have, want, shallow=None, progress=None,
                            ofs_delta=True):
         """Generate pack data objects for a set of wants/haves.
 
@@ -1169,9 +1169,11 @@ class MissingObjectFinder(object):
       tagged: dict of pointed-to sha -> tag sha for including tags
     """
 
-    def __init__(self, object_store, haves, wants, shallow, progress=None,
+    def __init__(self, object_store, haves, wants, shallow=None, progress=None,
                  get_tagged=None, get_parents=lambda commit: commit.parents):
         self.object_store = object_store
+        if shallow is None:
+            shallow = set()
         self._get_parents = get_parents
         # process Commits and Tags differently
         # Note, while haves may list commits/tags not available locally,

dulwich/porcelain.py

@@ -911,8 +911,8 @@ def push(repo, remote_location, refspecs,
         remote_location_bytes = client.get_url(path).encode(err_encoding)
         try:
             client.send_pack(
-                path, update_refs, r.get_shallow(),
-                generate_pack_data=r.object_store.generate_pack_data,
+                path, update_refs,
+                generate_pack_data=r.generate_pack_data,
                 progress=errstream.write)
             errstream.write(
                 b"Push to " + remote_location_bytes + b" successful.\n")

dulwich/repo.py

@@ -472,6 +472,19 @@ class BaseRepo(object):
               progress, get_tagged,
               get_parents=get_parents))
 
+    def generate_pack_data(self, have, want, progress=None, ofs_delta=None):
+        """Generate pack data objects for a set of wants/haves.
+
+        Args:
+          have: List of SHA1s of objects that should not be sent
+          want: List of SHA1s of objects that should be sent
+          ofs_delta: Whether OFS deltas can be included
+          progress: Optional progress reporting method
+        """
+        return self.object_store.generate_pack_data(
+            have, want, shallow=self.get_shallow(),
+            progress=progress, ofs_delta=ofs_delta)
+
     def get_graph_walker(self, heads=None):
         """Retrieve a graph walker.
 

dulwich/tests/compat/test_client.py

@@ -107,7 +107,7 @@ class DulwichClientTestBase(object):
             sendrefs = dict(src.get_refs())
             del sendrefs[b'HEAD']
             c.send_pack(self._build_path('/dest'), lambda _: sendrefs,
-                        src.get_shallow(), src.object_store.generate_pack_data)
+                        src.generate_pack_data)
 
     def test_send_pack(self):
         self._do_send_pack()
@@ -129,8 +129,6 @@ class DulwichClientTestBase(object):
         repo.object_store.add_object(tree)
         return tree.id
 
-    # Pushing from a shallow clone currently fails. See #705
-    @unittest.expectedFailure
     def test_send_pack_from_shallow_clone(self):
         c = self._client()
         server_new_path = os.path.join(self.gitroot, 'server_new.export')
@@ -153,8 +151,8 @@ class DulwichClientTestBase(object):
                     tree=tree_id)
             sendrefs = dict(local.get_refs())
             del sendrefs[b'HEAD']
-            c.send_pack(remote_path, lambda _: sendrefs, local.get_shallow(),
-                        local.object_store.generate_pack_data)
+            c.send_pack(remote_path, lambda _: sendrefs,
+                        local.generate_pack_data)
         with repo.Repo(server_new_path) as remote:
             self.assertEqual(remote.head(), commit_id)
 
@@ -166,7 +164,7 @@ class DulwichClientTestBase(object):
             sendrefs = dict(src.get_refs())
             del sendrefs[b'HEAD']
             c.send_pack(self._build_path('/dest'), lambda _: sendrefs,
-                        src.get_shallow(), src.object_store.generate_pack_data)
+                        src.generate_pack_data)
             self.assertDestEqualsSrc()
 
     def make_dummy_commit(self, dest):
@@ -193,7 +191,7 @@ class DulwichClientTestBase(object):
     def compute_send(self, src):
         sendrefs = dict(src.get_refs())
         del sendrefs[b'HEAD']
-        return sendrefs, src.object_store.generate_pack_data
+        return sendrefs, src.generate_pack_data
 
     def test_send_pack_one_error(self):
         dest, dummy_commit = self.disable_ff_and_make_dummy_commit()
@@ -204,7 +202,7 @@ class DulwichClientTestBase(object):
             c = self._client()
             try:
                 c.send_pack(self._build_path('/dest'), lambda _: sendrefs,
-                            src.get_shallow(), gen_pack)
+                            gen_pack)
             except errors.UpdateRefsError as e:
                 self.assertEqual('refs/heads/master failed to update',
                                  e.args[0])
@@ -223,7 +221,7 @@ class DulwichClientTestBase(object):
             c = self._client()
             try:
                 c.send_pack(self._build_path('/dest'), lambda _: sendrefs,
-                            src.get_shallow(), gen_pack)
+                            gen_pack)
             except errors.UpdateRefsError as e:
                 self.assertIn(
                         str(e),
@@ -316,12 +314,12 @@ class DulwichClientTestBase(object):
             sendrefs[b'refs/heads/abranch'] = b"00" * 20
             del sendrefs[b'HEAD']
 
-            def gen_pack(have, want, shallow, ofs_delta=False):
+            def gen_pack(have, want, ofs_delta=False):
                 return 0, []
             c = self._client()
             self.assertEqual(dest.refs[b"refs/heads/abranch"], dummy_commit)
             c.send_pack(
-                self._build_path('/dest'), lambda _: sendrefs, set(), gen_pack)
+                self._build_path('/dest'), lambda _: sendrefs, gen_pack)
             self.assertFalse(b"refs/heads/abranch" in dest.refs)
 
     def test_get_refs(self):

dulwich/tests/test_client.py

@@ -223,12 +223,12 @@ class GitClientTests(TestCase):
         def update_refs(refs):
             return {b'refs/foo/bar': commit.id, }
 
-        def generate_pack_data(have, want, shallow, ofs_delta=False):
+        def generate_pack_data(have, want, ofs_delta=False):
             return pack_objects_to_data([(commit, None), (tree, ''), ])
 
         self.assertRaises(UpdateRefsError,
                           self.client.send_pack, "blah",
-                          update_refs, set(), generate_pack_data)
+                          update_refs, generate_pack_data)
 
     def test_send_pack_none(self):
         self.rin.write(
@@ -244,7 +244,7 @@ class GitClientTests(TestCase):
                     b'310ca9477129b8586fa2afc779c1f57cf64bba6c'
             }
 
-        def generate_pack_data(have, want, shallow, ofs_delta=False):
+        def generate_pack_data(have, want, ofs_delta=False):
             return 0, []
 
         self.client.send_pack(b'/', update_refs, set(), generate_pack_data)
@@ -263,10 +263,10 @@ class GitClientTests(TestCase):
         def update_refs(refs):
             return {b'refs/heads/master': b'0' * 40}
 
-        def generate_pack_data(have, want, shallow, ofs_delta=False):
+        def generate_pack_data(have, want, ofs_delta=False):
             return 0, []
 
-        self.client.send_pack(b'/', update_refs, set(), generate_pack_data)
+        self.client.send_pack(b'/', update_refs, generate_pack_data)
         self.assertEqual(
             self.rout.getvalue(),
             b'008b310ca9477129b8586fa2afc779c1f57cf64bba6c '
@@ -285,10 +285,10 @@ class GitClientTests(TestCase):
         def update_refs(refs):
             return {b'refs/heads/master': b'0' * 40}
 
-        def generate_pack_data(have, want, shallow, ofs_delta=False):
+        def generate_pack_data(have, want, ofs_delta=False):
             return 0, []
 
-        self.client.send_pack(b'/', update_refs, set(), generate_pack_data)
+        self.client.send_pack(b'/', update_refs, generate_pack_data)
         self.assertEqual(
             self.rout.getvalue(),
             b'008b310ca9477129b8586fa2afc779c1f57cf64bba6c '
@@ -312,12 +312,12 @@ class GitClientTests(TestCase):
                     b'310ca9477129b8586fa2afc779c1f57cf64bba6c'
             }
 
-        def generate_pack_data(have, want, shallow, ofs_delta=False):
+        def generate_pack_data(have, want, ofs_delta=False):
             return 0, []
 
         f = BytesIO()
         write_pack_objects(f, {})
-        self.client.send_pack('/', update_refs, set(), generate_pack_data)
+        self.client.send_pack('/', update_refs, generate_pack_data)
         self.assertEqual(
             self.rout.getvalue(),
             b'008b0000000000000000000000000000000000000000 '
@@ -351,12 +351,12 @@ class GitClientTests(TestCase):
                     b'310ca9477129b8586fa2afc779c1f57cf64bba6c'
             }
 
-        def generate_pack_data(have, want, shallow, ofs_delta=False):
+        def generate_pack_data(have, want, ofs_delta=False):
             return pack_objects_to_data([(commit, None), (tree, b''), ])
 
         f = BytesIO()
-        write_pack_data(f, *generate_pack_data(None, None, set()))
-        self.client.send_pack(b'/', update_refs, set(), generate_pack_data)
+        write_pack_data(f, *generate_pack_data(None, None))
+        self.client.send_pack(b'/', update_refs, generate_pack_data)
         self.assertEqual(
             self.rout.getvalue(),
             b'008b0000000000000000000000000000000000000000 ' + commit.id +
@@ -378,12 +378,12 @@ class GitClientTests(TestCase):
         def update_refs(refs):
             return {b'refs/heads/master': b'0' * 40}
 
-        def generate_pack_data(have, want, shallow, ofs_delta=False):
+        def generate_pack_data(have, want, ofs_delta=False):
             return 0, []
 
         self.assertRaises(UpdateRefsError,
                           self.client.send_pack, b"/",
-                          update_refs, set(), generate_pack_data)
+                          update_refs, generate_pack_data)
         self.assertEqual(self.rout.getvalue(), b'0000')
 
 
@@ -873,8 +873,7 @@ class LocalGitClientTests(TestCase):
         ref_name = b"refs/heads/" + branch
         new_refs = client.send_pack(target.path,
                                     lambda _: {ref_name: local.refs[ref_name]},
-                                    local.get_shallow(),
-                                    local.object_store.generate_pack_data)
+                                    local.generate_pack_data)
 
         self.assertEqual(local.refs[ref_name], new_refs[ref_name])