Skip to content

Commit

Permalink
fix: add cycle detection for dependency's requirement
Browse files Browse the repository at this point in the history
if a cycled requirements found, raise an exception immediately or
the events will get stuck because of it.
  • Loading branch information
coolkiid authored and jianliang00 committed Oct 15, 2024
1 parent 3c04269 commit 0398abb
Show file tree
Hide file tree
Showing 3 changed files with 91 additions and 0 deletions.
5 changes: 5 additions & 0 deletions core/components/dependency_group.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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 = []
Expand Down
42 changes: 42 additions & 0 deletions core/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import subprocess
import sys
import traceback
from collections import defaultdict
from pathlib import Path
from zipfile import ZipFile

Expand Down Expand Up @@ -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)
44 changes: 44 additions & 0 deletions tests/test_sync.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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."

0 comments on commit 0398abb

Please sign in to comment.