Skip to content

Avoid panic for non-UTF-8 virtualenv paths - #20375

Merged
zanieb merged 4 commits into
mainfrom
codex/fix-venv-non-utf8-path
Jul 14, 2026
Merged

Avoid panic for non-UTF-8 virtualenv paths#20375
zanieb merged 4 commits into
mainfrom
codex/fix-venv-non-utf8-path

Conversation

@konstin

@konstin konstin commented Jul 14, 2026

Copy link
Copy Markdown
Member

A non-UTF-8 uv venv destination currently panics while generating activation scripts. Return a clear error at the activation-script conversion site instead.

@zanieb

zanieb commented Jul 14, 2026

Copy link
Copy Markdown
Member

Why does a virtual environment path need to be utf-8? Just for the activation scripts? I am unsure that should error?

@konstin

konstin commented Jul 14, 2026

Copy link
Copy Markdown
Member Author

We assume that our paths are UTF-8 serializable, so that we can use the location in UTF-8 files, which is why this code previously panicked; This isn't the first non-UTF-8 error variant we're adding.

@woodruffw

Copy link
Copy Markdown
Member

Probably too large of a change for just this PR, but maybe we can use camino for the subset of path operations that we want to enforce UTF-8 on? I think the APIs/error states it provides are pretty ergonomic.

(OTOH, it's another dep.)

@astral-sh-bot

astral-sh-bot Bot commented Jul 14, 2026

Copy link
Copy Markdown

uv test inventory changes

This PR changes the tests when compared with the latest main baseline.

  • Added tests: 1
  • Removed tests: 0
  • Changed suites: 1
uv::python: +1 / -0

Added:

  • uv::python::venv::non_utf8_path

Removed: none

@konstin

konstin commented Jul 14, 2026

Copy link
Copy Markdown
Member Author

camino would be nice, but it would require changing almost all of the path handling we do.

@konstin

konstin commented Jul 14, 2026

Copy link
Copy Markdown
Member Author

Regarding regression safety: This turns an existing panic in _ => escape_posix_for_single_quotes(location.simplified().to_str().unwrap()) into an error.

@konstin
konstin marked this pull request as ready for review July 14, 2026 19:27
@konstin
konstin requested review from woodruffw and zanieb July 14, 2026 20:17
@zanieb
zanieb merged commit f385b7a into main Jul 14, 2026
111 of 112 checks passed
@zanieb
zanieb deleted the codex/fix-venv-non-utf8-path branch July 14, 2026 23:33
zanieb pushed a commit that referenced this pull request Jul 15, 2026
[#20375](#20375) added a non-UTF-8
virtualenv-path test that fails on macOS because APFS rejects the path
during directory creation, before the activation-script validation is
reached ([failing CI
job](https://github.com/astral-sh/uv/actions/runs/29429195830/job/87399638975)).
Validate the absolute destination before modifying the filesystem and
assert that a rejected path does not leave a partially created
environment.
harrisonwithrow-cloud

This comment was marked as spam.

@harrisonwithrow-cloud

This comment was marked as spam.

harrisonwithrow-cloud

This comment was marked as spam.

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.

5 participants