Skip to content

bpo-29901: Improve support of path-like objects in zipapp. #815

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

Merged
serhiy-storchaka merged 1 commit into from
Mar 25, 2017
Merged

bpo-29901: Improve support of path-like objects in zipapp. #815

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions Doc/library/zipapp.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,11 +103,11 @@ The module defines two convenience functions:
Create an application archive from *source*. The source can be any
of the following:

* The name of a directory, or a :class:`pathlib.Path` object referring
* The name of a directory, or a :term:`path-like object` referring
to a directory, in which case a new application archive will be
created from the content of that directory.
* The name of an existing application archive file, or a :class:`pathlib.Path`
object referring to such a file, in which case the file is copied to
* The name of an existing application archive file, or a :term:`path-like object`
referring to such a file, in which case the file is copied to
the target (modifying it to reflect the value given for the *interpreter*
argument). The file name should include the ``.pyz`` extension, if required.
* A file object open for reading in bytes mode. The content of the
Expand All @@ -117,7 +117,7 @@ The module defines two convenience functions:
The *target* argument determines where the resulting archive will be
written:

* If it is the name of a file, or a :class:`pathlb.Path` object,
* If it is the name of a file, or a :term:`path-like object`,
the archive will be written to that file.
* If it is an open file object, the archive will be written to that
file object, which must be open for writing in bytes mode.
Expand Down
11 changes: 4 additions & 7 deletions Lib/zipapp.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,7 @@ class ZipAppError(ValueError):

@contextlib.contextmanager
def _maybe_open(archive, mode):
if isinstance(archive, pathlib.Path):
archive = str(archive)
if isinstance(archive, str):
if isinstance(archive, (str, os.PathLike)):
with open(archive, mode) as f:
yield f
else:
Expand Down Expand Up @@ -135,10 +133,9 @@ def create_archive(source, target=None, interpreter=None, main=None):
with _maybe_open(target, 'wb') as fd:
_write_file_prefix(fd, interpreter)
with zipfile.ZipFile(fd, 'w') as z:
root = pathlib.Path(source)
for child in root.rglob('*'):
arcname = str(child.relative_to(root))
z.write(str(child), arcname)
for child in source.rglob('*'):
arcname = child.relative_to(source).as_posix()
z.write(child, arcname)
Copy link
Member

@pfmoore pfmoore Mar 25, 2017

Choose a reason for hiding this comment

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

I assume that ZipFile.write() now takes a general path object, and doesn't require conversion to a string? The docs for that function don't explicitly state this, unlike some other functions in that module, but I assume that's just a documentation oversight.

Also, there's a minor change in behaviour here, as the archive name is always in POSIX format now, whereas before it was in OS-specific format. But IMO that's entirely reasonable (and was an oversight on my part in the original code) so I'm happy with that.
Copy link
Member Author

@serhiy-storchaka serhiy-storchaka Mar 25, 2017

Choose a reason for hiding this comment

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

There is no behavior change. ZipFile itself converts the archive name to POSIX format. But I think it is better to do this explicitly. Maybe implicit conversion will be deprecated in future.
Copy link
Member

@pfmoore pfmoore Mar 25, 2017

Choose a reason for hiding this comment

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

Ah, OK. I agree, explicit is better here.
if main_py:
z.writestr('__main__.py', main_py.encode('utf-8'))

Expand Down
3 changes: 3 additions & 0 deletions Misc/NEWS
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,9 @@ Extension Modules
Library
-------

- bpo-29901: The zipapp module now supports general path-like objects, not
just pathlib.Path.

- bpo-25803: Avoid incorrect errors raised by Path.mkdir(exist_ok=True)
when the OS gives priority to errors such as EACCES over EEXIST.

Expand Down