Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use uv pip sync and allow-existing instead of uv pip install #166

Merged
merged 1 commit into from
Nov 14, 2024

Conversation

ewianda
Copy link
Contributor

@ewianda ewianda commented Nov 6, 2024

Prior to this change, uv venv wipes out the virtual env target, resulting in longer installations when using private pypi indexes that don't support caching. Adding the allow-existing maintains the virtualenv if it exists, and using uv pip sync ensures the virtualenv doesn't retain unused dependencies.

@ewianda ewianda requested a review from a team as a code owner November 6, 2024 21:14
@ewianda
Copy link
Contributor Author

ewianda commented Nov 14, 2024

@mark-thm could please take a look

@mark-thm mark-thm merged commit 9598890 into theoremlp:main Nov 14, 2024
9 checks passed
@mark-thm
Copy link
Collaborator

@ewianda: I may need to revert this -- there's a slight behavior change which is breaking one of our use cases -- we use create_venv to make some tooling virtual environments without first expanding inputs to lockfiles, and pip sync seems to follow different semantics

@mark-thm mark-thm mentioned this pull request Nov 19, 2024
thm-automation bot pushed a commit that referenced this pull request Nov 19, 2024
In #166, create_venv was switched to perform a `uv pip sync`, but this has slightly different semantics than `uv pip install`. The latter will expand the input requirements as needed, while the former treats the input as a complete lockfile. To achieve both behaviors, revert `create_venv` to its old functionality and add a `sync_venv` macro that runs `uv pip sync`.
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