Skip to content

build(make): prefer .venv tooling for Python targets#273

Draft
armenzg wants to merge 2 commits into
mainfrom
armenzg/dev-env/makefile-venv
Draft

build(make): prefer .venv tooling for Python targets#273
armenzg wants to merge 2 commits into
mainfrom
armenzg/dev-env/makefile-venv

Conversation

@armenzg
Copy link
Copy Markdown
Member

@armenzg armenzg commented May 27, 2026

Makefile Python targets now prefer tools from .venv/bin when a local virtualenv exists.

Previously, build-py, test-py, docs, update-venv, and package-py called bare python/pip/pytest, which could silently use an ambient interpreter. This updates those targets to resolve .venv/bin/* first and fall back to PATH only when .venv is not present.

That keeps local and CI-like workflows aligned with the project environment without breaking first-run setups.

Made with Cursor

Resolve Python and pip from .venv/bin when available so local make targets
use the project virtualenv instead of ambient PATH tools.

Co-Authored-By: Codex <noreply@openai.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

Bugbot Autofix prepared a fix for the issue found in the latest run.

  • ✅ Fixed: Relative venv paths break after cd into subdirectories
    • Changed PYTHON, PIP, PYTEST, and SABLEDOCS variables to use absolute paths with $(CURDIR) prefix instead of relative .venv/bin/* paths.

Create PR

Or push these changes by commenting:

@cursor push a4edc7b3b8
Preview (a4edc7b3b8)
diff --git a/Makefile b/Makefile
--- a/Makefile
+++ b/Makefile
@@ -3,10 +3,10 @@
 
 # unstable protos are only included in local development and not part of release packages
 SENTRY_PROTOS_BUILD_UNSTABLE := 1
-PYTHON ?= $(if $(wildcard .venv/bin/python),.venv/bin/python,python)
-PIP ?= $(if $(wildcard .venv/bin/pip),.venv/bin/pip,pip)
-PYTEST ?= $(if $(wildcard .venv/bin/pytest),.venv/bin/pytest,pytest)
-SABLEDOCS ?= $(if $(wildcard .venv/bin/sabledocs),.venv/bin/sabledocs,sabledocs)
+PYTHON ?= $(if $(wildcard .venv/bin/python),$(CURDIR)/.venv/bin/python,python)
+PIP ?= $(if $(wildcard .venv/bin/pip),$(CURDIR)/.venv/bin/pip,pip)
+PYTEST ?= $(if $(wildcard .venv/bin/pytest),$(CURDIR)/.venv/bin/pytest,pytest)
+SABLEDOCS ?= $(if $(wildcard .venv/bin/sabledocs),$(CURDIR)/.venv/bin/sabledocs,sabledocs)
 
 .PHONY: update-venv
 update-venv:

You can send follow-ups to the cloud agent here.

Reviewed by Cursor Bugbot for commit 0710e7e. Configure here.

Comment thread Makefile Outdated
Comment thread Makefile
.PHONY: update-venv
update-venv:
pip install -r requirements.txt
$(PIP) install -r requirements.txt
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should we be switching over to uv run instead?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Sounds good 👍🏻

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Good callout. I kept this PR on the existing devenv/pip flow to keep scope tight, but I pushed a follow-up commit that fixes the path issue by resolving .venv tools with absolute $(CURDIR) paths so subdirectory recipes work reliably.

If you want, I can open a separate PR to evaluate moving these Make targets to uv run end-to-end.

Resolve .venv tool references from repository root so commands that cd into
subdirectories still run the intended virtualenv binaries.

Co-Authored-By: Codex <noreply@openai.com>
Co-authored-by: Cursor <cursoragent@cursor.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.

2 participants