ci: restore build cache for PR CI, with A2 wheel fix#483
Merged
iforgetmyname merged 3 commits intoMay 30, 2026
Conversation
Contributor
|
Note Gemini is unable to generate a review for this pull request due to the file types involved not being currently supported. |
iforgetmyname
previously approved these changes
May 18, 2026
JavaPythonAIForBAT
added a commit
to JavaPythonAIForBAT/sgl-kernel-npu
that referenced
this pull request
May 18, 2026
Per review on sgl-project#483: the cache key should not embed a CANN version string. The container's CANN version is already governed solely by the `image:` field of each job, so duplicating it in the key only creates a second source of truth that can drift (e.g. an image bump PR that forgets to update the key). When a forced cache invalidation is genuinely needed, bump the v2 -> v3 prefix instead. Removes the `cann8.5.0-` segment from all 5 cache keys (3 in pr-test-npu.yml, 2 in push_build_cache.yml). Both sides update in lockstep so push-side writes still match PR-side reads. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
JavaPythonAIForBAT
added a commit
to JavaPythonAIForBAT/sgl-kernel-npu
that referenced
this pull request
May 25, 2026
Follow-up to 0ab9915 and per review on sgl-project#483. The previous commit removed the cann version segment from the cache key entirely, which was wrong: BUILD_HASH only covers source files, so an image bump that changes only the container CANN version (without touching csrc/cmake/ python) would silently reuse stale .so artifacts and produce an ABI mismatch. Reviewer's intent was to avoid the hardcoded literal, not to drop the version dimension. Restore the version segment, but derive it at runtime from /usr/local/Ascend/ascend-toolkit/latest/version.cfg inside the container, so the YAML no longer contains the literal "8.5.0" and the cache key still tracks the actual CANN version of whichever image the job is running on. Falls back to "unknown" if the version file is unreadable; the echo in the step log makes the resolved value visible per run so we can spot regressions in image layout early. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
Author
关于 CI 失败test_internode_a2 失败原因当前 这是 Cache key 改动已完成已在 commit
这样既满足了"不硬编码版本"的要求,又保留了 cache key 中的版本维度,防止跨 CANN 版本的缓存误用。 关于 internode 测试失败,建议单独提 issue 或 PR 来修复 deepep 构建脚本的 git repo 依赖问题。 |
Follow-up to 0ab9915 and per review on sgl-project#483. The previous commit removed the cann version segment from the cache key entirely, which was wrong: BUILD_HASH only covers source files, so an image bump that changes only the container CANN version (without touching csrc/cmake/ python) would silently reuse stale .so artifacts and produce an ABI mismatch. Reviewer's intent was to avoid the hardcoded literal, not to drop the version dimension. Restore the version segment, but derive it at runtime from /usr/local/Ascend/ascend-toolkit/latest/version.cfg inside the container, so the YAML no longer contains the literal "8.5.0" and the cache key still tracks the actual CANN version of whichever image the job is running on. Falls back to "unknown" if the version file is unreadable; the echo in the step log makes the resolved value visible per run so we can spot regressions in image layout early. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
JavaPythonAIForBAT
force-pushed
the
ci/restore-build-cache
branch
from
May 29, 2026 04:13
aca45f4 to
cb59842
Compare
iforgetmyname
approved these changes
May 30, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
#479 rolled back the PR-side cache wiring after #469's cache scheme broke
test-build-deepep-a2. The push-side workflow file (push_build_cache.yml) was kept on main but still has the original bugs. This PR reintroduces the cache for PR CI with both bugs fixed and the CANN-version mismatch resolved.push_build_cache.ymlbash build.sh -a deepep2. The bare invocation defaults toBUILD_DEEPEP_OPS=ON+SOC_VERSION=Ascend910_9382, i.e. an A3 wheel — so the A2 cache entry used to contain an A3 wheel, which is what causedtest-build-deepep-a2to fail when restoring from the cache.Build (a3)/Build (a2). The previousdeepep2 for a3was self-contradictory.v1→v2so already-poisoned v1 entries on main are not reused.pr-test-npu.ymlGet build hash/Restore build cachetotest-all-build,test-build-deepep-a3,test-build-deepep-a2.Prepare Deepepskipsbash build.shand installs the cached wheel whencache-hit; otherwise falls back to the originalprepare_deepep_in_container.shpath.v2and the CANN tag in the key (cann8.5.0) matches bothpush_build_cache.yml's container andpr-test-npu.yml's containers on current main (after rollback build cache and cann 9.0.0 #479).No container images, test definitions, or other workflows are touched.
Test plan
prepare_deepep_in_container.sh, and succeedmainpush,push_build_cache.ymlpopulates v2 entries for both A3 and A2 (note: A2 entry is now an actual A2 wheel)test-build-deepep-a2installs the A2 wheel and tests passtest-all-build/test-build-deepep-a3cache reuse (both share the a3 cache key, both install onlydeep_ep*.whl)🤖 Generated with Claude Code