From 0398abbf719497aa81fbea175ea9ac3669f1ace3 Mon Sep 17 00:00:00 2001
From: coolkiid <73600915+coolkiid@users.noreply.github.com>
Date: Wed, 9 Oct 2024 11:29:31 +0800
Subject: [PATCH] fix: add cycle detection for dependency's requirement

if a cycled requirements found, raise an exception immediately or
the events will get stuck because of it.
---
 core/components/dependency_group.py |  5 ++++
 core/utils.py                       | 42 +++++++++++++++++++++++++++
 tests/test_sync.py                  | 44 +++++++++++++++++++++++++++++
 3 files changed, 91 insertions(+)

diff --git a/core/components/dependency_group.py b/core/components/dependency_group.py
index 0ea6987..a43d989 100644
--- a/core/components/dependency_group.py
+++ b/core/components/dependency_group.py
@@ -13,6 +13,7 @@
 from core.exceptions import HabitatException
 from core.fetchers.local_fetcher import LocalFetcher
 from core.settings import MAX_DEPENDENCY_WAIT_TIME
+from core.utils import cycle_detection
 
 
 async def fetch_child(child, *args, events=None, **kwargs):
@@ -113,6 +114,10 @@ async def fetch_children(self, root_dir, options, existing_sources=None, existin
 
         # Filter out components whose require has been skipped recursively
         get_final_components_to_fetch(components_to_fetch)
+
+        # cycle detection
+        cycle_detection(components_to_fetch)
+
         for name, child in components_to_fetch.items():
             require = getattr(child, 'require', [])
             events = []
diff --git a/core/utils.py b/core/utils.py
index 804cec6..b3be1bf 100644
--- a/core/utils.py
+++ b/core/utils.py
@@ -26,6 +26,7 @@
 import subprocess
 import sys
 import traceback
+from collections import defaultdict
 from pathlib import Path
 from zipfile import ZipFile
 
@@ -583,3 +584,44 @@ async def is_git_user_set() -> bool:
         return False
 
     return True
+
+
+class DependencyGraph:
+    def __init__(self, deps: dict) -> None:
+        self.node_requirements = defaultdict(list)
+        for dep_name, dep in deps.items():
+            requirements = getattr(dep, 'require', [])
+            self.node_requirements[dep_name].extend(requirements)
+
+
+def visit(graph, node, grey_nodes, black_nodes):
+    # mark the current node as a grey node, indicates that its child has not been traversed.
+    grey_nodes.add(node)
+
+    for require in graph.node_requirements[node]:
+        # cycle detected if current node needs a node that has been marked grey.
+        if require in grey_nodes:
+            raise HabitatException(
+                f"found a cicular dependency, please check {node}'s requirement {require}."
+            )
+
+        # traverse the unvisited node.
+        if require not in black_nodes:
+            visit(graph, require, grey_nodes, black_nodes)
+
+    # mark the current node as a black node, indicates that we don't need to visit it anymore.
+    grey_nodes.remove(node)
+    black_nodes.add(node)
+
+    return grey_nodes, black_nodes
+
+
+def cycle_detection(deps: dict):
+    graph = DependencyGraph(deps)
+
+    grey_nodes = set()
+    black_nodes = set()
+
+    for node in graph.node_requirements:
+        if (node not in grey_nodes) and (node not in black_nodes):
+            grey_nodes, black_nodes = visit(graph, node, grey_nodes, black_nodes)
diff --git a/tests/test_sync.py b/tests/test_sync.py
index 88ea681..ad665d8 100644
--- a/tests/test_sync.py
+++ b/tests/test_sync.py
@@ -2,8 +2,10 @@
 import json
 import os.path
 import subprocess
+from unittest.mock import patch
 
 from core.components.solution import load_entries_cache_from_git, store_entries_cache_to_git
+from core.exceptions import HabitatException
 from core.main import main
 from core.utils import rmtree
 from utils import create_zip_file, generate_habitat_config_file, make_change_in_repo, run_with_custom_argv
@@ -448,3 +450,45 @@ def test_sync_git_repo_with_patches(tmp_path):
     assert os.path.exists(f'{cwd}/main/lib-with-one-more-patch/hello.py')
     with open(f'{cwd}/main/lib-with-one-more-patch/hello.py') as f:
         assert f.read() == 'print("not yet")'
+
+
+@patch('core.main.DEBUG', True)
+def test_sync_dependency_with_cycled_requirement(tmp_path):
+    os.chdir(tmp_path)
+    cwd = os.getcwd()
+    subprocess.check_call(['git', 'init', 'dep', '--initial-branch=master'])
+    subprocess.check_call(['git', 'init', 'main-repo', '--initial-branch=master'])
+    solutions = [
+        {
+            'name': '.',
+            'deps_file': 'DEPS',
+            'url': f'file://{cwd}/main-repo/.git',
+            'branch': 'master'
+        }
+    ]
+    deps = {
+        'test_a': {
+            'type': 'git',
+            'url': f'file://{cwd}/dep/.git',
+            'branch': 'master',
+            'require': ['test_b']
+        },
+        'test_b': {
+            'type': 'git',
+            'url': f'file://{cwd}/dep/.git',
+            'branch': 'master',
+            'require': ['test_a']
+        }
+    }
+    make_change_in_repo(
+        f'{cwd}/main-repo', '.habitat', generate_habitat_config_file('solutions', solutions),
+        'add .habitat', 'w'
+    )
+    make_change_in_repo(
+        f'{cwd}/main-repo', 'DEPS', generate_habitat_config_file('deps', deps), 'add DEPS', 'w'
+    )
+    os.chdir('main-repo')
+    try:
+        run_with_custom_argv(main, ['hab', 'sync', '.'])
+    except HabitatException as e:
+        assert str(e) == "found a cicular dependency, please check test_b's requirement test_a."