Skip to content

Commit 010c7ed

Browse files
committed
CI: Add checks for new inter-module dependencies
1 parent 8fd81d4 commit 010c7ed

File tree

3 files changed

+212
-1
lines changed

3 files changed

+212
-1
lines changed

.github/workflows/CI_master.yml

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,13 +69,18 @@ jobs:
6969
changedPythonFiles: ${{ needs.Prepare.outputs.changedPythonFiles }}
7070
changedPythonLines: ${{ needs.Prepare.outputs.changedPythonLines }}
7171

72+
AdditionalChecks:
73+
needs: [ Prepare ]
74+
uses: ./.github/workflows/actions/additionalChecks/added_dependency.yml
75+
7276
WrapUp:
7377
needs: [
7478
Prepare,
7579
Pixi,
7680
Ubuntu,
7781
Windows,
78-
Lint
82+
Lint,
83+
AdditionalChecks
7984
]
8085
if: always()
8186
uses: ./.github/workflows/sub_wrapup.yml
Lines changed: 189 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,189 @@
1+
# SPDX-License-Identifier: LGPL-2.1-or-later
2+
3+
"""Script to parse git diff output and determine if any new C++ dependencies were added between FreeCAD modules."""
4+
5+
import os
6+
import re
7+
import sys
8+
9+
KNOWN_MODULES = [
10+
"app",
11+
"base",
12+
"gui",
13+
"addonmanager",
14+
"assembly",
15+
"bim",
16+
"cam",
17+
"cloud",
18+
"draft",
19+
"fem",
20+
"help",
21+
"idf",
22+
"import",
23+
"inspection",
24+
"jtreader",
25+
"material",
26+
"measure",
27+
"mesh",
28+
"meshpart",
29+
"openscad",
30+
"part",
31+
"partdesign",
32+
"plot",
33+
"points",
34+
"reverseengineering",
35+
"robot",
36+
"sandbox",
37+
"show",
38+
"sketcher",
39+
"spreadsheet",
40+
"start",
41+
"surface",
42+
"techdraw",
43+
"templatepymod",
44+
"test",
45+
"tux",
46+
"web",
47+
]
48+
49+
50+
def parse_diff_by_file(diff_content: str):
51+
"""
52+
Parse git diff output and return a dictionary mapping filenames to their diffs.
53+
54+
Returns:
55+
dict: {filename: diff_chunk}
56+
"""
57+
file_diffs = {}
58+
current_file = None
59+
current_chunk = []
60+
61+
lines = diff_content.split("\n")
62+
63+
for line in lines:
64+
if line.startswith("diff --git"):
65+
if current_file and current_chunk:
66+
file_diffs[current_file] = "\n".join(current_chunk)
67+
68+
match = re.search(r"diff --git a/(.*?) b/", line)
69+
if match:
70+
current_file = match.group(1)
71+
current_chunk = [line]
72+
else:
73+
current_file = None
74+
current_chunk = []
75+
elif current_file is not None:
76+
current_chunk.append(line)
77+
78+
if current_file and current_chunk:
79+
file_diffs[current_file] = "\n".join(current_chunk)
80+
81+
return file_diffs
82+
83+
84+
def check_module_compatibility(
85+
file_module: str, included_file_module: str, intermodule_dependencies: dict[str, list[str]]
86+
) -> bool:
87+
if file_module == included_file_module:
88+
return True
89+
if file_module not in KNOWN_MODULES or included_file_module not in KNOWN_MODULES:
90+
return True # We are only checking compatibility between modules *in FreeCAD*
91+
if file_module in intermodule_dependencies:
92+
return included_file_module in intermodule_dependencies[file_module]
93+
else:
94+
return False
95+
96+
97+
def load_intermodule_dependencies(cmake_file_path: str) -> dict[str, list[str]]:
98+
"""FreeCAD already has a file that defines the known dependencies between modules. The basic rule is that no NEW
99+
dependencies can be added (without extensive discussion with the core developers). This function loads that file
100+
and parses it such that we can use it to check if a new dependency was added."""
101+
dependencies = {}
102+
103+
if not os.path.exists(cmake_file_path):
104+
print(f"ERROR: {cmake_file_path} not found", file=sys.stderr)
105+
exit(1)
106+
107+
with open(cmake_file_path, "r") as f:
108+
content = f.read()
109+
110+
pattern = r"REQUIRES_MODS\(\s*(\w+)((?:\s+\w+)*)\s*\)"
111+
matches = re.finditer(pattern, content)
112+
for match in matches:
113+
dependent = match.group(1)
114+
prerequisites = match.group(2).split()
115+
module_name = dependent.replace("BUILD", "").replace("_", "").lower()
116+
prereq_names = [p.replace("BUILD", "").replace("_", "").lower() for p in prerequisites]
117+
dependencies[module_name] = prereq_names
118+
119+
print()
120+
print("Recognized intermodule dependencies")
121+
print("-----------------------------------")
122+
for module, deps in dependencies.items():
123+
print(f"{module} depends on: {', '.join(deps)}")
124+
print()
125+
126+
return dependencies
127+
128+
129+
def check_file_dependencies(
130+
file: str, diff: str, intermodule_dependencies: dict[str, list[str]]
131+
) -> bool:
132+
"""Returns true if the dependencies are OK, or false if they are not."""
133+
file_module = file.split("/")[1] # src/Gui, etc.
134+
if file_module == "Mod":
135+
file_module = file.split("/")[2] # src/Mod/Part, etc.
136+
diff_lines = diff.split("\n")
137+
failed = False
138+
for line in diff_lines:
139+
if file.endswith(".h") or file.endswith(".cpp"):
140+
include_file = (m := re.search(r'^\+\s*#include\s*[<"]([^>"]+)[>"]', line)) and m.group(
141+
1
142+
)
143+
if include_file:
144+
include_file_module = include_file.split("/")[0]
145+
if include_file_module == "Mod":
146+
include_file_module = include_file.split("/")[1]
147+
else:
148+
include_file_module = None
149+
elif file.endswith(".py") or file.endswith(".pyi"):
150+
include_file_module = (
151+
m := re.search(
152+
r"^\+\s*(?:from\s+([\w.]+)\s+)?import\s+([\w.]+(?:\s+as\s+\w+)?(?:\s*,\s*[\w.]+(?:\s+as\s+\w+)?)*)",
153+
line,
154+
)
155+
) and (m.group(1) or m.group(2))
156+
else:
157+
return True
158+
if not include_file_module:
159+
continue
160+
compatibility = check_module_compatibility(
161+
file_module.lower(), include_file_module.lower(), intermodule_dependencies
162+
)
163+
164+
if not compatibility:
165+
print(
166+
f" 👉 {file_module} added a new dependency on {include_file_module}",
167+
file=sys.stderr,
168+
)
169+
failed = True
170+
return not failed
171+
172+
173+
if __name__ == "__main__":
174+
dependencies = load_intermodule_dependencies(
175+
"cMake/FreeCAD_Helpers/CheckIntermoduleDependencies.cmake"
176+
)
177+
piped_diff = sys.stdin.read()
178+
file_diffs = parse_diff_by_file(piped_diff)
179+
failed_files = []
180+
for file, diff in file_diffs.items():
181+
print(f"Checking changed file {file} for dependency violations...")
182+
if not check_file_dependencies(file, diff, dependencies):
183+
print(f" ❌ ERROR: Dependency violation found in {file}")
184+
failed_files.append(file)
185+
else:
186+
print(f" ✅ No problems found in {file}")
187+
if failed_files:
188+
sys.exit(1)
189+
sys.exit(0)
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
# SPDX-License-Identifier: LGPL-2.1-or-later
2+
3+
name: Check for added dependencies
4+
5+
on:
6+
pull_request:
7+
branches: [main]
8+
9+
jobs:
10+
check_added_dependencies:
11+
runs-on: ubuntu-latest
12+
steps:
13+
- uses: actions/checkout@v6
14+
- name: Check for added C++ header file dependencies
15+
run: |
16+
git fetch origin ${{ github.base_ref }}:${{ github.base_ref }}
17+
git diff | python3 .github/workflows/actions/additionalChecks/added_dependency.py

0 commit comments

Comments
 (0)