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

Misc Windows fixes #1743

Merged
merged 7 commits into from
Feb 24, 2025
Merged

Misc Windows fixes #1743

merged 7 commits into from
Feb 24, 2025

Conversation

t-kalinowski
Copy link
Member

@t-kalinowski t-kalinowski commented Feb 22, 2025

I tested out py_require() on Windows and noticed a few issues:

  1. uv writes progress updates to stdout on Windows. This interfered with our ability to resolve the path to the ephemeral python, since we expected stdout to contain just the filepath.

    To fix, I refactored uv_get_or_create() to pass through both stderr and stdout from uv, and write the python executable path to a temporary file instead of stdout.

  2. The codepath that installed uv produced a file named 'Out-Null' in the current working directory.

    To fix, I refactored the system2() call to use stdout=FALSE

  3. The codepath that installed uv didn't seem to handle paths or mixed / in the file correctly.

    Rather than think too hard about how quoting needs to happen for cmd.exe calling powershell, I refactored the installer to pass filepaths via an envvar, and also, use shortPathName().

- Don't produce file named `Out-Null` in cwd
- Handle spaces, mixed \ and / in paths.
- handle filepaths with \ by passing constructing a raw string literal python expression
@t-kalinowski t-kalinowski merged commit d72e47b into main Feb 24, 2025
16 checks passed
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.

1 participant