fix(build): pass base-prefix Python to cmake for SWIG bindings (#216)#219
Conversation
…ASJIM#216) When running via pixi+uv, cmake's find_package(Python3) resolves the thin uv venv Python which lacks python3.XX-config. Pass -DPython3_EXECUTABLE pointing at the base-prefix interpreter (the pixi env) so cmake finds the config script it needs. Closes BANANASJIM#216
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 47 minutes and 46 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/rdc/_build_renderdoc.py (1)
380-384: Add a regression test for the newPython3_EXECUTABLEinjection path.Line 380–384 introduces the core fix, but current
configure_build()tests (for example intests/unit/test_build_renderdoc.pyaround Line 299–328 and Line 310–316) don’t assert this new flag. Please add a focused test to lock this behavior and prevent regressions.Proposed test addition
+def test_configure_sets_python3_executable_from_base_prefix(tmp_path: Path) -> None: + mock_run = MagicMock() + (tmp_path / "renderdoc").mkdir() + swig_dir = tmp_path / "renderdoc-swig" + + base_prefix = tmp_path / "base" + base_python = base_prefix / "bin" / "python3.12" + base_python.parent.mkdir(parents=True) + base_python.write_text("") + + with ( + patch("subprocess.run", mock_run), + patch.object(br.sys, "base_prefix", str(base_prefix)), + patch.object(br.sys, "executable", "/tmp/venv/bin/python3.12"), + ): + br.configure_build(tmp_path, swig_dir, "linux") + + args = mock_run.call_args[0][0] + assert f"-DPython3_EXECUTABLE={base_python}" in args🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/rdc/_build_renderdoc.py` around lines 380 - 384, The tests lack coverage asserting that configure_build() injects -DPython3_EXECUTABLE when sys.base_prefix's python executable exists; add a unit test in tests/unit/test_build_renderdoc.py that mocks Path.exists() (or creates a temporary fake base Python path) so that src/rdc/_build_renderdoc.py's configure_build() (or the function that builds cmd) runs and the returned command list contains f"-DPython3_EXECUTABLE={base_python}" (reference the base_python variable and the Python3_EXECUTABLE flag) to lock this behavior and prevent regressions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/rdc/_build_renderdoc.py`:
- Around line 380-384: The tests lack coverage asserting that configure_build()
injects -DPython3_EXECUTABLE when sys.base_prefix's python executable exists;
add a unit test in tests/unit/test_build_renderdoc.py that mocks Path.exists()
(or creates a temporary fake base Python path) so that
src/rdc/_build_renderdoc.py's configure_build() (or the function that builds
cmd) runs and the returned command list contains
f"-DPython3_EXECUTABLE={base_python}" (reference the base_python variable and
the Python3_EXECUTABLE flag) to lock this behavior and prevent regressions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f1ef01b5-08d5-4e89-bb8c-e049fb7459ce
📒 Files selected for processing (1)
src/rdc/_build_renderdoc.py
Resolves CVE-2026-40192 (pillow) and CVE-2025-71176 (pytest) that cause the pip-audit CI step to fail on all open PRs.
Without this, the cmake Python workaround silently degrades to the broken auto-detection it was meant to bypass.
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
2 participants
When running via pixi+uv, cmake's find_package(Python3) resolves the thin uv venv Python which lacks python3.XX-config. Pass -DPython3_EXECUTABLE pointing at the base-prefix interpreter (the pixi env) so cmake finds the config script it needs.
Closes #216