[AMD] ci: resolve the nightly image tag with git ls-remote instead of fetching tags - #36378
Open
michaelzhang-ai wants to merge 3 commits into
Open
michaelzhang-ai wants to merge 3 commits into
michaelzhang-ai wants to merge 3 commits into
Conversation
The AMD container scripts only need the release tag *name* to spell the nightly rocm/sgl-dev image, so `git ls-remote` is enough and transfers no objects into the depth-1 CI checkout. Ordering is imported from scripts/release/get_version_tag.py so the tag matches the one the nightly release workflow published the image under. Co-authored-by: quitenode <quitenode@users.noreply.github.com>
Both container scripts ran `git fetch --tags origin` on a depth-1 checkout just to learn the latest release tag. That pulled ~170MB of tag objects and history no later step reads, and could not make `git describe` work from a single-commit HEAD anyway, so the editable install resolved to the same fallback version either way. Read the tag name off the remote instead, and log how long the lookup took so nightly runs stay measurable. Co-authored-by: quitenode <quitenode@users.noreply.github.com>
michaelzhang-ai
marked this pull request as ready for review
August 25, 2026 21:00
The AMD workflows run these scripts against whatever ref they check out -- amd-aiter-scout.yml drives them at amd/aiter-ci, and release branches get cherry-picks -- so the helper has to survive a checkout where scripts/release/get_version_tag.py is not where #35196 put it. On release/v0.5.17 it still lives at python/tools/get_version_tag.py, and loading it by the new path raised FileNotFoundError, which the caller's `|| true` swallowed into a bare traceback. Report the missing file and return no tag instead, so the caller keeps its default the way it did before. Guessing an order without the shared helper could name an image the nightly never published, so it deliberately does not fall back. Co-authored-by: quitenode <quitenode@users.noreply.github.com>
This was referenced Sep 1, 2026
Jiminator
deleted the
cursor/amd-ci-resolve-image-tag-with-ls-remote-868a
branch
September 14, 2026 04:41
alexnails
restored the
cursor/amd-ci-resolve-image-tag-with-ls-remote-868a
branch
September 14, 2026 05:40
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.
Motivation
Both AMD container-start scripts ran
git fetch --tags originfor one reason: to learn the name of the latest release tag so they could spell the nightly image,rocm/sgl-dev:v0.5.18-rocm720-mi30x-<date>. They need the tag name and nothing the tag points at, but the fetch pulls every tag object and the history behind it into a checkout thatactions/checkoutcreated at depth 1. That is the single most expensive thing these scripts do before the image pull, and every AMD job pays it — a nightly runs ~90 of them concurrently against the same remote.This is the tag-lookup half of #34487, split out on its own as requested. The image-tarball cache measured in that PR is not included here; the benchmark came out negative for it.
Modifications
scripts/ci/amd/amd_ci_latest_release_tag.py, which lists tag names withgit ls-remote --tags --refs origin 'v*.*.*'and picks the highest. Tag ordering is imported fromscripts/release/get_version_tag.pyrather than reimplemented, so this resolves to the same tag the nightly release workflow published the image under (stable and post above rc). If the remote is unreachable it falls back to local tags; if it cannot order tags at all it returns nothing so the caller keeps its hard-coded default.amd_ci_start_container.shandamd_ci_start_container_disagg.shat that helper. Both scripts carried a byte-identical copy of the old block and have been changed in lockstep by every prior fix to it ([AMD] Fix nightly version tag selection #23644, [Chore] Move version tag helper to release scripts #35196), so leaving one behind on the fetch would have half-solved the problem and invited drift.test/registered/unit/tools/test_amd_ci_latest_release_tag.py, covering the helper plus a guard that neither container script goes back to fetching tags.No workflow YAML changes, and the flags both scripts accept are unchanged, so a workflow file from any ref still drives a checkout from any other ref. The helper and its caller always travel together in the same checkout.
scripts/release/get_version_tag.pyis untouched: it is what setuptools-scm invokes viagit_describe_commandin six pyprojects, and nothing here needs it to change.Accuracy Tests
N/A — CI setup plumbing, no model or kernel code.
Speed Tests and Profiling
Measured on a fresh
git clone --depth 1, which is whatactions/checkout@v4leaves the AMD jobs (1 commit, no tags, shallow):.gitsize aftergit ls-remote --tags --refs origin 'v*.*.*'git fetch --tags originSo the fetch transferred ~168MB of objects to answer a question
ls-remoteanswers in one round trip and 150 lines of output. The 29s above is a well-provisioned VM with a fast link to GitHub; a second run againstrelease/v0.5.18took 125s, in line with the 136s median measured on the AMD runners in #34487, where the worstls-remotelookup across all those runs was 20s.The end-to-end helper on a depth-1 checkout with no local tags returns in 0.29s:
The dropped fetch changes no version that anything downstream reads.
get_version_tag.pyin describe mode — the mode setuptools-scm actually calls — fails identically before and after the fetch, because a single-commit HEAD has no common ancestor with any release tag:Either way setuptools-scm falls back to the same tagless dev version. The only thing the fetch enabled was
--tag-onlyprintingv0.5.18, which is exactly what the new helper returns.Branch coverage
These scripts do not only run on
main.amd-aiter-scout.ymldrives all four AMD workflows atref: amd/aiter-ci, the PR workflows check outinputs.pr_head_sha, and release branches get cherry-picks. Becausegit ls-remoteasks the remote for its refs, the answer does not depend on which ref is checked out, and none of the AMD workflows overriderepository:ortoken:— sooriginis alwayssgl-project/sglang. Old and new paths were compared on a depth-1 clone of each branch:ls-remotehelperfetch --tags+ old helpermainv0.5.18(1s)v0.5.18(35s)amd/aiter-ci(aiter scout)v0.5.18(0s)v0.5.18(14s)release/v0.5.18v0.5.18(0s)v0.5.18(125s)release/v0.5.17Note this resolves the newest release tag on every branch, including release branches, exactly as the fetch did; picking a branch-scoped version was never the behavior and is not changed here.
That last row is a gap the branch sweep found and this PR now handles.
release/v0.5.17predates #35196, so its ordering helper is still atpython/tools/get_version_tag.py; loading it from the new path raisedFileNotFoundError, and the caller's|| trueturned that into a bare traceback. The helper now reports the missing file and returns no tag, so the caller keeps its default the way it did before:It deliberately does not guess an order in that case: without the shared helper,
v0.5.10rc0sorts abovev0.5.10understrverscmp, which is the bug #35196's helper exists to avoid, and that would name an image the nightly never published.nightly-test-amd-miles-rocm720.ymlpasses--custom-image, so it discards the resolved tag entirely — it was paying the full fetch for a value it never used, and now pays ~1s. Skipping the lookup outright for--custom-imagewould mean reordering the argument parsing that builds the default base tags, which is not worth the churn for a second.Verification
Fallback and failure paths, exercised on a depth-1 shallow checkout:
v0.5.18in 0.29sWarning: No version tags resolved; using default v0.5.5, exit 0 underset -euo pipefailAlso run:
git fetch --tagsis reintroduced into either script;scripts/lint/check_registered_tests.pyandcheck_no_bare_pytest_main.py;bash -non both container scripts;pre-commiton all four files.Checklist
Review and Merge Process
CI States
Latest PR Test (Base): ❌ Run #34829113888
Latest PR Test (Extra): ❌ Run #34829113577
Latest PR Test (AMD ROCm 10): ❌ Run #34829113943