This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: zipimport needs to support namespace packages when no 'directory' entry exists
Type: enhancement Stage: needs patch
Components: Library (Lib) Versions: Python 3.8
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: Arfrever, barry, eric.smith, eric.snow, gregory.p.smith, isoschiz, jerub, jpaugh, ncoghlan, pconnell, pje, ronaldoussoren, serhiy.storchaka, superluser
Priority: normal Keywords: patch

Created on 2012-05-25 00:27 by eric.smith, last changed 2022-04-11 14:57 by admin.

Files
File name Uploaded Description Edit
zipimport-issue14905.patch jerub, 2012-07-07 14:47 review
zipimport-issue14905-2.patch jerub, 2012-08-19 12:59 review
Messages (16)
msg161543 - (view) Author: Eric V. Smith (eric.smith) * (Python committer) Date: 2012-05-25 00:27
If a zip file contains "pkg/foo.py" but no "pkg/" entry, it will not be possible for "pkg" to be a namespace package portion.
msg161544 - (view) Author: Eric V. Smith (eric.smith) * (Python committer) Date: 2012-05-25 00:33
For a (very) brief discussion on the strategy to implement this, see: http://mail.python.org/pipermail/import-sig/2012-May/000528.html
msg161584 - (view) Author: Eric V. Smith (eric.smith) * (Python committer) Date: 2012-05-25 15:16
See also test_namespace_pkgs.py ZipWithMissingDirectory.test_missing_directory which is currently marked as expectedFailure.
msg164861 - (view) Author: Stephen Thorne (jerub) * Date: 2012-07-07 14:47
Here is a patch that synthesises the directory names at the point where file names are read in. The unit test now passes, and has had the expected failure removed.

Patch collaboration with Diarmuid Bourke <diarmuidbourke@gmail.com> at the europython sprint.
msg168568 - (view) Author: Stephen Thorne (jerub) * Date: 2012-08-19 12:59
Please see attached new patch, based on review comments.
msg182245 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2013-02-16 22:25
This can significant slowdown zipimport. I think we shouldn't support such broken zip files in zipimport.
msg182254 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2013-02-17 02:29
How common are such broken zip files? Like Serhiy, I'm concerned about the possible negative impact on the interpreter startup time as we try to second guess the contents of the zip file manifest.

It seems better to be explicit that we consider such zipfiles broken and they need to be regenerated with full manifests (perhaps providing a script in Tools that fixes them).
msg182255 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2013-02-17 02:31
OTOH, the scan time should be short relative to the time needed to read the manifest in the first place - an appropriate microbenchmark may also be adequate to address my concerns.
msg182279 - (view) Author: Eric V. Smith (eric.smith) * (Python committer) Date: 2013-02-17 17:26
I don't think such files are common: I've never seen such a file "in the wild". I created one, by accident, while testing PEP 420.

OTOH, it was surprisingly easy to create the malformed file with zipfile.
msg182326 - (view) Author: Ronald Oussoren (ronaldoussoren) * (Python committer) Date: 2013-02-18 15:33
Why are zipfiles without entries for directories broken? When you don't care about directory permissions (such as when the zipfile won't be extracted at all) the entries for directories are not necessary. Also, AFAIK the zipfile specification <http://www.pkware.com/documents/casestudies/APPNOTE.TXT> does not require adding directory entries to the zipfile.

FWIW: the zipfiles created by py2app do no contain entries for directories at the moment.  I'll probably add entries for directories in the next update to work around this issue.
msg185799 - (view) Author: PJ Eby (pje) * (Python committer) Date: 2013-04-02 03:25
Just a note: the zip files produced by the distutils and friends (sdist, bdist_dumb, eggs) do not include entries for plain directories.  I would guess that this is also true for wheels at the moment, unless something was specifically done to work around this property of distutils-generated zip files.  So ISTM the right thing to do is to synthesize the entries at directory read time, when they're being looped over anyway.

Reviewing the patch, there is a performance optimization possible by making a slight change to the algorithm.  Currently the patch loops from the start of the string to the end, looking for path prefixes.  This means that the total overall performance is determined by the length of the strings and especially the average directory depth.

However, there is a significant shortcut possible: looping from the *end* of each string to the beginning, it's possible to break out of the loop if the prefix has already been seen -- thus saving (depth-1) dictionary lookups in the average case, and only looking at the characters in the base filename, unless a new directory is encountered... for a typical overhead of one unicode substring, dictionary lookup, and strrchr per zipfile directory entry.  (Which is very small compared to what else is going on at that point in the process.)

To elaborate, if you have paths of the form:

x/y/a
x/y/b
x/y/c/d

Then when processing 'x/y/a', you would first process x/y -- it's not in the dict, add it.  Then x -- not in the dict, add it.  Then you go to x/y/b, your first parent is x/y again -- but since it's in the dict you skip it, and don't even bother with the x.  Next you see x/y/c, which is not in the dict, so you add it, then x/y, which is, so you break out of the loop for that item.

Basically, about all that would change would be the for() loop starting at the end of the string and going to the beginning, with the loop position still representing the end of the prefix to be extracted.  And the PyDict_Contains check would result in a break rather than a continue.

