Skip to content

Updated devcontainer, seamless switching between build environments#759

Open
aristarkhovNV wants to merge 2 commits into
mainfrom
aaristarkhov/devcontainer
Open

Updated devcontainer, seamless switching between build environments#759
aristarkhovNV wants to merge 2 commits into
mainfrom
aaristarkhov/devcontainer

Conversation

@aristarkhovNV

@aristarkhovNV aristarkhovNV commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator

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/python as a dangling symlink and FetchContent _deps sub-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 own build-devcontainer/ directory (via cmake.buildDirectory) so neither environment's baked-in absolute paths can collide with the other's. SetupPython.cmake now 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

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Testing

Checklist

  • I have read and understood the contribution guidelines
  • I have run the linter and formatter with SKIP=check-copyright-year pre-commit run --all-files
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix/feature works (or explained why not)
  • I have signed off all my commits (git commit -s) per the DCO

Summary by CodeRabbit

  • New Features
    • Enhanced the development container setup with additional tooling and a dedicated build output directory for CMake builds.
  • Bug Fixes
    • Improved Python build environment reliability by reusing working virtual environments when present and reporting more precise failures when the interpreter is missing or broken.
  • Chores
    • Updated dev container feature lock metadata and adjusted ignore rules to exclude the build output directory.

@coderabbitai

coderabbitai Bot commented Jul 8, 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: 304f0d07-af9e-4fd5-9177-ec8b40a85127

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:

  • ✅ Review completed - (🔄 Check again to review again)
📝 Walkthrough

Walkthrough

This 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)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the devcontainer and build-environment switching changes in the PR.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch aaristarkhov/devcontainer

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

@aristarkhovNV aristarkhovNV marked this pull request as ready for review July 9, 2026 16:27
@aristarkhovNV aristarkhovNV requested a review from jiwenc-nv July 9, 2026 16:27

@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: 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

📥 Commits

Reviewing files that changed from the base of the PR and between 90c92d6 and 5955c86.

📒 Files selected for processing (4)
  • .devcontainer/devcontainer-lock.json
  • .devcontainer/devcontainer.json
  • .gitignore
  • cmake/SetupPython.cmake

Comment thread cmake/SetupPython.cmake Outdated
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.

1 participant