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

feat: allow passing extra config k,v pairs for pyvenv.cfg when creating a venv #1852

Merged
merged 5 commits into from
Feb 22, 2024

Conversation

samypr100
Copy link
Collaborator

@samypr100 samypr100 commented Feb 22, 2024

Summary

This modifies gourgeist to allow passing additional k,v pairs to add to the pyvenv.cfg file as proposed in #1697.
I made it allow an arbitrary set of pairs (to decouple from uv since this is mainly a change to gourgeist) , but I can slim it down to just allow just a name and version strings if that's desired.

The pyvenv.cfg will also have a uv = <uv-crate-version> when a venv is created via uv venv and uv-build = <uv-build-crate-version> when it's created via SourceBuild::setup.

Example below via uv venv:

home = ...
implementation = CPython
version_info = 3.12
include-system-site-packages = false
base-prefix = ...
base-exec-prefix = ...
base-executable = ...
uv = 0.1.6
prompt = uv

Open to any suggestions, thanks!

Closes #1697

Test Plan

Added new test in tests/venv.rs called verify_pyvenv_cfg to verify that it contains the right uv version string. I didn't see tests configured in gourgeist itself, so I didn't add any there.

@samypr100 samypr100 force-pushed the uv-pyvenv-cfg-custom-fields branch 2 times, most recently from 47c8c42 to 03eaffd Compare February 22, 2024 04:01
@samypr100 samypr100 marked this pull request as ready for review February 22, 2024 04:03
@konstin konstin added the enhancement New feature or improvement to existing functionality label Feb 22, 2024
crates/gourgeist/src/bare.rs Outdated Show resolved Hide resolved
crates/gourgeist/src/bare.rs Outdated Show resolved Hide resolved
crates/uv-build/src/lib.rs Outdated Show resolved Hide resolved
crates/gourgeist/src/bare.rs Outdated Show resolved Hide resolved
crates/gourgeist/src/bare.rs Outdated Show resolved Hide resolved
crates/gourgeist/src/bare.rs Show resolved Hide resolved
@zanieb zanieb added the virtualenv Related to virtual environments label Feb 22, 2024
@konstin konstin merged commit 2fa67ea into astral-sh:main Feb 22, 2024
7 checks passed
@konstin
Copy link
Member

konstin commented Feb 22, 2024

Thanks!

@samypr100 samypr100 deleted the uv-pyvenv-cfg-custom-fields branch February 22, 2024 19:53
@henryiii
Copy link
Contributor

Shouldn’t gourgeist still be there too? I am checking for that in nox (though not merged yet due to the coverage CI being broken). I can check for either one easily enough.

@konstin
Copy link
Member

konstin commented Feb 22, 2024

I think the "API" should be uv with gourgeist being more an implementation detail. What's your use case for checking who created the venv?

@samypr100
Copy link
Collaborator Author

samypr100 commented Feb 22, 2024

@henryiii Can you look for uv starting with 0.1.9 instead? I believe that's what you desired for nox based on your issue.

@henryiii
Copy link
Contributor

I can look for either one. I need to know if we need to regenerate the environment if a user changes the provider. This is especially important for uv vs. venv/virtualenv, since we are installing pip into those environments but not uv.

@henryiii
Copy link
Contributor

Technically, since it didn't matter before (unless someone switched venv or virtualenv with conda or mamba, which would have been a mess but not something very common), the mechanism is weirdly opt-in via a secret environment variable, but I think I can fix that.

konstin added a commit that referenced this pull request Feb 23, 2024
Read the key read for uv from `pyenv.cfg` from `gourgeist` to `uv`. I missed that we're also reading pyenv.cfg when reviewing #1852.
konstin added a commit that referenced this pull request Feb 23, 2024
Read the key read for uv from `pyenv.cfg` from `gourgeist` to `uv`. I missed that we're also reading pyenv.cfg when reviewing #1852.
zanieb pushed a commit that referenced this pull request Feb 23, 2024
Read the key read for uv from `pyenv.cfg` from `gourgeist` instead of
`uv`. I missed that we're also reading pyenv.cfg when reviewing #1852.
We could check for gourgeist for backwards compatibility, but i think
it's fine this way.
charliermarsh pushed a commit that referenced this pull request Apr 23, 2024
## Summary

This is mainly a cleanup PR to leverage uv-version in uv-virtualenv
instead of passing it via `uv`.
In #1852 I introduced the ability to pass extra cfg to `gourgeist` for
the primary purpose of passing the uv version, but since the dawn of the
uv-version crate dynamically passing more values to pyvenv.cfg is no
longer needed.

## Test Plan

Existing `uv` tests should still verify `uv = <version>` exists in the
venv and make sure no regressions were introduced.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or improvement to existing functionality virtualenv Related to virtual environments
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Report uv version in pyvenv.cfg
4 participants