Skip to content

[None][chore] upgrade setuptools#12415

Open
tburt-nv wants to merge 1 commit intoNVIDIA:mainfrom
tburt-nv:user/tburt/setuptools_upgrade
Open

[None][chore] upgrade setuptools#12415
tburt-nv wants to merge 1 commit intoNVIDIA:mainfrom
tburt-nv:user/tburt/setuptools_upgrade

Conversation

@tburt-nv
Copy link
Collaborator

@tburt-nv tburt-nv commented Mar 20, 2026

Summary by CodeRabbit

  • Chores
    • Updated setuptools version constraints.
    • Changes cutlass_kernels/CMakeLists.txt to avoid using setuptools

Description

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.

@tburt-nv tburt-nv requested a review from a team as a code owner March 20, 2026 16:22
@tburt-nv
Copy link
Collaborator Author

/bot run

@tburt-nv tburt-nv enabled auto-merge (squash) March 20, 2026 16:26
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 20, 2026

📝 Walkthrough

Walkthrough

The changes modify the CUTLASS kernel build configuration and dependency constraints. The CMake build process no longer runs the setup_library.py develop installation step; instead, kernel generation now explicitly sets PYTHONPATH to access CUTLASS modules from the local build tree. The requirements.txt updates the setuptools constraint from an upper bound to a lower bound.

Changes

Cohort / File(s) Summary
Build Configuration
cpp/tensorrt_llm/kernels/cutlass_kernels/CMakeLists.txt
Removed setup_library.py develop --user execution step. Modified kernel generation to set PYTHONPATH=${cutlass_source_dir}/python via cmake -E env instead of relying on prior setup installation.
Dependencies
requirements.txt
Updated setuptools version constraint from setuptools<80 (upper-bounded) to setuptools>=78.1.1 (lower-bounded).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description is blank except for the template structure; it lacks actual content explaining the setuptools upgrade rationale, changes, testing, or impact. Provide a clear description of why setuptools is being upgraded (from <80 to >=78.1.1), what issues it resolves, relevant test coverage, and any breaking changes or migration notes.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: upgrading setuptools, which directly corresponds to the requirements.txt modification from setuptools<80 to setuptools>=78.1.1.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

Tip

CodeRabbit can generate a title for your PR based on the changes.

Add @coderabbitai title placeholder anywhere in the title of your PR and CodeRabbit will replace it with a title based on the changes in the PR. You can change the placeholder by changing the reviews.auto_title_placeholder setting.

@tensorrt-cicd
Copy link
Collaborator

PR_Github #39751 [ run ] triggered by Bot. Commit: bc49431 Link to invocation

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
requirements.txt (1)

52-52: Consider adding an upper bound to the setuptools version constraint to prevent potential breaking changes in future major releases.

The constraint setuptools>=78.1.1 currently allows any future version without restriction. While this provides maximum flexibility, it leaves the build vulnerable to breaking changes as new versions are released (currently at 82.0.1). Consider using a bounded constraint like setuptools>=78.1.1,<82 or setuptools>=78.1.1,<85 to balance forward compatibility with stability.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@requirements.txt` at line 52, The current requirements pin
"setuptools>=78.1.1" allows unbounded future major upgrades; change this to
include an upper bound to avoid breaking changes (for example use a range like
"setuptools>=78.1.1,<82" or "setuptools>=78.1.1,<85") so CI/builds remain
stable; update the setuptools requirement line in requirements.txt accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@requirements.txt`:
- Line 52: The current requirements pin "setuptools>=78.1.1" allows unbounded
future major upgrades; change this to include an upper bound to avoid breaking
changes (for example use a range like "setuptools>=78.1.1,<82" or
"setuptools>=78.1.1,<85") so CI/builds remain stable; update the setuptools
requirement line in requirements.txt accordingly.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 74405924-eee2-45f5-bc8c-d18ed40dfecc

📥 Commits

Reviewing files that changed from the base of the PR and between 68001ce and bc49431.

📒 Files selected for processing (2)
  • cpp/tensorrt_llm/kernels/cutlass_kernels/CMakeLists.txt
  • requirements.txt

@tburt-nv
Copy link
Collaborator Author

/bot run --disable-fail-fast

@tensorrt-cicd
Copy link
Collaborator

PR_Github #39756 [ run ] triggered by Bot. Commit: bc49431 Link to invocation

Signed-off-by: Tyler Burt <195370667+tburt-nv@users.noreply.github.com>
@tburt-nv tburt-nv force-pushed the user/tburt/setuptools_upgrade branch from bc49431 to f8c3f11 Compare March 20, 2026 19:25
@tburt-nv
Copy link
Collaborator Author

/bot run --disable-fail-fast

@tensorrt-cicd
Copy link
Collaborator

PR_Github #39761 [ run ] triggered by Bot. Commit: f8c3f11 Link to invocation

@tensorrt-cicd
Copy link
Collaborator

PR_Github #39761 [ run ] completed with state SUCCESS. Commit: f8c3f11
/LLM/main/L0_MergeRequest_PR pipeline #30956 completed with status: 'SUCCESS'

CI Report

Link to invocation

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants