Skip to content

Conversation

@guyueh1
Copy link
Contributor

@guyueh1 guyueh1 commented Feb 5, 2026

What does this PR do ?

Add a script to support build custom flash-infer

Issues

List issues that this PR closes (syntax):

Usage

  • You can potentially add a usage example below
# Add a code snippet demonstrating how to use this

Before your PR is "Ready for review"

Pre checks:

  • Make sure you read and followed Contributor guidelines
  • Did you write any new necessary tests?
  • Did you run the unit tests and functional tests locally? Visit our Testing Guide for how to run tests
  • Did you add or update any necessary documentation? Visit our Document Development Guide for how to write, build and test the docs.

Additional Information

  • ...

Summary by CodeRabbit

  • New Features
    • Added support for optional custom FlashInfer builds. Users can enable this by setting the BUILD_CUSTOM_FLASHINFER environment variable. The build system automatically handles dependency management and configuration for the custom integration.

Signed-off-by: Guyue Huang <guyueh@nvidia.com>
@guyueh1 guyueh1 requested a review from a team as a code owner February 5, 2026 17:14
Signed-off-by: Guyue Huang <guyueh@nvidia.com>
@guyueh1 guyueh1 requested a review from a team as a code owner February 5, 2026 17:15
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 5, 2026

📝 Walkthrough

Walkthrough

This PR adds support for building custom FlashInfer integration by introducing a new bash script that automates cloning, validation, and configuration of FlashInfer repositories. The script is integrated into the Docker build process via a conditional BUILD_CUSTOM_FLASHINFER flag, mirroring the existing BUILD_CUSTOM_VLLM mechanism.

Changes

Cohort / File(s) Summary
Docker Build Integration
docker/Dockerfile
Adds 4 lines to copy the build-custom-flashinfer.sh script and conditionally execute it during the hermetic build stage when BUILD_CUSTOM_FLASHINFER is set.
FlashInfer Build Script
tools/build-custom-flashinfer.sh
New 89-line Bash script that automates custom FlashInfer integration: clones a specified repository, validates structure, updates pyproject.toml with uv and tomlkit to add local source configuration, installs build dependencies, and refreshes lockfiles.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~15 minutes

🚥 Pre-merge checks | ✅ 3 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Test Results For Major Changes ⚠️ Warning PR introduces major changes but provides no test results, testing information, or validation evidence despite the test checklist being incomplete. Document comprehensive test results and validation approach in the PR description; resolve all identified issues and mark the test checklist complete.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'feat: Support build custom flashinfer' directly and clearly describes the main change: adding support for building custom FlashInfer, which matches the core purpose of both files modified (Dockerfile additions and new build script).
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 docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

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.

Actionable comments posted: 4

🤖 Fix all issues with AI agents
In `@docker/Dockerfile`:
- Around line 109-111: The Dockerfile uses the shell variable
BUILD_CUSTOM_FLASHINFER in the conditional block but never declares it as a
build ARG, so add an ARG declaration (e.g., add "ARG BUILD_CUSTOM_FLASHINFER" or
"ARG BUILD_CUSTOM_FLASHINFER=0") above the if-statement to ensure the build-arg
is accepted and propagated; keep the existing conditional that calls
tools/build-custom-flashinfer.sh when BUILD_CUSTOM_FLASHINFER is non-empty so
builds can enable this via --build-arg BUILD_CUSTOM_FLASHINFER=1.

In `@tools/build-custom-flashinfer.sh`:
- Around line 38-41: The script currently forces SSH host key verification off
via GIT_SSH_COMMAND which is insecure; change it to only disable verification
when an explicit opt-in env var is set (e.g. ALLOW_INSECURE_SSH=true or
INTERNAL_BUILD=true) and only for SSH-style URLs (check GIT_URL starts with
"ssh://" or matches an SSH host:path pattern) before setting GIT_SSH_COMMAND;
otherwise run a normal git clone to preserve host key checking. Update the logic
around GIT_SSH_COMMAND, GIT_URL and BUILD_DIR so the insecure options are
conditional on the env var and SSH URL detection.
- Around line 58-79: The script fails because project is never defined and it
blindly appends into vllm causing duplicates; fix by initializing project =
doc.setdefault("project", {}) then opt =
project.setdefault("optional-dependencies", {}) and vllm_list =
opt.setdefault("vllm", []) and only append "flashinfer-python" if it's not
already present; also make the sources addition idempotent by checking
sources.get("flashinfer-python") and only set it when missing or different (use
the existing symbols pyproject_path, doc, project, opt, vllm_list, sources, and
the "flashinfer-python" key).
- Around line 26-30: The script currently runs realpath on a target that may not
exist, causing the script to abort under set -e; update the BUILD_DIR assignment
so it does not call realpath on a non-existent path (either use
BUILD_DIR="$SCRIPT_DIR/../3rdparty/flashinfer" or use realpath -m
"$SCRIPT_DIR/../3rdparty/flashinfer") and keep the subsequent existence check
that echoes the error and exits; modify the line that defines BUILD_DIR (and any
usages relying on its absolute value) to use one of these safer alternatives.

@guyueh1 guyueh1 self-assigned this Feb 5, 2026
Signed-off-by: root <root@gpu-93.slurm-workers-slurm.slurm.svc.cluster.local>
@guyueh1 guyueh1 added the CI:L2 Run doctests, unit tests, functional tests, and convergence tests label Feb 9, 2026
Signed-off-by: Guyue Huang <140554423+guyueh1@users.noreply.github.com>
@guyueh1 guyueh1 added CI:L2 Run doctests, unit tests, functional tests, and convergence tests and removed CI:L2 Run doctests, unit tests, functional tests, and convergence tests labels Feb 10, 2026
root and others added 4 commits February 10, 2026 00:57
Signed-off-by: root <root@gpu-93.slurm-workers-slurm.slurm.svc.cluster.local>
Signed-off-by: Guyue Huang <140554423+guyueh1@users.noreply.github.com>
Signed-off-by: Guyue Huang <140554423+guyueh1@users.noreply.github.com>
@guyueh1 guyueh1 added CI:L2 Run doctests, unit tests, functional tests, and convergence tests and removed CI:L2 Run doctests, unit tests, functional tests, and convergence tests labels Feb 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CI:L2 Run doctests, unit tests, functional tests, and convergence tests super-v3

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants