Strip UNC prefix when setting working directory #1277
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
charliermarsh
added
bug
charliermarsh
commented
Feb 12, 2024
| @@ -6,7 +6,7 @@ use fs_err as fs; | |||
| use url::Url; | |||
|
|
|||
| use pep440_rs::Version; | |||
| use puffin_fs::NormalizedDisplay; | |||
| use puffin_fs::Normalized; | |||
There was a problem hiding this comment.
I renamed this trait.
charliermarsh
commented
Feb 12, 2024
| @@ -771,7 +772,7 @@ async fn run_python_script( | |||
| }; | |||
| Command::new(venv.python_executable()) | |||
| .args(["-c", script]) | |||
| .current_dir(source_tree) | |||
| .current_dir(source_tree.normalized()) | |||
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
For PEP 517 builds, the current working directory needs to be set to the directory of the source distribution. It turns out that on Windows, if you use a UNC path for the working directory, then relative paths are interpreted relative to the root of the current drive (source). So, when builds attempted to resolve relative paths, they always errored...
This PR ensures that we remove the UNC prefix when setting the current working directory.
Closes #1238.
Test Plan
I tested this on my Windows machine by installing
ujsonwith--no-binary ujson. (I don't want to add that specific test, since it's really slow to build.)