os.path.expanduser should not use HOME on windows

[asottile]

BPO	36264
Nosy	@pfmoore, @jaraco, @blueyed, @tjguk, @zware, @eryksun, @zooba, @lazka, @asottile, @miss-islington
PRs	bpo-36264: Don't honor POSIX HOME in os.path.expanduser on windows #12282 bpo-36264: Updates documentation for change to expanduser on Windows #12294 bpo-38883: Don't honor POSIX HOME in pathlib.Path.home/expanduser on Windows #17961 [3.8] bpo-38883: Don't use POSIX $HOME in pathlib.Path.home/expanduser on Windows (GH-17961) #18229

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/zooba'
closed_at = <Date 2019-03-12.22:16:16.883>
created_at = <Date 2019-03-11.16:09:57.334>
labels = ['3.8', 'type-bug', 'library', 'OS-windows']
title = 'os.path.expanduser should not use HOME on windows'
updated_at = <Date 2020-05-16.17:18:51.381>
user = 'https://github.com/asottile'

bugs.python.org fields:

activity = <Date 2020-05-16.17:18:51.381>
actor = 'jaraco'
assignee = 'steve.dower'
closed = True
closed_date = <Date 2019-03-12.22:16:16.883>
closer = 'steve.dower'
components = ['Library (Lib)', 'Windows']
creation = <Date 2019-03-11.16:09:57.334>
creator = 'Anthony Sottile'
dependencies = []
files = []
hgrepos = []
issue_num = 36264
keywords = ['patch']
message_count = 27.0
messages = ['337683', '337750', '337752', '337754', '337755', '337756', '337763', '337781', '337784', '337813', '338162', '357182', '357192', '357194', '357197', '360846', '360853', '368931', '368932', '368933', '368935', '368963', '369025', '369034', '369045', '369049', '369060']
nosy_count = 10.0
nosy_names = ['paul.moore', 'jaraco', 'blueyed', 'tim.golden', 'zach.ware', 'eryksun', 'steve.dower', 'lazka', 'Anthony Sottile', 'miss-islington']
pr_nums = ['12282', '12294', '17961', '18229']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue36264'
versions = ['Python 3.8']

[asottile]

The current code for os.path.expanduser in ntpath uses HOME in preference to USERPROFILE / HOMEDRIVE\\HOMEPATH

I can't find any documentation of HOME being relevant on windows (only on posix). For example, wikipedia only mentions USERPROFILE / HOMEDRIVE\\HOMEPATH options: https://en.wikipedia.org/wiki/Home_directory#Default_home_directory_per_operating_system

Seems to be (one of) the direct causes of a downstream bug for me: pre-commit/pre-commit#960

(msys sets HOME to a bogus value if HOMEDRIVE is set, then python uses it)

My proposal is to remove these lines and change the elif to an if:

cpython/Lib/ntpath.py

Lines 302 to 304 in d9bd8ec

	if 'HOME' in os.environ:
	userhome = os.environ['HOME']
	elif 'USERPROFILE' in os.environ:

[zooba]

I like the patch, but I'm not sure all the tests are properly preserving the real value of USERPROFILE.

Modifying this value could have a real impact on the rest of the process, so we should be very careful to undo it regardless of test result. (Modifying HOME is not a as big a deal since, as you point out, it's not "real" ;) )

Also, it's probably better to get the current value and check against that, rather than setting it to a known value. One day we might do the right thing and switch expanduser() from the environment variables to the correct API, which would break the test.

[asottile]

Modifying this value could have a real impact on the rest of the process, so we should be very careful to undo it regardless of test result. (Modifying HOME is not a as big a deal since, as you point out, it's not "real" ;) )

Agreed, though I'd like to write it off as an existing problem (if it is a problem, it looks to me like the tests are doing proper environment cleanup). If they are broken, they'd already be leaking environment variables on posix since these tests run on both

Auditing the tests I modified:

• test_netrc: uses `with support.EnvironmentVarGuard() as environ:` (OK!)
• test_ntpath: uses `with support.EnvironmentVarGuard() as env:` (OK!)
• test_dist: `MetadataTestCase` extends `support.EnvironGuard` (OK!)
• test_config: `BasePyPIRCCommandTestCase` extends `support.EnvironGuard` (OK!)

so actually, looks like this is already good to go on that front!

Definitely agree on using the true value in the long term -- I'd like to defer that until that's actually happening though as it'll be a much more involved patch!

[zooba]

Great, thanks for checking! I'll merge.

[zooba]

New changeset 25ec4a4 by Steve Dower (Anthony Sottile) in branch 'master':
bpo-36264: Don't honor POSIX HOME in os.path.expanduser on Windows (GH-12282)
25ec4a4

[zware]

If we're going ahead with this, it's worth a mention in whatsnew as this is going to break things for some users.

[zooba]

If we're going ahead with this, it's worth a mention in whatsnew

Good call. I'll do it

[eryksun]

os.path.expanduser in ntpath uses HOME in preference to
USERPROFILE / HOMEDRIVE\\HOMEPATH

Guido intentionally added support for HOME in ntpath.expanduser way back in Python 1.5 (circa 1997), and now we're removing it over 20 years later. I expect this to break some deployments, but it's not a serious problem.

My feeling here is "meh". Practically all use of expanduser() is dubious in Windows, varying from going against convention to being completely wrong. We're long due for a module in the standard library that abstracts access to platform-specific configuration and special directories, but attempts to adopt one keep losing momentum. I guess it's too prone to bike shedding and arguments.

[zooba]

Guido intentionally added support for HOME in ntpath.expanduser way back in Python 1.5 (circa 1997), and now we're removing it over 20 years later.

Wouldn't be the first thing to be removed :)

[zooba]

New changeset 8ef864d by Steve Dower in branch 'master':
bpo-36264: Updates documentation for change to expanduser on Windows (GH-12294)
8ef864d