gitclone: Add support for cloning submodules by default

Also, add functionality checkout the submodule(s) repos to the HASH
parent repo points to.

Also, some indentation cleanups.

README.rst

@@ -121,7 +121,7 @@ try to redownload resource with wrong md5sum.
  slapos.recipe.build:gitclone
 ==============================
 
-Checkout a git repository.
+Checkout a git repository and its submodules by default.
 Supports slapos.libnetworkcache if present, and if boolean 'use-cache' option
 is true.
 
@@ -236,6 +236,7 @@ Then let's run the buildout::
   Running uninstall recipe.
   Installing git-clone.
   Cloning into '/sample-buildout/parts/git-clone'...
+  HEAD is now at 2566127 ...
 
 Let's take a look at the buildout parts directory now::
 
@@ -249,15 +250,12 @@ And let's see that current revision is "2566127"::
   >>> print(subprocess.check_output(['git', 'rev-parse', '--short', 'HEAD'], universal_newlines=True))
   2566127
 
-When updating, it will do a "git fetch; git reset revision"::
+When updating, it shouldn't do anything as revision is mentioned::
 
   >>> cd(sample_buildout)
   >>> print(system(buildout))
   Updating git-clone.
   ...
-  HEAD is now at 2566127 ... 
-  ...
-  <BLANKLINE>
 
 Empty revision/branch
 ~~~~~~~~~~~~~~~~~~~~~
@@ -327,7 +325,7 @@ and branch parameter is ignored::
 Setup a "develop" repository
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
-If you need to setup a repository that will be manually alterated over time for
+If you need to setup a repository that will be manually altered over time for
 development purposes, you need to make sure buildout will NOT alter it and NOT
 erase your local modifications by specifying the "develop" flag::
 
@@ -366,7 +364,6 @@ repository::
   >>> cd(sample_buildout)
   >>> print(system(buildout))
   Updating git-clone.
-  Fetching origin
   ...
   <BLANKLINE>
 
@@ -386,17 +383,7 @@ Then, when update occurs, nothing is done::
   >>> cd(sample_buildout)
   >>> print(system(buildout))
   Updating git-clone.
-  Fetching origin
-  Fetching broken
-  Unable to update:
-  Traceback (most recent call last):
-  ...
-  ...CalledProcessError: Command '['git', 'fetch', '--all']' returned non-zero exit status 1
-  <BLANKLINE>
   ...
-  fatal: unable to access 'http://git.erp5.org/repos/nowhere/': The requested URL returned error: 500
-  error: Could not fetch broken
-  <BLANKLINE> 
 
   >>> cd(sample_buildout, 'parts', 'git-clone')
   >>> print(system('cat local_change'))
@@ -450,6 +437,21 @@ boolean option::
   repository = https://example.net/example.git/
   ignore-ssl-certificate = true
 
+Ignore cloning submodules
+~~~~~~~~~~~~~~~~~~~~~~~~~
+
+By default, cloning the repository will clone its submodules also. You can force
+git to ignore cloinig submodules by defining `ignore-cloning-submodules` boolean
+option to 'true'::
+
+  [buildout]
+  parts = git-clone
+
+  [git-clone]
+  recipe = slapos.recipe.build:gitclone
+  repository = https://lab.nexedi.com/tiwariayush/test_erp5
+  ignore-cloning-submodules = true
+
 Other options
 ~~~~~~~~~~~~~
 

slapos/recipe/build/tests.py

 import doctest
 from zope.testing import renormalizing
 
+import logging
 import os
 import re
 import shutil
@@ -8,6 +9,9 @@ import stat
 import tempfile
 import unittest
 import zc.buildout.testing
+from zc.buildout.testing import buildoutTearDown
+from functools import wraps
+from subprocess import check_call, check_output, CalledProcessError
 from slapos.recipe.gitclone import GIT_CLONE_ERROR_MESSAGE, \
     GIT_CLONE_CACHE_ERROR_MESSAGE
 
@@ -26,12 +30,54 @@ def setUp(test):
   zc.buildout.testing.buildoutSetUp(test)
   zc.buildout.testing.install_develop('slapos.recipe.build', test)
 
+def with_buildout(wrapped):
+  def wrapper(self):
+    self.globs = {}
+    setUp(self)
+    try:
+      wrapped(self, **self.globs)
+    finally:
+      buildoutTearDown(self)
+  return wraps(wrapped)(wrapper)
+
 class GitCloneNonInformativeTests(unittest.TestCase):
 
   def setUp(self):
     self.dir = os.path.realpath(tempfile.mkdtemp())
     self.parts_directory_path = os.path.join(self.dir, 'test_parts')
 
