feat: only clone init templates when required

samcli/commands/init/init_templates.py

@@ -8,6 +8,7 @@
 import re
 from enum import Enum
 from pathlib import Path
+from subprocess import STDOUT, CalledProcessError, check_output
 from typing import Dict, Optional
 
 import requests
@@ -87,13 +88,16 @@ def clone_templates_repo(self):
             os_name = system().lower()
             cloned_folder_name = APP_TEMPLATES_REPO_NAME_WINDOWS if os_name == "windows" else APP_TEMPLATES_REPO_NAME
 
+            to_upsert = self._check_upsert_templates(shared_dir, cloned_folder_name)
+
             try:
-                self._git_repo.clone(
-                    clone_dir=shared_dir,
-                    clone_name=cloned_folder_name,
-                    replace_existing=True,
-                    commit=APP_TEMPLATES_REPO_COMMIT,
-                )
+                if to_upsert:
+                    self._git_repo.clone(
+                        clone_dir=shared_dir,
+                        clone_name=cloned_folder_name,
+                        replace_existing=True,
+                        commit=APP_TEMPLATES_REPO_COMMIT,
+                    )
             except CloneRepoUnstableStateException as ex:
                 raise AppTemplateUpdateException(str(ex)) from ex
             except (OSError, CloneRepoException):
@@ -102,6 +106,37 @@ def clone_templates_repo(self):
                 if expected_previous_clone_local_path.exists():
                     self._git_repo.local_path = expected_previous_clone_local_path
 
+    def _check_upsert_templates(self, shared_dir: Path, cloned_folder_name: str) -> bool:
+        """
+        Check if the app templates repository should be cloned, or if cloning should be skipped.
+
+        Parameters
+        ----------
+        shared_dir: Path
+            Folder containing the aws-sam-cli shared data
+
+        cloned_folder_name: str
+            Name of the directory into which the app templates will be copied
+
+        Returns
+        -------
+        bool
+            True if the cache should be updated, False otherwise
+
+        """
+        cache_dir = shared_dir / cloned_folder_name
+        git_executable = self._git_repo.get_git_executable()
+        command = [git_executable, "rev-parse", "--verify", "HEAD"]
+        try:
+            existing_hash = check_output(command, cwd=cache_dir, stderr=STDOUT).decode("utf-8").strip()
+        except CalledProcessError:
+            LOG.debug("Unable to check existing cache hash")
+            return True
+        except FileNotFoundError:
+            LOG.debug("Cache directory does not yet exist, creating one.")
+            return True
+        return not existing_hash == APP_TEMPLATES_REPO_COMMIT
+
     def _init_options_from_manifest(self, package_type, runtime, base_image, dependency_manager):
         manifest_path = self.get_manifest_path()
         with open(str(manifest_path)) as fp:

samcli/lib/utils/git_repo.py

@@ -70,6 +70,9 @@ def _ensure_clone_directory_exists(clone_dir: Path) -> None:
             LOG.warning("WARN: Unable to create clone directory.", exc_info=ex)
             raise
 
+    def get_git_executable(self):
+        return self._git_executable()
+
     @staticmethod
     def _git_executable() -> str:
         if platform.system().lower() == "windows":
