[MRG] Fix submodule check out (#809)

[MRG] Fix submodule check out

docs/source/changelog.rst

@@ -19,7 +19,7 @@ API changes
 
 Bug fixes
 ---------
-
+- Fix for submodule check out in :pr:`809` by :user:`davidbrochart`.
 
 
 Version 0.10.0

repo2docker/contentproviders/git.py

@@ -21,15 +21,23 @@ class Git(ContentProvider):
 
         # make a, possibly shallow, clone of the remote repository
         try:
-            cmd = ["git", "clone", "--recursive"]
+            cmd = ["git", "clone"]
             if ref is None:
+                # check out of HEAD is performed after the clone is complete
                 cmd.extend(["--depth", "1"])
+            else:
+                # don't check out HEAD, the given ref will be checked out later
+                # this prevents HEAD's submodules to be cloned if ref doesn't have them
+                cmd.extend(["--no-checkout"])
             cmd.extend([repo, output_dir])
             for line in execute_cmd(cmd, capture=yield_output):
                 yield line
 
         except subprocess.CalledProcessError as e:
-            msg = "Failed to clone repository from {repo}.".format(repo=repo)
+            msg = "Failed to clone repository from {repo}".format(repo=repo)
+            if ref is not None:
+                msg += " (ref {ref})".format(ref=ref)
+            msg += "."
             raise ContentProviderException(msg) from e
 
         # check out the specific ref given by the user
@@ -40,6 +48,11 @@ class Git(ContentProvider):
                     "Failed to check out ref %s", ref, extra=dict(phase="failed")
                 )
                 raise ValueError("Failed to check out ref {}".format(ref))
+            # check out ref as it has not been done yet
+            for line in execute_cmd(
+                ["git", "checkout", hash], cwd=output_dir, capture=yield_output
+            ):
+                yield line
             # If the hash is resolved above, we should be able to reset to it
             for line in execute_cmd(
                 ["git", "reset", "--hard", hash], cwd=output_dir, capture=yield_output

tests/conftest.py

@@ -145,6 +145,10 @@ def repo_with_submodule():
         submod_sha1_b = _get_sha1(git_b_dir)
         _add_content_to_git(git_b_dir)
 
+        # create a new branch in the parent without any submodule
+        subprocess.check_call(
+            ["git", "checkout", "-b", "branch-without-submod"], cwd=git_a_dir
+        )
         # create a new branch in the parent to add the submodule
         subprocess.check_call(
             ["git", "checkout", "-b", "branch-with-submod"], cwd=git_a_dir

tests/unit/contentproviders/test_git.py

@@ -23,6 +23,18 @@ def test_submodule_clone(repo_with_submodule):
     """Test git clone containing a git submodule."""
     upstream, expected_sha1_upstream, expected_sha1_submod = repo_with_submodule
 
+    # check that checking out a branch where there are no submodule
+    # indeed doesn't get any submodule, even though they are in master
+    with TemporaryDirectory() as clone_dir2:
+        submod_dir = os.path.join(clone_dir2, "submod")  # set by fixture
+        spec = {"repo": upstream, "ref": "branch-without-submod"}
+        git_content = Git()
+        for _ in git_content.fetch(spec, clone_dir2):
+            pass
+
+        assert os.path.exists(os.path.join(clone_dir2, "test"))
+        assert not os.path.exists(os.path.join(submod_dir, "test"))
+
     with TemporaryDirectory() as clone_dir:
         submod_dir = os.path.join(clone_dir, "submod")  # set by fixture
         spec = {"repo": upstream}