+  def setUpSubmoduleRepository(self):
+    """
+    This function sets up repositories and config for parent repo and a
+    submodule repo and create small commits as well as links the 2 repos.
+    """
+    # Create parent and submodule directory
+    self.project_dir = os.path.join(self.dir, 'main_repo')
+    self.submodule_dir = os.path.join(self.dir ,'submodule_repo')
+    os.mkdir(self.project_dir)
+    os.mkdir(self.submodule_dir)
+
+    # Add files in submodule repo and initialize git in it
+    check_call(['git', 'init'], cwd=self.submodule_dir)
+    self.setUpGitConfig(self.submodule_dir)
+    self.touch(self.submodule_dir, 'file1.py')
+    self.gitAdd(self.submodule_dir)
+    self.gitCommit(self.submodule_dir, msg='Add file1 in submodule repo')
+
+    # Add files and folder in parent repo and initialize git in it
+    check_call(['git', 'init'], cwd=self.project_dir)
+    self.setUpGitConfig(self.project_dir)
+    submodule_dir_main_repo = os.path.join(self.project_dir, 'dir1')
+    os.mkdir(submodule_dir_main_repo)
+    self.touch(self.project_dir, 'file1.py')
+    self.gitAdd(self.project_dir)
+    self.gitCommit(self.project_dir, msg='Add file and folder in main repo')
+
+    # Add submodule to main repo and commit
+    check_call(['git', 'submodule', 'add', self.submodule_dir],
+                cwd=submodule_dir_main_repo)
+    self.gitCommit(self.project_dir, msg='Add submodule repo')
+
   def tearDown(self):
     shutil.rmtree(self.dir)
     for var in list(os.environ):
@@ -46,6 +92,54 @@ class GitCloneNonInformativeTests(unittest.TestCase):
     os.chmod(path, mode)
     return path
 
