Repository navigation
gh-161788: Handle path-like commands in shutil.which() - #161789
Conversation
Resolve command objects through the path protocol before selecting text or bytes handling. This preserves the documented bytes return type for path-like objects backed by bytes. Ref python#161788
picnixz
left a comment
picnixz
left a comment
There was a problem hiding this comment.
I don't think we ever actually accepted path-like objects for shutil.which() btw. So this is not an issue and rather a new feature. However, I don't think it's correct to accept pathlike because of the way we treat bytes for now. shutil.which(s: T) returns an object type T and returning a bytes instead of a path of bytes seems also wrong.
|
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 |
| rv = shutil.which(self.file, path=self.dir) | ||
| self.assertEqual(rv, self.filepath) | ||
|
|
||
| def test_pathlike_cmd(self): |
There was a problem hiding this comment.
Good: this test is run on Unicode string (TestWhich) and bytes string (TestWhichBytes).
Honestly, it doesn't matter since it would be about maybe relying on some non-common details (I think it's documented but having an explicit call to |
|
Ok, fine, I merged the PR as it is! Thanks @Builder106 for your fix! |
|
I consider that this change adds a new feature: support path-like objects which return bytes. So I don't think that it should be backported to stable branches: we don't backport new features. |
|
Yes, I think it's a new feature as well. |
|
Oh maybe, we should reword the NEWS entry and add a |
|
@Builder106 Can you open a follow-up PR to fix the formulation of the NEWS entry and update the docs actually please? |
Resolve
cmdwithos.fspath()beforeshutil.which()decides whether to use text or bytes paths. This makes bytes-backed path-like commands behave like directbytescommands and preserves the existing return type.The regression test runs through the existing text and bytes
TestWhichvariants.Tests
All 240 tests passed; 52 platform-specific tests were skipped on Linux ARM64.