fix: check interpreter exists for VIRTUAL_ENV and CONDA_PREFIX#6178
fix: check interpreter exists for VIRTUAL_ENV and CONDA_PREFIX#6178trim21 wants to merge 2 commits into
Conversation
When VIRTUAL_ENV or CONDA_PREFIX is set but points to a deleted or Python-less environment, pyo3-build-config would blindly use the derived interpreter path and fail with 'No such file or directory'. Now we check that the interpreter exists before using it, falling back to other detection methods with a warning if it does not. Fixes PyO3#6177
davidhewitt
left a comment
davidhewitt
left a comment
There was a problem hiding this comment.
Thanks for the PR. I'm not convinced that this relaxation is a good idea; it seems to me like this makes it easier for users to accidentally misconfigure their system (e.g. typo in `VIRTUAL_ENV)
The warn! system from build scripts is sadly quite limited, it is not visible to users unless the build fails. In this case it's quite likely that some other python will be found and probably built against, so the misconfiguration could easily be silent.
On balance, I think it's preferable for PyO3 to continue to be strict here, and in the (hopefully rare) cases where PyO3 is attempting to use an invalid interpreter we give users the right nudge.
See suggestion below
| if path.exists() { | ||
| Some(path) | ||
| } else { | ||
| warn!( | ||
| "VIRTUAL_ENV is set to `{}` but no Python interpreter found at `{}`; ignoring", | ||
| dir.to_string_lossy(), | ||
| path.display() | ||
| ); | ||
| None | ||
| } |
There was a problem hiding this comment.
If we do it this way, then the build should almost immediately fail to configure, users will see the warning, and they can decide what to do differently.
| if path.exists() { | |
| Some(path) | |
| } else { | |
| warn!( | |
| "VIRTUAL_ENV is set to `{}` but no Python interpreter found at `{}`; ignoring", | |
| dir.to_string_lossy(), | |
| path.display() | |
| ); | |
| None | |
| } | |
| if !path.exists() { | |
| warn!( | |
| "VIRTUAL_ENV is set to `{}` but no Python interpreter found at `{}`", | |
| dir.to_string_lossy(), | |
| path.display() | |
| ); | |
| } | |
| Some(path) |
Same for CONDA_PREFIX below.
2 participants
Fixes #6177.
When
VIRTUAL_ENVorCONDA_PREFIXis set but points to a deleted or Python-less environment, pyo3-build-config would blindly use the derived interpreter path and fail withNo such file or directory.Now we check that the interpreter exists before using it, falling back to other detection methods with a warning if it does not.