Support Path objects in the posix module #70215

serhiy-storchaka · 2016-01-06T21:05:30Z

BPO	26027
Nosy	@brettcannon, @pfmoore, @tjguk, @ethanfurman, @berkerpeksag, @vadmium, @zware, @serhiy-storchaka, @eryksun, @zooba, @JelleZijlstra
Dependencies	bpo-26671: Clean up path_converter in posixmodule.c bpo-26800: Don't accept bytearray as filenames part 2
Files	path_converter_path.patch issue27186-os_path_t.patch path_converter.diff: Inline of PyOS_FSPath() path_converter.diff: Updated patch since adding the warning about bytearrays path_converter.diff: Incorporate Serhiy's review feedback

^{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/brettcannon'
closed_at = <Date 2016-08-27.19:59:15.038>
created_at = <Date 2016-01-06.21:05:29.971>
labels = ['extension-modules', 'type-feature', 'OS-windows']
title = 'Support Path objects in the posix module'
updated_at = <Date 2016-09-06.22:59:12.215>
user = 'https://github.com/serhiy-storchaka'

bugs.python.org fields:

activity = <Date 2016-09-06.22:59:12.215>
actor = 'python-dev'
assignee = 'brett.cannon'
closed = True
closed_date = <Date 2016-08-27.19:59:15.038>
closer = 'berker.peksag'
components = ['Extension Modules', 'Windows']
creation = <Date 2016-01-06.21:05:29.971>
creator = 'serhiy.storchaka'
dependencies = ['26671', '26800']
files = ['42382', '43238', '44029', '44090', '44159']
hgrepos = []
issue_num = 26027
keywords = ['patch']
message_count = 27.0
messages = ['257642', '262963', '267427', '269212', '269220', '269797', '269801', '272061', '272556', '272650', '272677', '273161', '273734', '273735', '273745', '273748', '273749', '273754', '273786', '273788', '273791', '273792', '273793', '273799', '273800', '274650', '274654']
nosy_count = 12.0
nosy_names = ['brett.cannon', 'paul.moore', 'tim.golden', 'ethan.furman', 'python-dev', 'berker.peksag', 'martin.panter', 'zach.ware', 'serhiy.storchaka', 'eryksun', 'steve.dower', 'JelleZijlstra']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue26027'
versions = ['Python 3.6']

serhiy-storchaka · 2016-01-06T21:05:30Z

path_converter should be changed to accept objects with the "path" attribute. See bpo-22570 for details.

serhiy-storchaka · 2016-04-06T20:33:56Z

Here is preliminary patch without tests. Writing tests will be tiresome.

JelleZijlstra · 2016-06-05T16:34:21Z

The patch is obsolete because PEP-519 ended up going with a different approach. I submitted a new patch for the path_t converter in bpo-27186. That patch probably fits better here, so I'm resubmitting it.

brettcannon · 2016-06-24T21:03:24Z

Did you still want to handle this, Serhiy, or should I assign the issue to myself?

serhiy-storchaka · 2016-06-25T05:41:27Z

I'll take a look.

serhiy-storchaka · 2016-07-04T16:50:47Z

At first glance bpo-27186-os_path_t.patch looks good.

But with the patch applied the error message in case of incorrect argument type is always "expected str, bytes or os.PathLike object, not ...". Currently it is more detailed and specific: contains the function and the argument names, and is aware that some functions accept an integer or None. I think the best way is to inline the code of PyOS_FSPath in path_converter.

Also we first resolve bpo-26800.

brettcannon · 2016-07-04T17:28:30Z

I have no issues inlining -- with a comment about the inlining -- if it buys us better error messages in this critical case.

brettcannon · 2016-08-05T21:56:29Z

Here is a version of Jelle's patch but with PyOS_FSPath() inlined.

Serhiy, does this work for you?

brettcannon · 2016-08-12T18:50:11Z

Here is an updated patch that adds in change to posixmodule.c stemming from the new warning about using bytearrays. It also makes type checking more stringent for what __fspath__() returns.

serhiy-storchaka · 2016-08-14T06:03:59Z

Added comments on Rietveld. Needed tests for supporting path-like objects. And it would be nice to have few tests for error messages.

brettcannon · 2016-08-14T16:00:36Z

Thanks for the review! I'll probably update the patch next week based on your feedback (which I agree with).

As for error messages and tests, they exist in my patches on issues dependent on this one (e.g. the tests included in issues bpo-27524 and bpo-27182 which pull in Jelle's test from his original patch); I have been doing all of my os package-related changes in a single checkout and so this one isn't wholly written in isolation.

brettcannon · 2016-08-19T22:09:42Z

Here is a patch that incorporates Serhiy's feedback.

python-dev · 2016-08-26T21:45:25Z

New changeset b64f83d6ff24 by Brett Cannon in branch 'default':
Issue bpo-26027, bpo-27524: Add PEP 519/fspath() support to os and
https://hg.python.org/cpython/rev/b64f83d6ff24

brettcannon · 2016-08-26T21:46:04Z

Thanks to Jelle for the initial commit and Serhiy for the code review!

vadmium · 2016-08-27T01:56:37Z

This change causes test_os to produce warnings, and can fail:

$ hg update b64f83d6ff24
$ ./python -bWerror -m test -u all -W test_os
[. . .]

======================================================================
ERROR: test_path_t_converter (test.test_os.PathTConverterTests) (name='stat', path=bytearray(b'@test_12055_tmp'))
----------------------------------------------------------------------

Traceback (most recent call last):
  File "/media/disk/home/proj/python/cpython/Lib/test/test_os.py", line 2865, in test_path_t_converter
    result = fn(path, *extra_args)
DeprecationWarning: stat: path should be string, bytes, os.PathLike or integer, not bytearray

Similar warnings:
DeprecationWarning: lstat: path should be string, bytes or os.PathLike, not bytearray
DeprecationWarning: access: path should be string, bytes, os.PathLike or integer, not bytearray
DeprecationWarning: open: path should be string, bytes or os.PathLike, not bytearray

python-dev · 2016-08-27T02:30:20Z

New changeset 32b93ba32aa0 by Brett Cannon in branch 'default':
Issue bpo-26027: Don't test for bytearray in path_t as that's now
https://hg.python.org/cpython/rev/32b93ba32aa0

brettcannon · 2016-08-27T02:31:11Z

Thanks for catching that, Martin. I removed the test for bytearray as it was originally written before the deprecation.

vadmium · 2016-08-27T03:16:57Z

One more thing, ;) the Windows buildbots are failing to removing a temporary file:

http://buildbot.python.org/all/builders/AMD64%20Windows7%20SP1%203.x/builds/8173/steps/test/logs/stdio

======================================================================
ERROR: test_path_t_converter (test.test_os.PathTConverterTests)
----------------------------------------------------------------------

Traceback (most recent call last):
  File "C:\buildbot.python.org\3.x.kloth-win64\build\lib\test\support\__init__.py", line 365, in unlink
    _unlink(filename)
  File "C:\buildbot.python.org\3.x.kloth-win64\build\lib\test\support\__init__.py", line 336, in _unlink
    _waitfor(os.unlink, filename)
  File "C:\buildbot.python.org\3.x.kloth-win64\build\lib\test\support\__init__.py", line 304, in _waitfor
    func(pathname)
PermissionError: [WinError 32] The process cannot access the file because it is being used by another process: '@test_5716_tmp'

Subsequently, other tests fail, probably because this file already exists.

brettcannon · 2016-08-27T16:43:19Z

Hopefully https://hg.python.org/cpython/rev/775158408ecb will fix the problem.

brettcannon · 2016-08-27T17:11:47Z

It's still failing: http://buildbot.python.org/all/builders/AMD64%20Windows7%20SP1%203.x/builds/8176/steps/test/logs/stdio .

Don't have time to look at why right now and I'm on a Mac ATM so I can't test locally to try and fix it until I'm at work on Monday. If someone has an idea as to why this is only happening on Windows I'm open to understanding.

python-dev · 2016-08-27T18:25:18Z

New changeset 8ec5a00e5d75 by Berker Peksag in branch 'default':
Issue bpo-26027: Fix test_path_t_converter on Windows
https://hg.python.org/cpython/rev/8ec5a00e5d75

berkerpeksag · 2016-08-27T18:31:36Z

test_path_t_converter failure looks similar to http://bugs.python.org/issue27493#msg271047 (fixed by 5424252ce174.) I've tested a fix on my Windows box and the test passed for me. Hopefully 8ec5a00e5d75 will fix the problem on buildbots too :)

eryksun · 2016-08-27T18:39:36Z

I wish the name was "pushCleanup" to emphasize that cleanup functions are popped and called in LIFO order.

berkerpeksag · 2016-08-27T19:59:15Z

Builtbots look happy now:

Windows 7 SP1: http://buildbot.python.org/all/builders/AMD64%20Windows7%20SP1%203.x/builds/8177
Windows 8: http://buildbot.python.org/all/builders/AMD64%20Windows8%203.x/builds/2476
Windows 10: http://buildbot.python.org/all/builders/AMD64%20Windows10%203.x/builds/1377

brettcannon · 2016-08-27T21:09:13Z

Thanks for fixing it, Berker!

On Sat, Aug 27, 2016, 12:59 Berker Peksag <report@bugs.python.org> wrote:

Berker Peksag added the comment:

Builtbots look happy now:

Windows 7 SP1:
http://buildbot.python.org/all/builders/AMD64%20Windows7%20SP1%203.x/builds/8177

Windows 8:
http://buildbot.python.org/all/builders/AMD64%20Windows8%203.x/builds/2476

Windows 10:
http://buildbot.python.org/all/builders/AMD64%20Windows10%203.x/builds/1377

----------
status: open -> closed

Python tracker <report@bugs.python.org>
<https://bugs.python.org/issue26027\>

python-dev · 2016-09-06T22:50:48Z

New changeset d0d9d7f55cb5 by Brett Cannon in branch 'default':
Issue bpo-26027: Support path-like objects in PyUnicode-FSConverter().
https://hg.python.org/cpython/rev/d0d9d7f55cb5

python-dev · 2016-09-06T22:59:12Z

New changeset 9be0286772bf by Brett Cannon in branch 'default':
Issue bpo-26027, bpo-27524: Document the support for path-like objects in os and os.path.
https://hg.python.org/cpython/rev/9be0286772bf

serhiy-storchaka self-assigned this Jan 6, 2016

serhiy-storchaka added extension-modules C modules in the Modules dir type-feature A feature request or enhancement labels Jan 6, 2016

serhiy-storchaka assigned brettcannon and unassigned serhiy-storchaka Aug 14, 2016

brettcannon closed this as completed Aug 26, 2016

vadmium reopened this Aug 27, 2016

brettcannon closed this as completed Aug 27, 2016

vadmium reopened this Aug 27, 2016

brettcannon closed this as completed Aug 27, 2016

brettcannon added the OS-windows label Aug 27, 2016

brettcannon reopened this Aug 27, 2016

berkerpeksag closed this as completed Aug 27, 2016

ezio-melotti transferred this issue from another repository Apr 10, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support Path objects in the posix module #70215

Support Path objects in the posix module #70215

serhiy-storchaka commented Jan 6, 2016

serhiy-storchaka commented Jan 6, 2016

serhiy-storchaka commented Apr 6, 2016

JelleZijlstra commented Jun 5, 2016

brettcannon commented Jun 24, 2016

serhiy-storchaka commented Jun 25, 2016

serhiy-storchaka commented Jul 4, 2016

brettcannon commented Jul 4, 2016

brettcannon commented Aug 5, 2016

brettcannon commented Aug 12, 2016

serhiy-storchaka commented Aug 14, 2016

brettcannon commented Aug 14, 2016

brettcannon commented Aug 19, 2016

python-dev mannequin commented Aug 26, 2016

brettcannon commented Aug 26, 2016

vadmium commented Aug 27, 2016

python-dev mannequin commented Aug 27, 2016

brettcannon commented Aug 27, 2016

vadmium commented Aug 27, 2016

brettcannon commented Aug 27, 2016

brettcannon commented Aug 27, 2016

python-dev mannequin commented Aug 27, 2016

berkerpeksag commented Aug 27, 2016

eryksun commented Aug 27, 2016

berkerpeksag commented Aug 27, 2016

brettcannon commented Aug 27, 2016

python-dev mannequin commented Sep 6, 2016

python-dev mannequin commented Sep 6, 2016

Support Path objects in the posix module #70215

Support Path objects in the posix module #70215

Comments

serhiy-storchaka commented Jan 6, 2016

serhiy-storchaka commented Jan 6, 2016

serhiy-storchaka commented Apr 6, 2016

JelleZijlstra commented Jun 5, 2016

brettcannon commented Jun 24, 2016

serhiy-storchaka commented Jun 25, 2016

serhiy-storchaka commented Jul 4, 2016

brettcannon commented Jul 4, 2016

brettcannon commented Aug 5, 2016

brettcannon commented Aug 12, 2016

serhiy-storchaka commented Aug 14, 2016

brettcannon commented Aug 14, 2016

brettcannon commented Aug 19, 2016

python-dev mannequin commented Aug 26, 2016

brettcannon commented Aug 26, 2016

vadmium commented Aug 27, 2016

python-dev mannequin commented Aug 27, 2016

brettcannon commented Aug 27, 2016

vadmium commented Aug 27, 2016

brettcannon commented Aug 27, 2016

brettcannon commented Aug 27, 2016

python-dev mannequin commented Aug 27, 2016

berkerpeksag commented Aug 27, 2016

eryksun commented Aug 27, 2016

berkerpeksag commented Aug 27, 2016

brettcannon commented Aug 27, 2016

python-dev mannequin commented Sep 6, 2016

python-dev mannequin commented Sep 6, 2016