Do not clone the same repository multiple times, re-use already cloned repository

Clone the directory to the compose tmp directory
Update the test_scm in order to create real Compose object. Mock objects are not allowed
to create/delete files for preventing multiple clones

JIRA: RHELCMP-5250

Signed-off-by: Ozan Unsal <ounsal@redhat.com>

pungi/wrappers/scm.py

@@ -19,7 +19,9 @@ from __future__ import absolute_import
 import os
 import shutil
 import glob
+import threading
 import six
+import tempfile
 from six.moves import shlex_quote
 from six.moves.urllib.request import urlretrieve
 from fnmatch import fnmatch
@@ -29,6 +31,8 @@ from kobo.shortcuts import run, force_list
 from pungi.util import explode_rpm_package, makedirs, copy_all, temp_dir, retry
 from .kojiwrapper import KojiWrapper
 
+scm_lock = threading.Lock()
+
 
 class ScmBase(kobo.log.LoggingBase):
     def __init__(self, logger=None, command=None, compose=None):
@@ -372,10 +376,33 @@ def get_file_from_scm(scm_dict, target_path, compose=None):
     scm = _get_wrapper(scm_type, logger=logger, command=command, compose=compose)
 
     files_copied = []
-    for i in force_list(scm_file):
+    branch = scm_branch if scm_branch else "master"
-        with temp_dir(prefix="scm_checkout_") as tmp_dir:
+    delete_after_flag = False
-            scm.export_file(scm_repo, i, scm_branch=scm_branch, target_dir=tmp_dir)
+    with scm_lock:
-            files_copied += copy_all(tmp_dir, target_path)
+        if compose and scm_repo:
+            repo = scm_repo.rsplit("/")[-1]
+            tmp_dir = compose.paths.work.tmp_dir()
+            tmp_dir = os.path.join(tmp_dir, repo, branch)
+        else:
+            tmp_dir = tempfile.mkdtemp(prefix="scm_checkout_")
+            delete_after_flag = True
+
+        if not os.path.isdir(tmp_dir):
+            makedirs(tmp_dir)
+
+        for i in force_list(scm_file):
+            # Check the files which are included with subdirectories
+            check_file = os.path.join(tmp_dir, i[i.rfind("/") + 1 :])
+            if (
+                type(scm_dict) is not dict
+                or command is not None
+                or not compose
+                or not os.path.isfile(check_file)
+            ):
+                scm.export_file(scm_repo, i, scm_branch=scm_branch, target_dir=tmp_dir)
+        files_copied += copy_all(tmp_dir, target_path)
+        if delete_after_flag:
+            shutil.rmtree(tmp_dir)
     return files_copied
 
 
@@ -459,14 +486,34 @@ def get_dir_from_scm(scm_dict, target_path, compose=None):
 
     logger = compose._logger if compose else None
     scm = _get_wrapper(scm_type, logger=logger, command=command, compose=compose)
+    branch = scm_branch if scm_branch else "master"
+    delete_after_flag = False
 
-    with temp_dir(prefix="scm_checkout_") as tmp_dir:
+    with scm_lock:
-        scm.export_dir(scm_repo, scm_dir, scm_branch=scm_branch, target_dir=tmp_dir)
+        if compose and scm_repo:
+            repo = scm_repo.rsplit("/")[-1]
+            tmp_dir = compose.paths.work.tmp_dir()
+            tmp_dir = os.path.join(tmp_dir, repo, branch)
+        else:
+            tmp_dir = tempfile.mkdtemp(prefix="scm_checkout_")
+            delete_after_flag = True
+
+        if not os.path.isdir(tmp_dir):
+            makedirs(tmp_dir)
+            scm.export_dir(scm_repo, scm_dir, scm_branch=scm_branch, target_dir=tmp_dir)
+        elif (
+            type(scm_dict) is not dict
+            or command is not None
+            or not scm_repo
+            or not compose
+        ):
+            scm.export_dir(scm_repo, scm_dir, scm_branch=scm_branch, target_dir=tmp_dir)
         files_copied = copy_all(tmp_dir, target_path)
 
-    # Make sure the directory has permissions set to 755. This is a workaround
+        # Make sure the directory has permissions set to 755. This is a workaround
-    # for a problem where sometimes the directory will be 700 and it will not
+        # for a problem where sometimes the directory will be 700 and it will not
-    # be accessible via httpd.
+        # be accessible via httpd.
-    os.chmod(target_path, 0o755)
+        os.chmod(target_path, 0o755)
-
+        if delete_after_flag:
+            shutil.rmtree(tmp_dir)
     return files_copied