@@ -172,7 +175,7 @@ def _persist_local_repo(temp_path: str, dest_dir: Path, dest_name: str, replace_
 
             LOG.debug("Copying from %s to %s", temp_path, dest_path)
             # Todo consider not removing the .git files/directories
-            shutil.copytree(temp_path, dest_path, ignore=shutil.ignore_patterns("*.git"))
+            shutil.copytree(temp_path, dest_path)
             return Path(dest_path)
         except (OSError, shutil.Error) as ex:
             # UNSTABLE STATE

tests/integration/init/test_init_command.py

@@ -4,6 +4,7 @@
 
 from click.testing import CliRunner
 
+from samcli.cli.global_config import GlobalConfig
 from samcli.commands.init import cli as init_cmd
 from unittest import TestCase
 
@@ -838,6 +839,62 @@ def test_interactive_init_default_runtime(self):
             self.assertTrue(Path(expected_output_folder, "hello_world", "app.py").is_file())
 
 
+class TestSubsequentInitCaching(TestCase):
+    def test_subsequent_init_skips_cloning(self):
+        with tempfile.TemporaryDirectory() as temp:
+            project_directory = Path(temp, "sam-app")
+            cache_dir = GlobalConfig().config_dir / "aws-sam-cli-app-templates"
+
+            # Run the first time, get cache last modification time
+            self._run_init(temp)
+            first_modification_time = self._last_modification_time(cache_dir)
+            self.assertTrue(project_directory.is_dir())
+            shutil.rmtree(project_directory)
+
+            # Run init again, get the second modification time
+            self._run_init(temp)
+            second_modification_time = self._last_modification_time(cache_dir)
+            self.assertTrue(project_directory.is_dir())
+
+            self.assertEqual(first_modification_time, second_modification_time)
+
+    def _run_init(self, cwd):
+        process = Popen(
+            [
+                get_sam_command(),
+                "init",
+                "--runtime",
+                "nodejs14.x",
+                "--dependency-manager",
+                "npm",
+                "--architecture",
+                "arm64",
+                "--app-template",
+                "hello-world",
+                "--name",
+                "sam-app",
+                "--no-interactive",
+                "-o",
+                cwd,
+            ],
+            stdout=PIPE,
+            stderr=PIPE,
+        )
+        try:
+            stdout_data, stderr_data = process.communicate(timeout=TIMEOUT)
+            stderr = stderr_data.decode("utf-8")
+        except TimeoutExpired:
+            process.kill()
+            raise
+
+        self.assertEqual(process.returncode, 0)
+        self.assertNotIn(COMMIT_ERROR, stderr)
+
+    @staticmethod
+    def _last_modification_time(file):
+        return os.path.getmtime(file)
+
+
 class TestInitProducesSamconfigFile(TestCase):
     def test_zip_template_config(self):
         with tempfile.TemporaryDirectory() as temp:

tests/unit/commands/init/test_templates.py

@@ -1,8 +1,11 @@
+from subprocess import STDOUT
+
 import json
+from parameterized import parameterized
 from pathlib import Path
 from re import search
 from unittest import TestCase
-from unittest.mock import mock_open, patch, PropertyMock
+from unittest.mock import mock_open, patch, PropertyMock, Mock
 
 from samcli.commands.init.init_templates import InitTemplates
 from samcli.lib.utils.packagetype import IMAGE, ZIP
@@ -15,6 +18,7 @@ class TestTemplates(TestCase):
     @patch("shutil.copytree")
     def test_location_from_app_template_zip(self, subprocess_mock, git_exec_mock, cd_mock, copy_mock):
         it = InitTemplates()
+        it._check_upsert_templates = Mock()
 
         manifest = {
             "ruby2.7": [
@@ -42,6 +46,7 @@ def test_location_from_app_template_zip(self, subprocess_mock, git_exec_mock, cd
     @patch("shutil.copytree")
     def test_location_from_app_template_image(self, subprocess_mock, git_exec_mock, cd_mock, copy_mock):
         it = InitTemplates()
+        it._check_upsert_templates = Mock()
 
         manifest = {
             "ruby2.7-image": [
@@ -64,3 +69,17 @@ def test_location_from_app_template_image(self, subprocess_mock, git_exec_mock,
                     IMAGE, None, "ruby2.7-image", "bundler", "hello-world-lambda-image"
                 )
                 self.assertTrue(search("mock-ruby-image-template", location))
+
+    @parameterized.expand([("hash_a", "hash_a", False), ("hash_a", "hash_b", True)])
+    @patch("samcli.lib.utils.git_repo.GitRepo.get_git_executable")
+    @patch("samcli.commands.init.init_templates.check_output")
+    def test_check_upsert_templates(self, first_hash, second_hash, expected_value, check_output_mock, git_exec_mock):
+        it = InitTemplates()
+        git_exec_mock.return_value = "git"
+        check_output_mock.return_value = second_hash.encode("utf-8")
+        with patch("samcli.commands.init.init_templates.APP_TEMPLATES_REPO_COMMIT", first_hash):
+            return_value = it._check_upsert_templates(Path("shared_dir"), Path("cloned_folder_dir"))
+        check_output_mock.assert_called_once_with(
+            ["git", "rev-parse", "--verify", "HEAD"], cwd=Path("shared_dir/cloned_folder_dir"), stderr=STDOUT
+        )
+        self.assertEqual(return_value, expected_value)

tests/unit/lib/utils/test_git_repo.py

@@ -62,8 +62,7 @@ def test_clone_happy_case(self, platform_mock, popen_mock, check_output_mock, sh
             [call(["git", "clone", self.repo.url, REPO_NAME], cwd=ANY, stderr=subprocess.STDOUT)]
         )
         shutil_mock.rmtree.assert_not_called()
-        shutil_mock.copytree.assert_called_with(ANY, EXPECTED_DEFAULT_CLONE_PATH, ignore=ANY)
-        shutil_mock.ignore_patterns.assert_called_with("*.git")
+        shutil_mock.copytree.assert_called_with(ANY, EXPECTED_DEFAULT_CLONE_PATH)
 
     @patch("samcli.lib.utils.git_repo.Path.exists")
     @patch("samcli.lib.utils.git_repo.shutil")
@@ -76,8 +75,7 @@ def test_clone_create_new_local_repo(
         path_exist_mock.return_value = False
         self.repo.clone(clone_dir=self.local_clone_dir, clone_name=REPO_NAME)
         shutil_mock.rmtree.assert_not_called()
-        shutil_mock.copytree.assert_called_with(ANY, EXPECTED_DEFAULT_CLONE_PATH, ignore=ANY)
-        shutil_mock.ignore_patterns.assert_called_with("*.git")
+        shutil_mock.copytree.assert_called_with(ANY, EXPECTED_DEFAULT_CLONE_PATH)
 
     @patch("samcli.lib.utils.git_repo.Path.exists")
     @patch("samcli.lib.utils.git_repo.shutil")
@@ -91,8 +89,7 @@ def test_clone_replace_current_local_repo_if_replace_existing_flag_is_set(
         self.repo.clone(clone_dir=self.local_clone_dir, clone_name=REPO_NAME, replace_existing=True)
         self.local_clone_dir.mkdir.assert_called_once_with(mode=0o700, parents=True, exist_ok=True)
         shutil_mock.rmtree.assert_called_with(EXPECTED_DEFAULT_CLONE_PATH, onerror=rmtree_callback)
-        shutil_mock.copytree.assert_called_with(ANY, EXPECTED_DEFAULT_CLONE_PATH, ignore=ANY)
-        shutil_mock.ignore_patterns.assert_called_with("*.git")
+        shutil_mock.copytree.assert_called_with(ANY, EXPECTED_DEFAULT_CLONE_PATH)
 
     @patch("samcli.lib.utils.git_repo.Path.exists")
     @patch("samcli.lib.utils.git_repo.check_output")
@@ -186,8 +183,7 @@ def test_clone_when_failed_to_move_cloned_repo_from_temp_to_final_destination(
         except Exception:
             pass
         shutil_mock.rmtree.assert_called_once_with(EXPECTED_DEFAULT_CLONE_PATH, onerror=rmtree_callback)
-        shutil_mock.copytree.assert_called_once_with(ANY, EXPECTED_DEFAULT_CLONE_PATH, ignore=ANY)
-        shutil_mock.ignore_patterns.assert_called_once_with("*.git")
+        shutil_mock.copytree.assert_called_once_with(ANY, EXPECTED_DEFAULT_CLONE_PATH)
 
     @patch("samcli.lib.utils.git_repo.LOG")
     @patch("samcli.lib.utils.git_repo.check_output")
@@ -221,8 +217,7 @@ def test_clone_with_commit(self, platform_mock, popen_mock, check_output_mock, s
             [call(["git", "checkout", COMMIT], cwd=ANY, stderr=subprocess.STDOUT)],
         )
         shutil_mock.rmtree.assert_not_called()
-        shutil_mock.copytree.assert_called_with(ANY, EXPECTED_DEFAULT_CLONE_PATH, ignore=ANY)
-        shutil_mock.ignore_patterns.assert_called_with("*.git")
+        shutil_mock.copytree.assert_called_with(ANY, EXPECTED_DEFAULT_CLONE_PATH)
 
     @patch("samcli.lib.utils.git_repo.Path.exists")
     @patch("samcli.lib.utils.git_repo.shutil")
@@ -247,8 +242,7 @@ def test_clone_with_longpaths_configured_in_windows(
             ]
         )
         shutil_mock.rmtree.assert_not_called()
-        shutil_mock.copytree.assert_called_with(ANY, EXPECTED_DEFAULT_CLONE_PATH, ignore=ANY)
-        shutil_mock.ignore_patterns.assert_called_with("*.git")
+        shutil_mock.copytree.assert_called_with(ANY, EXPECTED_DEFAULT_CLONE_PATH)
 
     @patch("samcli.lib.utils.git_repo.Path")
     @patch("samcli.lib.utils.git_repo.platform.system")