gh-92675: venv: Fix ensure_directories() to again accept a Path for env_dir #92676
Conversation
Lib/venv/__init__.py
Outdated
| @@ -116,7 +116,7 @@ def create_if_needed(d): | |||
| elif os.path.islink(d) or os.path.isfile(d): | |||
| raise ValueError('Unable to create directory %r' % d) | |||
|
|
|||
| if os.pathsep in env_dir: | |||
| if isinstance(env_dir, str) and os.pathsep in env_dir: | |||
There was a problem hiding this comment.
It looks like the intent here is to enforce that env_dir is only a single path segment. Should we preserve that behavior for path-likes too? For example, we could call env_dir = os.fspath(env_dir) first.
I'm not sure why we need to check for this at all though. What is the reason we have this check?
There was a problem hiding this comment.
It's checking for os.pathsep, not os.sep: it was a fix for bpo-43218. I agree, we should retain the behaviour for path-likes too - otherwise the problem originally solved for will come back.
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
|
I have made the requested changes; please review again |
|
Thanks for making the requested changes! @vsajip: please review the changes made to this pull request. |
|
Thanks for the effort fixing this issue! Any update on its status? |
Thanks to JelleZijlstra for the feedback! Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
|
@JelleZijlstra thanks for the feedback! Did apply your suggestion. |
Fixes: #92675