+  def setUpGitConfig(self, proj_dir):
+    """
+    Setup user and email for given git repository
+    """
+    check_call(['git', 'config', 'user.email', 'test@example.com'], cwd=proj_dir)
+    check_call(['git', 'config', 'user.name', 'Test'], cwd=proj_dir)
+
+  def gitAdd(self, proj_dir):
+    """runs a 'git add .' in the provided git repo
+    :param proj_dir:    path to a git repo
+    """
+    try:
+      check_call(['git', 'add', '.'], cwd=proj_dir)
+    except CalledProcessError as e:
+      logging.error("'git add .' failed with '%s'" % e.returncode)
+
+  def gitCommit(self, proj_dir, msg):
+    """runs a 'git commit -m msg' in the provided git repo
+    :param proj_dir:  path to a git repo
+    """
+    try:
+      check_call(['git', 'commit', '-m', msg], cwd=proj_dir)
+    except CalledProcessError as e:
+      logging.error("'git commit' failed with '%s'" % e.returncode)
+
+  def getRepositoryHeadCommit(self, proj_dir):
+    """
+    Returns the sha of HEAD of a git repo
+    """
+    try :
+      output = check_output(['git', 'rev-parse', 'HEAD'],
+                            cwd=proj_dir).decode("utf-8")
+      return output.rstrip()
+    except CalledProcessError:
+      logging.error("failed to call 'git rev-parse'")
+      return None
+
+  def touch(self, *parts):
+    os.close(os.open(os.path.join(*parts), os.O_CREAT, 0o666))
+
+  def checkLocalChanges(self, parent_local_change_path,
+                        submodule_local_change_path):
+    # Check if the file are created at the expected position and check contents
+    self.assertTrue(os.path.exists(parent_local_change_path))
+    self.assertTrue(os.path.exists(submodule_local_change_path))
+    self.assertEqual(check_output(['cat', parent_local_change_path]), 'foo')
+    self.assertEqual(check_output(['cat', submodule_local_change_path]), 'bar')
+
   def makeGitCloneRecipe(self, options):
     from slapos.recipe.gitclone import Recipe
     bo = {
@@ -90,6 +184,148 @@ class GitCloneNonInformativeTests(unittest.TestCase):
     self.assertTrue(os.path.exists(git_repository_path))
     self.assertFalse(os.path.exists(bad_file_path), "pyc file not removed")
 
+  @with_buildout
+  def test_clone_and_update_submodule(self, buildout, write, sample_buildout,
+                                      **kw):
+    """
+    Remote:
+      Repositories status:  Parent repo(M1) and Submodule repo (S1)
+      Parent repo (M1) ---references---> Submodule(S1)
+    Local:
+      Buildout should install(using branch) at M1+S1
+
+    Remote:
+      Repositories status:  Parent repo(M2) and Submodule repo (S2)
+      Parent repo (M2) ---references---> Submodule(S1)
+    Local:
+      Buildout should uninstall/install(using revision M2) at M2+S1
+
+    Remote:
+      Repositories status:  Parent repo(M3) and Submodule repo (S2)
+      Parent repo (M3) ---references---> Submodule(S2)
+    Local:
+      Buildout should uninstall/install(using revision M3) at M3+S2
+    """
+    self.setUpSubmoduleRepository()
+
+    # Clone repositories in status M1 and S1 (M1---->S1)
+    write(sample_buildout, 'buildout.cfg',
+"""
+[buildout]
+parts = git-clone
+
+[git-clone]
+recipe = slapos.recipe.build:gitclone
+repository = %s
+""" % self.project_dir)
+    check_call([buildout])
+
+    main_repo_path = os.path.join(sample_buildout, 'parts', 'git-clone')
+    self.assertTrue(os.path.exists(main_repo_path))
+    submodule_repo_path = os.path.join(main_repo_path, 'dir1',
+                                       'submodule_repo')
+    # Check if the submodule is not empty
+    self.assertTrue(os.listdir(submodule_repo_path))
+
+    # Get the head commit of the submodule repo
+    head_commit_submodule_after_clone = self.getRepositoryHeadCommit(
+                                              submodule_repo_path)
+
+    # Add untracked files as markers to check that the part
+    # was updated rather than removed+reinstalled.
+    write(main_repo_path, 'local_change_main', 'foo')
+    write(submodule_repo_path, 'local_change_submodule', 'bar')
+    parent_local_change_path = os.path.join(main_repo_path,
+                                            'local_change_main')
+    submodule_local_change_path = os.path.join(submodule_repo_path,
+                                               'local_change_submodule')
+    self.checkLocalChanges(parent_local_change_path, submodule_local_change_path)
+
+    # Trigger `update` method call for gitclone recipe
+    check_call([buildout])
+
+    # The local changes should be still there after update
+    self.checkLocalChanges(parent_local_change_path, submodule_local_change_path)
+
+    # On REMOTE, update submodule repository and parent repo with new commit,
+    # but do not update the pointer to submodule on parent repo
+    self.touch(self.project_dir, 'file2.py')
+    self.gitAdd(self.project_dir)
+    self.gitCommit(self.project_dir, msg='Add file2 in main repo')
+    self.touch(self.submodule_dir, 'file2.py')
+    self.gitAdd(self.submodule_dir)
+    self.gitCommit(self.submodule_dir, msg='Add file2 in submodule repo')
+
+    # Clone repositories in status M2 and S2 (M2---->S1)
+    # Update the recipe with new revision for parent and trigger uninstall/
+    # install
+    write(sample_buildout, 'buildout.cfg',
+"""
+[buildout]
+parts = git-clone
+
+[git-clone]
+recipe = slapos.recipe.build:gitclone
+repository = %s
+revision = %s
+""" % (self.project_dir, str(self.getRepositoryHeadCommit(self.project_dir))))
+    check_call([buildout])
+
+    self.assertTrue(os.path.exists(main_repo_path))
+    # Check if the submodule is not empty
+    self.assertTrue(os.listdir(submodule_repo_path))
+
+    # Since calling buildout should've reinstalled, we expect the local changes
+    # to be gone
+    self.assertFalse(os.path.exists(parent_local_change_path))
+    self.assertFalse(os.path.exists(submodule_local_change_path))
+
+    # Get the head commit of the submodule repo
+    head_commit_submodule_after_first_revision = self.getRepositoryHeadCommit(
+                                                      submodule_repo_path)
+
+    # On REMOTE, add new commit to submodule and then update the submodule
+    # pointer on parent repo and commit it
+    submodule_dir_main_repo = os.path.join(self.project_dir, 'dir1',
+                                           'submodule_repo')
+    check_call(['git', 'checkout', 'master'], cwd=submodule_dir_main_repo)
+    check_call(['git', 'pull', '--ff'], cwd=submodule_dir_main_repo)
+    self.gitAdd(self.project_dir)
+    self.gitCommit(self.project_dir, msg='Update submodule version')
+
+    # Clone repositories in status M3 and S2 (M3---->S2)
+    # Update the recipe with new revision which points to submodule new revision
+    # and run uninstall/install again
+    write(sample_buildout, 'buildout.cfg',
+"""
+[buildout]
+parts = git-clone
+
+[git-clone]
+recipe = slapos.recipe.build:gitclone
+repository = %s
+revision = %s
+""" % (self.project_dir, str(self.getRepositoryHeadCommit(self.project_dir))))
+    check_call([buildout])
+
+    self.assertTrue(os.path.exists(main_repo_path))
+    # Check if the submodule is not empty
+    self.assertTrue(os.listdir(submodule_repo_path))
+    # Get the head commit of the submodule repo
+    head_commit_submodule_after_second_revision = self.getRepositoryHeadCommit(
+                                                      submodule_repo_path)
+
+    # Check the HEAD of the submodule
+    submodule_head_commit = self.getRepositoryHeadCommit(self.submodule_dir)
+    self.assertEqual(head_commit_submodule_after_clone,
+                     head_commit_submodule_after_first_revision)
+
+    self.assertNotEqual(head_commit_submodule_after_first_revision,
+                        head_commit_submodule_after_second_revision)
+
+    self.assertEqual(head_commit_submodule_after_second_revision,
+                     submodule_head_commit)
+
   def test_ignore_ssl_certificate(self, ignore_ssl_certificate=True):
     import slapos.recipe.gitclone
 
@@ -117,7 +353,6 @@ class GitCloneNonInformativeTests(unittest.TestCase):
 
     # Check git clone parameters
     _ = self.assertIn if ignore_ssl_certificate else self.assertNotIn
-    _("--config", check_call_parameter_list[0][0])
     _("http.sslVerify=false", check_call_parameter_list[0][0])
 
     # Restore original check_call method
@@ -126,6 +361,74 @@ class GitCloneNonInformativeTests(unittest.TestCase):
   def test_ignore_ssl_certificate_false(self):
     self.test_ignore_ssl_certificate(ignore_ssl_certificate=False)
 
+  def test_clone_submodules_by_default(self, ignore_cloning_submodules=False):
+    self.setUpSubmoduleRepository()
+    recipe = self.makeGitCloneRecipe(
+      {'repository': self.project_dir,
+        'ignore-cloning-submodules': str(ignore_cloning_submodules).lower()}
+    )
+    recipe.install()
+    main_repo_path = os.path.join(self.parts_directory_path, "test")
+    self.assertTrue(os.path.exists(main_repo_path))
+    submodule_repo_path = os.path.join(main_repo_path, 'dir1',
+                                       'submodule_repo')
+    # Check if the folder exists
+    self.assertTrue(os.path.exists(main_repo_path))
+    # Check is there is anything in submodule repository path
+    self.assertNotEqual(bool(ignore_cloning_submodules),
+                        bool(os.listdir(submodule_repo_path)))
+
+  def test_ignore_cloning_submodules(self):
+    self.test_clone_submodules_by_default(ignore_cloning_submodules=True)
+
+  def test_fetch_submodules_with_main_repo(self):
+    """
+    Test to check the fetch of submodules while fetching main repo. Updating
+    should udpate the main repo as well as submodule repo if the reference of
+    submodule in main repo has been udpated.
+    """
+    self.setUpSubmoduleRepository()
+    recipe = self.makeGitCloneRecipe(
+      {'repository': self.project_dir}
+    )
+    recipe.install()
+    main_repo_path = os.path.join(self.parts_directory_path, "test")
+    self.assertTrue(os.path.exists(main_repo_path))
+    submodule_repo_path = os.path.join(main_repo_path, 'dir1',
+                                       'submodule_repo')
+    # Check if the submodule is not empty
+    self.assertTrue(os.listdir(submodule_repo_path))
+
+    # Get the head commit of the submodule repo
+    head_commit_submodule_before_fetch = check_output(['git', 'rev-parse',
+                                        'HEAD'],
+                                        cwd=submodule_repo_path).strip()
+
+    # Now, let's update the main repo as well as submodule repository.
+    # Updating the repo will fetch the updated branch but doesn't checkout
+    # to the updated master branch.
+    recipe.update()
+    head_commit_after_udpate_before_checkout = check_output(['git', 'submodule',
+                                          'status', submodule_repo_path],
+                                          cwd=main_repo_path).split()[0]
+
+    # Checkout the submodule repository and get the head commit
+    check_call(['git', 'checkout', 'master'], cwd=submodule_repo_path)
+    head_commit_after_update_after_checkout = check_output(['git', 'rev-parse',
+                                        'HEAD'],
+                                        cwd=submodule_repo_path).strip()
+
+    # Make another git clone of submodule and check the HEAD of the repo.
+    submodule_recipe = self.makeGitCloneRecipe(
+      {'repository': self.submodule_dir}
+    )
+    submodule_recipe.install()
+    # Check the HEAD of the submodule
+    head_commit = check_output(['git', 'rev-parse', 'HEAD'],
+                                cwd=main_repo_path).strip()
+
+    self.assertEqual(head_commit_after_update_after_checkout, head_commit)
+
 def test_suite():
   suite = unittest.TestSuite((
       doctest.DocFileSuite(

slapos/recipe/gitclone.py

@@ -34,7 +34,7 @@ import time
 import traceback
 
 from zc.buildout import UserError
-from subprocess import call, check_call, CalledProcessError
+from subprocess import call, check_call, check_output, CalledProcessError
 import subprocess
 
 try:
@@ -145,7 +145,8 @@ class Recipe(object):
       self.git_command = 'git'
     self.sparse = options.get('sparse-checkout', '').strip()
     # Set boolean values
-    for key in ('develop', 'shared', 'use-cache', 'ignore-ssl-certificate'):
+    for key in ('develop', 'shared', 'use-cache', 'ignore-ssl-certificate',
+                'ignore-cloning-submodules'):
       setattr(self, key.replace('-', '_'), options.get(key, '').lower() in TRUE_VALUES)
     if self.shared:
       self.use_cache = False
@@ -171,7 +172,6 @@ class Recipe(object):
       command.append(revision)
     check_call(command, cwd=self.location)
 
-
   def install(self):
     """
     Do a git clone.
@@ -188,6 +188,7 @@ class Recipe(object):
         shutil.rmtree(self.location)
       else:
         # If develop is set, assume that this is a valid working copy
+        self._update()
         return [self.location]
 
     if getattr(self, 'branch_overrided', None):
@@ -213,6 +214,13 @@ class Recipe(object):
 
     if config and self.use_cache:
       raise NotImplementedError
+
+    if not self.ignore_cloning_submodules:
+      # `--recurse-submodules` to the git clone command will automatically
+      # initialize and update each submodule in the repository.
+      config.append('submodule.recurse=true')
+      git_clone_command.append('--recurse-submodules')
+
     for config in config:
       git_clone_command += '--config', config
 
@@ -230,6 +238,11 @@ class Recipe(object):
       if self.use_cache:
         upload_network_cached(os.path.join(self.location, '.git'),
                               self.repository, self.revision, self.networkcache)
+      if not self.ignore_cloning_submodules:
+        # Update submodule repository to the commit which is being pointed to
+        # in main repo
+        check_call([self.git_command, 'submodule', 'update', '--recursive'],
+                    cwd=self.location)
     except CalledProcessError:
       print("Unable to download from git repository."
             " Trying from network cache...")
@@ -258,31 +271,42 @@ class Recipe(object):
               raise
 
   def update(self):
+    if not self.revision:
+      self._update()
+
+  def _update(self):
     """
     Do a git fetch.
     If user doesn't develop, reset to remote revision (or branch if revision is
     not specified).
+    With support for submodules, we also update the submodule repository if
+    there has been update to the pointer of the submodules in the parent repo.
+    This however puts the updated submodules in a DETACHED state.
     """
     try:
       # first cleanup pyc files
       self.deletePycFiles(self.location)
 
-      # then update,
-      # but, to save time, only if we don't have the revision already
-      revision_already_fetched = \
-            self.revision and   \
-            call([self.git_command, 'rev-parse', '--verify', self.revision],
-                    cwd=self.location) == 0
-      if not revision_already_fetched:
-        check_call([self.git_command, 'fetch', '--all'], cwd=self.location)
-
       # If develop parameter is set, don't reset/update.
       # Otherwise, reset --hard
       if not self.develop:
         if self.revision:
           self.gitReset(self.revision)
         else:
+          check_call([self.git_command, 'fetch', '--all'], cwd=self.location)
           self.gitReset('@{upstream}')
+
+        if not self.ignore_cloning_submodules:
+          # Update the submodule to the commit which parent repo points to if
+          # there has been revision is present for the parent repo.
+          # According to man-page of submodule, update also clones missing
+          # submodules and updating the working tree of the submodules. This is
+          # why we do update here only after we are ok with revision of parent
+          # repo being checked out to the desired one.
+          # NOTE: This will put the submodule repo in a `Detached` state.
+          check_call([self.git_command, 'submodule', 'update', '--recursive'],
+                      cwd=self.location)
+
     except:
       if not self.develop:
         raise