So, if the only concern keeping the patch from being accepted is that it adds to startup time, this approach would cut down quite a bit on the overhead for generating the path information, in cases of repeated prefixes.  (And in the common cases for zipfile use on sys.path, one would expect to see a lot of common prefixes, if only for package names.)
msg185936 - (view) Author: Phil Connell (pconnell) * Date: 2013-04-03 16:47
The problem appears to be more general. zipimport fails for deeper hierarchies, even with directory entries.

With the supplied patch (zipimport-issue14905-2.patch) I see the following:

$ unzip -l foo.zip
Archive:  foo.zip
  Length      Date    Time    Name
---------  ---------- -----   ----
        0  2013-04-03 17:28   a/b/c/foo.py
        0  2013-04-03 17:34   a/
        0  2013-04-03 17:34   a/b/
        0  2013-04-03 17:34   a/b/c/
---------                     -------
        0                     4 files
$ ls
foo.zip
$ PYTHONPATH=foo.zip ~/dev/cpython/python
Python 3.4.0a0 (default:3b1dbe7a2aa0+, Apr  3 2013, 17:31:54) 
[GCC 4.8.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import a
>>> import a.b
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ImportError: No module named 'a.b'
>>>
msg186026 - (view) Author: Phil Connell (pconnell) * Date: 2013-04-04 10:12
I've raised issue17633 to track the issue in my last message.
msg325724 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-09-19 07:31
zipimport have been rewritten in pure Python (issue25711).
msg325764 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-09-19 13:54
Issue34738 fixes distutils.
msg375307 - (view) Author: Phil Connell (pconnell) * Date: 2020-08-13 13:47
One version of the bug described here (and fixed in the old implementation under issue17633) exists in the Python implementation of zipimport:

$ unzip -l namespace1.zip
Archive:  namespace1.zip
  Length      Date    Time    Name
---------  ---------- -----   ----
        0  08-13-2020 06:30   one/
        0  08-13-2020 06:30   one/two/
        0  08-13-2020 06:30   one/two/three.py
---------                     -------
        0                     3 files
$ unzip -l namespace2.zip
Archive:  namespace2.zip
  Length      Date    Time    Name
---------  ---------- -----   ----
        0  08-13-2020 06:37   alpha/beta/gamma.py
---------                     -------
        0                     1 file
$ PYTHONPATH=namespace1.zip:namespace2.zip ./python
Python 3.10.0a0 (heads/master:c51db0ea40, Aug 13 2020, 06:41:20)
[GCC 4.8.5 20150623 (Red Hat 4.8.5-39)] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import one
>>> import alpha
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ModuleNotFoundError: No module named 'alpha'
>>>

In short, imports where there's no separate entry for directories in the zip file don't work.

Any opinions on whether this *is* the problem this issue is trying to track?
History
Date User Action Args
2022-04-11 14:57:30adminsetgithub: 59110
2020-08-13 13:47:03pconnellsetmessages: + msg375307
2019-04-30 22:54:33barrysetnosy: + barry
2019-04-28 20:52:26jaracolinkissue36740 superseder
2018-09-19 15:12:18ppperrysettitle: zipimport.c needs to support namespace packages when no 'directory' entry exists -> zipimport needs to support namespace packages when no 'directory' entry exists
2018-09-19 13:54:17serhiy.storchakasetmessages: + msg325764
2018-09-19 07:31:28serhiy.storchakasetstage: patch review -> needs patch
type: behavior -> enhancement
components: + Library (Lib), - Extension Modules
versions: + Python 3.8, - Python 3.6
2018-09-19 07:31:05serhiy.storchakasetmessages: + msg325724
2015-08-05 16:07:32eric.snowsetnosy: + gregory.p.smith
2015-08-05 16:06:15eric.snowsetnosy: + superluser

versions: + Python 3.6, - Python 3.3, Python 3.4
2013-04-04 10:12:12pconnellsetmessages: + msg186026
2013-04-03 16:47:11pconnellsetmessages: + msg185936
2013-04-03 16:33:21isoschizsetnosy: + isoschiz
2013-04-03 16:11:12pconnellsetnosy: + pconnell
2013-04-02 03:25:16pjesetnosy: + pje
messages: + msg185799
2013-02-18 18:33:44brett.cannonsetnosy: - brett.cannon
2013-02-18 15:33:24ronaldoussorensetnosy: + ronaldoussoren
messages: + msg182326
2013-02-17 17:26:09eric.smithsetmessages: + msg182279
2013-02-17 02:31:36ncoghlansetmessages: + msg182255
2013-02-17 02:29:44ncoghlansetmessages: + msg182254
2013-02-16 22:25:51serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg182245
2013-02-16 22:14:02pitrousetnosy: + brett.cannon, ncoghlan

versions: + Python 3.4
2012-08-19 12:59:24jerubsetfiles: + zipimport-issue14905-2.patch

messages: + msg168568
2012-08-18 13:36:05Arfreversetnosy: + Arfrever
2012-08-18 03:22:40eric.snowsetstage: needs patch -> patch review
2012-08-03 23:06:36eric.snowsetnosy: + eric.snow
2012-08-02 16:20:27jpaughsetnosy: + jpaugh
2012-07-07 14:47:49jerubsetfiles: + zipimport-issue14905.patch

nosy: + jerub
messages: + msg164861

keywords: + patch
2012-05-25 15:16:55eric.smithsetmessages: + msg161584
2012-05-25 00:33:02eric.smithsetmessages: + msg161544
2012-05-25 00:27:06eric.smithcreate