Skip to content
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

Jump to bottom

bpo-30218: support PathLike in shutil.unpack_archive #1367

Merged
merged 5 commits into from
May 5, 2017
Merged

bpo-30218: support PathLike in shutil.unpack_archive #1367

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
c3a2a9f
bpo-30218: support PathLike in shutil.unpack_archive
JelleZijlstra Apr 30, 2017
3bd1c43
call fspath on extract_dir too
JelleZijlstra May 2, 2017
e3dcfad
add Misc/NEWS entry
JelleZijlstra May 3, 2017
6293174
put it in the right section
JelleZijlstra May 3, 2017
11a987a
add doc note
JelleZijlstra May 5, 2017
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
3 changes: 3 additions & 0 deletions Lib/shutil.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -958,6 +958,9 @@ def unpack_archive(filename, extract_dir=None, format=None):
if extract_dir is None:
extract_dir = os.getcwd()

extract_dir = os.fspath(extract_dir)
filename = os.fspath(filename)
Copy link
Member

Choose a reason for hiding this comment

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

I notice that the tests show extract_dir working for a path-like object, but there's no explicit conversion here. If it's because something else explicitly covers that then please add a comment. Otherwise just explicitly do the os.fspath() conversion for extract_dir.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it works because it's only passed to other functions that already accept PathLikes. However, it is passed to a handler function that is specific to a file format and maybe some handlers don't accept PathLike, so I'll just add os.fspath for it anyway.


if format is not None:
try:
format_info = _UNPACK_FORMATS[format]
Expand Down
21 changes: 17 additions & 4 deletions Lib/test/test_shutil.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import os.path
import errno
import functools
import pathlib
import subprocess
from shutil import (make_archive,
register_archive_format, unregister_archive_format,
Expand Down Expand Up @@ -1223,6 +1224,18 @@ def test_register_archive_format(self):
self.assertNotIn('xxx', formats)

def check_unpack_archive(self, format):
self.check_unpack_archive_with_converter(format, lambda path: path)
self.check_unpack_archive_with_converter(format, pathlib.Path)

class MyPath:
def __init__(self, path):
self.path = path
def __fspath__(self):
return self.path

self.check_unpack_archive_with_converter(format, MyPath)

def check_unpack_archive_with_converter(self, format, converter):
root_dir, base_dir = self._create_files()
expected = rlistdir(root_dir)
expected.remove('outer')
Expand All @@ -1232,16 +1245,16 @@ def check_unpack_archive(self, format):

# let's try to unpack it now
tmpdir2 = self.mkdtemp()
unpack_archive(filename, tmpdir2)
unpack_archive(converter(filename), converter(tmpdir2))
self.assertEqual(rlistdir(tmpdir2), expected)

# and again, this time with the format specified
tmpdir3 = self.mkdtemp()
unpack_archive(filename, tmpdir3, format=format)
unpack_archive(converter(filename), converter(tmpdir3), format=format)
self.assertEqual(rlistdir(tmpdir3), expected)

self.assertRaises(shutil.ReadError, unpack_archive, TESTFN)
self.assertRaises(ValueError, unpack_archive, TESTFN, format='xxx')
self.assertRaises(shutil.ReadError, unpack_archive, converter(TESTFN))
self.assertRaises(ValueError, unpack_archive, converter(TESTFN), format='xxx')

def test_unpack_archive_tar(self):
self.check_unpack_archive('tar')
Expand Down