Skip to content

Make uv_fs::PythonExt::escape_for_python completely robust#20470

Open
EliteTK wants to merge 4 commits into
mainfrom
tk/robust-fs-python-escape
Open

Make uv_fs::PythonExt::escape_for_python completely robust#20470
EliteTK wants to merge 4 commits into
mainfrom
tk/robust-fs-python-escape

Conversation

@EliteTK

@EliteTK EliteTK commented Jul 16, 2026

Copy link
Copy Markdown
Member

Summary

Previously we only handled a subset of paths and silently broke them if they contained invalid unicode.

This now handles all possible paths in a way that python would accept losslessly.

In a lot of cases we assume paths are valid unicode elsewhere, but making this work properly is a one-off exercise, and now we no longer need to worry about it.

This also cleans up uv-build-frontend using its own custom implementation, it now pulls from uv_fs.

Test Plan

Some testing was manual to ensure we correctly handle the result in Python.

There's an exhaustive test for *nix, and a selective test for Windows.

EliteTK added 2 commits July 16, 2026 09:57
The filter is very broad to the point that it cannot be used to verify
expectations. But making it narrow enough for that would require
parametrization or a second filter, and only one test exercises this
path at the moment, so I've decided to inline it.
Ty is the only consumer, it doesn't unescape the result.

This probably worked fine for Windows because it eats duplicate
backslashes, it would only break there if someone put a double-quote in
their path (which nobdy does). On *nix it would break in both cases but
again nobody does that.

Regardless, it shouldn't be escaped, and eating the utf-8 error silently
would just produce a weird broken result. So I opted to make it loud...
Could make it a warning?

[^ty]: <https://github.com/astral-sh/ruff/blob/db195f3a494c55feeef225daa5de0b803a5253ac/crates/ty_site_packages/src/lib.rs#L1081-L1117>
@EliteTK EliteTK added bug Something isn't working internal A refactor or improvement that is not user-facing labels Jul 16, 2026
@EliteTK
EliteTK requested a review from konstin July 16, 2026 14:03
@EliteTK
EliteTK force-pushed the tk/robust-fs-python-escape branch from 113cd66 to 4dee44f Compare July 16, 2026 14:11
@EliteTK
EliteTK force-pushed the tk/robust-fs-python-escape branch from 4dee44f to 242a5f4 Compare July 16, 2026 14:21
@EliteTK EliteTK removed the bug Something isn't working label Jul 16, 2026
@EliteTK
EliteTK force-pushed the tk/robust-fs-python-escape branch from 242a5f4 to 72542b1 Compare July 16, 2026 15:01
@EliteTK
EliteTK marked this pull request as ready for review July 16, 2026 15:01
@EliteTK
EliteTK force-pushed the tk/robust-fs-python-escape branch from 72542b1 to 6988bdd Compare July 16, 2026 15:33
@EliteTK
EliteTK force-pushed the tk/fix-extends-environment-path branch from 7367964 to e4a8528 Compare July 16, 2026 16:02
Previously we only handled a subset of paths and silently broke them if
they contained invalid unicode.

This now handles all possible paths in a way that python would accept
losslessly.

In a lot of cases we assume paths are valid unicode elsewhere, but
making this work properly is a one-off exercise, and now we no longer
need to worry about it.

This also cleans up uv-build-frontend using its own custom
implementation, it now pulls from uv_fs.
@EliteTK
EliteTK force-pushed the tk/robust-fs-python-escape branch from 6988bdd to 57c584a Compare July 16, 2026 16:09
@EliteTK
EliteTK requested a review from zanieb July 16, 2026 16:09
Base automatically changed from tk/fix-extends-environment-path to main July 16, 2026 21:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

internal A refactor or improvement that is not user-facing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant