Updated devcontainer, seamless switching between build environments#759
Updated devcontainer, seamless switching between build environments#759aristarkhovNV wants to merge 2 commits into
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR updates devcontainer configuration by pinning feature lock versions/digests, adding the patchelf apt package, setting a cmake.buildDirectory VS Code setting, and ignoring the new build directory in .gitignore. It also reworks the CMake Python build venv logic to detect a usable existing venv via interpreter version checks, and reports symlink-aware errors when the interpreter is broken, replacing the prior stale-venv detection. Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cmake/SetupPython.cmake`:
- Around line 169-189: The fatal error in SetupPython.cmake is incorrectly
appending “(target does not exist)” whenever _venv_python is a symlink, even
though _venv_usable may be false for other reasons. Update the message
construction near the _venv_symlink_detail block so it only reports a missing
symlink target when the target is actually absent, and otherwise describe the
interpreter failure generically; keep the logic around _venv_usable, IS_SYMLINK,
and file(READ_SYMLINK) aligned with the real failure mode.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: c1222644-fd49-40b2-bb30-032124bac293
📒 Files selected for processing (4)
.devcontainer/devcontainer-lock.json.devcontainer/devcontainer.json.gitignorecmake/SetupPython.cmake
jiwenc-nv
left a comment
jiwenc-nv
left a comment
There was a problem hiding this comment.
I think the devcontainer workflow probably would be useful for some testing. @aristarkhovNV can you add a new rst file under https://nvidia.github.io/IsaacTeleop/main/getting_started/build_from_source/ to describe how to use the devcontainer? Once we have that we can ask QA to draft a test case for it.
tbh, i have never successfully went over the devcontainer we had in the repo... once you had the doc I can help go over it with you as well. |
2 participants
Description
Make container↔host dev switching seamless for the build tree. Previously a single shared build/ directory was reused across the dev container and the host, whose repo mount paths and uv-managed Python locations differ. This left the NumPy build venv's
bin/pythonas a dangling symlink andFetchContent _depssub-caches pointing at the other environment's paths, surfacing as a confusing "stale venv" CMake error on every switch. The dev container now builds into its ownbuild-devcontainer/directory (viacmake.buildDirectory) so neither environment's baked-in absolute paths can collide with the other's.SetupPython.cmakenow detects the dangling bin/python symlink, names its missing target, explains it came from another environment, and directs the user to remove the build directory (or run cmake --fresh) and reconfigure.Fixes #(issue)
Type of change
Testing
Checklist
SKIP=check-copyright-year pre-commit run --all-filesgit commit -s) per the DCOSummary by CodeRabbit