Skip to content

ci: restore build cache for PR CI, with A2 wheel fix#483

Merged
iforgetmyname merged 3 commits into
sgl-project:mainfrom
JavaPythonAIForBAT:ci/restore-build-cache
May 30, 2026
Merged

ci: restore build cache for PR CI, with A2 wheel fix#483
iforgetmyname merged 3 commits into
sgl-project:mainfrom
JavaPythonAIForBAT:ci/restore-build-cache

Conversation

@JavaPythonAIForBAT

Copy link
Copy Markdown
Contributor

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.yml

  • A2 build now runs bash build.sh -a deepep2. The bare invocation defaults to BUILD_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 caused test-build-deepep-a2 to fail when restoring from the cache.
  • Rename Build step labels to Build (a3) / Build (a2). The previous deepep2 for a3 was self-contradictory.
  • Bump cache key v1v2 so already-poisoned v1 entries on main are not reused.

pr-test-npu.yml

  • Re-add Get build hash / Restore build cache to test-all-build, test-build-deepep-a3, test-build-deepep-a2.
  • Prepare Deepep skips bash build.sh and installs the cached wheel when cache-hit; otherwise falls back to the original prepare_deepep_in_container.sh path.
  • Cache keys use v2 and the CANN tag in the key (cann8.5.0) matches both push_build_cache.yml's container and pr-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

  • First PR after merge: all three NPU jobs cache-miss on v2, fall back to prepare_deepep_in_container.sh, and succeed
  • After a subsequent main push, push_build_cache.yml populates v2 entries for both A3 and A2 (note: A2 entry is now an actual A2 wheel)
  • A later PR run hits the v2 cache; test-build-deepep-a2 installs the A2 wheel and tests pass
  • No regression in test-all-build / test-build-deepep-a3 cache reuse (both share the a3 cache key, both install only deep_ep*.whl)

🤖 Generated with Claude Code

@gemini-code-assist

Copy link
Copy Markdown
Contributor

Note

Gemini is unable to generate a review for this pull request due to the file types involved not being currently supported.

iforgetmyname
iforgetmyname previously approved these changes May 18, 2026
Comment thread .github/workflows/pr-test-npu.yml Outdated
Comment thread .github/workflows/pr-test-npu.yml Outdated
Comment thread .github/workflows/pr-test-npu.yml Outdated
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>
@JavaPythonAIForBAT

Copy link
Copy Markdown
Contributor Author

关于 CI 失败

test_internode_a2 失败原因

当前 multi-node (test_internode_a2, 2, tests/python/deepep/test_internode_a2.py) / test_internode_a2 失败的根本原因是:

fatal: not a git repository (or any of the parent directories): .git

这是 internode.yml workflow 在构建 deepep 时试图更新 git submodule(third-party/pto-isa),但当前工作目录不是 git repo 导致的。这个问题与本 PR 的 cache key 改动无关,是预先存在的 bug。

Cache key 改动已完成

已在 commit aca45f4 中实现:

  • ✅ 移除了 YAML 中的硬编码 cann8.5.0- 字面量
  • ✅ 在运行时从容器内 /usr/local/Ascend/ascend-toolkit/latest/version.cfg 动态派生 CANN 版本
  • ✅ 所有 5 个 cache key(pr-test-npu.yml × 3,push_build_cache.yml × 2)都已更新
  • ✅ 版本来源现在完全由 image: 字段控制,消除了第二份真相源

这样既满足了"不硬编码版本"的要求,又保留了 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>
@iforgetmyname
iforgetmyname merged commit 861a5c5 into sgl-project:main May 30, 2026
7 checks passed
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