ref(dependencies): Adding ability to install nested dependencies (#77)

* ref(dependencies): Adding ability to install nested dependencies

* fixing dependency version error

* fixing tests

* Improving test coverage

* Fixing naming

devservices/constants.py

@@ -7,9 +7,10 @@
 CONFIG_FILE_NAME = "config.yml"
 DOCKER_USER_PLUGIN_DIR = os.path.expanduser("~/.docker/cli-plugins/")
 
+DEVSERVICES_CACHE_DIR = os.path.expanduser("~/.cache/sentry-devservices")
 DEVSERVICES_LOCAL_DIR = os.path.expanduser("~/.local/share/sentry-devservices")
-DEVSERVICES_LOCAL_DEPENDENCIES_DIR = os.path.join(DEVSERVICES_LOCAL_DIR, "dependencies")
-DEVSERVICES_LOCAL_DEPENDENCIES_DIR_KEY = "DEVSERVICES_LOCAL_DEPENDENCIES_DIR"
+DEVSERVICES_DEPENDENCIES_CACHE_DIR = os.path.join(DEVSERVICES_CACHE_DIR, "dependencies")
+DEVSERVICES_DEPENDENCIES_CACHE_DIR_KEY = "DEVSERVICES_DEPENDENCIES_CACHE_DIR"
 
 DEPENDENCY_CONFIG_VERSION = "v1"
 DEPENDENCY_GIT_PARTIAL_CLONE_CONFIG_OPTIONS = {

devservices/exceptions.py

@@ -68,6 +68,18 @@ def __init__(self, repo_name: str, repo_link: str, branch: str):
         self.branch = branch
 
 
+class InvalidDependencyConfigError(DependencyError):
+    """Raised when a dependency's config is invalid."""
+
+    pass
+
+
+class DependencyNotInstalledError(DependencyError):
+    """Raised when a dependency is not installed correctly."""
+
+    pass
+
+
 class GitConfigError(Exception):
     """Base class for git config related errors."""
 

devservices/utils/dependencies.py

@@ -3,20 +3,28 @@
 import os
 import shutil
 import subprocess
+import tempfile
 from concurrent.futures import as_completed
 from concurrent.futures import ThreadPoolExecutor
 from typing import TextIO
 from typing import TypeGuard
 
 from devservices.configs.service_config import Dependency
+from devservices.configs.service_config import load_service_config_from_file
 from devservices.configs.service_config import RemoteConfig
 from devservices.constants import CONFIG_FILE_NAME
 from devservices.constants import DEPENDENCY_CONFIG_VERSION
 from devservices.constants import DEPENDENCY_GIT_PARTIAL_CLONE_CONFIG_OPTIONS
+from devservices.constants import DEVSERVICES_DEPENDENCIES_CACHE_DIR
 from devservices.constants import DEVSERVICES_DIR_NAME
-from devservices.constants import DEVSERVICES_LOCAL_DEPENDENCIES_DIR
+from devservices.exceptions import ConfigNotFoundError
+from devservices.exceptions import ConfigParseError
+from devservices.exceptions import ConfigValidationError
 from devservices.exceptions import DependencyError
+from devservices.exceptions import DependencyNotInstalledError
 from devservices.exceptions import FailedToSetGitConfigError
+from devservices.exceptions import InvalidDependencyConfigError
+from devservices.utils.file_lock import lock
 
 
 class SparseCheckoutManager:
@@ -90,27 +98,29 @@ def _set_config(self, key: str, value: str) -> None:
             raise FailedToSetGitConfigError from e
 
 
+def verify_local_dependency(remote_config: RemoteConfig) -> bool:
+    local_dependency_path = os.path.join(
+        DEVSERVICES_DEPENDENCIES_CACHE_DIR,
+        DEPENDENCY_CONFIG_VERSION,
+        remote_config.repo_name,
+        DEVSERVICES_DIR_NAME,
+        CONFIG_FILE_NAME,
+    )
+    return os.path.exists(local_dependency_path)
+
+
 def verify_local_dependencies(dependencies: list[Dependency]) -> bool:
     remote_configs = _get_remote_configs(dependencies)
 
     # Short circuit to avoid doing unnecessary work
     if len(remote_configs) == 0:
         return True
 
-    if not os.path.exists(DEVSERVICES_LOCAL_DEPENDENCIES_DIR):
+    if not os.path.exists(DEVSERVICES_DEPENDENCIES_CACHE_DIR):
         return False
 
     return all(
-        os.path.exists(
-            os.path.join(
-                DEVSERVICES_LOCAL_DEPENDENCIES_DIR,
-                DEPENDENCY_CONFIG_VERSION,
-                remote_config.repo_name,
-                DEVSERVICES_DIR_NAME,
-                CONFIG_FILE_NAME,
-            )
-        )
-        for remote_config in remote_configs
+        verify_local_dependency(remote_config) for remote_config in remote_configs
     )
 
 
@@ -121,7 +131,7 @@ def install_dependencies(dependencies: list[Dependency]) -> None:
     if len(remote_configs) == 0:
         return
 
-    os.makedirs(DEVSERVICES_LOCAL_DEPENDENCIES_DIR, exist_ok=True)
+    os.makedirs(DEVSERVICES_DEPENDENCIES_CACHE_DIR, exist_ok=True)
 
     with ThreadPoolExecutor() as executor:
         futures = [
@@ -131,25 +141,67 @@ def install_dependencies(dependencies: list[Dependency]) -> None:
         for future in as_completed(futures):
             try:
                 future.result()
-            except Exception as e:
-                print(e)
+            except DependencyError as e:
+                raise e
 
 
 def install_dependency(dependency: RemoteConfig) -> None:
     dependency_repo_dir = os.path.join(
-        DEVSERVICES_LOCAL_DEPENDENCIES_DIR,
+        DEVSERVICES_DEPENDENCIES_CACHE_DIR,
         DEPENDENCY_CONFIG_VERSION,
         dependency.repo_name,
     )
 
-    if (
-        os.path.exists(dependency_repo_dir)
-        and _is_valid_repo(dependency_repo_dir)
-        and _has_valid_config_file(dependency_repo_dir)
-    ):
-        _update_dependency(dependency, dependency_repo_dir)
-    else:
-        _checkout_dependency(dependency, dependency_repo_dir)
+    os.makedirs(DEVSERVICES_DEPENDENCIES_CACHE_DIR, exist_ok=True)
+
+    # Ensure that only one process is installing a specific dependency at a time
+    # TODO: This is a very broad lock, we should consider making it more granular to enable faster installs
+    # TODO: Ideally we would simply not re-install something that is being currently being installed or was recently installed
+    lock_path = os.path.join(
+        DEVSERVICES_DEPENDENCIES_CACHE_DIR, f"{dependency.repo_name}.lock"
+    )
+    with lock(lock_path):
+        if (
+            os.path.exists(dependency_repo_dir)
+            and _is_valid_repo(dependency_repo_dir)
+            and _has_valid_config_file(dependency_repo_dir)
+        ):
+            _update_dependency(dependency, dependency_repo_dir)
+        else:
+            _checkout_dependency(dependency, dependency_repo_dir)
+
+        if not verify_local_dependency(dependency):
+            # TODO: what should we do if the local dependency isn't installed correctly?
+            raise DependencyNotInstalledError(
+                repo_name=dependency.repo_name,
+                repo_link=dependency.repo_link,
+                branch=dependency.branch,
+            )
+
+        # Once the dependency is installed, install its dependencies (recursively)
+        try:
+            installed_config = load_service_config_from_file(dependency_repo_dir)
+        except (ConfigNotFoundError, ConfigParseError, ConfigValidationError) as e:
+            # TODO: This happens when the dependency has an invalid config
+            raise InvalidDependencyConfigError(
+                repo_name=dependency.repo_name,
+                repo_link=dependency.repo_link,
+                branch=dependency.branch,
+            ) from e
+
+    nested_dependencies = list(installed_config.dependencies.values())
+    nested_remote_configs = _get_remote_configs(nested_dependencies)
+
+    with ThreadPoolExecutor() as nested_executor:
+        nested_futures = [
+            nested_executor.submit(install_dependency, nested_remote_config)
+            for nested_remote_config in nested_remote_configs
+        ]
+        for nested_future in as_completed(nested_futures):
+            try:
+                nested_future.result()
+            except DependencyError as e:
+                raise e
 
 
 def _update_dependency(
@@ -174,12 +226,12 @@ def _update_dependency(
             ["git", "fetch", "origin", dependency.branch, "--filter=blob:none"],
             cwd=dependency_repo_dir,
         )
-    except subprocess.CalledProcessError:
+    except subprocess.CalledProcessError as e:
         raise DependencyError(
             repo_name=dependency.repo_name,
             repo_link=dependency.repo_link,
             branch=dependency.branch,
-        )
+        ) from e
 
     # Check if the local repo is up-to-date
     local_commit = subprocess.check_output(
@@ -206,41 +258,58 @@ def _checkout_dependency(
     dependency: RemoteConfig,
     dependency_repo_dir: str,
 ) -> None:
-    if os.path.exists(dependency_repo_dir):
-        shutil.rmtree(dependency_repo_dir)
-    os.makedirs(dependency_repo_dir, exist_ok=False)
-
-    _run_command(
-        [
-            "git",
-            "clone",
-            "--filter=blob:none",
-            "--no-checkout",
-            dependency.repo_link,
-            dependency_repo_dir,
-        ],
-        cwd=dependency_repo_dir,
-    )
+    with tempfile.TemporaryDirectory() as temp_dir:
+        try:
+            _run_command(
+                [
+                    "git",
+                    "clone",
+                    "--filter=blob:none",
+                    "--no-checkout",
+                    dependency.repo_link,
+                    temp_dir,
+                ],
+                cwd=temp_dir,
+            )
+        except subprocess.CalledProcessError as e:
+            raise DependencyError(
+                repo_name=dependency.repo_name,
+                repo_link=dependency.repo_link,
+                branch=dependency.branch,
+            ) from e
+
+        # Setup config for partial clone and sparse checkout
+        git_config_manager = GitConfigManager(
+            temp_dir,
+            DEPENDENCY_GIT_PARTIAL_CLONE_CONFIG_OPTIONS,
+            f"{DEVSERVICES_DIR_NAME}/",
+        )
+        try:
+            git_config_manager.ensure_config()
+        except FailedToSetGitConfigError as e:
+            raise DependencyError(
+                repo_name=dependency.repo_name,
+                repo_link=dependency.repo_link,
+                branch=dependency.branch,
+            ) from e
 
-    # Setup config for partial clone and sparse checkout
-    git_config_manager = GitConfigManager(
-        dependency_repo_dir,
-        DEPENDENCY_GIT_PARTIAL_CLONE_CONFIG_OPTIONS,
-        f"{DEVSERVICES_DIR_NAME}/",
-    )
-    try:
-        git_config_manager.ensure_config()
-    except FailedToSetGitConfigError as e:
-        raise DependencyError(
-            repo_name=dependency.repo_name,
-            repo_link=dependency.repo_link,
-            branch=dependency.branch,
-        ) from e
+        _run_command(
+            ["git", "checkout", dependency.branch],
+            cwd=temp_dir,
+        )
 
-    _run_command(
-        ["git", "checkout", dependency.branch],
-        cwd=dependency_repo_dir,
-    )
+        # Clean up the existing directory if it exists
+        if os.path.exists(dependency_repo_dir):
+            shutil.rmtree(dependency_repo_dir)
+        # Copy the cloned repo to the dependency cache directory
+        try:
+            shutil.copytree(temp_dir, dst=dependency_repo_dir)
+        except FileExistsError as e:
+            raise DependencyError(
+                repo_name=dependency.repo_name,
+                repo_link=dependency.repo_link,
+                branch=dependency.branch,
+            ) from e
 
 
 def _is_valid_repo(path: str) -> bool:

devservices/utils/docker_compose.py

@@ -9,9 +9,10 @@
 from packaging import version
 
 from devservices.constants import CONFIG_FILE_NAME
+from devservices.constants import DEPENDENCY_CONFIG_VERSION
+from devservices.constants import DEVSERVICES_DEPENDENCIES_CACHE_DIR
+from devservices.constants import DEVSERVICES_DEPENDENCIES_CACHE_DIR_KEY
 from devservices.constants import DEVSERVICES_DIR_NAME
-from devservices.constants import DEVSERVICES_LOCAL_DEPENDENCIES_DIR
-from devservices.constants import DEVSERVICES_LOCAL_DEPENDENCIES_DIR_KEY
 from devservices.constants import DOCKER_COMPOSE_DOWNLOAD_URL
 from devservices.constants import DOCKER_USER_PLUGIN_DIR
 from devservices.constants import MINIMUM_DOCKER_COMPOSE_VERSION
@@ -158,15 +159,16 @@ def run_docker_compose_command(
             #       since the dependencies may have changed since the service was started.
             install_dependencies(dependencies)
     relative_local_dependency_directory = os.path.relpath(
-        DEVSERVICES_LOCAL_DEPENDENCIES_DIR, service.repo_path
+        os.path.join(DEVSERVICES_DEPENDENCIES_CACHE_DIR, DEPENDENCY_CONFIG_VERSION),
+        service.repo_path,
     )
     service_config_file_path = os.path.join(
         service.repo_path, DEVSERVICES_DIR_NAME, CONFIG_FILE_NAME
     )
     # Set the environment variable for the local dependencies directory to be used by docker compose
     current_env = os.environ.copy()
     current_env[
-        DEVSERVICES_LOCAL_DEPENDENCIES_DIR_KEY
+        DEVSERVICES_DEPENDENCIES_CACHE_DIR_KEY
     ] = relative_local_dependency_directory
     cmd = [
         "docker",

devservices/utils/file_lock.py

@@ -0,0 +1,24 @@
+from __future__ import annotations
+
+import contextlib
+import fcntl
+from collections.abc import Generator
+
+
+@contextlib.contextmanager
+def lock(path: str) -> Generator[None, None, None]:
+    with open(path, mode="a+") as f:
+        with _locked(f.fileno()):
+            yield
+
+
+@contextlib.contextmanager
+def _locked(fileno: int) -> Generator[None, None, None]:
+    try:
+        fcntl.flock(fileno, fcntl.LOCK_EX | fcntl.LOCK_NB)
+    except OSError:
+        fcntl.flock(fileno, fcntl.LOCK_EX)
+    try:
+        yield
+    finally:
+        fcntl.flock(fileno, fcntl.LOCK_UN)

testing/resources/basic_repo/devservices/config.yml

@@ -1,6 +1,6 @@
 x-sentry-service-config:
-    version: 0.1,
-    service_name: basic,
-    dependencies:
+    version: 0.1
+    service_name: basic
+    dependencies: {}
     modes:
-      default:
+      default: []

testing/resources/blank_repo/README.md

@@ -0,0 +1 @@
+# This is a blank repository for testing purposes

testing/resources/invalid_repo/README.md

@@ -0,0 +1 @@
+# This is an invalid repository for testing purposes

testing/resources/invalid_repo/devservices/config.yml

@@ -0,0 +1,7 @@
+x-sentry-service-config:
+    version: 0.1
+    service_name: invalid
+    dependencies: {}
+    modes:
+      default: []
+    invalid