Skip to content
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

os.path.relpath() assumes '//' root is equivalent to '/' #92737

Open
iynehz opened this issue May 12, 2022 · 5 comments · May be fixed by #103798
Open

os.path.relpath() assumes '//' root is equivalent to '/' #92737

iynehz opened this issue May 12, 2022 · 5 comments · May be fixed by #103798
Labels
stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@iynehz
Copy link

iynehz commented May 12, 2022

Bug report

pathlib's Path.relative_to() and os.path.relpath(), the two have different behavior: On Linux os.path.relpath() understands double-slash in path is same as single slash, but Path.relative_to() does not.

For example,

import os
os.path.relpath('//foo/bar', '/foo')                 # results 'bar'

from pathlib import Path
pathlib.PosixPath('//foo/bar').relative_to('/foo')   # raises ValueError

pytest issue pytest-dev/pytest#9894 relates to this.

Your environment

  • CPython versions tested on: 3.9.2
  • Operating system and architecture: Debian Bullseye

Linked PRs

@iynehz iynehz added the type-bug An unexpected behavior, bug, or error label May 12, 2022
@eryksun
Copy link
Contributor

eryksun commented May 12, 2022

Cross-platform POSIX applications should not assume that "//foo" is the same as "/foo". A path that begins with two slashes is reserved in POSIX for use by implementations. Linux handles "//foo" the same as "/foo". Cygwin, on the other hand, supports UNC paths such as "//server/share".

A strict=False parameter could be added to posixpath.relpath() in order to force POSIX-ly correct behavior. For pathlib.PosixPath.relative_to() the default could be strict=True.

@eryksun eryksun added the stdlib Python modules in the Lib dir label May 12, 2022
@achhina
Copy link
Contributor

achhina commented Mar 10, 2023

@eryksun Would you still think adding a strict parameter be a good way to move forward with this? If so, I'd like to attempt to implement that.

@barneygale
Copy link
Contributor

barneygale commented Mar 12, 2023

I'd argue that posixpath.relpath() should simply be fixed and that there's no need for a new parameter. I recall GvR saying he cringes a little every time we add a new parameter that users need to set to get correct behaviour.

The implementation should call splitroot() and raise if the roots are different, just like ntpath.relpath() does.

@achhina
Copy link
Contributor

achhina commented Apr 24, 2023

@barneygale Would in this case fixing posixpath.relpath mean no longer assuming paths beginning // are the same as /? If that's the case, would we need to introduce a deprecation cycle for users who are relying on the current assumption?

@barneygale
Copy link
Contributor

would we need to introduce a deprecation cycle for users who are relying on the current assumption?

It's a bugfix, so not necessarily. Quoth PEP 387:

This policy applies to all public APIs. These include: [...] Given a set of arguments, the return value, side effects, and raised exceptions of a function. This does not preclude changes from reasonable bug fixes.

But if we think fixing it will cause sadness, we could skip backporting or perhaps hide it behind an argument as Eryk suggests.

achhina added a commit to achhina/cpython that referenced this issue Apr 24, 2023
ambv added a commit to achhina/cpython that referenced this issue Apr 25, 2023
achhina added a commit to achhina/cpython that referenced this issue Apr 29, 2023
achhina added a commit to achhina/cpython that referenced this issue May 28, 2023
achhina added a commit to achhina/cpython that referenced this issue Jun 4, 2023
achhina added a commit to achhina/cpython that referenced this issue Aug 27, 2023
achhina added a commit to achhina/cpython that referenced this issue Dec 3, 2023
@barneygale barneygale changed the title Inconsistent behavior os.path.relpath() vs pathlib's relative_to() os.path.relpath() assumes '//' root is equivalent to '/' Jan 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants