Skip to content

Replace deprecated os.path.commonprefix with os.path.commonpath #13847

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.

Sign up for GitHub

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

Merged
ichard26 merged 1 commit into from
Mar 22, 2026
+11 −8
Merged

Replace deprecated os.path.commonprefix with os.path.commonpath #13847

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions news/13847.bugfix.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Use a path-segment prefix comparison, not char-by-char.
16 changes: 9 additions & 7 deletions src/pip/_internal/network/auth.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
import urllib.parse
from abc import ABC, abstractmethod
from functools import cache
from os.path import commonprefix
from os.path import commonpath
from pathlib import Path
from typing import Any, NamedTuple

Expand Down Expand Up @@ -325,12 +325,14 @@ def _get_index_url(self, url: str) -> str | None:

candidates.sort(
reverse=True,
key=lambda candidate: commonprefix(
[
parsed_url.path,
candidate.path,
]
).rfind("/"),
key=lambda candidate: len(
commonpath(
[
parsed_url.path,
candidate.path,
]
)
),
Comment on lines +328 to +335

@ichard26 ichard26 Mar 15, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't seem exactly behaviour equivalent to the old logic? Although I'm not sure what the old logic is supposed to do either. Pick the least deep common prefixing path?

@hugovk hugovk Mar 15, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's mostly the same, and the case it's different doesn't matter.

Each of these lambdas is returning an integer to show how close each of the candidate URLs are to the input one, then using the integer to sort the list and pick the first one as the closest match.

  • Before, commonprefix matched by character. So given something like "/path/path2/" and "/path/path3/", it'd give "/path/path". Then rfind("/") gives us the index of that last "/", or 5.

  • After, commonpath matches by full path parts. With the same inputs, we get "/path". len of that is also 5.

There is one slight difference, but it doesn't matter, and it's for the case when there isn't really any match.

  • If we have two paths like "/path/path2/" and "/path4/path3/", before commonprefix would give "/path", and `rfind("/") would give the index of the only "/", or 0.

  • Afterwards, commonpath gives us "/", whose len is 1.

That's okay, because in both cases, the shortest value we get for a meaningful common match is 2. For example, with "/p/path2/" and "/p/path3/":

  • Before: commonprefix(["/p/path2/", "/p/path3/"]) -> "/p/path" -> rfind("/") -> 2

  • After: commonpath(["/p/path2/", "/p/path3/"]) -> "/p" -> len -> 2

@ichard26 ichard26 Mar 22, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see. I did misread the original code, so that didn't help. Thanks!

)

return urllib.parse.urlunsplit(candidates[0])
Expand Down
2 changes: 1 addition & 1 deletion tests/conftest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ def pytest_collection_modifyitems(config: Config, items: list[pytest.Function])

module_file = item.module.__file__
module_path = os.path.relpath(
module_file, os.path.commonprefix([__file__, module_file])
module_file, os.path.commonpath([__file__, module_file])
)

module_root_dir = module_path.split(os.pathsep)[0]
Expand Down
Loading