Skip to content

Wheel Builder#5222

Open
myurasov-nv wants to merge 43 commits intoisaac-sim:developfrom
myurasov-nv:my-wheel-building
Open

Wheel Builder#5222
myurasov-nv wants to merge 43 commits intoisaac-sim:developfrom
myurasov-nv:my-wheel-building

Conversation

@myurasov-nv
Copy link
Copy Markdown
Member

@myurasov-nv myurasov-nv commented Apr 10, 2026

Ports the external wheel builder (from the internal GitLab CI/CD repo) into the Isaac Lab, mostly replicating it's functionality, but fixing few issues with it.

Adds a test for Install CI.

This is step 2 (1 - install ci) on the path to resolve our installation bugs. Step 3 will be making project structure compatible with python -m wheel build ., after GA.

Type of change

  • New feature (non-breaking change which adds functionality)

Checklist

  • I have read and understood the contribution guidelines
  • I have run the pre-commit checks with ./isaaclab.sh --format
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • I have updated the changelog and the corresponding version in the extension's config/extension.toml file
  • I have added my name to the CONTRIBUTORS.md or my name already exists there

@github-actions github-actions bot added isaac-lab Related to Isaac Lab team infrastructure labels Apr 10, 2026
Copy link
Copy Markdown

@isaaclab-review-bot isaaclab-review-bot bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: Wheel Builder + Installation CI

Solid infrastructure PR — porting the wheel builder into the monorepo and adding a proper installation test framework is a big step forward for CI/CD hygiene. The test runner (run) is well-designed with Docker/native modes, cache volume reuse, and flexible pytest passthrough. A few things to address below, mostly around security, robustness, and some cleanup from the port.

Summary

  • Scope: Adds tools/wheel_builder/ (build scripts, pyproject generator, package manifests), tools/install_ci/ (pytest-based installation tests with Docker runner), a GH Actions workflow, and minor fixups to .gitignore/envs.py
  • Architecture: Clean separation between the wheel builder (build.sh + gen_pyproject.py) and the CI test harness (conftest.py + utils.py + test files). The Docker-based test isolation is the right approach.
  • Quality: Generally good. Tests cover smoke (env creation, extension install) and heavy (full training loop) scenarios. The UV_Utils mixin pattern keeps test code DRY.

Key Issues

  1. build.sh uses pip install --break-system-packages inside the build stage — this modifies the host system Python when run natively (not just in Docker). Consider using a venv or --user install, or at minimum document that native execution is unsafe.
  2. res/__init__.py has dead code and uncertainty markersbootstrap_kernel() is defined but commented out with # ??? and # disable as we probably dont need it. Either remove it or add a proper TODO with context.
  3. The .gitignore change from tests/ to /tests/ is a broad scope change that affects the entire repo — worth calling out in the PR description since it could unignore nested tests/ dirs that were previously hidden.
  4. python_packages.toml has duplicate entries (numpy>=2, torch>=2.10, protobuf, tensorboard appear multiple times) — the dedup in gen_pyproject.py handles this, but the source file should be clean.
  5. Wheel retagging is commented out in build.sh — if this is intentional for the initial port, add a TODO issue reference.

@myurasov-nv myurasov-nv marked this pull request as ready for review April 11, 2026 09:48
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Apr 11, 2026

Greptile Summary

This PR ports an internal GitLab wheel builder into the IsaacLab repo and adds an integration test (Test_Wheel_Builder) that builds the wheel, installs it in a fresh uv environment, and verifies key imports and the CLI. Issues flagged in the prior review round (missing main(), duplicate copyright, inconsistent python/python3 calls) have been addressed.

  • P1 — Silent constraint loss in gen_pyproject.py: The first-wins deduplication drops packaging<24 (replaced by unconstrained packaging) and the tighter torchvision>=0.25.0 (replaced by >=0.0.25.0). Installing with incompatible versions of these packages can silently break isaaclab_rl at runtime.

Confidence Score: 4/5

Safe to merge after the constraint-deduplication bug in gen_pyproject.py is fixed; remaining findings are non-blocking.

One P1 finding: the first-wins dependency deduplication in gen_pyproject.py silently drops packaging<24 and the tighter torchvision lower bound, which can cause runtime breakage for isaaclab_rl users installing the wheel. All previously flagged issues are resolved. The test infrastructure and build script are otherwise solid.

tools/wheel_builder/gen_pyproject.py (constraint deduplication logic) and tools/wheel_builder/res/python_packages.toml (conflicting constraints + dead inventory section)

Important Files Changed

Filename Overview
tools/wheel_builder/build.sh Orchestrates wheel staging and build; Python invocations are now consistently python3, but pip install --break-system-packages remains, modifying the system Python in CI.
tools/wheel_builder/gen_pyproject.py Generates pyproject.toml from python_packages.toml; first-wins deduplication silently drops version constraints (e.g. packaging<24, torchvision>=0.25.0).
tools/wheel_builder/res/init.py Custom wheel __init__.py that exposes __version__, extends __path__ for sub-package discovery, and defines main() entry point; issues from previous review are resolved.
tools/wheel_builder/res/main.py CLI entry point with main() function; duplicate copyright removed; toml import is satisfied by the declared dependency.
tools/wheel_builder/res/python_packages.toml Consolidated dependency manifest; contains conflicting constraints for packaging and torchvision that the deduplication silently resolves in favor of the first entry, plus a dead inventory section with non-existent paths.
source/isaaclab/test/install_ci/test_wheel_builder.py New class-scoped integration test that builds the wheel, installs it into a fresh uv environment, and verifies key imports and the CLI; structure and fixtures look correct.
source/isaaclab/test/install_ci/utils.py Shared test utilities providing run_cmd and UV_Mixin; well-structured with proper streaming, timeout, and cleanup handling.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[build.sh starts] --> B[Copy apps/ and source/ to BUILD_DIR/src/isaaclab/]
    B --> C[Ensure apps/ sub-dirs have __init__.py]
    C --> D[For each isaaclab_* sub-package: promote to top-level src/]
    D --> D1[Copy inner Python package to src/pkg/]
    D1 --> D2[Copy config/ and data/ resource dirs]
    D2 --> D3[Patch EXT_DIR references in __init__.py]
    D3 --> D4[Remove original from isaaclab bundle]
    D4 --> E[Clean __pycache__ / egg-info / .pyc]
    E --> F[Copy res/__init__.py and res/__main__.py]
    F --> G[gen_pyproject.py: read python_packages.toml]
    G --> G1[Deduplicate deps - first-wins]
    G1 --> G2[Write pyproject.toml to BUILD_DIR]
    G2 --> H[python3 -m pip install build wheel]
    H --> I[python3 -m build --wheel]
    I --> J[Wheel written to DIST_DIR/]
    J --> K{Test: Test_Wheel_Builder}
    K --> L[Install wheel into uv venv]
    L --> M[test_import_isaaclab]
    L --> N[test_version_matches_wheel]
    L --> O[test_import_isaaclab_app]
    L --> P[test_import_isaaclab_assets]
    L --> Q[test_cli_help]
Loading

Reviews (2): Last reviewed commit: "--format" | Re-trigger Greptile

@myurasov-nv
Copy link
Copy Markdown
Member Author

@greptile-apps

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

infrastructure isaac-lab Related to Isaac Lab team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant