Skip to content

Commit 76193b4

Browse files
committed
[TRTLLM-9228][infra] Verify thirdparty C++ process
This change adds two tests to help ensure that thirdparty C++ code is integrated according to the process descripted in 3rdparty/cpp-thirdparty.md. The desired process requires folks to use FetchContent_Declare in 3rdparty/CMakeLists.txt. The two tests added here search the following deviations of the desired process: 1. uses of FetchContent_Declare or ExternalProject_Add outside of 3rdparty/CMakeLists.txt 2. new git-submodules added to the repo Both scripts have the ability to allow-list exemptions if there are any cases in the future where a deviation is warranted. The scripts live in 3rdparty/* where CODEOWNERS ensures that process reviewers will be required on any new exemptions added to the allowlists. Signed-off-by: Josh Bialkowski <[email protected]>
1 parent 9b2abb8 commit 76193b4

File tree

3 files changed

+272
-0
lines changed

3 files changed

+272
-0
lines changed

3rdparty/test_cmake_third_party.py

Lines changed: 164 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,164 @@
1+
"""
2+
This script searches for cmake function invocations that might indicate
3+
the addition of new third-party dependencies outside of the intended
4+
process (3rdparty/README.md).
5+
"""
6+
7+
import argparse
8+
import collections
9+
import logging
10+
import pathlib
11+
import sys
12+
from typing import Generator
13+
14+
logger = logging.getLogger(__name__)
15+
16+
IGNORE_PATTERNS = [
17+
".*", # Hidden files and directories, like .git
18+
# This is where we actually want third-party stuff to go
19+
"3rdparty/CMakeLists.txt",
20+
# Historical use of ExternalProject_Add that is not yet migrated to 3rdparty
21+
"cpp/tensorrt_llm/deep_ep/CMakeLists.txt",
22+
# Historical build that is not included in the wheel build and thus exempt
23+
# from the third-party process.
24+
"triton_backend/inflight_batcher_llm/*",
25+
"build", # Default build directory
26+
"cpp/build", # Default extension module build directory
27+
]
28+
29+
30+
class DirectoryFilter:
31+
"""
32+
Callable filter for directories that excludes any paths matching
33+
IGNORE_PATTERNS.
34+
"""
35+
36+
def __init__(self, parent: pathlib.Path):
37+
self.parent = parent
38+
39+
def __call__(self, name: str) -> bool:
40+
path = self.parent / name
41+
if any(path.match(pat) for pat in IGNORE_PATTERNS):
42+
return False
43+
return True
44+
45+
46+
class FileFilter:
47+
"""
48+
Callable filter for file entries that (in order of precedence):
49+
50+
1. excludes any paths matching IGNORE_PATTERNS
51+
2. includes only CMakeLists.txt and *.cmake files
52+
"""
53+
54+
def __init__(self, parent: pathlib.Path):
55+
self.parent = parent
56+
57+
def __call__(self, name: str) -> bool:
58+
path = self.parent / name
59+
if any(path.match(pat) for pat in IGNORE_PATTERNS):
60+
return False
61+
62+
if name == "CMakeLists.txt":
63+
return True
64+
elif name.endswith(".cmake"):
65+
return True
66+
67+
return False
68+
69+
70+
def yield_sources(src_tree: pathlib.Path):
71+
"""
72+
Perform a filesystem walk and yield any paths that should be scanned
73+
"""
74+
for parent, dirs, files in src_tree.walk(src_tree):
75+
relpath_parent = parent.relative_to(src_tree)
76+
77+
# Filter out ignored directories
78+
dirs[:] = sorted(filter(DirectoryFilter(relpath_parent), dirs))
79+
80+
for file in sorted(filter(FileFilter(relpath_parent), files)):
81+
yield parent / file
82+
83+
84+
ThirdpartyViolation = collections.namedtuple(
85+
"ThirdpartyViolation", ["srcfile", "lineno", "note", "line"]
86+
)
87+
88+
89+
def yield_potential_thirdparty(
90+
fullpath: pathlib.Path, relpath: pathlib.Path
91+
) -> Generator[ThirdpartyViolation, None, None]:
92+
"""
93+
Look for patterns that might indicate the addition of new third-party
94+
sources.
95+
"""
96+
with fullpath.open("r", encoding="utf-8") as infile:
97+
for lineno, line in enumerate(infile):
98+
lineno += 1 # Make line numbers 1-based
99+
100+
if "FetchContent_Declare" in line:
101+
note = "Invalid use of FetchContent_Declare outside of 3rdparty/CMakeLists.txt"
102+
yield ThirdpartyViolation(relpath, lineno, note, line.strip())
103+
104+
if "ExternalProject_Add" in line:
105+
note = "Invalid use of ExternalProject_Add outside of 3rdparty/CMakeLists.txt"
106+
yield ThirdpartyViolation(relpath, lineno, note, line.strip())
107+
108+
109+
def check_sources(src_tree: pathlib.Path) -> int:
110+
"""
111+
Common entry-point between main() and pytest. Prints any violations to
112+
stderr and returns non-zero if any violations are found.
113+
"""
114+
violations = []
115+
for filepath in yield_sources(src_tree):
116+
for violation in yield_potential_thirdparty(
117+
filepath, filepath.relative_to(src_tree)
118+
):
119+
violations.append(violation)
120+
121+
if not violations:
122+
return 0
123+
124+
for violation in sorted(violations):
125+
sys.stderr.write(
126+
f"{violation.srcfile}:{violation.lineno}: {violation.note}\n"
127+
+ f" {violation.line}\n"
128+
)
129+
130+
logger.error(
131+
"Found %d potential third-party violations. "
132+
"If you are trying to add a new third-party dependency, "
133+
"please follow the instructions in 3rdparty/cpp-thirdparty.md",
134+
len(violations),
135+
)
136+
return 1
137+
138+
139+
class TestThirdPartyInCmake:
140+
def test_cmake_listfiles(self):
141+
"""
142+
Test that no third-party violations are found in the source tree.
143+
"""
144+
source_tree = pathlib.Path(__file__).parents[1]
145+
result = check_sources(source_tree)
146+
assert result == 0
147+
148+
149+
def main():
150+
parser = argparse.ArgumentParser(description="__doc__")
151+
parser.add_argument(
152+
"--src-tree",
153+
default=pathlib.Path.cwd(),
154+
type=pathlib.Path,
155+
help="Path to the source tree, defaults to current directory",
156+
)
157+
args = parser.parse_args()
158+
result = check_sources(args.src_tree)
159+
sys.exit(result)
160+
161+
162+
if __name__ == "__main__":
163+
logging.basicConfig(level=logging.INFO)
164+
main()

3rdparty/test_git_modules.py

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
"""
2+
This script audit the .gitmodules file to make sure that new git submodules
3+
are not added without following the proper process
4+
(cpp/3rdparty/cpp-thirdparty.md)
5+
"""
6+
7+
import argparse
8+
import collections
9+
import logging
10+
import pathlib
11+
import configparser
12+
import sys
13+
14+
logger = logging.getLogger(__name__)
15+
16+
ALLOWLIST_SUBMODULES = [
17+
# NOTE: please do not add new sobmodules here without following the process
18+
# in 3rdparty/cpp-thirdparty.md. Prefer to use FetchContent or other methods
19+
# to avoid adding new git submodules unless absolutely necessary.
20+
]
21+
22+
ThirdpartyViolation = collections.namedtuple(
23+
"ThirdpartyViolation", ["section", "path", "note"]
24+
)
25+
26+
27+
def find_violations(config: configparser.ConfigParser) -> list[str]:
28+
violations = []
29+
for section in config.sections():
30+
if not section.startswith("submodule "):
31+
raise ValueError(f"Unexpected section in .gitmodules: {section}")
32+
33+
path = config[section].get("path", "")
34+
if not path:
35+
raise ValueError(f"Missing path for submodule {section}")
36+
37+
if path not in ALLOWLIST_SUBMODULES:
38+
yield ThirdpartyViolation(
39+
section=section,
40+
path=path,
41+
note="Submodule not in allowlist (see test_git_modules.py)",
42+
)
43+
44+
if not path.startswith("3rdparty/"):
45+
yield ThirdpartyViolation(
46+
section=section,
47+
path=path,
48+
note="Submodule path must be under 3rdparty/",
49+
)
50+
return violations
51+
52+
53+
def check_modules_file(git_modules_path: pathlib.Path) -> int:
54+
"""
55+
Common entry-point between main() and pytest. Prints any violations to
56+
stderr and returns non-zero if any violations are found.
57+
"""
58+
config = configparser.ConfigParser()
59+
config.read(git_modules_path)
60+
61+
violations = list(find_violations(config))
62+
63+
if violations:
64+
for violation in violations:
65+
sys.stderr.write(
66+
f"{violation.section} (path={violation.path}): {violation.note}\n"
67+
)
68+
69+
logger.error(
70+
"Found %d potential third-party violations. "
71+
"If you are trying to add a new third-party dependency, "
72+
"please follow the instructions in cpp/3rdparty/cpp-thirdparty.md",
73+
len(violations),
74+
)
75+
return 1
76+
return 0
77+
78+
79+
class TestGitModules:
80+
def test_gitmodules(self):
81+
"""
82+
Test that no git submodules are added to .gitmodules without
83+
following the defined process.
84+
"""
85+
git_modules_path = pathlib.Path(__file__).parents[1] / ".gitmodules"
86+
result = check_modules_file(git_modules_path)
87+
assert result == 0
88+
89+
90+
def main():
91+
parser = argparse.ArgumentParser(description="__doc__")
92+
parser.add_argument(
93+
"--git-modules-path",
94+
default=pathlib.Path(".gitmodules"),
95+
type=pathlib.Path,
96+
help="Path to the .gitmodules file, defaults to .gitmodules in current directory",
97+
)
98+
args = parser.parse_args()
99+
result = check_modules_file(args.git_modules_path)
100+
sys.exit(result)
101+
102+
103+
if __name__ == "__main__":
104+
logging.basicConfig(level=logging.INFO)
105+
main()

tests/integration/test_lists/test-db/l0_a10.yml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,9 @@ l0_a10:
7676
- unittest/llmapi/apps/test_chat_utils.py
7777
- unittest/llmapi/apps/test_tool_parsers.py
7878
- unittest/llmapi/apps/test_harmony_channel_validation.py
79+
# third-party policy checks CPU-only
80+
- 3rdparty/test_cmake_third_party.py
81+
- 3rdparty/test_git_modules.py
7982
- condition:
8083
ranges:
8184
system_gpu_count:

0 commit comments

Comments
 (0)