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.

Sign up for GitHub

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

Jump to bottom

Add warning when VIRTUAL_ENV is set but will not be respected in project commands #6864

Merged
merged 1 commit into from
Sep 3, 2024
Merged

Add warning when VIRTUAL_ENV is set but will not be respected in project commands #6864

merged 1 commit into from
Sep 3, 2024

Conversation

zanieb
Copy link
Member

@zanieb zanieb commented Aug 30, 2024 •
edited
Loading

Following #6834

@zanieb zanieb added the cli Related to the command line interface label Aug 30, 2024
@zanieb zanieb mentioned this pull request Aug 30, 2024
Merged
@jamesharris-garmin jamesharris-garmin mentioned this pull request Aug 30, 2024
Open
zanieb added a commit that referenced this pull request Sep 3, 2024
@zanieb
Allow customizing the project environment path with `UV_PROJECT_ENVIR…
1234b6d 
…ONMENT` (#6834)

Allows configuration of the (currently hard-coded) path to the virtual
environment in projects using the `UV_PROJECT_ENVIRONMENT` environment
variable.

If empty, we'll ignore it. If a relative path, it will be resolved
relative to the workspace root. If an absolute path, we'll use that.

This feature targets use in Docker images and CI. The variable is
intended to be set once in an isolated system and used for all uv
operations.

We do not expose a CLI option or configuration file setting — we may
pursue those later but I see them as lower priority. I think a
system-level environment variable addresses the most pressing use-cases
here.

This doesn't special-case the system environment. Which means that you
can use this to write to the system Python environment. I would
generally strongly recommend against doing so. The insightful comment
from @edmorley at
#5229 (comment)
provides some context on why. More generally, `uv sync` will remove
packages from the environment by default. This means that if the system
environment contains any packages relevant to the operation of the
system (that are not dependencies of your project), `uv sync` will break
it. I'd only use this in Docker or CI, if anywhere. Virtual environments
have lots of benefits, and it's only [one line to "activate"
them](https://docs.astral.sh/uv/guides/integration/docker/#using-the-environment).

If you are considering using this feature to use Docker bind mounts for
developing in containers, I would highly recommend reading our [Docker
container development
documentation](https://docs.astral.sh/uv/guides/integration/docker/#developing-in-a-container)
first. If the solutions there do not work for you, please open an issue
describing your use-case and why.

We do not read `VIRTUAL_ENV` and do not have plans to at this time.
Reading `VIRTUAL_ENV` is high-risk, because users can easily leave an
environment active and use the uv project interface today. Reading
`VIRTUAL_ENV` would be a breaking change. Additionally, uv is
intentionally moving away from the concept of "active environments" and
I don't think syncing to an "active" environment is the right behavior
while managing projects. I plan to add a warning if `VIRTUAL_ENV` is
set, to avoid confusion in this area (see
#6864).

This does not directly enable centrally managed virtual environments. If
you set `UV_PROJECT_ENVIRONMENT` to an absolute path and use it across
multiple projects, they will clobber each other's environments. However,
you could use this with something like `direnv` to achieve "centrally
managed" environments. I intend to build a prototype of this eventually.
See #1495 for more details on this use-case.

Lots of discussion about this feature in:

- astral-sh/rye#371
- astral-sh/rye#1222
- astral-sh/rye#1211
- #5229
- #6669
- #6612

Follow-ups:

- #6835 
- #6864
- Document this in the project concept documentation (can probably
re-use some of this post)

Closes #6669
Closes #5229
Closes #6612
Base automatically changed from zb/venv-path to main September 3, 2024 17:52
@zanieb zanieb mentioned this pull request Sep 3, 2024
Merged
zanieb added a commit that referenced this pull request Sep 3, 2024
@zanieb
Remove VIRTUAL_ENV from project commmands by default (#6976)
1e89d3e 
And use test context helpers for commands consistently.

Needed for #6864
@zanieb zanieb force-pushed the zb/project-venv-warn branch 3 times, most recently from 7b279cf to 4b1c43b Compare September 3, 2024 19:27
@zanieb zanieb mentioned this pull request Sep 3, 2024
Open
@zanieb zanieb force-pushed the zb/project-venv-warn branch 2 times, most recently from 03aeb5e to 921acdb Compare September 3, 2024 20:31
zanieb
zanieb commented Sep 3, 2024
View reviewed changes
crates/uv-workspace/src/workspace.rs
Comment on lines +486 to +487
// Often, one of the directories won't exist yet so perform the comparison up a level
if let (Some(left_parent), Some(right_parent), Some(left_name), Some(right_name)) = (
Copy link
Member Author

@zanieb zanieb Sep 3, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

A little awkward, but important for cases where .venv is being created in a project — it'd be weird not to warn on the first invocation but warn on later ones.

Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any value to the previous check when this one's already here? This just feels like a more general version of the previous one. The only thing I can think of is that this one will trigger on case-insensitive filesystems if the casing differs, while I don't think that is_same_file will.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure honestly. Seems safe to keep it.

cc @BurntSushi for a post-merge review if you have time.

@charliermarsh charliermarsh merged commit 2a42515 into main Sep 3, 2024
58 checks passed
@charliermarsh charliermarsh deleted the zb/project-venv-warn branch September 3, 2024 23:51
@BrewTestBot BrewTestBot mentioned this pull request Sep 4, 2024
Merged
tmeijn pushed a commit to tmeijn/dotfiles that referenced this pull request Sep 4, 2024
@tmeijn
build(deps): update astral-sh/uv to v0.4.4
59d659b 
This MR contains the following updates:

| Package | Update | Change |
|---|---|---|
| [astral-sh/uv](https://github.com/astral-sh/uv) | patch | `0.4.0` -> `0.4.4` |

MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot).

**Proposed changes to behavior should be submitted there as MRs.**

---

### Release Notes

<details>
<summary>astral-sh/uv (astral-sh/uv)</summary>

### [`v0.4.4`](https://github.com/astral-sh/uv/blob/HEAD/CHANGELOG.md#044)

[Compare Source](astral-sh/uv@0.4.3...0.4.4)

##### Enhancements

-   Allow customizing the project environment path with `UV_PROJECT_ENVIRONMENT` ([#&#8203;6834](astral-sh/uv#6834))
-   Warn when `VIRTUAL_ENV` is set but will not be respected in project commands ([#&#8203;6864](astral-sh/uv#6864))
-   Add `--no-hashes` to `uv export` ([#&#8203;6954](astral-sh/uv#6954))
-   Make HTTP headers title case for backward compatibility ([#&#8203;6887](astral-sh/uv#6887))
-   Pin `.python-version` in `uv init` ([#&#8203;6869](astral-sh/uv#6869))
-   Support `file://` URLs for `UV_PYTHON_INSTALL_MIRROR` ([#&#8203;6950](astral-sh/uv#6950))
-   Introduce more docker tags for uv ([#&#8203;6053](astral-sh/uv#6053))

##### Bug fixes

-   Avoid canonicalizing the cache directory ([#&#8203;6949](astral-sh/uv#6949))
-   Show all PyPy versions in `uv python list --all-versions` ([#&#8203;6917](astral-sh/uv#6917))
-   Avoid incorrect `requires-python` marker simplifications ([#&#8203;6268](astral-sh/uv#6268))

##### Documentation

-   Add documentation for `UV_PROJECT_ENVIRONMENT` ([#&#8203;6987](astral-sh/uv#6987))
-   Add optional dependencies section to the lockfile document ([#&#8203;6982](astral-sh/uv#6982))
-   Document use of the `file://` scheme in Python installation mirrors ([#&#8203;6984](astral-sh/uv#6984))
-   Fix outdated references to the help menu documentation in the first steps page ([#&#8203;6980](astral-sh/uv#6980))
-   Show env option in CLI reference documentation ([#&#8203;6863](astral-sh/uv#6863))
-   Add bind mount example to `docker.md` ([#&#8203;6921](astral-sh/uv#6921))

### [`v0.4.3`](https://github.com/astral-sh/uv/blob/HEAD/CHANGELOG.md#043)

[Compare Source](astral-sh/uv@0.4.2...0.4.3)

##### Enhancements

-   Show build backend output when `--verbose` is provided ([#&#8203;6903](astral-sh/uv#6903))
-   Allow `uv sync --frozen --package` without copying member `pyproject.toml` ([#&#8203;6943](astral-sh/uv#6943))

##### Bug fixes

-   Avoid panic with missing temporary directory ([#&#8203;6929](astral-sh/uv#6929))
-   Avoid updating incorrect dependencies for sorted `uv add` ([#&#8203;6939](astral-sh/uv#6939))
-   Use lower-bound semantics for all Python compatibility comparisons ([#&#8203;6882](astral-sh/uv#6882))

### [`v0.4.2`](https://github.com/astral-sh/uv/blob/HEAD/CHANGELOG.md#042)

[Compare Source](astral-sh/uv@0.4.1...0.4.2)

##### Enhancements

-   Adding support for `.pyc`  files in `uv run` ([#&#8203;6886](astral-sh/uv#6886))
-   Treat missing `top_level.txt` as non-fatal ([#&#8203;6881](astral-sh/uv#6881))

##### Bug fixes

-   Fix `is_disjoint` check for supported environments ([#&#8203;6902](astral-sh/uv#6902))
-   Remove dangling archives in `uv cache clean ${package}` ([#&#8203;6915](astral-sh/uv#6915))
-   Error when discovered Python is incompatible with `--isolated` workspace ([#&#8203;6885](astral-sh/uv#6885))
-   Warn when discovered Python is incompatible with PEP 723 script ([#&#8203;6884](astral-sh/uv#6884))

### [`v0.4.1`](https://github.com/astral-sh/uv/blob/HEAD/CHANGELOG.md#041)

[Compare Source](astral-sh/uv@0.4.0...0.4.1)

##### Enhancements

-   Add `uv export --format requirements-txt` ([#&#8203;6778](astral-sh/uv#6778))
-   Allow `@` references in `uv tool install --from` ([#&#8203;6842](astral-sh/uv#6842))
-   Normalize version specifiers by sorting ([#&#8203;6333](astral-sh/uv#6333))
-   Respect the user's upper-bound in `requires-python` ([#&#8203;6824](astral-sh/uv#6824))
-   Use Windows registry to discover Python on Windows directly ([#&#8203;6761](astral-sh/uv#6761))
-   Hint at `--no-workspace` in `uv init` failures ([#&#8203;6815](astral-sh/uv#6815))
-   Update to last PyPy releases ([#&#8203;6784](astral-sh/uv#6784))

##### Bug fixes

-   Avoid deadlocks when multiple uv processes lock resources ([#&#8203;6790](astral-sh/uv#6790))
-   Expand tildes when matching against `PATH` ([#&#8203;6829](astral-sh/uv#6829))
-   Fix `uv init --no-project` alias ([#&#8203;6837](astral-sh/uv#6837))
-   Ignore pre-release segments when discovering via `requires-python` ([#&#8203;6813](astral-sh/uv#6813))
-   Support inline optional tables in `uv add` and `uv remove` ([#&#8203;6787](astral-sh/uv#6787))
-   Update default `hello.py` to pass `ruff format` ([#&#8203;6811](astral-sh/uv#6811))
-   Avoid stripping root for user path display ([#&#8203;6865](astral-sh/uv#6865))
-   Error when user-provided environments are disjoint with Python ([#&#8203;6841](astral-sh/uv#6841))
-   Retain alphabetical sorting for `pyproject.toml` in `uv add` operations ([#&#8203;6388](astral-sh/uv#6388))))

##### Documentation

-   Add a link to the multiple index docs in the alternative index guide ([#&#8203;6826](astral-sh/uv#6826))
-   Add docs for inline exclude newer in PEP 723 scripts ([#&#8203;6831](astral-sh/uv#6831))
-   Enumerate available Docker tags ([#&#8203;6768](astral-sh/uv#6768))
-   Omit `[pip]` section from configuration file docs ([#&#8203;6814](astral-sh/uv#6814))
-   Update `project.urls` in `pyproject.toml`  ([#&#8203;6844](astral-sh/uv#6844))
-   Add docs for AWS CodeArtifact usage ([#&#8203;6816](astral-sh/uv#6816))

##### Other changes

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this MR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box

---

This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy40NDAuNyIsInVwZGF0ZWRJblZlciI6IjM3LjQ0MC43IiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJSZW5vdmF0ZSBCb3QiXX0=-->
konstin added a commit that referenced this pull request Sep 16, 2024
@konstin
Ignore activate venv in tests
c3594ff 
Fix a regression in running tests from #6864 that would cause test failures due to added "does not match the project environment path `.venv` and will be ignored" warning when running the tests inside an active venv,
@konstin konstin mentioned this pull request Sep 16, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MithicSpirit MithicSpirit MithicSpirit left review comments

@charliermarsh charliermarsh Awaiting requested review from charliermarsh

@BurntSushi BurntSushi Awaiting requested review from BurntSushi

Assignees
No one assigned
Labels
cli Related to the command line interface
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@zanieb @MithicSpirit @charliermarsh