Skip to content

bpo-33660: Fix corner-case in PosixPath.resolve resulting in "//path"#21971

Closed
mbarkhau wants to merge 1 commit into
python:masterfrom
mbarkhau:fix-pathlib-resolve-at-root
Closed

bpo-33660: Fix corner-case in PosixPath.resolve resulting in "//path"#21971
mbarkhau wants to merge 1 commit into
python:masterfrom
mbarkhau:fix-pathlib-resolve-at-root

Conversation

@mbarkhau

@mbarkhau mbarkhau commented Aug 26, 2020 •
edited by bedevere-bot
Loading

Copy link
Copy Markdown
Contributor

This PR is provisional, a bpo doesn't exist yet.

On POSIX, when Path.cwd() == Path("/"), then Path("path").resolve() == Path("//var")

This is because the normal case is that sep must be added between the base path and the relative path, as the prefix/base does not usually end with sep itself. In this respect, the root directory Path("/") is a corner-case that I believe was previously missed.

A consequence of this can be that the relative_to method raises an error, even though the intent was to just normalize different both absolute and relative paths to relative paths:

In [9]: Path.cwd()
Out[9]: PosixPath('/')

In [10]: Path("path").resolve().relative_to(Path("/"))
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
<ipython-input-10-02608bb55fa0> in <module>
----> 1 Path("path").resolve().relative_to(Path("/"))

~/miniconda3/envs/py38/lib/python3.8/pathlib.py in relative_to(self, *other)
    897         if (root or drv) if n == 0 else cf(abs_parts[:n]) != cf(to_abs_parts):
    898             formatted = self._format_parsed_parts(to_drv, to_root, to_parts)
--> 899             raise ValueError("{!r} does not start with {!r}"
    900                              .format(str(self), str(formatted)))
    901         return self._from_parsed_parts('', root if n == 1 else '',

ValueError: '//path' does not start with '/'

https://bugs.python.org/issue33660

@mbarkhau
initial fix and test Path.resolve() at /
9b0724c 
On posix, when `Path.cwd() == "/"`, the `Path("var").resolve()`
produced `"//var"`. This is because the sep usually needs to be
added together with a prefix/base to a relative path, as the
prefix/base does not usually end with the sep itself. In this
respect, the root directory is a corner-case that was previously
missed.
@mbarkhau

mbarkhau commented Aug 26, 2020

Copy link
Copy Markdown
Contributor Author

I see now this relates to https://bugs.python.org/issue33660

@mbarkhau mbarkhau changed the title Fix corner-case in PosixPath.resolve resulting in "//path" bpo-33660: Fix corner-case in PosixPath.resolve resulting in "//path" Aug 26, 2020
@mbarkhau

mbarkhau commented Aug 26, 2020

Copy link
Copy Markdown
Contributor Author

Related: psf/black#1631

@ambv

ambv commented Aug 26, 2020

Copy link
Copy Markdown
Contributor

Thanks for this but I'd rather merge the original PR by Dong-Hee Na.

@ambv ambv closed this Aug 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

awaiting review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@mbarkhau @ambv @the-knights-who-say-ni @bedevere-bot