gh-79125: Update _dirnameW to accept long path names

[jopamer]

The fix for issue 32557 updated os__getdiskusage_impl to use _dirnameW for obtaining the parent directory of a file. This would cause a regression if the path exceeded 260 characters, since _dirnameW currently returns -1 if given a path >= MAX_PATH in length.

As suggested in the issue's comments, _dirnameW should be updated to use PathCchRemoveFileSpec when available (on Win8.1 or greater) to avoid throwing an unnecessary error on a long path.

Note:
If PathCchRemoveFileSpec isn't available, we can call through PathRemoveFileSpecW, which is otherwise deprecated. What's interesting there is that while the docs say it expects a buffer of size MAX_PATH, analysis of the function shows that it doesn't make assumptions about the size of the path other than it's less than 32k characters in length. It calls through PathCchRemoveFileSpec under the hood on Win8 and greater, passing in 0x8000h as the cch argument. PathCchRemoveFileSpec then scans through the path for '' via wcschr, stops when it hits the last one and inserts a NULL. (Analysis of PathRemoveFileSpecW on a Win7 VM shows that it does basically the same thing, and is also resilient to paths greater than MAX_PATH in length.)

https://bugs.python.org/issue34944

• Issue: Update _dirnameW to accept long path names #79125

[zooba]

I'm not excited about adding a fatal error in the midst of a Python call, especially since we have an error code here that could be returned instead (inside getpathp.c is different, because we aren't initialized enough to raise an exception at that stage).

Setting an error here and returning non-zero should work fine. I believe all calls to this are already holding the GIL, but that would be the only issue.

[jopamer]

Thanks - I wasn't aware of the difference there. I'll update the diff.

[jopamer]

I've updated the diffs to address the concerns, though I'm a bit puzzled by the CI failures, since I'm not seeing any test failures locally.

[jopamer]

I think I see where things are going wrong - taking a look.

[jopamer]

Ok: os.symlink doesn't seem to have the GIL, hence the crash in some of the CI tests when they hit Py_BEGIN_ALLOW_THREADS in _dirnameW.

[jopamer]

Ok - I think I've addressed all the issues, and all tests are passing.

[jopamer]

Hi everyone - it's been a couple of weeks, so I just wanted to see if there was any feedback :)

[zware]

Hi @jopamer. Unfortunately this has wound up with a conflict since the last time @zooba reviewed it; would you mind to rebase it?

[jopamer]

Thanks for taking a look! I'll be happy to - I just need to spin up a new Windows environment.

[csabella]

@jopamer friendly ping :-)

[github-actions]

This PR is stale because it has been open for 30 days with no activity.

[zooba]

Just going to close this. If someone wants to work on it, we can now assume that the function is available, so it'll be a much simpler change than this one.