[Workflow] Add isaacsim source install uv workflow - #6762
Conversation
github-actions
Bot
added
documentation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if build_dir.is_dir(): | ||
| print_info(f"Using the existing Isaac Sim build in {build_dir}.") | ||
| print_info(f"To rebuild, run {build_script} yourself before this command.") | ||
| else: | ||
| print_info("Building Isaac Sim from source. This takes a while...") | ||
| run_command([str(build_script)], cwd=isaacsim_root) |
There was a problem hiding this comment.
When an interrupted, stale, or wrong-platform build has already created _build, this branch treats it as complete and skips build.sh, causing packaging to fail or produce unusable wheels. Check for the expected platform release artifacts rather than only the top-level directory.
| ] | ||
| # Isaac Sim (PhysX backend); co-resolves with the base install via the [tool.uv] conflicts table. | ||
| isaacsim = ["isaacsim[all,extscache]==6.0.0.1"] | ||
| isaacsim-local = ["isaacsim[all,extscache]>=6.0.0.1"] |
There was a problem hiding this comment.
The new isaacsim-local extra and its conflict metadata were added without regenerating the tracked uv.lock. A clean checkout using locked or frozen resolution therefore rejects the stale lockfile instead of running the documented local-wheel workflow.
Greptile SummaryAdds an automated uv-based Isaac Sim source-build workflow.
Confidence Score: 2/5This PR should not merge until partial builds are no longer mistaken for completed builds and the uv lockfile includes the new extra. The automated path can skip compilation for an incomplete Files Needing Attention: source/isaaclab/isaaclab/cli/commands/misc.py, pyproject.toml, uv.lock Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart LR
A["isaaclab --isaacsim_source PATH"] --> B{"_build exists?"}
B -- No --> C["Run build script"]
B -- Yes --> D["Reuse build output"]
C --> E["Create Python wheels"]
D --> E
E --> F["Link _isaac_sim_wheels"]
F --> G["uv lock --upgrade-package isaacsim"]
G --> H["uv run --extra isaacsim-local"]
Reviews (1): Last reviewed commit: "Add isaacsim source install pure uv" | Re-trigger Greptile |
![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
Summary
Adds a --isaacsim_source CLI workflow that builds Isaac Sim from a source checkout, packages it as wheels, links them as _isaac_sim_wheels, re-locks isaacsim against them, plus a new isaacsim-local extra and a rewritten source-build documentation section. The direction is good and the docs are a clear improvement over the removed source_details.inc, but the resolver-side changes look incomplete: isaacsim-local carries the same isaacsim[all,extscache] dependency as isaacsim yet only declares a conflict with isaacsim itself, and the tracked uv.lock is not regenerated for the new extra. The CLI helper also has a few silent-fallback paths (relative UV_FIND_LINKS with no fixed working directory, treating any _build/ directory as a completed build) that can make the command report success while the user still runs the published Isaac Sim.
Architecture impact
Scoped to packaging metadata, the install CLI, and docs; no runtime/simulation API surface is touched. The new extra participates in uv's universal resolution, so its conflict declarations affect every user of the repository, not only those using the source-build path. The CLI gains a new top-level option handled in the existing elif dispatch chain, consistent with the surrounding style.
Test coverage
No tests are added. command_build_isaacsim contains several pure-Python branches (invalid checkout, existing build, empty wheel directory, existing path vs. symlink at the link target, missing uv) that are cheap to cover with tmp_path plus a mocked run_command/shutil.which, and the new --isaacsim_source parsing/dispatch is likewise untested. Per the repository's testing guidance these belong in source/isaaclab/test/cli/ (unit tests on the install CLI), not install_ci/.
Implementation verdict
Significant concerns. Posted 7 actionable findings inline.
Automated comment-only review; human maintainers own approval decisions.
| [{ extra = "isaacsim" }, { extra = "mimic" }], | ||
| [{ extra = "isaacsim" }, { extra = "all" }], | ||
| [{ extra = "isaacsim" }, { extra = "test" }], | ||
| [{ extra = "isaacsim-local" }, { extra = "isaacsim" }], |
There was a problem hiding this comment.
🔴 Critical — isaacsim-local needs the same conflict entries as isaacsim
isaacsim-local resolves the same isaacsim[all,extscache] distribution as the isaacsim extra, and the comment directly above this table records that those pins cannot co-resolve with teleop, ov, viser, mimic, all, or test. Only the isaacsim-local/isaacsim pair is declared here, so uv must still co-resolve isaacsim-local with those extras when producing the universal lock — exactly the combinations already known to be unsatisfiable. This can break uv lock/uv sync/uv run for all users, including the uv lock --upgrade-package isaacsim step this PR itself performs. Mirror the full set of isaacsim conflict pairs.
| [{ extra = "isaacsim-local" }, { extra = "isaacsim" }], | |
| [{ extra = "isaacsim-local" }, { extra = "isaacsim" }], | |
| [{ extra = "isaacsim-local" }, { extra = "teleop" }], | |
| [{ extra = "isaacsim-local" }, { extra = "ov" }], | |
| [{ extra = "isaacsim-local" }, { extra = "viser" }], | |
| [{ extra = "isaacsim-local" }, { extra = "mimic" }], | |
| [{ extra = "isaacsim-local" }, { extra = "all" }], | |
| [{ extra = "isaacsim-local" }, { extra = "test" }], |
| ] | ||
| # Isaac Sim (PhysX backend); co-resolves with the base install via the [tool.uv] conflicts table. | ||
| isaacsim = ["isaacsim[all,extscache]==6.0.0.1"] | ||
| isaacsim-local = ["isaacsim[all,extscache]>=6.0.0.1"] |
There was a problem hiding this comment.
🟡 Warning — New extra added without regenerating the tracked uv.lock
Adding the isaacsim-local optional-dependency group changes the project's extra set, but uv.lock is not part of this PR. The docs added here explicitly treat uv.lock as tracked, so any workflow using uv sync --locked / uv run --locked / --frozen (CI, install tests) will reject the lockfile as out of date, while a plain uv run will rewrite it unexpectedly. Regenerate and commit uv.lock after fixing the conflicts table.
| print_info(f"Open with: xdg-open {index_path}") | ||
|
|
||
|
|
||
| def command_build_isaacsim(source_path: str) -> None: |
There was a problem hiding this comment.
🟡 Warning — Add unit tests for the new source-build command
command_build_isaacsim introduces filesystem mutation and several external-command branches that never require a real build: invalid checkout -> SystemExit(1), existing build -> skip build.sh, empty dist -> SystemExit(1), existing real directory at _isaac_sim_wheels -> SystemExit(1) vs. symlink -> replaced, and missing uv -> warning instead of uv lock. None are covered, so inverting a check or dropping a SystemExit would ship unnoticed. Add tests under source/isaaclab/test/cli/ driving the function with tmp_path and monkeypatched run_command, is_windows, shutil.which, and ISAACLAB_ROOT, asserting the exact command sequence (build.sh, then repo.sh python_package --create, repo.sh comment_archive_deps, repo.sh python_package --wheel) and the uv lock environment. Also cover --isaacsim_source parsing/dispatch in the CLI entry point.
| raise SystemExit(1) | ||
|
|
||
| build_dir = isaacsim_root / "_build" | ||
| if build_dir.is_dir(): |
There was a problem hiding this comment.
🟡 Warning — Existence of _build is not proof of a completed Isaac Sim build
The Isaac Sim build tooling creates _build early (packman links/staging), so an interrupted or failed build.sh leaves the directory behind. On the next invocation this branch prints "Using the existing Isaac Sim build", skips the build entirely, and proceeds to packaging, which then fails with a confusing repo-tool error or produces incomplete wheels — and the advertised retry never rebuilds. Gate on the platform release output produced by a successful build (e.g. _build/linux-x86_64/release / _build/windows-x86_64/release containing isaac-sim.sh/isaac-sim.bat) and fall through to the build script when it is missing.
| # Re-resolve Isaac Sim so the lock file picks the local wheels over the published release. | ||
| if shutil.which("uv") is not None: | ||
| print_info("Re-resolving Isaac Sim from the local wheels...") | ||
| run_command(["uv", "lock", "--upgrade-package", "isaacsim"], env={**os.environ, "UV_FIND_LINKS": find_links}) |
There was a problem hiding this comment.
🟡 Warning — uv lock runs with a relative UV_FIND_LINKS and the caller's working directory
When the symlink is created, find_links is the bare name _isaac_sim_wheels, and this uv lock invocation inherits the caller's CWD. UV_FIND_LINKS with a relative value is resolved against that CWD, so invoking the command from anywhere other than the repository root (a subdirectory, or from the parent directory next to the IsaacSim clone — which the docs' git clone ... && uv run isaaclab --isaacsim_source ./IsaacSim flow encourages) means uv never sees the local wheels and silently re-locks isaacsim from pypi.nvidia.com. Without cwd, it may also target a different uv project entirely. Pin the working directory to ISAACLAB_ROOT and pass the absolute wheel directory; keep the short relative form only for the hint printed to the user (or print an absolute path there too).
| run_command(["uv", "lock", "--upgrade-package", "isaacsim"], env={**os.environ, "UV_FIND_LINKS": find_links}) | |
| run_command( | |
| ["uv", "lock", "--upgrade-package", "isaacsim"], | |
| cwd=ISAACLAB_ROOT, | |
| env={**os.environ, "UV_FIND_LINKS": str(wheel_dir)}, | |
| ) |
| elif args.uv: | ||
| command_setup_uv(args.uv) | ||
|
|
||
| elif args.isaacsim_source: |
There was a problem hiding this comment.
🔵 Suggestion — --isaacsim_source is silently ignored when combined with an earlier flag
This branch sits in the elif chain after --install, --format, --conda, and --uv. Invoking, for example, isaaclab -i --isaacsim_source ./IsaacSim runs only the install and exits without building or linking anything, with no warning; the user discovers it only when uv run --extra isaacsim-local picks the published release. Since this is an option (not a subcommand) that takes a value, either handle the combination explicitly or fail with a clear mutually-exclusive message.
| run_command([str(repo_script)] + repo_args, cwd=isaacsim_root) | ||
|
|
||
| wheel_dir = build_dir / "packages" / "dist" | ||
| wheels = sorted(wheel_dir.glob("*.whl")) if wheel_dir.is_dir() else [] |
There was a problem hiding this comment.
🔵 Suggestion — Wheel check passes on stale or unrelated wheels
wheel_dir.glob("*.whl") matches leftovers from a previous packaging run and succeeds even when the current python_package --wheel step produced no isaacsim meta wheel. The command then reports "Built N wheel(s)" and continues, while uv lock quietly falls back to the published release. Verify the Isaac Sim meta wheel itself is present (e.g. wheel_dir.glob("isaacsim-*.whl")) so a stale directory surfaces as an error rather than a success.
Author uv find-links in pyproject.toml when using isaacsim source install
StafaH
changed the title
AntoineRichard
left a comment
AntoineRichard
left a comment
There was a problem hiding this comment.
[AI review] Requesting changes for one blocking resolver conflict and four user-facing or documentation issues. Verification: ./isaaclab.sh -f passed and 13 targeted CLI/uv-project tests passed. An actual Isaac Sim source build was not run.
| conflicts = [ | ||
| [{ extra = "isaacsim-local" }, { extra = "ov" }], | ||
| [{ extra = "isaacsim-local" }, { extra = "ovphysx" }], | ||
| [{ extra = "isaacsim-local" }, { extra = "isaacsim" }], |
There was a problem hiding this comment.
[AI review][Critical] command_build_isaacsim rewrites isaacsim-local to an exact source-build version, while teleop still requires isaacsim==6.0.1.0. Because this table does not declare isaacsim-local and teleop as conflicting, uv must co-resolve incompatible exact versions during the subsequent universal uv lock, blocking the main workflow. Add the missing conflict pair (or make teleop compatible with the local extra) and add a regression test that locks after applying a representative local-version pin.
| .. code-block:: bash | ||
|
|
||
| git clone https://github.com/isaac-sim/IsaacSim.git | ||
| uv run isaaclab --isaacsim_source ../IsaacSim |
There was a problem hiding this comment.
[AI review][Important] This sequence cannot be followed from the stated Isaac Lab root: the clone command creates ./IsaacSim, but this command points at the sibling ../IsaacSim. Either clone directly to ../IsaacSim or include explicit cd commands from the common parent directory.
| print_info("Isaac Sim is ready. Run Isaac Lab against it with:") | ||
| print_info( | ||
| " uv run --extra isaacsim-local isaaclab train --rl_library rsl_rl --task Isaac-Cartpole-Direct" | ||
| " presets=isaacsim_physx" |
There was a problem hiding this comment.
[AI review][Moderate] This completion command should use physics=isaacsim_physx, matching the installation docs and the repository-wide physics selector. presets=isaacsim_physx targets the wrong Hydra configuration key, so copying the command will fail instead of starting PhysX training.
|
|
||
| # Replace through a callable: a Windows path reaches this as a literal and ``re`` would read | ||
| # its backslashes as escapes in a replacement string. | ||
| section, count = re.subn(r"^find-links = \[.*\]$", lambda _: entry, section, count=1, flags=re.MULTILINE) |
There was a problem hiding this comment.
[AI review][Moderate] This pattern only recognizes a single-line TOML array. If the user already has a valid multiline find-links entry, count remains zero and the code inserts a duplicate key into [tool.uv]; uv then rejects the project before locking. The single-line replacement also discards any existing wheel locations. Preserve or extend the existing array, or at minimum reject unsupported formatting without modifying the file, and cover both single-line and multiline forms in tests.
| --isaacsim_source PATH | ||
| Build Isaac Sim from the source checkout at PATH, package it as wheels, and link | ||
| them as '_isaac_sim_wheels' for 'uv run --extra isaacsim-local'. | ||
| Skips the build when the checkout is already built. |
There was a problem hiding this comment.
[AI review][Moderate] This checked-in help text is stale. The implementation now always invokes the incremental build, and the live parser says it runs on every invocation. Please update both the Linux and Windows help snapshots so the documentation matches the command behavior.
|
Feel free to discard's the AI comments, I think this is nice and needed for GA. |
…ource_install # Conflicts: # uv.lock
# Description Fix the recurring `isaaclab_rl` LEAPP export failure for `Isaac-Reach-Franka` on Newton MJWarp. The existing workaround passes `limit_cpu_threads=1` to `SimulationApp`, but the failing stack is in OpenUSD's concurrent parser and OpenUSD reads `PXR_WORK_THREAD_LIMIT` during process startup. Set that environment variable on every LEAPP child process so USD is serialized before any USD module is imported, while retaining the existing Kit-side limit. This keeps the current task and Newton backend coverage. It also adds a deterministic subprocess probe for the environment contract. Observed in unrelated PRs: - #6762: https://github.com/isaac-sim/IsaacLab/actions/runs/32343107298/job/96578439022 - #6673: https://github.com/isaac-sim/IsaacLab/actions/runs/32419190550/job/96590692454 - #7207: https://github.com/isaac-sim/IsaacLab/actions/runs/32418984574/job/96590568310 OpenUSD documents `PXR_WORK_THREAD_LIMIT=1` as single-threaded mode: https://openusd.org/dev/api/thread_limits_8h.html ## Type of change - Bug fix (non-breaking change which fixes an issue) ## Validation - `uv run --extra sb3 --extra skrl --extra rl-games --extra leapp python -m pytest source/isaaclab_rl/test/export/test_leapp_export_flow.py -k 'openusd_thread_limit or rsl_rl-Isaac-Reach-Franka' -vv` (2 passed) - `uv run isaaclab -f` - `uv run python tools/changelog/cli.py check develop` ## Checklist - [x] I have read and understood the contribution guidelines - [x] I have run the pre-commit checks with `uv run isaaclab -f` - [x] Documentation is not required for this test-only mitigation - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective - [x] I have added a changelog fragment for every touched package - [x] My name is already in `CONTRIBUTORS.md`
## Description Bundled backport to `release/3.0.0` to reduce CI load. Source PRs reviewed for this bundle: - #7020 — already represented in `release/3.0.0`; its cherry-pick was empty, so no duplicate commit was added. - #7207 - #7229 - #7227 - #7231 - #6762 - #7208 - #7168 — backports the current PR head while the source PR is still open. - #7157 - #7216 ## Type of change - Bug fix - Documentation update - Workflow / packaging update ## Checklist - [x] I have read and understood the contribution guidelines. - [x] I have run formatting checks. - [x] Documentation changes are included. - [x] Documentation build generates no new warnings. - [x] Focused regression coverage passed. - [x] Required changelog fragments are included by the source PRs. - [x] Contributors are already listed or included by the source PRs. --------- Signed-off-by: Kelly Guo <kellyg@nvidia.com> Co-authored-by: Mustafa H <34825877+StafaH@users.noreply.github.com> Co-authored-by: Richard Lei <rilei@nvidia.com> Co-authored-by: Mustafa Haiderbhai <mhaiderbhai@nvidia.com> Co-authored-by: Kelly Guo <kellyg@nvidia.com>
4 participants
Description
Add a pure uv workflow for isaacsim
Easy to use:
Clone your isaacsim repo somewhere.
run
uv run isaaclab --isaacsim_source <path_to_isaacsim>run training with isaacsim
uv run isaaclab train --task Isaac-Cartpole physics=isaacsim_physxChecklist
pre-commitchecks with./isaaclab.sh --formatsource/<pkg>/changelog.d/for every touched package (do not editCHANGELOG.rstor bumpextension.toml— CI handles that)CONTRIBUTORS.mdor my name already exists there