tests/test_scm.py

@@ -16,14 +16,17 @@ import six
 from pungi.wrappers import scm
 from tests.helpers import touch
 from kobo.shortcuts import run
+from pungi.compose import Compose
 
 
 class SCMBaseTest(unittest.TestCase):
     def setUp(self):
         self.destdir = tempfile.mkdtemp()
+        self.tmp_dir = tempfile.mkdtemp()
 
     def tearDown(self):
         shutil.rmtree(self.destdir)
+        shutil.rmtree(self.tmp_dir)
 
     def assertStructure(self, returned, expected):
         # Check we returned the correct files
@@ -143,7 +146,8 @@ class GitSCMTestCase(SCMBaseTest):
 
     @mock.patch("pungi.wrappers.scm.run")
     def test_get_file_function(self, run):
-        compose = mock.Mock(conf={})
+        with mock.patch("pungi.compose.ComposeInfo"):
+            compose = Compose({}, self.tmp_dir)
 
         def process(cmd, workdir=None, **kwargs):
             touch(os.path.join(workdir, "some_file.txt"))
@@ -338,6 +342,8 @@ class GitSCMTestCaseReal(SCMBaseTest):
         shutil.rmtree(self.gitRepositoryLocation)
 
     def test_get_file_function(self):
+        with mock.patch("pungi.compose.ComposeInfo"):
+            compose = Compose({}, self.tmp_dir)
         sourceFileLocation = random.choice(list(self.files.keys()))
         sourceFilename = os.path.basename(sourceFileLocation)
         destinationFileLocation = os.path.join(self.destdir, "other_file.txt")
@@ -348,7 +354,7 @@ class GitSCMTestCaseReal(SCMBaseTest):
                 "file": sourceFilename,
             },
             os.path.join(self.destdir, destinationFileLocation),
-            compose=self.compose,
+            compose=compose,
         )
         self.assertEqual(destinationFileActualLocation, destinationFileLocation)
         self.assertTrue(os.path.isfile(destinationFileActualLocation))
@@ -361,6 +367,8 @@ class GitSCMTestCaseReal(SCMBaseTest):
         self.assertEqual(sourceFileContent, destinationFileContent)
 
     def test_get_file_function_with_overwrite(self):
+        with mock.patch("pungi.compose.ComposeInfo"):
+            compose = Compose({}, self.tmp_dir)
         sourceFileLocation = random.choice(list(self.files.keys()))
         sourceFilename = os.path.basename(sourceFileLocation)
         destinationFileLocation = os.path.join(self.destdir, "other_file.txt")
@@ -375,7 +383,7 @@ class GitSCMTestCaseReal(SCMBaseTest):
                 "file": sourceFilename,
             },
             os.path.join(self.destdir, destinationFileLocation),
-            compose=self.compose,
+            compose=compose,
             overwrite=True,
         )
         self.assertEqual(destinationFileActualLocation, destinationFileLocation)
@@ -603,7 +611,8 @@ class KojiSCMTestCase(SCMBaseTest):
 
     @mock.patch("pungi.wrappers.scm.KojiWrapper")
     def test_doesnt_get_dirs(self, KW, dl):
-        compose = mock.Mock(conf={"koji_profile": "koji"})
+        with mock.patch("pungi.compose.ComposeInfo"):
+            compose = Compose({"koji_profile": "koji"}, self.tmp_dir)
 
         with self.assertRaises(RuntimeError) as ctx:
             scm.get_dir_from_scm(
@@ -628,7 +637,8 @@ class KojiSCMTestCase(SCMBaseTest):
 
     @mock.patch("pungi.wrappers.scm.KojiWrapper")
     def test_get_from_build(self, KW, dl):
-        compose = mock.Mock(conf={"koji_profile": "koji"})
+        with mock.patch("pungi.compose.ComposeInfo"):
+            compose = Compose({"koji_profile": "koji"}, self.tmp_dir)
 
         def download(src, dst):
             touch(dst)
@@ -659,7 +669,8 @@ class KojiSCMTestCase(SCMBaseTest):
 
     @mock.patch("pungi.wrappers.scm.KojiWrapper")
     def test_get_from_latest_build(self, KW, dl):
-        compose = mock.Mock(conf={"koji_profile": "koji"})
+        with mock.patch("pungi.compose.ComposeInfo"):
+            compose = Compose({"koji_profile": "koji"}, self.tmp_dir)
 
         def download(src, dst):
             touch(dst)