[None][infra] Generate json with cmake fetched contents in build stage#13607
Conversation
|
/bot run |
📝 WalkthroughWalkthroughThe changes introduce a new Python script to generate a Changes
Sequence Diagram(s)sequenceDiagram
actor Jenkins
participant Script as generate_cpp_dependency_json.py
participant FileSystem
participant Metadata as fetch_content.json
participant GitLab API
participant Output as third-party-sources.json
Jenkins->>Script: Invoke with --deps-dir and --output-dir
Script->>FileSystem: Enumerate *-src directories
Script->>Metadata: Load dependency metadata index
loop For each dependency
Script->>GitLab API: Check if mirror exists<br/>(nvidia/tensorrt-llm/oss-components)
alt Mirror not found
Script->>GitLab API: Create new mirror project<br/>(upstream URL, public)
GitLab API-->>Script: Mirror creation initiated
Script->>GitLab API: Poll mirror status<br/>(until import completes or timeout)
end
Script->>GitLab API: Verify tag/ref exists in mirror
Script->>Script: Build updated dependency record<br/>(git_repository → mirror URL)
end
Script->>Output: Write collected records as indented JSON
Script-->>Jenkins: Script completes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Review rate limit: 9/10 reviews remaining, refill in 6 minutes. Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
scripts/generate_cpp_dependency_json.py (2)
93-93: Remove debug print statement.This
print(upstream_url)appears to be debug output. Uselogger.info()for consistency with the rest of the script, or remove it entirely.Proposed fix
def create_oss_component( package_name: str, namespace_id: int, upstream_url: str ) -> tuple[str, int]: """Create a new project under oss-components with a pull mirror and return (web_url, project_id).""" - print(upstream_url) + logger.debug("Creating OSS component from upstream: %s", upstream_url) payload = json.dumps(🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/generate_cpp_dependency_json.py` at line 93, Remove the stray debug print by deleting the print(upstream_url) call or replacing it with a structured logger call (e.g., logger.info("upstream_url=%s", upstream_url)) to match the script's existing logging; locate the print statement in generate_cpp_dependency_json.py (the print(upstream_url) line) and either remove it or convert it to use logger.info so output is consistent with other log messages.
134-134: Add return type annotation tomain().As per coding guidelines, always annotate Python function return types.
Proposed fix
-def main(): +def main() -> None:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/generate_cpp_dependency_json.py` at line 134, The function signature for main() lacks a return type annotation; update the declaration of main (def main()) in generate_cpp_dependency_json.py to include an explicit return type (e.g., def main() -> None:) and ensure any return statements inside main conform to that type (or remove returns that return values) so the function signature and implementation remain consistent with the codebase typing guidelines.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@jenkins/Build.groovy`:
- Line 402: The shell invocation running generate_cpp_dependency_json.py is
executed from the workspace root so relative paths fail; update the command that
currently calls sh "python3 scripts/generate_cpp_dependency_json.py --deps-dir
cpp/build/_deps --output-dir ./" to first change to ${LLM_ROOT} (e.g., use cd
${LLM_ROOT} && ...) so both the script path
scripts/generate_cpp_dependency_json.py and the deps-dir cpp/build/_deps resolve
correctly and the expected output third-party-sources.json is created under
${LLM_ROOT}; ensure the same pattern is used wherever the script is invoked so
later references (line 417) find the output.
In `@scripts/generate_cpp_dependency_json.py`:
- Around line 1-8: This file is missing the required NVIDIA copyright header;
add the SPDX/Apache-2.0 copyright block at the very top of the Python module
(above the existing module docstring) using the provided text and the latest
modification year (2026) so the header precedes the triple-quoted docstring that
currently begins the file.
- Around line 183-187: The code assumes dep_source_index[package_name] exists
and can raise KeyError when a *-src dir has no entry in fetch_content.json;
modify the block handling tag confirmation (the code that calls
commit_exists_in_project and appends to third_party_source_list) to first check
for the package key (e.g., using package_name in dep_source_index or
dep_source_index.get(package_name)), and if missing, log a clear warning
mentioning package_name/project_id and skip appending; otherwise merge the dict
as before ({**dep_source_index[package_name], "git_repository": oss_url}) so you
avoid the KeyError.
- Line 20: Replace the hardcoded GitLab PAT assigned to the TOKEN variable in
generate_cpp_dependency_json.py by reading it from an environment variable
(e.g., GITLAB_OSS_TOKEN) and fail fast with a clear error if the env var is
missing; update any CI/Jenkins usage to pass the secret via credentials (e.g.,
withCredentials setting GITLAB_OSS_TOKEN) rather than embedding the token in the
script, and remove the literal token string so it is not stored in the repo or
history.
---
Nitpick comments:
In `@scripts/generate_cpp_dependency_json.py`:
- Line 93: Remove the stray debug print by deleting the print(upstream_url) call
or replacing it with a structured logger call (e.g.,
logger.info("upstream_url=%s", upstream_url)) to match the script's existing
logging; locate the print statement in generate_cpp_dependency_json.py (the
print(upstream_url) line) and either remove it or convert it to use logger.info
so output is consistent with other log messages.
- Line 134: The function signature for main() lacks a return type annotation;
update the declaration of main (def main()) in generate_cpp_dependency_json.py
to include an explicit return type (e.g., def main() -> None:) and ensure any
return statements inside main conform to that type (or remove returns that
return values) so the function signature and implementation remain consistent
with the codebase typing guidelines.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: f0f47d07-8343-4773-9d55-012b6f60f433
📒 Files selected for processing (3)
3rdparty/fetch_content.jsonjenkins/Build.groovyscripts/generate_cpp_dependency_json.py
|
PR_Github #46107 [ run ] triggered by Bot. Commit: |
|
PR_Github #46107 [ run ] completed with state
|
230ef2b to
42c32f1
Compare
|
/bot run --stage-list "A10-PackageSanityCheck-PY310-UB2204" |
|
PR_Github #46184 [ run ] triggered by Bot. Commit: |
Signed-off-by: Yuanjing Xue <197832395+yuanjingx87@users.noreply.github.com>
42c32f1 to
f836d75
Compare
|
/bot run --stage-list "A10-PackageSanityCheck-PY310-UB2204" |
|
PR_Github #46203 [ run ] triggered by Bot. Commit: |
|
PR_Github #46184 [ run ] completed with state |
Signed-off-by: Yuanjing Xue <197832395+yuanjingx87@users.noreply.github.com>
|
PR_Github #46203 [ run ] completed with state
|
|
/bot run --stage-list "Build-Docker-Images" |
|
PR_Github #46220 [ run ] triggered by Bot. Commit: |
53adf7f to
74450e2
Compare
|
PR_Github #46220 [ run ] completed with state
|
Signed-off-by: Yuanjing Xue <197832395+yuanjingx87@users.noreply.github.com>
74450e2 to
bc749bf
Compare
945b26c to
4088659
Compare
Signed-off-by: Yuanjing Xue <197832395+yuanjingx87@users.noreply.github.com>
c19e286 to
fa84f0c
Compare
|
/bot run |
|
PR_Github #47120 [ run ] triggered by Bot. Commit: |
|
PR_Github #47120 [ run ] completed with state
|
Signed-off-by: Yuanjing Xue <197832395+yuanjingx87@users.noreply.github.com>
|
/bot skip --comment "No need to run CI" |
|
PR_Github #50909 [ skip ] triggered by Bot. Commit: |
|
PR_Github #50909 [ skip ] completed with state |
Summary by CodeRabbit
Release Notes
New Features
Chores
Description
When build the wheel, generate a json with all cmake fetched packages, copy that json file to release containers
Test Coverage
PR Checklist
Please review the following before submitting your PR:
PR description clearly explains what and why. If using CodeRabbit's summary, please make sure it makes sense.
PR Follows TRT-LLM CODING GUIDELINES to the best of your knowledge.
Test cases are provided for new code paths (see test instructions)
Any new dependencies have been scanned for license and vulnerabilities
CODEOWNERS updated if ownership changes
Documentation updated as needed
Update tava architecture diagram if there is a significant design change in PR.
The reviewers assigned automatically/manually are appropriate for the PR.
Please check this after reviewing the above items as appropriate for this PR.
GitHub Bot Help
To see a list of available CI bot commands, please comment
/bot help.