Repository navigation
Fix unresolved Git LFS assets in CI wheels - #1215
Conversation
Signed-off-by: Jiwen Cai <jiwenc@nvidia.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (2)
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review. 📝 WalkthroughWalkthroughThe Ubuntu build and editable-install validation checkouts now enable Git LFS. The Windows checkout also enables Git LFS and retains recursive submodule checkout. Priority: ➖ Normal Estimated code review effort: 1 (Trivial) | ~4 minutes Merge Risk: ⚪ Minimal · up to The listed build workflows now fetch Git LFS assets, and no concrete merge-blocking risk is evident. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
c271333 to
d80d3ea
Compare
There was a problem hiding this comment.
Summary
Turns on LFS for the three checkouts that build wheels, so controller-guide-*.png get packaged as real images and not as 132-byte pointer files. CI is green. I pulled the arm64/py3.12 wheel from the #1216 run: both PNGs are in it, about 1.5 MB each, with a valid PNG header. Good to merge as the hotfix. The two suggestions below can be follow-ups.
Legend: 🚫 BLOCKER = merge-blocking | 💡 SUGGESTION = meaningful improvement | 🧹 NIT = precise small correction
| Finding | |
|---|---|
| 🚫 | none |
| 💡 | .github/workflows/build-ubuntu.yml:297 - Nothing checks that a built wheel contains no LFS pointers. The bad 1.5.166rc1 wheel passed every CI gate and was published. Any checkout without LFS (a new workflow, a local build without git-lfs, pip install git+…) would ship a broken wheel again without anything failing. Add a step before upload that fails if any file in install/wheels/*.whl starts with version https://git-lfs.github.com/spec/v1. |
| 💡 | .github/workflows/build-ubuntu.yml:48 (also L325, windows L40) - lfs: true downloads every LFS object in the repo, about 80 MB of docs images, GIFs and meshes. These two PNGs (3 MB) are the only LFS files that end up in a wheel. With 14 checkouts per run, that is roughly 1.1 GB of LFS bandwidth per run, and every merge to main runs CI twice because of the release/1.6.x sync. If the LFS quota runs out, every one of these checkouts fails. Either drop lfs: true and run git lfs pull --include="src/python/**", or exempt these two PNGs in .gitattributes, as is already done for webxr_client/src/icons/*.svg. The .gitattributes exemption also means no builder needs LFS for them. |
| 🧹 | none |
Actionables (copy-paste-ready for implementation agents)
Validate these suggestions in context before applying them. Skip anything already addressed, incorrect, or not worth the churn.
.github/workflows/build-ubuntu.yml:297- Before "Upload wheel artifacts", add a step that unzips eachinstall/wheels/*.whland fails if any member starts withversion https://git-lfs.github.com/spec/v1..gitattributes- Addsrc/python/*/viz/robot/controller-guide-*.png -filter binary, re-add both PNGs as regular blobs, and removelfs: truefrom the three build checkouts.
The 1.5.166rc1 wheel contained Git LFS pointer text instead of the controller-guide PNGs, causing Pillow to fail during SO-101 XR teleoperation.
Enable
lfs: trueon the Ubuntu, pip, and Windows build checkouts so packaging receives the actual assets. This change adds only three workflow lines.Validation: full pre-commit suite passes. The local ARM64 visualization wheel was rebuilt; both PNGs decode and their sizes match
RECORD.Pending: install the corrected CI wheel on Orin and rerun the original
lerobot-teleoperatecommand with the robot twin enabled.Summary by CodeRabbit