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.

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

bpo-25711: zipimport rewritten in Python #4023

Closed
wants to merge 2 commits into from

Conversation

warsaw
Copy link
Member

@warsaw warsaw commented Oct 17, 2017

This starts from @serhiy-storchaka 's zipimport-2.patch from bpo-25711. It does not yet work, but it will be easier to comment on and fix as a PR rather than a patch.

https://bugs.python.org/issue25711

@@ -0,0 +1 @@
zipimport has been rewritten in pure Python.
Copy link
Contributor

Choose a reason for hiding this comment

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

A separate news entry for the DeprecationWarning -> TypeError change for creating zip importers directly from bytes paths would probably make sense. (Or at least mentioning it in this one)


_importing_zlib = False

# Return the zlib.decompress function object, or NULL if zlib couldn't
Copy link

Choose a reason for hiding this comment

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

NULL doesn't exist in python. Should be "raises a ZipImportError ..."

@warsaw
Copy link
Member Author

warsaw commented Jan 24, 2018

I wonder if it's really so useful to keep this PR around, especially in light of @Yhg1s promised rewrite?

@Yhg1s
Copy link
Member

Yhg1s commented Jan 24, 2018

It can always be re-opened. I do still plan to open-source my zipimport rewrite, hopefully before PyCon, but it is in C, not Python. If people value having a pure-Python version of zipimport in the standard library, that's a different matter.

(I do recommend not using zipimport.c as a guide for a pure-Python version of zipimport, as this version has done... It has a number of bugs, like how it handles offsets in the central directory and how it closes and then re-opens the ZIP file, which may have changed, without re-reading the central directory. Also, no zip64 support. And as written, zipimport.py (partially) reimplements zipfile, which is a bit of a shame... Although I also have plenty of issues with zipfile :-P)

@warsaw
Copy link
Member Author

warsaw commented Jan 24, 2018

Alright, I'm closing this since I don't think it has a viable future. It was mainly an exercise in trying to convert the original bpo patch into a branch+PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants