Skip to content

Commit 1c1e343

Browse files
committed
Merge branch 'FreeCAD:main' into addedDependencyChecker
2 parents 010c7ed + e147e5a commit 1c1e343

File tree

4 files changed

+83
-34
lines changed

4 files changed

+83
-34
lines changed

.github/ISSUE_TEMPLATE/1-FUNCTIONAL_PROBLEM_REPORT.yml

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,15 @@ body:
77
value: |
88
Thanks for taking the time to fill out this problem report! Please [search](https://github.com/FreeCAD/FreeCAD/issues) if a similar issue already exists and check out [how to report issues](https://github.com/FreeCAD/FreeCAD?tab=readme-ov-file#reporting-issues). By submitting this issue, you agree to follow our [Code of Conduct](https://github.com/FreeCAD/FreeCAD/blob/main/CODE_OF_CONDUCT.md).
99
10+
- type: textarea
11+
id: description
12+
attributes:
13+
label: Problem description
14+
description: Describe the problem and how it impacts user experience, workflow, maintainability or performance. You can attach images or log files by clicking this area to highlight it and then dragging files in. To attach a FCStd file, ZIP it first.
15+
placeholder: Describe your problem briefly.
16+
validations:
17+
required: true
18+
1019
- type: dropdown
1120
id: wb
1221
attributes:
@@ -28,15 +37,6 @@ body:
2837
- TechDraw
2938
- Other (specify in description)
3039

31-
- type: textarea
32-
id: description
33-
attributes:
34-
label: Problem description
35-
description: Describe the problem and how it impacts user experience, workflow, maintainability or performance. You can attach images or log files by clicking this area to highlight it and then dragging files in. To attach a FCStd file, ZIP it first.
36-
placeholder: Describe your problem briefly.
37-
validations:
38-
required: true
39-
4040
- type: textarea
4141
id: steps_to_reproduce
4242
attributes:

.pre-commit-config.yaml

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,3 +77,11 @@ repos:
7777
rev: 317810f3c6a0ad3572367dc86cb6e41863e16e08 # frozen: v21.1.5
7878
hooks:
7979
- id: clang-format
80+
- repo: local
81+
hooks:
82+
- id: added_dependency
83+
name: check for new FreeCAD inter-module dependencies
84+
entry: python tools/lint/added_dependency.py
85+
language: python
86+
files: \.(cpp|h|py|pyi)$
87+
pass_filenames: true

cMake/FreeCAD_Helpers/CheckInterModuleDependencies.cmake

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ macro(CheckInterModuleDependencies)
2121
REQUIRES_MODS(BUILD_BIM BUILD_PART BUILD_MESH BUILD_MESH_PART BUILD_DRAFT)
2222
REQUIRES_MODS(BUILD_DRAFT BUILD_SKETCHER BUILD_TECHDRAW)
2323
REQUIRES_MODS(BUILD_DRAWING BUILD_PART BUILD_SPREADSHEET)
24-
REQUIRES_MODS(BUILD_FEM BUILD_PART)
24+
REQUIRES_MODS(BUILD_FEM BUILD_PART BUILD_MESH BUILD_PLOT)
2525
REQUIRES_MODS(BUILD_IMPORT BUILD_PART BUILD_PART_DESIGN)
2626
REQUIRES_MODS(BUILD_INSPECTION BUILD_MESH BUILD_POINTS BUILD_PART)
2727
REQUIRES_MODS(BUILD_JTREADER BUILD_MESH)
@@ -30,7 +30,7 @@ macro(CheckInterModuleDependencies)
3030
REQUIRES_MODS(BUILD_OPENSCAD BUILD_MESH_PART BUILD_DRAFT)
3131
REQUIRES_MODS(BUILD_MATERIAL_EXTERNAL BUILD_MATERIAL)
3232
REQUIRES_MODS(BUILD_MEASURE BUILD_PART)
33-
REQUIRES_MODS(BUILD_PART BUILD_MATERIAL)
33+
REQUIRES_MODS(BUILD_PART BUILD_MATERIAL BUILD_SHOW)
3434
REQUIRES_MODS(BUILD_PART_DESIGN BUILD_SKETCHER)
3535
# REQUIRES_MODS(BUILD_CAM BUILD_PART BUILD_MESH BUILD_ROBOT)
3636
REQUIRES_MODS(BUILD_CAM BUILD_PART BUILD_MESH)

.github/workflows/actions/additionalChecks/added_dependency.py renamed to tools/lint/added_dependency.py

Lines changed: 64 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,22 @@
1+
#!/usr/bin/env python3
12
# SPDX-License-Identifier: LGPL-2.1-or-later
23

3-
"""Script to parse git diff output and determine if any new C++ dependencies were added between FreeCAD modules."""
4+
"""Script to parse changed files and determine if any new C++ dependencies were added between
5+
FreeCAD modules."""
46

7+
import argparse
58
import os
69
import re
710
import sys
811

12+
from utils import (
13+
init_environment,
14+
write_file,
15+
append_file,
16+
emit_problem_matchers,
17+
add_common_arguments,
18+
)
19+
920
KNOWN_MODULES = [
1021
"app",
1122
"base",
@@ -98,7 +109,11 @@ def load_intermodule_dependencies(cmake_file_path: str) -> dict[str, list[str]]:
98109
"""FreeCAD already has a file that defines the known dependencies between modules. The basic rule is that no NEW
99110
dependencies can be added (without extensive discussion with the core developers). This function loads that file
100111
and parses it such that we can use it to check if a new dependency was added."""
101-
dependencies = {}
112+
dependencies = {
113+
"base": [],
114+
"app": ["base"],
115+
"gui": ["app", "base"]
116+
}
102117

103118
if not os.path.exists(cmake_file_path):
104119
print(f"ERROR: {cmake_file_path} not found", file=sys.stderr)
@@ -114,30 +129,54 @@ def load_intermodule_dependencies(cmake_file_path: str) -> dict[str, list[str]]:
114129
prerequisites = match.group(2).split()
115130
module_name = dependent.replace("BUILD", "").replace("_", "").lower()
116131
prereq_names = [p.replace("BUILD", "").replace("_", "").lower() for p in prerequisites]
132+
prereq_names.append("app") # Everything in this list depends on (or is allowed to depend on) App
117133
dependencies[module_name] = prereq_names
118134

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()
135+
for KNOWN_MODULE in KNOWN_MODULES:
136+
if KNOWN_MODULE not in dependencies:
137+
dependencies[KNOWN_MODULE] = ["app", "base"] # NOT Gui, which must be handled separately
138+
139+
resolve_transitive_dependencies(dependencies)
140+
141+
#print()
142+
#print("Recognized intermodule dependencies")
143+
#print("-----------------------------------")
144+
#for module, deps in dependencies.items():
145+
# print(f"{module} depends on: {', '.join(deps)}")
146+
#print()
125147

126148
return dependencies
127149

150+
def resolve_transitive_dependencies(dependencies: dict[str, list[str]]) -> None:
151+
changed = True
152+
while changed:
153+
changed = False
154+
for module, deps in dependencies.items():
155+
for dep in list(deps): # copy to avoid mutation during iteration
156+
for transitive in dependencies.get(dep, []):
157+
if transitive not in deps:
158+
deps.append(transitive)
159+
changed = True
160+
161+
128162

129163
def check_file_dependencies(
130-
file: str, diff: str, intermodule_dependencies: dict[str, list[str]]
164+
file: str, intermodule_dependencies: dict[str, list[str]]
131165
) -> bool:
132166
"""Returns true if the dependencies are OK, or false if they are not."""
133167
file_module = file.split("/")[1] # src/Gui, etc.
134168
if file_module == "Mod":
135169
file_module = file.split("/")[2] # src/Mod/Part, etc.
136-
diff_lines = diff.split("\n")
170+
is_gui = "gui" in [x.lower() for x in file.split("/")]
171+
with open(file, "r", encoding="utf-8") as f:
172+
content = f.read()
173+
lines = content.splitlines()
137174
failed = False
138-
for line in diff_lines:
175+
line_number = 0
176+
for line in lines:
177+
line_number += 1
139178
if file.endswith(".h") or file.endswith(".cpp"):
140-
include_file = (m := re.search(r'^\+\s*#include\s*[<"]([^>"]+)[>"]', line)) and m.group(
179+
include_file = (m := re.search(r'^\s*#include\s*[<"]([^>"]+)[>"]', line)) and m.group(
141180
1
142181
)
143182
if include_file:
@@ -149,41 +188,43 @@ def check_file_dependencies(
149188
elif file.endswith(".py") or file.endswith(".pyi"):
150189
include_file_module = (
151190
m := re.search(
152-
r"^\+\s*(?:from\s+([\w.]+)\s+)?import\s+([\w.]+(?:\s+as\s+\w+)?(?:\s*,\s*[\w.]+(?:\s+as\s+\w+)?)*)",
191+
r"^\s*(?:from\s+([\w.]+)\s+)?import\s+([\w.]+(?:\s+as\s+\w+)?(?:\s*,\s*[\w.]+(?:\s+as\s+\w+)?)*)",
153192
line,
154193
)
155194
) and (m.group(1) or m.group(2))
156195
else:
157196
return True
158197
if not include_file_module:
159198
continue
199+
if is_gui and include_file_module.lower() == "gui":
200+
# Special case: anything with "gui" in its path is allowed to include Gui
201+
continue
160202
compatibility = check_module_compatibility(
161203
file_module.lower(), include_file_module.lower(), intermodule_dependencies
162204
)
163205

164206
if not compatibility:
165207
print(
166-
f" 👉 {file_module} added a new dependency on {include_file_module}",
208+
f" --> {file_module} added a new dependency on {include_file_module} in {file} line {line_number}",
167209
file=sys.stderr,
168210
)
169211
failed = True
170212
return not failed
171213

172214

173-
if __name__ == "__main__":
215+
def main():
216+
174217
dependencies = load_intermodule_dependencies(
175218
"cMake/FreeCAD_Helpers/CheckIntermoduleDependencies.cmake"
176219
)
177-
piped_diff = sys.stdin.read()
178-
file_diffs = parse_diff_by_file(piped_diff)
179220
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}")
221+
for file in sys.argv[1:]:
222+
if not check_file_dependencies(file, dependencies):
184223
failed_files.append(file)
185-
else:
186-
print(f" ✅ No problems found in {file}")
187224
if failed_files:
188225
sys.exit(1)
189226
sys.exit(0)
227+
228+
229+
if __name__ == "__main__":
230+
main()

0 commit comments

Comments
 (0)