GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — bpo-39899: Make pathlib use `os.path.expanduser()` to expand home directories by barneygale · Pull Request #18841 · python/cpython

Conversation

@barneygale

@barneygale barneygale commented Mar 8, 2020 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

os.path.expanduser(): don't guess other Windows users' home directories if the basename of the current user's home directory doesn't match their username.

This makes ntpath.expanduser() match pathlib.Path.expanduser() in this regard, and is more in line with posixpath.expanduser()'s cautious approach.

Also remove the near-duplicate implementation of expanduser() in pathlib, and by doing so fix a bug where KeyError could be raised when expanding another user's home directory on Windows.

https://bugs.python.org/issue39899

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@the-knights-who-say-ni the-knights-who-say-ni added the CLA not signed label Mar 8, 2020
@the-knights-who-say-ni

the-knights-who-say-ni commented Mar 8, 2020

Copy link
Copy Markdown

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept this contribution by verifying everyone involved has signed the PSF contributor agreement (CLA).

CLA Missing

Our records indicate the following people have not signed the CLA:

@barneygale

For legal reasons we need all the people listed to sign the CLA before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

If you have recently signed the CLA, please wait at least one business day
before our records are updated.

You can check yourself to see if the CLA has been received.

Thanks again for the contribution, we look forward to reviewing it!
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot bedevere-bot added the awaiting review label Mar 8, 2020
@the-knights-who-say-ni the-knights-who-say-ni added CLA signed and removed CLA not signed labels Mar 9, 2020
@barneygale
barneygale force-pushed the bpo-39899-pathlib-expanduser-reimpl branch 3 times, most recently from 0e82850 to b9b9801 Compare March 10, 2020 20:23
pitrou
pitrou reviewed Apr 17, 2020
View reviewed changes

@pitrou pitrou left a comment

Copy link
Copy Markdown
Member

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.

I see no problem with this personally but I would like advice from our Windows experts, @zooba and @pfmoore, especially on the os.path.expanduser changes.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Lib/ntpath.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
@barneygale
barneygale force-pushed the bpo-39899-pathlib-expanduser-reimpl branch from b9b9801 to 2a49892 Compare January 21, 2021 23:52
@barneygale

barneygale commented Jan 22, 2021

Copy link
Copy Markdown
Contributor Author

Updated to use os.fsdecode() per review comments. Thanks for your help so far @pitrou.

@pfmoore @zooba flagging this with you again per pitrou's recommendation!
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@zooba

zooba commented Apr 7, 2021

Copy link
Copy Markdown
Member

I think this one is okay, but I need to spend a bit more time reading through it (it's pretty late here now and I'm too tired).
barneygale reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@barneygale

barneygale commented Apr 7, 2021

Copy link
Copy Markdown
Contributor Author

Yep understood! If it helps, the rough process here is:

  1. Bring os.path.expanduser() in line with pathlib's home directory resolution. They were already almost identical, but pathlib was slightly stricter in the rare scenario:
  • We're on Windows, and
  • USERNAME is set, and
  • It doesn't match the last component of the current user's home directory, and
  • We're querying for another user's home directory
  1. Delete pathlib's implementation in the name of DRY.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
zooba
zooba reviewed Apr 7, 2021
View reviewed changes
Comment thread Lib/ntpath.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
@barneygale
barneygale force-pushed the bpo-39899-pathlib-expanduser-reimpl branch from 2a49892 to d7074c2 Compare April 7, 2021 22:03
@barneygale

barneygale commented Apr 7, 2021

Copy link
Copy Markdown
Contributor Author

Rebased to fix conflicts
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@zooba
zooba merged commit 3f3d82b into python:master Apr 7, 2021
@bedevere-bot bedevere-bot removed the awaiting review label Apr 7, 2021
@barneygale
bpo-39899: os.path.expanduser(): don't guess other Windows users' hom…
d7074c2 
…e directories if the basename of the current user's home directory doesn't match their username.

This makes `ntpath.expanduser()` match `pathlib.Path.expanduser()` in this regard, and is more in line with `posixpath.expanduser()`'s cautious approach.

Also remove the near-duplicate implementation of `expanduser()` in pathlib, and by doing so fix a bug where KeyError could be raised when expanding another user's home directory.
@barneygale barneygale mentioned this pull request Apr 8, 2021
bpo-39899: ntpath.expanduser(): don't check the basename of the directory matches the username if we're requesting the current user's home directory #25277
Merged
@alexdelorenzo alexdelorenzo mentioned this pull request Apr 16, 2021
Python 3.10 alpha 7 introduces new changes to pathlib alexdelorenzo/aiopath#4
Open
@barneygale barneygale mentioned this pull request May 15, 2021
bpo-44136: remove pathlib._Flavour #26141
Closed
@akulakov akulakov mentioned this pull request Sep 6, 2021
[3.9] bpo-41082: Add note on errors that may be raised by home() and expanduser() #28186
Merged
@barneygale barneygale mannequin mentioned this pull request Apr 10, 2022
pathlib.Path.expanduser() does not call os.path.expanduser() #84080
Closed
This was referenced Aug 4, 2026
fix(hermes): follow wrapper-script exec target when locating Hermes Python mnemosyne-oss/mnemosyne#620
Merged
fix(cli): cover the population file's content in the run's identity Leuven-Gravity-Institute/gwmock#337
Merged
[Feature] Add 'datus package' command: export a self-contained project zip Datus-ai/Datus-agent#1262
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@zooba zooba zooba left review comments
@pitrou pitrou pitrou left review comments

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

5 participants

@barneygale @the-knights-who-say-ni @zooba @pitrou @bedevere-bot
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.