Skip to content

Commit 4f07f75

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 4f07f75

File tree

3 files changed

+278
-0
lines changed

3 files changed

+278
-0
lines changed

3rdparty/test_cmake_third_party.py

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

3rdparty/test_git_modules.py

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
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+
violations.append(
39+
ThirdpartyViolation(
40+
section=section,
41+
path=path,
42+
note="Submodule not in allowlist (see test_git_modules.py)",
43+
)
44+
)
45+
46+
if not path.startswith("3rdparty/"):
47+
violations.append(
48+
ThirdpartyViolation(
49+
section=section,
50+
path=path,
51+
note="Submodule path must be under 3rdparty/",
52+
)
53+
)
54+
return violations
55+
56+
57+
def check_modules_file(git_modules_path: pathlib.Path) -> int:
58+
"""
59+
Common entry-point between main() and pytest. Prints any violations to
60+
stderr and returns non-zero if any violations are found.
61+
"""
62+
config = configparser.ConfigParser()
63+
config.read(git_modules_path)
64+
65+
violations = find_violations(config)
66+
67+
if violations:
68+
for violation in violations:
69+
sys.stderr.write(
70+
f"{violation.section} (path={violation.path}): {violation.note}\n"
71+
)
72+
73+
logger.error(
74+
"Found %d potential third-party violations. "
75+
"If you are trying to add a new third-party dependency, "
76+
"please follow the instructions in cpp/3rdparty/cpp-thirdparty.md",
77+
len(violations),
78+
)
79+
return 1
80+
return 0
81+
82+
83+
class TestGitModules:
84+
def test_gitmodules(self):
85+
"""
86+
Test that no git submodules are added to .gitmodules without
87+
following the defined process.
88+
"""
89+
git_modules_path = pathlib.Path(__file__).parents[1] / ".gitmodules"
90+
result = check_modules_file(git_modules_path)
91+
assert result == 0
92+
93+
94+
def main():
95+
parser = argparse.ArgumentParser(description="__doc__")
96+
parser.add_argument(
97+
"--git-modules-path",
98+
default=pathlib.Path(".gitmodules"),
99+
type=pathlib.Path,
100+
help="Path to the .gitmodules file, defaults to .gitmodules in current directory",
101+
)
102+
args = parser.parse_args()
103+
result = check_modules_file(args.git_modules_path)
104+
sys.exit(result)
105+
106+
107+
if __name__ == "__main__":
108+
logging.basicConfig(level=logging.INFO)
109+
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)