classification
Title: _zip_directory_cache untested and undocumented
Type: behavior Stage: needs patch
Components: Documentation, Extension Modules Versions: Python 3.3, Python 2.7
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: eric.araujo, eric.snow, fijal, georg.brandl, ncoghlan, tarek
Priority: normal Keywords:

Created on 2008-05-23 20:26 by fijal, last changed 2013-04-07 15:22 by terry.reedy.

Messages (19)
msg67267 - (view) Author: Maciek Fijalkowski (fijal) Date: 2008-05-23 20:25
_zip_directory_cache has no single test nor piece of documentation. The
only place where it's mentioned is a call to method .clear() on it in
tests. Besides, it can be whatever. Despite _ prefix, which I would
consider to be private, distutils crash obscurely if it's not there or
does not contain sequences as keys.
msg84384 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2009-03-29 12:56
I can't find a reference to _zip_directory_cache in distutils.  Do you
mean setuptools?
msg84397 - (view) Author: Maciek Fijalkowski (fijal) Date: 2009-03-29 13:46
Seems I meant setuptools indeed. Note that brett's importlib contains
some tests for _zip_directory_cache.
msg84911 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2009-03-31 21:17
That's fortunate, because I can just reassign this to him :)
msg85069 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2009-04-01 18:44
Actually importlib does not test this; that is an old thing from an old
sandbox version of importlib that I no longer use.

I am assigning to Tarek as the better solution is for distutils to stop
using this "private" attribute.
msg85585 - (view) Author: Tarek Ziadé (tarek) * (Python committer) Date: 2009-04-05 23:18
_zip_directory_cache is part of the zipimport module (C module) you can
find in Modules/zipimport.c.

The tests are located here: ./Lib/test/test_zipimport.py

_zip_directory_cache is a dict used by zipimporter_init to speed up the
lookup.

Notice that if it is successfully compiled, zipimport is used by pkgutil.

So Maciek, did you have a bug with this ?
msg85586 - (view) Author: Maciek Fijalkowski (fijal) Date: 2009-04-05 23:29
The bug with this is that setuptools uses it (and even relies on what is
there and in what order), while it has no single tests and no documentation.
msg85591 - (view) Author: Tarek Ziadé (tarek) * (Python committer) Date: 2009-04-05 23:56
> while it has no single tests and no documentation."

what "it" refers to ? zipimport, distutils or setuptools ?
msg85595 - (view) Author: Maciek Fijalkowski (fijal) Date: 2009-04-06 00:18
it == _zip_directory_cache
msg85597 - (view) Author: Tarek Ziadé (tarek) * (Python committer) Date: 2009-04-06 00:33
I am not sure what you would like to see added.

in zipimport.c:

"- _zip_directory_cache: a dict, mapping archive paths to zip directory
  info dicts, as used in zipimporter._files.
"

while there are no direct test, Lib/test/test_zipimport.py covers it
becauseit is used when you instanciate a zipimport class.

would you like to see a section explaining how it works at 
http://docs.python.org/library/zipimport.html ?
msg85598 - (view) Author: Tarek Ziadé (tarek) * (Python committer) Date: 2009-04-06 00:35
maybe what's missing is a public API for the clear() method since
setuptools uses it by calling it directly
msg85599 - (view) Author: Maciek Fijalkowski (fijal) Date: 2009-04-06 00:59
setuptools at least deletes stuff from _zip_directory_cache (via _uncache
function in easy_install.py). it also iterates over it and checks for
existance of elements.

What about documenting details about _zip_directory_cache:
* does it need to be a normal python dictionary or anything?
* what methods it needs to support?
* what api zipimporter exposes?

Right now the situation is:
if I want my library not to be cached any more, I need to grab object
called _zip_directory_cache, and remove things from it. That's a very
non-obvious API for caching. Is it documented in some PEP?

The way I stomped on it - just passing the tests of zipimporter module
actually does not guarantee that you've implemented it correctly. You
also need to expose at least _zip_directory_cache otherwise setuptools
won't be too happy.

Stdlib is not only about testing cpython, it's also about what you
define as python these days.

I propose to have a clean api on zipimporter how to invalidate it's
caches. Maybe a general mechanism for that, I don't know.
msg85608 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2009-04-06 04:03
Does distutils use _zip_directory_cache in any way? If not then setuptools
is in the wrong here for using a private attribute and it is on them to make
it work if the attribute does not exist, period.

