[Docs/Workflow] Fix installation bugs/issues for conda/uv/venv - #7466
Conversation
github-actions
Bot
added
documentation
Greptile SummaryThis PR rejects downloaded Isaac Sim packages used with conda, uv, or venv interpreters while retaining uv support for marked live source builds. It also aligns installation documentation and skills with the breaking workflow change and adds launcher and CLI regression tests.
Confidence Score: 4/5The inactive default uv-environment case should be fixed before merging because it prevents downloaded-package users from switching back to the supported bundled Python after deactivation. The Linux launcher now hard-fails whenever Files Needing Attention: isaaclab.sh Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
Start[Invoke Isaac Lab] --> LocalSim{_isaac_sim present?}
LocalSim -- No --> Pip[Use pip or resolved Python]
LocalSim -- Yes --> Marker{Source-build marker present?}
Marker -- Yes --> Venv[Allow active uv environment]
Marker -- No --> Env{Virtual environment selected?}
Env -- Yes --> Reject[Reject unsupported combination]
Env -- No --> Bundled[Use Isaac Sim bundled Python]
Reviews (1): Last reviewed commit: "Fix downloaded Isaac Sim virtual environ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| # Downloaded Isaac Sim packages must run with their bundled Python. Live source | ||
| # builds created by --isaacsim_source carry a marker and support active environments. | ||
| if [ -d "$ISAACLAB_PATH/_isaac_sim" ] && [ ! -f "$ISAACLAB_PATH/_isaac_sim/.isaaclab_source_build" ]; then | ||
| if [ -n "$VIRTUAL_ENV" ] || [ -n "$CONDA_PREFIX" ] || [ -f "$ISAACLAB_PATH/env_isaaclab/bin/python" ]; then |
There was a problem hiding this comment.
When a downloaded Isaac Sim package is linked and a previously created env_isaaclab/bin/python remains after deactivation, this condition treats the inactive directory as a selected virtual environment and exits, preventing every isaaclab.sh command from using the supported bundled Python until the directory is manually removed or renamed.
![isaaclab-review-bot[bot]](https://avatars.githubusercontent.com/u/157846462?s=60&v=4){kind=small}
There was a problem hiding this comment.
Isaac Lab Review Bot
The source-build marker and enforcement paths are coherent, but the Linux launcher guard mishandles two existing-install cases: it prevents the command needed to mark a pre-existing source build, and it rejects an inactive local env_isaaclab despite the documented deactivation guidance.
- Design and architecture: Using
.isaaclab_source_buildto distinguish live source trees from downloaded packages is consistent across the CLI and launchers. However, placing the Linux guard before argument dispatch means an unmarked legacy source link cannot be repaired throughisaaclab.sh --isaacsim_source. - API: The breaking workflow change is documented with migration guidance, but the Linux behavior does not fully match that guidance: merely deactivating
env_isaaclabis insufficient while its interpreter remains on disk. The launcher should avoid selecting that inactive environment for downloaded packages, or the documentation must explicitly require its removal. - Implementation: The marker producer and Python-side consumers are aligned and covered by regression tests. The Linux launcher needs targeted handling for
--isaacsim_sourceand for its implicitenv_isaaclab/bin/pythonselection so users can recover legacy source links and use downloaded packages with bundled Python as documented.
Minor fixes needed. Posted 2 actionable findings inline.
Automated review; human maintainers own approval decisions.
|
|
||
| # Downloaded Isaac Sim packages must run with their bundled Python. Live source | ||
| # builds created by --isaacsim_source carry a marker and support active environments. | ||
| if [ -d "$ISAACLAB_PATH/_isaac_sim" ] && [ ! -f "$ISAACLAB_PATH/_isaac_sim/.isaaclab_source_build" ]; then |
There was a problem hiding this comment.
🟡 Warning · Api — Guard blocks re-linking unmarked source builds
The guard runs before argument dispatch, so a source build linked before this change (no marker) plus an active uv/conda env makes every ./isaaclab.sh invocation exit 1 — including --isaacsim_source PATH, the command that writes the marker. The error text then advises removing _isaac_sim, which is wrong for source builds. Exempt --isaacsim_source from the check, or name the re-link command in the message.
| # Downloaded Isaac Sim packages must run with their bundled Python. Live source | ||
| # builds created by --isaacsim_source carry a marker and support active environments. | ||
| if [ -d "$ISAACLAB_PATH/_isaac_sim" ] && [ ! -f "$ISAACLAB_PATH/_isaac_sim/.isaaclab_source_build" ]; then | ||
| if [ -n "$VIRTUAL_ENV" ] || [ -n "$CONDA_PREFIX" ] || [ -f "$ISAACLAB_PATH/env_isaaclab/bin/python" ]; then |
There was a problem hiding this comment.
🟡 Warning · Implementation — Inactive env_isaaclab directory blocks launcher
On Linux the guard also fires when env_isaaclab/bin/python merely exists, even after deactivation, while isaaclab.bat only checks VIRTUAL_ENV/CONDA_PREFIX. A user following the new release note ("use isaaclab.sh … without an active virtual environment") still gets exit 1 and must delete the directory. Either limit rejection to active environments and skip auto-selecting env_isaaclab when _isaac_sim is unmarked, or state the removal requirement in the release note.
|
run-ci |
isaaclab-bot
Bot
added
ci:run-docker
AntoineRichard
left a comment
AntoineRichard
left a comment
There was a problem hiding this comment.
Note
🤖 AI-generated review. This review was produced by Claude Code (Opus 5) at my request and lightly edited by me. Findings 1 and 3 were reproduced by executing the branch's own code; the rest are read-based. Please verify before acting — and push back on anything that looks wrong.
Summary
The core approach is sound: a .isaaclab_source_build marker distinguishes live Isaac Sim source builds from downloaded packages, and downloaded-package + conda/uv/venv is rejected in the CLI and both platform launchers. The nine orphaned installation/include/*.rst pages are removed cleanly. My concerns are that the guard is broader than the stated intent in several dimensions, and that the release note for the breaking change was lost between commits.
Blocking
1. source/isaaclab/isaaclab/cli/utils.py:628 — the guard fires with no active environment.
using_virtual_environment includes _is_virtualenv_python(extract_python_exe()), and extract_python_exe() prefers a repo-local env_isaaclab/.venv over the bundled Kit Python. So a stale env_isaaclab/.venv left in the checkout makes ./isaaclab.sh -p ... hard-exit 1 on a downloaded Isaac Sim even when nothing is activated — and the error message tells the user to do exactly what they already did. Reproduced: SystemExit: 1. This also defeats commit 2's launcher fix, since the launcher hands off to a Python path that then rejects itself.
Suggestion: gate on an active environment (VIRTUAL_ENV / CONDA_PREFIX / UV_PROJECT_ENVIRONMENT) rather than on the resolved interpreter path.
2. docs/source/refs/release_notes.rst:182 — the breaking change ships with no release note.
Commit 1 added the "Downloaded Isaac Sim packages" entry under Breaking Changes; commit 2 deleted it while keeping the behavior change. The net PR therefore ships a breaking workflow change with no release note, contrary to the PR description. Looks like an unintended revert during rebase.
3. isaaclab.bat:19 — the escape hatch is Linux-only.
Commit 2 added the --isaacsim_source passthrough to isaaclab.sh only. On Windows, isaaclab.bat --isaacsim_source PATH is rejected before dispatch, so the documented recovery path (write the marker via a source-build link) is unreachable from an active environment. The two launchers also print contradictory remedies for the same situation.
Should fix
4. isaaclab.sh:18 — --isaacsim_source=/path is not recognized.
The argument scan matches only the bare --isaacsim_source token. The =-joined form is valid argparse and is what many users will type; it still exits 1. Verified by running the branch's script.
5. source/isaaclab/isaaclab/cli/commands/misc.py:167 — pre-existing source builds are misclassified.
The marker is written only by command_build_isaacsim. But index.rst still documents linking a source build by hand (ln -s ${ISAACSIM_PATH} _isaac_sim), and every --isaacsim_source link created before this PR has no marker. Those genuine source builds are now rejected with a message calling them "downloaded packages", and the implied recovery is a full rebuild. Consider detecting a source build structurally (or documenting a one-line touch of the marker in the release notes/migration guidance).
6. isaaclab.sh:27 — the guard blocks non-Python subcommands.
It runs before dispatch, so -h, -f and -i are rejected too. Since conda auto-activates base on many setups, CONDA_PREFIX is set for users who are not in an Isaac Lab environment at all — ./isaaclab.sh -f previously worked and now exits 1. Restricting the guard to the Python-execution paths would avoid this.
Verified clean
- The nine deleted
installation/include/*.rstfiles have no remaining references; thedocs/conf.pyexclude entry and the skill docs were updated correctly. - Container workflows are unaffected:
docker/Dockerfile.basesets noVIRTUAL_ENV/CONDA_PREFIX, andDockerfile.kitlessasserts_isaac_simis absent. --conda/--uv/--isaacsim_sourcedispatch is mutually exclusive, so the new_reject_downloaded_isaac_simcalls cannot shadow the relink command.- The marker check correctly follows the
_isaac_simsymlink for marked source builds.
|
Backported to |
# Description Downloaded Isaac Sim packages linked through `_isaac_sim` were still allowed to select conda, uv, or venv Python environments even though that workflow is no longer supported. The CLI warnings and launcher behavior also contradicted the updated installation documentation. This PR: - rejects downloaded Isaac Sim packages combined with conda, uv, or venv environments in the CLI and platform launchers; - marks live Isaac Sim source builds so their supported uv workflow remains available; - removes the nine orphaned installation include pages and cleans up stale references; - documents the breaking workflow change in the 3.0 release notes and package changelog; - synchronizes the installation skill guidance; and - adds regression coverage for downloaded-package and source-build behavior. No additional dependencies are required. ## Type of change - Bug fix - Breaking change - Documentation update ## Release backport - [x] <!-- backport-active-release --> Backport this pull request to the active release branch after it merges into `develop` ## Screenshots Not applicable. ## Validation - `uv run --frozen python -m pytest --confcutdir=source/isaaclab/test/cli source/isaaclab/test/cli/test_env_commands.py source/isaaclab/test/cli/test_install.py source/isaaclab/test/cli/test_misc_commands.py` — 34 passed - `uv run --isolated --extra test -- make -C docs current-docs` — passed without warnings - `uv run isaaclab -f` — passed - `uv run --no-project python tools/skills/cli.py check` — passed - `uv run --frozen python tools/changelog/cli.py check upstream-develop --include-worktree` — passed ## Checklist - [x] I have read and understood the contribution guidelines - [x] I have run the pre-commit checks - [x] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [x] I have added tests that prove the fix is effective - [x] I have added a changelog fragment for the touched package - [x] My name already exists in `CONTRIBUTORS.md` (cherry picked from commit 37f6cba)
…m only in Kit images (#7540) ## Summary Every first CI run of a new commit on a dependency-stable branch has failed since #7405 landed (develop included): `Build Base Docker Image` dies in 20 s and skips the 17 test jobs behind it, and `Multi-GPU training smoke (kit-less)` exits before pytest. Both are CI plumbing, fixed here. # Description ## 1. Build Base Docker Image #7405 added a "Verify image invariants" step that runs `docker image inspect` on the commit tag after `ecr-build-push-pull`. The action leaves an image on the runner only after a full build or an exact-tag pull. On a deps-cache hit it just aliases the commit tag onto the `deps-<hash>` manifest in ECR and pulls nothing, so the inspect fails: ``` Error response from daemon: No such image: isaac-lab-ci:develop-8ca264bd8e5528d7d9b7b6f58a9d8cd051c5933a ``` develop runs 33733992373, 33740164999 and 33781460422 fail this way; PRs pass only after a deps change (full build) or on a re-run of an already built head (exact-tag pull). The step also ran too late to be a gate. The action pushes the commit tag and the deps tag inside itself, so an image that fails the check is already published. That is not hypothetical: on `newton-world-prefixes` (run 33735349483) `deps-6aebe721c56a91f7` was pushed at 09:05:07 and the invariant failed at 09:05:11, leaving an image whose Isaac Sim extensions cannot load in the shared cache. Any later run whose install inputs hash the same inherits it. Fix: move the assertion into the action, before every push, behind a `verify-command` input that the base image job passes. A failure now publishes nothing, so the next run rebuilds and fails again rather than serving the bad image from the cache. Skipping it on a cache hit is then correct by construction — every published image passed when it was built, and the deps hash covers exactly the install inputs the invariant asserts against. ## 2. Multi-GPU training smoke (kit-less) `run_tests.sh` bind-mounts nine writable runtime directories under `/isaac-sim/…` into every container, so `/isaac-sim` exists as a directory in the kit-less image too, and the container script then runs `ln -s /isaac-sim _isaac_sim` unconditionally. #7466 taught `isaaclab.sh` to treat a `_isaac_sim` directory without a source-build marker as a downloaded Isaac Sim and to refuse it next to an active venv; the kit-less image sets `VIRTUAL_ENV=/opt/isaaclab-venv`, so every run stops with: ``` [ERROR] Downloaded Isaac Sim packages cannot be combined with a Python virtual environment. ``` #7405 reworked the same check (venv-on-bundled-Python exemption) and the run-tests script without exempting the kit-less lane. Every PR's smoke run since 11:43Z on 2026-09-03 fails identically. Fix: plant the link only when `/isaac-sim/python.sh` exists. Kit images are unchanged; the kit-less image resolves its interpreter through `VIRTUAL_ENV`, as it did before #7466. ## 3. Audit of every `_isaac_sim` site Every place that creates the link, and how it interacts with the guard: | Site | Creates | Verdict | | --- | --- | --- | | `run-tests/run_tests.sh` | `/isaac-sim` in every image | **Fixed here**: guarded on `python.sh` | | `multi-gpu/multi_gpu_host_launcher.sh` | `/isaac-sim`, unguarded | Correct today: its only caller passes `CI_IMAGE_TAG`, the Kit image. The kit-less lane goes through `run-tests` | | `Dockerfile.base`, `Dockerfile.curobo` | `ISAACSIM_ROOT_PATH`, alongside `VIRTUAL_ENV` | Correct: `UV_PYTHON=${ISAACSIM_ROOT_PATH}/kit/python/bin/python3`, so the venv is on the bundled Python and #7405's exemption applies | | `Dockerfile.kitless` | never; asserts `test ! -e "${ISAACLAB_PATH}/_isaac_sim"` | Correct | | `cli/commands/misc.py` (`--isaacsim_source`) | source-build link | Correct: writes `.isaaclab_source_build` | | `docs/.../installation/index.rst` ×3 | user-run `ln -s` | Out of scope, see below | ## 4. Verification - `docker/test/test_container_profiles.py` gains two regression tests — one pinning the kit-less guard, one pinning that verification precedes both push steps. Each fails without its fix. 15 pass in that file, 20 with `test_carb_env_shim.py`. - `yaml.safe_load` on both YAML files, `bash -n` on the script, pre-commit on the changed files. - This PR touches `.github/actions/{ecr-build-push-pull,run-tests}/` and `build.yaml`, which the detect-changes patterns of both workflows match, so its own CI exercises both paths: the base image job takes the deps-cache-hit path (no dependency change here) and the kit-less smoke job runs. ## 5. Out of scope Three checkers implement the same "downloaded Isaac Sim vs. virtual environment" rule and #7405 updated only two, leaving user-facing divergences that this PR does not touch: - `isaaclab.bat` has neither the venv-on-bundled-Python exemption nor the `--isaacsim_source` escape, so on Windows the remedy the Linux error message recommends is still rejected. - `cli/commands/envs.py::_reject_downloaded_isaac_sim` has no exemption either, so `isaaclab -c/-u` rejects a venv created on the bundled Python that `isaaclab.sh` accepts. - The docs link a *source build* with a bare `ln -s`, which leaves no `.isaaclab_source_build` marker, so the same tree is treated as "downloaded" unless linked via `--isaacsim_source`. `isaaclab.sh` also lacks the `python.sh` precondition its Python twin (`cli/utils.py`) has, but that is deliberate: `test_launcher_rejects_downloaded_isaac_sim_with_active_environment` pins the fail-closed behaviour on a bare directory. That is why the fix here is in the CI script. ## Type of change - Bug fix (non-breaking change which fixes an issue) ## Release backport - [x] <!-- backport-active-release --> Backport this pull request to the active release branch after it merges into `develop` ## Checklist - [x] I have read and understood the [contribution guidelines](https://isaac-sim.github.io/IsaacLab/main/source/refs/contributing.html) - [x] I have run the [`pre-commit` checks](https://pre-commit.com/) with `./isaaclab.sh --format` - [x] I have made corresponding changes to the documentation (the action README documents the new output) - [x] My changes generate no new warnings - [ ] Tests: CI-only change; the workflows' own runs on this PR are the test - [ ] Changelog: no source package changed - [x] I have added my name to the `CONTRIBUTORS.md` or my name already exists there
# Description Fixes two release CI failures without backporting #7405. ## Kitless multi-GPU tests Backports only the release-relevant multi-GPU test fix from #7540. PR #7466 is present on `release/3.0.0`, so `isaaclab.sh` rejects a downloaded-Isaac-Sim link alongside the kitless image's active virtual environment. The test runner currently creates `_isaac_sim -> /isaac-sim` unconditionally because runtime mounts make `/isaac-sim` exist even in the kitless image. This change creates the link only when `/isaac-sim/python.sh` exists. Kit-based test images retain their existing behavior, while the kitless multi-GPU smoke test resolves Python from `VIRTUAL_ENV`. This intentionally excludes #7540's image-invariant/cache changes and all of #7405, which was not backported to the release branch. Original PR: #7540 ## Documentation link check The automatic backport's link check failed twice because `www.ros.org` and `docs.ros.org` return HTTP 403 to the GitHub runner. Both links remain valid. The link-check workflow already excludes known crawl blockers, so this adds a single `ros.org` exclusion covering both hosts without changing the documentation destinations or weakening checks for other domains. Failed run: https://github.com/isaac-sim/IsaacLab/actions/runs/33828925399 ## Type of change - Bug fix (non-breaking change which fixes an issue) ## Release backport Not applicable: this PR directly targets `release/3.0.0`. ## Validation - Added a regression test that failed against the unmodified release branch and passes with the multi-GPU fix. - `uv run --no-project --with pytest --with pyyaml python -m pytest docker/test/test_container_profiles.py -q` - 14 passed. - `bash -n .github/actions/run-tests/run_tests.sh` - passed. - Parsed `.github/workflows/check-links.yml` with PyYAML and verified the exclusion matches both failing URLs. - Applicable file-scoped pre-commit hooks - passed. The branch-wide changelog hook is not applicable to a release backport because it compares historical release differences against `develop`. - The canonical `uv run isaaclab -f` command cannot resolve this branch's Linux/Windows-only lockfile on macOS; the equivalent file-scoped hooks were run directly. ## Checklist - [x] I have read and understood the contribution guidelines. - [x] I have run the applicable pre-commit checks. - [x] Documentation destinations remain unchanged. - [x] My changes generate no new warnings. - [x] I have added a regression test for the multi-GPU behavior. - [x] No changelog fragment is required because no source package changed. - [x] The contributors already exist in `CONTRIBUTORS.md`. --------- Co-authored-by: hujc <jichuanh@nvidia.com>
3 participants
Description
Downloaded Isaac Sim packages linked through
_isaac_simwere still allowed to select conda, uv, or venv Python environments even though that workflow is no longer supported. The CLI warnings and launcher behavior also contradicted the updated installation documentation.This PR:
No additional dependencies are required.
Type of change
Release backport
developScreenshots
Not applicable.
Validation
uv run --frozen python -m pytest --confcutdir=source/isaaclab/test/cli source/isaaclab/test/cli/test_env_commands.py source/isaaclab/test/cli/test_install.py source/isaaclab/test/cli/test_misc_commands.py— 34 passeduv run --isolated --extra test -- make -C docs current-docs— passed without warningsuv run isaaclab -f— passeduv run --no-project python tools/skills/cli.py check— passeduv run --frozen python tools/changelog/cli.py check upstream-develop --include-worktree— passedChecklist
CONTRIBUTORS.md