Refactor to simplify clone logic, and avoid circular imports.

Move some ref-only operations to dulwich.refs.

NEWS

@@ -3,6 +3,13 @@
  * Support staging submodules.
    (Jelmer Vernooij)
 
+ * Drop deprecated Index.iterblobs and iter_fresh_blobs.
+   (Jelmer Vernooij)
+
+ * Unify clone behaviour of ``Repo.clone`` and
+   ``porcelain.clone``, and add branch parameter for
+   clone. (Peter Rowlands, #851)
+
 0.20.28	2022-01-05
 
  * Fix hook test on Mac OSX / Linux when dulwich is

dulwich/clone.py

@@ -1,187 +0,0 @@
-# clone.py
-# Copyright (C) 2021 Jelmer Vernooij <jelmer@samba.org>
-#
-# Dulwich is dual-licensed under the Apache License, Version 2.0 and the GNU
-# General Public License as public by the Free Software Foundation; version 2.0
-# or (at your option) any later version. You can redistribute it and/or
-# modify it under the terms of either of these two licenses.
-#
-# Unless required by applicable law or agreed to in writing, software
-# distributed under the License is distributed on an "AS IS" BASIS,
-# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-# See the License for the specific language governing permissions and
-# limitations under the License.
-#
-# You should have received a copy of the licenses; if not, see
-# <http://www.gnu.org/licenses/> for a copy of the GNU General Public License
-# and <http://www.apache.org/licenses/LICENSE-2.0> for a copy of the Apache
-# License, Version 2.0.
-#
-
-"""Repository clone handling."""
-
-import os
-import shutil
-from typing import TYPE_CHECKING, Callable, Tuple
-
-from dulwich.objects import (
-    Tag,
-)
-from dulwich.refs import (
-    LOCAL_BRANCH_PREFIX,
-    LOCAL_TAG_PREFIX,
-)
-
-if TYPE_CHECKING:
-    from dulwich.repo import Repo
-
-
-def do_clone(
-    source_path,
-    target_path,
-    clone_refs: Callable[["Repo", bytes], Tuple[bytes, bytes]] = None,
-    mkdir=True,
-    bare=False,
-    origin=b"origin",
-    checkout=None,
-    errstream=None,
-    branch=None,
-):
-    """Clone a repository.
-
-    Args:
-      source_path: Source repository path
-      target_path: Target repository path
-      clone_refs: Callback to handle setting up cloned remote refs in
-        the target repo
-      mkdir: Create the target directory
-      bare: Whether to create a bare repository
-      checkout: Whether or not to check-out HEAD after cloning
-      origin: Base name for refs in target repository
-        cloned from this repository
-      branch: Optional branch or tag to be used as HEAD in the new repository
-        instead of the source repository's HEAD.
-    Returns: Created repository as `Repo`
-    """
-    from dulwich.repo import Repo
-
-    if not clone_refs:
-        raise ValueError("clone_refs callback is required")
-
-    if mkdir:
-        os.mkdir(target_path)
-
-    try:
-        target = None
-        if not bare:
-            target = Repo.init(target_path)
-            if checkout is None:
-                checkout = True
-        else:
-            if checkout:
-                raise ValueError("checkout and bare are incompatible")
-            target = Repo.init_bare(target_path)
-
-        target_config = target.get_config()
-        target_config.set((b"remote", origin), b"url", source_path)
-        target_config.set(
-            (b"remote", origin),
-            b"fetch",
-            b"+refs/heads/*:refs/remotes/" + origin + b"/*",
-        )
-        target_config.write_to_path()
-
-        ref_message = b"clone: from " + source_path
-        origin_head, origin_sha = clone_refs(target, ref_message)
-        if origin_sha and not origin_head:
-            # set detached HEAD
-            target.refs[b"HEAD"] = origin_sha
-
-        _set_origin_head(target, origin, origin_head)
-        head_ref = _set_default_branch(
-            target, origin, origin_head, branch, ref_message
-        )
-
-        # Update target head
-        if head_ref:
-            head = _set_head(target, head_ref, ref_message)
-        else:
-            head = None
-
-        if checkout and head is not None:
-            if errstream:
-                errstream.write(b"Checking out " + head + b"\n")
-            target.reset_index()
-    except BaseException:
-        if target is not None:
-            target.close()
-        if mkdir:
-            shutil.rmtree(target_path)
-        raise
-
-    return target
-
-
-def _set_origin_head(r, origin, origin_head):
-    # set refs/remotes/origin/HEAD
-    origin_base = b"refs/remotes/" + origin + b"/"
-    if origin_head and origin_head.startswith(LOCAL_BRANCH_PREFIX):
-        origin_ref = origin_base + b"HEAD"
-        target_ref = origin_base + origin_head[len(LOCAL_BRANCH_PREFIX) :]
-        if target_ref in r.refs:
-            r.refs.set_symbolic_ref(origin_ref, target_ref)
-
-
-def _set_default_branch(r, origin, origin_head, branch, ref_message):
-    origin_base = b"refs/remotes/" + origin + b"/"
-    if branch:
-        origin_ref = origin_base + branch
-        if origin_ref in r.refs:
-            local_ref = LOCAL_BRANCH_PREFIX + branch
-            r.refs.add_if_new(
-                local_ref, r.refs[origin_ref], ref_message
-            )
-            head_ref = local_ref
-        elif LOCAL_TAG_PREFIX + branch in r.refs:
-            head_ref = LOCAL_TAG_PREFIX + branch
-        else:
-            raise ValueError(
-                "%s is not a valid branch or tag" % os.fsencode(branch)
-            )
-    elif origin_head:
-        head_ref = origin_head
-        if origin_head.startswith(LOCAL_BRANCH_PREFIX):
-            origin_ref = origin_base + origin_head[len(LOCAL_BRANCH_PREFIX) :]
-        else:
-            origin_ref = origin_head
-        try:
-            r.refs.add_if_new(
-                head_ref, r.refs[origin_ref], ref_message
-            )
-        except KeyError:
-            pass
-    return head_ref
-
-
-def _set_head(r, head_ref, ref_message):
-    if head_ref.startswith(LOCAL_TAG_PREFIX):
-        # detach HEAD at specified tag
-        head = r.refs[head_ref]
-        if isinstance(head, Tag):
-            _cls, obj = head.object
-            head = obj.get_object(obj).id
-        del r.refs[b"HEAD"]
-        r.refs.set_if_equals(
-            b"HEAD", None, head, message=ref_message
-        )
-    else:
-        # set HEAD to specific branch
-        try:
-            head = r.refs[head_ref]
-            r.refs.set_symbolic_ref(b"HEAD", head_ref)
-            r.refs.set_if_equals(
-                b"HEAD", None, head, message=ref_message
-            )
-        except KeyError:
-            head = None
-    return head

dulwich/porcelain.py

@@ -86,9 +86,6 @@ from dulwich.archive import (
 from dulwich.client import (
     get_transport_and_path,
 )
-from dulwich.clone import (
-    do_clone,
-)
 from dulwich.config import (
     StackedConfig,
 )
@@ -444,36 +441,15 @@ def clone(
 
     mkdir = not os.path.exists(target)
 
-    if not isinstance(source, bytes):
-        source = source.encode(DEFAULT_ENCODING)
-
-    def clone_refs(target_repo, ref_message):
-        fetch_result = fetch(
-            target_repo,
-            origin,
-            errstream=errstream,
-            message=ref_message,
-            depth=depth,
-            **kwargs
+    with open_repo_closing(source) as r:
+        return r.clone(
+            target,
+            mkdir=mkdir,
+            bare=bare,
+            origin=origin,
+            checkout=checkout,
+            branch=branch,
         )
-        head_ref = fetch_result.symrefs.get(b"HEAD", None)
-        try:
-            head_sha = target_repo[fetch_result.refs[b"HEAD"]].id
-        except KeyError:
-            head_sha = None
-        return head_ref, head_sha
-
-    return do_clone(
-        source,
-        target,
-        clone_refs=clone_refs,
-        mkdir=mkdir,
-        bare=bare,
-        origin=origin,
-        checkout=checkout,
-        errstream=errstream,
-        branch=branch,
-    )
 
 
 def add(repo=".", paths=None):

dulwich/refs.py

@@ -32,6 +32,7 @@ from dulwich.objects import (
     git_line,
     valid_hexsha,
     ZERO_SHA,
+    Tag,
 )
 from dulwich.file import (
     GitFile,
@@ -1203,3 +1204,68 @@ def strip_peeled_refs(refs):
         for (ref, sha) in refs.items()
         if not ref.endswith(ANNOTATED_TAG_SUFFIX)
     }
+
+
+def _set_origin_head(refs, origin, origin_head):
+    # set refs/remotes/origin/HEAD
+    origin_base = b"refs/remotes/" + origin + b"/"
+    if origin_head and origin_head.startswith(LOCAL_BRANCH_PREFIX):
+        origin_ref = origin_base + b"HEAD"
+        target_ref = origin_base + origin_head[len(LOCAL_BRANCH_PREFIX) :]
+        if target_ref in refs:
+            refs.set_symbolic_ref(origin_ref, target_ref)
+
+
+def _set_default_branch(refs, origin, origin_head, branch, ref_message):
+    origin_base = b"refs/remotes/" + origin + b"/"
+    if branch:
+        origin_ref = origin_base + branch
+        if origin_ref in refs:
+            local_ref = LOCAL_BRANCH_PREFIX + branch
+            refs.add_if_new(
+                local_ref, refs[origin_ref], ref_message
+            )
+            head_ref = local_ref
+        elif LOCAL_TAG_PREFIX + branch in refs:
+            head_ref = LOCAL_TAG_PREFIX + branch
+        else:
+            raise ValueError(
+                "%s is not a valid branch or tag" % os.fsencode(branch)
+            )
+    elif origin_head:
+        head_ref = origin_head
+        if origin_head.startswith(LOCAL_BRANCH_PREFIX):
+            origin_ref = origin_base + origin_head[len(LOCAL_BRANCH_PREFIX) :]
+        else:
+            origin_ref = origin_head
+        try:
+            refs.add_if_new(
+                head_ref, refs[origin_ref], ref_message
+            )
+        except KeyError:
+            pass
+    return head_ref
+
+
+def _set_head(refs, head_ref, ref_message):
+    if head_ref.startswith(LOCAL_TAG_PREFIX):
+        # detach HEAD at specified tag
+        head = refs[head_ref]
+        if isinstance(head, Tag):
+            _cls, obj = head.object
+            head = obj.get_object(obj).id
+        del refs[b"HEAD"]
+        refs.set_if_equals(
+            b"HEAD", None, head, message=ref_message
+        )
+    else:
+        # set HEAD to specific branch
+        try:
+            head = refs[head_ref]
+            refs.set_symbolic_ref(b"HEAD", head_ref)
+            refs.set_if_equals(
+                b"HEAD", None, head, message=ref_message
+            )
+        except KeyError:
+            head = None
+    return head

dulwich/repo.py

@@ -42,9 +42,6 @@ if TYPE_CHECKING:
     from dulwich.config import StackedConfig, ConfigFile
     from dulwich.index import Index
 
-from dulwich.clone import (
-    do_clone,
-)
 from dulwich.errors import (
     NoIndexPresent,
     NotBlobError,
@@ -101,6 +98,9 @@ from dulwich.refs import (  # noqa: F401
     read_packed_refs_with_peeled,
     write_packed_refs,
     SYMREF,
+    _set_default_branch,
+    _set_head,
+    _set_origin_head,
 )
 
 
@@ -1404,35 +1404,71 @@ class Repo(BaseRepo):
         Returns: Created repository as `Repo`
         """
 
-        def clone_refs(target_repo, ref_message):
-            self.fetch(target_repo)
-            target_repo.refs.import_refs(
+        encoded_path = self.path
+        if not isinstance(encoded_path, bytes):
+            encoded_path = os.fsencode(encoded_path)
+
+        if mkdir:
+            os.mkdir(target_path)
+
+        try:
+            target = None
+            if not bare:
+                target = Repo.init(target_path)
+                if checkout is None:
+                    checkout = True
+            else:
+                if checkout:
+                    raise ValueError("checkout and bare are incompatible")
+                target = Repo.init_bare(target_path)
+
+            target_config = target.get_config()
+            target_config.set((b"remote", origin), b"url", encoded_path)
+            target_config.set(
+                (b"remote", origin),
+                b"fetch",
+                b"+refs/heads/*:refs/remotes/" + origin + b"/*",
+            )
+            target_config.write_to_path()
+
+            ref_message = b"clone: from " + encoded_path
+            self.fetch(target)
+            target.refs.import_refs(
                 b"refs/remotes/" + origin,
                 self.refs.as_dict(b"refs/heads"),
                 message=ref_message,
             )
-            target_repo.refs.import_refs(
+            target.refs.import_refs(
                 b"refs/tags", self.refs.as_dict(b"refs/tags"), message=ref_message
             )
 
-            head_chain, sha = self.refs.follow(b"HEAD")
-            head_chain = head_chain[-1] if head_chain else None
-            return head_chain, sha
+            head_chain, origin_sha = self.refs.follow(b"HEAD")
+            origin_head = head_chain[-1] if head_chain else None
+            if origin_sha and not origin_head:
+                # set detached HEAD
+                target.refs[b"HEAD"] = origin_sha
 
-        encoded_path = self.path
-        if not isinstance(encoded_path, bytes):
-            encoded_path = os.fsencode(encoded_path)
+            _set_origin_head(target.refs, origin, origin_head)
+            head_ref = _set_default_branch(
+                target.refs, origin, origin_head, branch, ref_message
+            )
 
-        return do_clone(
-            encoded_path,
-            target_path,
-            clone_refs=clone_refs,
-            mkdir=mkdir,
-            bare=bare,
-            origin=origin,
-            checkout=checkout,
-            branch=branch,
-        )
+            # Update target head
+            if head_ref:
+                head = _set_head(target.refs, head_ref, ref_message)
+            else:
+                head = None
+
+            if checkout and head is not None:
+                target.reset_index()
+        except BaseException:
+            if target is not None:
+                target.close()
+            if mkdir:
+                import shutil
+                shutil.rmtree(target_path)
+            raise
+        return target
 
     def reset_index(self, tree=None):
         """Reset the index back to a specific tree.