Skip to content

Commit cdb0dd3

Browse files
cheshirekowusberkeley
authored andcommitted
[TRTLLM-9228][infra] Verify thirdparty C++ process (NVIDIA#9367)
Signed-off-by: Josh Bialkowski <[email protected]> Co-authored-by: Josh Bialkowski <[email protected]>
1 parent 04a4283 commit cdb0dd3

File tree

3 files changed

+271
-0
lines changed

3 files changed

+271
-0
lines changed
Lines changed: 163 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,163 @@
1+
"""Find bad third-party usage in cmake.
2+
3+
This script searches for cmake function invocations that might indicate
4+
the addition of new third-party dependencies outside of the intended
5+
process (3rdparty/README.md).
6+
"""
7+
8+
import argparse
9+
import collections
10+
import logging
11+
import os
12+
import pathlib
13+
import sys
14+
from typing import Generator
15+
16+
logger = logging.getLogger(__name__)
17+
18+
IGNORE_PATTERNS = [
19+
".*", # Hidden files and directories, like .git
20+
# This is where we actually want third-party stuff to go
21+
"3rdparty/CMakeLists.txt",
22+
# Historical use of ExternalProject_Add that is not yet migrated to 3rdparty
23+
"cpp/tensorrt_llm/deep_ep/CMakeLists.txt",
24+
# Historical build that is not included in the wheel build and thus exempt
25+
# from the third-party process.
26+
"triton_backend/inflight_batcher_llm/*",
27+
"build", # Default build directory
28+
"cpp/build", # Default extension module build directory
29+
]
30+
31+
32+
class DirectoryFilter:
33+
"""Callable filter for directories.
34+
35+
This filter excludes any paths matching IGNORE_PATTERNS.
36+
"""
37+
38+
def __init__(self, parent: pathlib.Path):
39+
self.parent = parent
40+
41+
def __call__(self, name: str) -> bool:
42+
path = self.parent / name
43+
if any(path.match(pat) for pat in IGNORE_PATTERNS):
44+
return False
45+
return True
46+
47+
48+
class FileFilter:
49+
"""Callable filter for file entries.
50+
51+
In order of precedence:
52+
53+
1. excludes any paths matching IGNORE_PATTERNS
54+
2. includes only CMakeLists.txt and *.cmake files
55+
"""
56+
57+
def __init__(self, parent: pathlib.Path):
58+
self.parent = parent
59+
60+
def __call__(self, name: str) -> bool:
61+
path = self.parent / name
62+
if any(path.match(pat) for pat in IGNORE_PATTERNS):
63+
return False
64+
65+
if name == "CMakeLists.txt":
66+
return True
67+
elif name.endswith(".cmake"):
68+
return True
69+
70+
return False
71+
72+
73+
def yield_sources(src_tree: pathlib.Path):
74+
"""Perform a filesystem walk and yield any paths that should be scanned."""
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+
"""Look for bad patterns with third-party sources.
95+
96+
Look for patterns that might indicate the addition of new third-party
97+
sources.
98+
"""
99+
with fullpath.open("r", encoding="utf-8") as infile:
100+
for lineno, line in enumerate(infile):
101+
lineno += 1 # Make line numbers 1-based
102+
103+
if "FetchContent_Declare" in line:
104+
note = "Invalid use of FetchContent_Declare outside of 3rdparty/CMakeLists.txt"
105+
yield ThirdpartyViolation(relpath, lineno, note, line.strip())
106+
107+
if "ExternalProject_Add" in line:
108+
note = "Invalid use of ExternalProject_Add outside of 3rdparty/CMakeLists.txt"
109+
yield ThirdpartyViolation(relpath, lineno, note, line.strip())
110+
111+
112+
def check_sources(src_tree: pathlib.Path) -> int:
113+
"""Common entry-point between main() and pytest.
114+
115+
Prints any violations to stderr and returns non-zero if any violations are
116+
found.
117+
"""
118+
violations = []
119+
for filepath in yield_sources(src_tree):
120+
for violation in yield_potential_thirdparty(filepath, filepath.relative_to(src_tree)):
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+
def test_cmake_listfiles():
142+
"""Test that no third-party violations are found in the source tree."""
143+
source_tree = pathlib.Path(__file__).parents[1]
144+
result = check_sources(source_tree)
145+
assert result == 0
146+
147+
148+
def main():
149+
parser = argparse.ArgumentParser(description="__doc__")
150+
parser.add_argument(
151+
"--src-tree",
152+
default=pathlib.Path.cwd(),
153+
type=pathlib.Path,
154+
help="Path to the source tree, defaults to current directory",
155+
)
156+
args = parser.parse_args()
157+
result = check_sources(args.src_tree)
158+
sys.exit(result)
159+
160+
161+
if __name__ == "__main__":
162+
logging.basicConfig(level=logging.INFO)
163+
main()
Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
"""This script audits the .gitmodules file.
2+
3+
... to make sure that new git submodules are not added without following the
4+
proper process (cpp/3rdparty/cpp-thirdparty.md)
5+
"""
6+
7+
import argparse
8+
import collections
9+
import configparser
10+
import logging
11+
import pathlib
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("ThirdpartyViolation", ["section", "path", "note"])
23+
24+
25+
def find_violations(config: configparser.ConfigParser) -> list[str]:
26+
violations = []
27+
for section in config.sections():
28+
if not section.startswith("submodule "):
29+
raise ValueError(f"Unexpected section in .gitmodules: {section}")
30+
31+
path = config[section].get("path", "")
32+
if not path:
33+
raise ValueError(f"Missing path for submodule {section}")
34+
35+
if path not in ALLOWLIST_SUBMODULES:
36+
violations.append(
37+
ThirdpartyViolation(
38+
section=section,
39+
path=path,
40+
note="Submodule not in allowlist (see test_git_modules.py)",
41+
)
42+
)
43+
44+
if not path.startswith("3rdparty/"):
45+
violations.append(
46+
ThirdpartyViolation(
47+
section=section,
48+
path=path,
49+
note="Submodule path must be under 3rdparty/",
50+
)
51+
)
52+
return violations
53+
54+
55+
def check_modules_file(git_modules_path: pathlib.Path) -> int:
56+
"""Common entry-point between main() and pytest.
57+
58+
Prints any violations to stderr and returns non-zero if any violations are
59+
found.
60+
"""
61+
config = configparser.ConfigParser()
62+
config.read(git_modules_path)
63+
64+
violations = find_violations(config)
65+
66+
if violations:
67+
for violation in violations:
68+
sys.stderr.write(f"{violation.section} (path={violation.path}): {violation.note}\n")
69+
70+
logger.error(
71+
"Found %d potential third-party violations. "
72+
"If you are trying to add a new third-party dependency, "
73+
"please follow the instructions in cpp/3rdparty/cpp-thirdparty.md",
74+
len(violations),
75+
)
76+
return 1
77+
return 0
78+
79+
80+
def test_gitmodules():
81+
"""Test that no git submodules are added to .gitmodules.
82+
83+
... without 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
@@ -95,6 +95,9 @@ l0_a10:
9595
- llmapi/test_llm_api_connector.py::test_connector_disagg_prefill[False]
9696
- llmapi/test_llm_api_connector.py::test_connector_disagg_prefill[True]
9797
- llmapi/test_llm_api_connector.py::test_connector_multi_request
98+
# third-party policy checks CPU-only
99+
- thirdparty/test_cmake_third_party.py::test_cmake_listfiles
100+
- thirdparty/test_git_modules.py::test_gitmodules
98101
- condition:
99102
ranges:
100103
system_gpu_count:

0 commit comments

Comments
 (0)