Otherwise someone will need to come up with a very simple API or set or
rules to follow for clearing the cache and get setuptools to switch to it
(IMO it should be handled by simply deleting all entries for a module in
sys.modules, removing the path hook from sys.path_hooks, and then clear out
sys.path_importer_cache for all paths associated with the module). Be aware,
though, this is all leading down the road of special-casing zip imports when
you may very well run into the same issues if people start to develop other
importers -- e.g. tarball, dbm, or sqlite3 importers -- and this will all
start up again.
msg85610 - (view) Author: Tarek Ziadé (tarek) * (Python committer) Date: 2009-04-06 04:32
> Does distutils use _zip_directory_cache in any way?

Not at all. But pkgutil does, and uses to get a dirlist,
so I guess a new API should be created in zipimport to handle this case
as well. (I think it's some setuptools code originallyt, that landed in
pkgutil)
msg85660 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2009-04-06 18:16
2009/4/5 <"Tarek Ziadé <report@bugs.python.org>"@psf.upfronthosting.co.za>

>
> Tarek Ziadé <ziade.tarek@gmail.com> added the comment:
>
> > Does distutils use _zip_directory_cache in any way?
>
> Not at all. But pkgutil does,

Ah, OK.

> and uses to get a dirlist,

Couldn't one just open the zip file with zipfile to get the directory list?
Is there any real requirement that the cache must be used?

>
> so I guess a new API should be created in zipimport to handle this case
> as well. (I think it's some setuptools code originallyt, that landed in
> pkgutil)

Probably.
msg118403 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2010-10-12 05:05
Could the recently-added functools.lru_cache help here?
msg175807 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2012-11-17 20:44
Eric, as you know importlib, can you comment about this?  If zipimport now works with the invalidate_caches system, then we do have a documented way of clearing the cache, and I would recommend closing this bug (the existing attribute should stay for compat, but we should not document or test it more IMO).
msg175808 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2012-11-17 20:53
I'll take a look but can't say when.  My wife's is having a baby in the next couple days so I'm a bit swamped.  :)
msg175810 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2012-11-17 21:28
No problem (and congratulations :)
History
Date User Action Args
2013-04-07 15:22:57terry.reedysetversions: + Python 3.3, - Python 3.1, Python 3.2
2012-11-17 21:28:01eric.araujosetnosy: + ncoghlan
messages: + msg175810
2012-11-17 20:53:07eric.snowsetmessages: + msg175808
2012-11-17 20:44:23eric.araujosetassignee: tarek ->

messages: + msg175807
nosy: + eric.snow
2012-11-17 17:51:26brett.cannonsetnosy: - brett.cannon
2010-10-12 05:05:52eric.araujosetversions: + Python 3.2
nosy: + eric.araujo

messages: + msg118403

components: - Distutils
2010-10-12 05:04:33eric.araujosetfiles: - unnamed
2010-10-12 05:04:19eric.araujosetfiles: - unnamed
2009-04-06 18:16:17brett.cannonsetfiles: + unnamed

messages: + msg85660
2009-04-06 04:32:42tareksetmessages: + msg85610
2009-04-06 04:03:44brett.cannonsetfiles: + unnamed

messages: + msg85608
2009-04-06 00:59:01fijalsetmessages: + msg85599
2009-04-06 00:35:09tareksetmessages: + msg85598
2009-04-06 00:33:05tareksetmessages: + msg85597
2009-04-06 00:18:46fijalsetmessages: + msg85595
2009-04-05 23:56:32tareksetmessages: + msg85591
2009-04-05 23:29:59fijalsetmessages: + msg85586
2009-04-05 23:18:44tareksetmessages: + msg85585
2009-04-04 08:21:25tareksetpriority: normal
versions: + Python 3.1, Python 2.7, - Python 2.6
2009-04-01 18:44:42brett.cannonsetassignee: brett.cannon -> tarek
messages: + msg85069
stage: needs patch
2009-03-31 21:17:56georg.brandlsetassignee: georg.brandl -> brett.cannon

messages: + msg84911
nosy: + brett.cannon
2009-03-29 13:46:54fijalsetmessages: + msg84397
2009-03-29 12:56:27georg.brandlsetmessages: + msg84384
2009-02-11 04:45:26ajaksu2setnosy: + tarek
2008-05-23 20:26:03fijalcreate