Source sglang and Megatron-LM from third_party submodules in docker build - #1169
Source sglang and Megatron-LM from third_party submodules in docker build#1169Shi-Dong wants to merge 3 commits into
Conversation
sglang -> sgl-project/sglang @ sglang-miles megatron -> radixark/Megatron-LM @ miles-main Step 1 of moving these deps in-tree. No build wiring yet: docker/Dockerfile still clones into the image at the same paths it always has, so runtime behavior is unchanged. Follow-up PR will switch the Dockerfile to COPY from third_party/.
…uild Dockerfile: replace the in-build git clones of sglang and Megatron-LM with COPY from the new third_party/sglang and third_party/Megatron-LM submodules. The submodule pointer is now the source of truth for which sglang/Megatron-LM revision the image carries; the SGLANG_BRANCH, SGLANG_COMMIT, MEGATRON_REPO, MEGATRON_BRANCH build args are removed. docker-build.yml: checkout submodules recursively; strip the gitlink .git files in third_party/ before docker build (they would otherwise copy into the image as dangling references); extend the push path-trigger to include third_party/sglang and third_party/Megatron-LM so a submodule bump auto-fires a rebuild.
The lmsysorg/sglang base image ships a .git/ directory at /sgl-workspace/sglang from its own upstream checkout. After our COPY overlays the submodule source on top, that .git/ no longer matches the working tree (the index references files from the base image checkout, not the submodule snapshot). Drop it explicitly so the resulting image does not carry a misleading git state.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the Dockerfile to use git submodules for the sglang and Megatron-LM dependencies, replacing the previous method of using build arguments and runtime git clones. This change ensures that the submodule pointers serve as the source of truth for dependency versions. A review comment suggests that instead of only removing the .git directory in /sgl-workspace/sglang, the entire directory should be deleted before the COPY command to prevent stale files from the base image from persisting.
| # base image ships a stale `.git/` dir at this path — drop it before COPY | ||
| # so the resulting image isn't carrying a git index that disagrees with | ||
| # the working tree. | ||
| RUN rm -rf /sgl-workspace/sglang/.git |
There was a problem hiding this comment.
The COPY command merges the source directory contents into the destination if it already exists. Since the base image lmsysorg/sglang already contains an installation in /sgl-workspace/sglang, simply removing the .git directory might leave behind stale source files or artifacts that are not present in the submodule (e.g., if files were renamed or removed in the submodule version). To ensure a clean state where the directory contains exactly what is in the submodule, it is safer to remove the entire directory before copying.
RUN rm -rf /sgl-workspace/sglang
573294d to
af93e38
Compare
af93e38 to
b1217cb
Compare
|
Closing as obsolete. This PR is superseded by a thin packaging slice cut fresh off current `main`:
Why this one is no longer needed: see the per-PR note below. The branch is left in place; nothing is lost. |
1 participant
Summary
docker/Dockerfile: replace the in-buildgit cloneof sglang and Megatron-LM withCOPY third_party/{sglang,Megatron-LM}from the new submodules; drop the now-unused build args (SGLANG_BRANCH,SGLANG_COMMIT,MEGATRON_REPO,MEGATRON_BRANCH); alsorm -rf /sgl-workspace/sglang/.gitbefore COPY so the resulting image doesn't carry the base image's stale gitdir..github/workflows/docker-build.yml: checkout withsubmodules: recursive; add a step that writes each submodule's SHA into a.miles-submodule-shasidecar inside the submodule worktree and then strips the gitlink.gitfile (so docker COPY doesn't carry a dangling reference); extend the push path-trigger to includethird_party/sglangandthird_party/Megatron-LMso bumping a submodule pointer auto-fires a rebuild.Why
After step 1, the submodule pointer is the authoritative version pin for sglang and Megatron-LM. This PR makes the docker image build from that pointer (
COPYfrom the submodule snapshot) instead of issuing its owngit fetchagainstsgl-project/sglang/radixark/Megatron-LMat build time. The image is now reproducible from a commit alone: "same miles commit, same image content" without depending on remote branch tips moving.The sidecar file (
/sgl-workspace/sglang/.miles-submodule-sha,/root/Megatron-LM/.miles-submodule-sha) carries the SHA the COPY came from, so it's trivial to confirm at runtime what was baked in.Test plan
docker-build.ymlrun 26169858537 (variant=primary, image_tag=custom, custom_tag=dev-submodule-test) succeeded → imageradixark/miles:dev-submodule-testpushed to Docker Hub. Production:devwas not touched.shi-img-smoke, imageradixark/miles:dev-submodule-test):/sgl-workspace/sglang/.miles-submodule-sha=363403a2…← matches step 1's submodule pointer./root/Megatron-LM/.miles-submodule-sha=23924a0b…← matches step 1's submodule pointer.pip show sglangEditable location =/sgl-workspace/sglang/python✓pip show megatron-coreEditable location =/root/Megatron-LM✓import sglang✓,import megatron.core✓ (withPYTHONPATH=/root/Megatron-LM, same constraint as production),import miles_megatron_plugins.true_on_policy.contracts✓.git/at/sgl-workspace/sglang/is gone; submodule.gitfiles were correctly stripped before COPY.Stacked on
#1168 (step 1: add submodules). Once that merges, this PR's base auto-updates to
main.