Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 34k
bpo-38671: Make sure to return an absolute path in pathlib._WindowsFlavour.resolve()#17716
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
Uh oh!
There was an error while loading. Please reload this page.
Conversation
uranusjr commented Dec 27, 2019 • edited by bedevere-bot
Loading Uh oh!
There was an error while loading. Please reload this page.
edited by bedevere-bot
Uh oh!
There was an error while loading. Please reload this page.
78953ba to 34c68f7CompareUh oh!
There was an error while loading. Please reload this page.
ofek commented Jul 14, 2020
Rebase? |
uranusjr commented Jul 15, 2020 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
I’ll rebase and change the call to Edit: Done. I ended up adding the |
80498e7 to 54b8063CompareUh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
uranusjr commented Jul 19, 2020
OK, I’ve
I split each of these changes into individual commits so they can be easily reverted if needed. |
6b7ebcc to 42fd0b0Compareofek commented Aug 11, 2020
Will this be in 3.9? |
ofek commented Sep 23, 2020
Any progress? |
ofek commented Oct 5, 2020
Is this just blocked on reviews? |
Lib/pathlib.py Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lines 189-191 make a bad assumption that the current working directory (CWD) is a final path. The CWD is an absolute path but not a final path. The assignment should be something like s = str(path) or '.'. In practice it may never be an issue since PurePath.__str__ assigns and returns self._str = self._format_parsed_parts(self._drv, self._root, self._parts) or '.'.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The CWD is an absolute path but not a final path.
TIL.
I like the or '.' approach since it avoids special-casing things, but want @pitrou to take a look since this is inherited from the initial pathlib implementation.
Uh oh!
There was an error while loading. Please reload this page.
prescod commented Mar 9, 2021
Is this a case of letting the best be the enemy of the good? This bug is a real time-waster and can easily slip into cross-platform code. If the issues @eryksun highlights are pre-existing then perhaps you should merge it and then fix those separately? |
eryksun commented Mar 9, 2021 • edited by bedevere-bot
Loading Uh oh!
There was an error while loading. Please reload this page.
edited by bedevere-bot
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
ofek commented Mar 10, 2021
Is it too late for this to be in 3.10? |
eryksun commented Mar 10, 2021
@ofek, this is a bug fix, not a new feature. Even if it were, new features can be added to 3.10 until the first beta release in May. Hopefully it gets merged into the 3.8 branch in time for the 3.8.9 release in May, since it's the last bug-fix release for 3.8. |
This guarentees we are returning an absolute path when the input `path` is relative. Nothing would change if `path` is already absolute.
42fd0b0 to 8f20c10Compareuranusjr commented Apr 27, 2021
Strong nudge.
This is our last chance. |
ofek commented May 2, 2021
Needs one final rebase |
uranusjr commented May 2, 2021 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
I’ve rebased this multiple times and this time the new incoming changes is too much for me. I give up. Someone else can take over. |
https://bugs.python.org/issue38671