Skip to content

feat(build): scikit-build-core from-source install (#735) - #820

Open
jiwenc-nv wants to merge 2 commits into
NVIDIA:mainfrom
jiwenc-nv:jiwenc/scikit-build-core
Open

feat(build): scikit-build-core from-source install (#735)#820
jiwenc-nv wants to merge 2 commits into
NVIDIA:mainfrom
jiwenc-nv:jiwenc/scikit-build-core

Conversation

@jiwenc-nv

@jiwenc-nv jiwenc-nv commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator

Summary

Adds a root pyproject.toml using the scikit-build-core (PEP 517/660) backend, so isaacteleop builds and installs from source with standard Python tooling — the from-source / developer path requested in #735:

pip install .            # build + install the compiled wheel from source
pip install -e .         # editable / developer install
python -m build --sdist  # source distribution

It drives the same top-level CMakeLists.txt as the classic cmake -B build flow, so the two coexist; the classic flow (which CI uses to produce install/wheels/ and the released wheels) is unchanged.

What's in it

  • pyproject.toml — scikit-build-core backend; extras mirrored from src/core/python/requirements*.txt. Version is derived from VERSION as MAJOR.MINOR+local. The build tree is kept per-Python at build/wheel-<cache_tag>/, and CMake is capped to a 3.x line (>=3.20,<4) so pip/isolated builds match the Ubuntu 22.04/24.04 line instead of pulling CMake 4.x.
  • Full build + executables on PATH — a pip install builds the default target (extensions, examples, C++ tests, and plugins — a plugin without its SDK skips gracefully) and installs every built executable onto the venv PATH via a SKBUILD-gated loop in the root CMakeLists.txt (${SKBUILD_SCRIPTS_DIR}). Shared libraries (Python extension modules, plugin .so) are not executables and stay out of bin/. clang-format is enforced — the build fails if clang-format-14 is missing or any C++ file is unformatted.
  • CMakePresets.jsonpy3.10py3.13 presets that set ISAAC_TELEOP_PYTHON_VERSION and build into build/cmake-<cache_tag>/, sharing the same per-version tag as the wheel build dir so the two never collide.
  • cmake/SetupPython.cmake — under SKBUILD, compile against the interpreter running the build so the wheel's ABI tag matches; the classic uv-managed path is unchanged, and the python_wheel target is gated to non-SKBUILD.
  • CI — a pip-editable-install job (sdist build + pip install -e . + import/completeness smoke), with clang-format-14 installed.
  • Docs — build-from-source guide (pip/editable/sdist, presets, the executables-on-PATH and clang-format behavior) and build reference, plus the aarch64 retargetersretargeters-lite note.

Behavior / scope notes

  • Version on the pip path is MAJOR.MINOR+local (a from-source/dev build); git-aware release versioning (a1/rc1/.devN, tags) stays with the classic flow.
  • Editable installs don't rebuild on import; because the pure-Python subpackages are copy-staged, editing src/retargeters/ isn't live without re-staging. Making those edits live (a src-layout relocation) is a follow-up.
  • The pip-built wheel omits .pyi stubs (stub generation shells out to uv, not guaranteed in pip build isolation); the classic/released wheel still ships them.

Validation

Verified locally: pip install -e . builds and installs the example + test executables into the venv bin/ (they run self-contained), and cmake --preset py3.12 configures. The pip-editable-install CI job is the gate for a clean-environment build.

Closes #735.

@coderabbitai

coderabbitai Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: c39c8df6-7a4f-4391-a329-7d45d4084f8a

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

The project adds scikit-build-core PEP 517/660 packaging with project metadata, dependency groups, CMake options, and editable-install settings. CMake now uses the backend-selected Python interpreter and stages the Python package tree for SKBUILD wheels while preserving the legacy path otherwise. Documentation covers pip source installs, editable behavior, and aarch64 extras. Ubuntu CI builds an sdist, performs an editable install, and runs import and package-discovery smoke tests.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Developer
  participant pip
  participant scikit_build_core
  participant CMake
  participant isaacteleop
  Developer->>pip: install from source or editable
  pip->>scikit_build_core: invoke PEP 517/660 backend
  scikit_build_core->>CMake: configure SKBUILD build
  CMake->>isaacteleop: stage compiled and pure-Python package
  isaacteleop-->>Developer: provide installed package
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The PR addresses #735 by enabling source/editable installs and documenting the aarch64 retargeters-lite note.
Out of Scope Changes check ✅ Passed The changes stay focused on packaging, editable-install support, CI validation, and related docs.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly reflects the main change: adding scikit-build-core support for source installation and build workflow updates.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown
Contributor

📝 Docs preview is not auto-deployed for fork PRs.

A maintainer with write access to NVIDIA/IsaacTeleop can deploy a preview by
commenting /preview-docs on this PR. Once deployed, the preview
will live at:

https://nvidia.github.io/IsaacTeleop/preview/pr-820/

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 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 @.github/workflows/build-ubuntu.yml:
- Around line 295-307: Update the pip-editable-install job to set job-level
permissions to contents: read, and configure its actions/checkout@v6 step with
persist-credentials: false while preserving the existing shallow checkout
behavior.

In `@docs/source/references/build.rst`:
- Around line 78-82: Replace the python -m build --sdist command with uv build
--sdist in both docs/source/references/build.rst lines 78-82 and
docs/source/getting_started/build_from_source/index.rst lines 306-310, using the
already required uv frontend and preserving the documented source-distribution
workflow.
🪄 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: 61fa3db5-c8ad-4ad8-ad52-8e8ef321ee5a

📥 Commits

Reviewing files that changed from the base of the PR and between 6258095 and 27d63ed.

📒 Files selected for processing (6)
  • .github/workflows/build-ubuntu.yml
  • cmake/SetupPython.cmake
  • docs/source/getting_started/build_from_source/index.rst
  • docs/source/references/build.rst
  • pyproject.toml
  • src/core/python/CMakeLists.txt

Comment thread .github/workflows/build-ubuntu.yml
Comment thread docs/source/references/build.rst Outdated
@jiwenc-nv
jiwenc-nv force-pushed the jiwenc/scikit-build-core branch from 27d63ed to 5de3626 Compare July 24, 2026 17:31
@jiwenc-nv

Copy link
Copy Markdown
Collaborator Author

/preview-docs

@github-actions

Copy link
Copy Markdown
Contributor

✅ Preview deployed: https://NVIDIA.github.io/IsaacTeleop/preview/pr-820/

@jiwenc-nv
jiwenc-nv force-pushed the jiwenc/scikit-build-core branch 3 times, most recently from 405b6f1 to c6c948a Compare July 25, 2026 00:24
@jiwenc-nv

Copy link
Copy Markdown
Collaborator Author

/preview-docs

@github-actions

Copy link
Copy Markdown
Contributor

✅ Preview deployed: https://NVIDIA.github.io/IsaacTeleop/preview/pr-820/

@jiwenc-nv
jiwenc-nv force-pushed the jiwenc/scikit-build-core branch from b006dc5 to d244c7e Compare July 25, 2026 18:21
@jiwenc-nv jiwenc-nv changed the title Add scikit-build-core backend for pip install-from-source (#735) feat(build): scikit-build-core from-source install (#735) Jul 25, 2026
@jiwenc-nv
jiwenc-nv force-pushed the jiwenc/scikit-build-core branch from d244c7e to ecb6a1b Compare July 25, 2026 22:20
@jiwenc-nv

Copy link
Copy Markdown
Collaborator Author

/preview-docs

@github-actions

Copy link
Copy Markdown
Contributor

✅ Preview deployed: https://NVIDIA.github.io/IsaacTeleop/preview/pr-820/

@jiwenc-nv
jiwenc-nv force-pushed the jiwenc/scikit-build-core branch 3 times, most recently from 019475c to e2b9590 Compare July 25, 2026 23:10
@jiwenc-nv

Copy link
Copy Markdown
Collaborator Author

/preview-docs

@github-actions

Copy link
Copy Markdown
Contributor

✅ Preview deployed: https://NVIDIA.github.io/IsaacTeleop/preview/pr-820/

@jiwenc-nv
jiwenc-nv force-pushed the jiwenc/scikit-build-core branch from e2b9590 to e1da93e Compare July 25, 2026 23:20
@jiwenc-nv

Copy link
Copy Markdown
Collaborator Author

/preview-docs

@github-actions

Copy link
Copy Markdown
Contributor

✅ Preview deployed: https://NVIDIA.github.io/IsaacTeleop/preview/pr-820/

Add a scikit-build-core (PEP 517/660) backend so isaacteleop builds and
installs from source with standard tooling (pip install . / -e . / sdist),
driving the same top-level CMakeLists.txt as the classic cmake flow.

- Version derived from VERSION as MAJOR.MINOR+local; per-Python build dir
  build/wheel-<cache_tag>; CMake capped to a 3.x line (>=3.20,<4) to match
  the Ubuntu 22.04/24.04 line.
- Full build: a pip install builds the default target -- extensions,
  examples, C++ tests and plugins (a plugin without its SDK skips) -- and
  installs every built executable onto the venv PATH (SKBUILD_SCRIPTS_DIR);
  clang-format is enforced.
- CMakePresets.json: py3.10-3.13 presets (build/cmake-<cache_tag>) matching
  the wheel build-dir tag.
- SetupPython.cmake: under SKBUILD, build against the invoking interpreter
  so the wheel ABI tag matches; python_wheel gated to non-SKBUILD.
- tool.uv.package = false so `uv run` / `uv sync` don't implicitly build the
  project into their env (which turned every `uv run` from the repo into a
  source build); explicit pip install / uv build still compile.
- CI pip-editable-install job (sdist + editable install + import smoke);
  build-from-source guide and build reference docs.

Signed-off-by: Jiwen Cai <jiwenc@nvidia.com>
@jiwenc-nv
jiwenc-nv force-pushed the jiwenc/scikit-build-core branch from e1da93e to 087f693 Compare July 28, 2026 05:08
Switch the Ubuntu and Windows build jobs to the preset-based flow added in
NVIDIA#735, so CI configures/builds the same way the build-from-source docs tell
users to:

  cmake --preset py3.12
  cmake --build --preset py3.12 --parallel
  cmake --install build/cmake-cpython-312

- build-ubuntu derives CMAKE_PRESET / BUILD_DIR once from the matrix Python
  version; the preset supplies ISAAC_TELEOP_PYTHON_VERSION, so the explicit
  -D is gone. Every path that hardcoded build/ now follows BUILD_DIR: ctest,
  cmake --install, the gcovr object/exclude dirs (a stale --object-directory
  silently reports 0% coverage rather than failing) and the viz test-binary
  packaging step.
- build-windows has no Python matrix, so it pins py3.11 -- already the
  CMakeLists default, so behaviour is unchanged -- via job-level env.
- vcpkg caches follow the build directory, since manifest ports install into
  ${CMAKE_BINARY_DIR}/vcpkg_installed. The preset is part of the cache key:
  without it, entries saved from the old build/ path would still hit exactly,
  restore to a path the job never reads, and block the re-save. On Ubuntu this
  makes the cache per-Python (8 entries instead of 2); Windows stays at 1.

test-viz-sanitizers keeps cmake -B build-san: it has no Python dimension and
builds with BUILD_PYTHON_BINDINGS=OFF, so no preset applies.

Signed-off-by: Jiwen Cai <jiwenc@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Developer experience: editable-install story for pure-Python subpackages (+ aarch64 'retargeters' extra note)

1 participant