If you call reload() with a module that was imported from 
a zip, it fails with a "no such module" error.  Although 
zips are typically read-only, it is possible that a zip could 
be modified during a run, and a reload be necessary.  If 
this is considered unnecessary, then I think a more 
informative "can't reload from zip" error would be better 
than a 'no such module" one.

"""
>set PYTHONPATH=path/to/spambayes.zip
>python

>>> from spambayes import Options
>>> Options
<module 'spambayes.Options' 
from 'c:\spambayes\windows\py2exe\dist\lib\spambayes
.zip\spambayes\Options.pyc'>
>>> reload(Options)
Traceback (most recent call last):
  File "<stdin>", line 1, in ?
ImportError: No module named Options
"""

This is with Python 2.3.2 and WinXP.

Logged In: YES 
user_id=642545

Here's a patch that fixes the problem so that modules from
zip files can be reloaded.

The problem was the PyImport_ReloadModulefunction not
passing a loader around. Perhaps this is naive, and the
PyImport_ReloadModule was purposefully not using a loader,
but, again, it works for me.

cvs diff -u dist\src\Python\import.c (in directory
C:\cvs\python\)
Index: dist/src/Python/import.c
===================================================================
RCS file: /cvsroot/python/python/dist/src/Python/import.c,v
retrieving revision 2.230
diff -u -r2.230 import.c
--- dist/src/Python/import.c	1 Apr 2004 02:45:22 -0000	2.230
+++ dist/src/Python/import.c	2 Apr 2004 00:18:46 -0000
@@ -2217,7 +2217,7 @@
 PyImport_ReloadModule(PyObject *m)
 {
 	PyObject *modules = PyImport_GetModuleDict();
-	PyObject *path = NULL;
+	PyObject *path, *loader = NULL;
 	char *name, *subname;
 	char buf[MAXPATHLEN+1];
 	struct filedescr *fdp;
@@ -2259,11 +2259,12 @@
 			PyErr_Clear();
 	}
 	buf[0] = '\0';
-	fdp = find_module(name, subname, path, buf, MAXPATHLEN+1,
&fp, NULL);
+	fdp = find_module(name, subname, path, buf, MAXPATHLEN+1,
&fp, &loader);
 	Py_XDECREF(path);
 	if (fdp == NULL)
 		return NULL;
-	m = load_module(name, fp, buf, fdp->type, NULL);
+	m = load_module(name, fp, buf, fdp->type, loader);
+    Py_XDECREF(loader);
 	if (fp)
 		fclose(fp);
 	return m;

Logged In: YES 
user_id=44345

attached is the patch to protect it from the vagaries of cut-n-
paste.  I also added a simple testReload test to the 
test_zipimport.py file.  I can't get zip imports to work.  Perhaps 
Just can test this out.

Logged In: YES 
user_id=92689

The import.c patch looks fine (although I didn't test it yet). Skip, 
what doesn't work for you? "I can't get zip imports to work" is 
quite broad...

Logged In: YES 
user_id=44345

> what doesn't work for you?

Sorry.  Pilot error.  I was adding foozip.zip to sys.path then trying

    from foozip import somemodule

instead of just

    import somemodule

(Obvious, not a feature I use a lot...)

Seems to be working for me now.  I'll try a couple more tests then 
check it in.

Logged In: YES 
user_id=642545

Actually, it doesn't seem to be working fully.

I've been playing with Skip's test patch and it looks like
the code it reloads uses...the same buffer size as it did
with the original read? You can play with replacing 'return
__name__' with different strings and it will change the
error you get.

E.g. with 'return __name__+"modified"', it parses fine, but
get_foo() is not found (which is added after get_name()).

E.g. with 'return "new"', it does not parse fine, it returns
'NameError: name 'de' is not defined'. My guess is that it
got part way through reading "def get_foo(): return 1" and
hit the end of a buffer.

Or something. At this point, its beyond my C/Python code
skills to track it down. Hopefully its just some minor error
in re-loading the code from the new zip file you guys can
easily find.

I can't see where to upload a patch file, so you can get
what I'm currently playing with at:

http://sh5.beachead.com:8080/~stephen/patch.txt

Note the import.c patch is the same, the new patch just adds
stuff to Skip's testReload function to try loading code from
a new zip file.

Logged In: YES 
user_id=642545

So, upon some more investigation, it looks like the problem
is that the toc_entry from the old zip file is being used,
which has the old file size, not the new file size.

It looks like in zipimport.c, line 126, where the
zip_directory_cache is used to cache all of the toc_entry's
for a zip file, a check on the zip file modification should
be made, just as the modification check is done vs. .py
files, I would imagine.

This would require storing another a tuple in
zip_directory_cache of (modification_time, list of toc_entry's).

Let me know if you want me to take a shot at implementing
this. Otherwise I'd prefer deferring to you guys, as I
assume you can do it with much less time and much higher
quality than I could.

Thanks.

Logged In: YES 
user_id=92689

Hm, I forgot about the caching of directory info. I don't think 
properly supporting reloading from zip files is worth the effort. It 
would be nice if it failed nicer, though.

The reload patch is still cool, though, since it should fix reloading 
with PEP 302-style importers in general (yet zipimport may still 
choose not to support it). A reload test in test_importhooks.py 
would be a great addition.

Logged In: YES 
user_id=642545

Okay, so you talked me in to it. Besides the previous
PyImport_ReloadModule patch, I modified zipimport.c to add
another cache (zip_directory_cache of toc_entry's and
zip_mtime_cache of modification times).

On creation of a new ZipImporter or call to get_module_code,
check_archive_mtime is called, which gets the new mtime and
compares to the cached one. If they are different, it calls
the old read_directory.

read_directory was modified to, instead of creating a brand
new path: [toc_entry] dictionary, clear and re-populate the
same 'files' dictionary. This is so that if multiple
ZipImporters are sharing an archive, and hence already
sharing the same 'files' entry in zip_directory_cache, if
one of them detects a new zip file and has the toc_entry's
reloaded, the other ZipImporters will see the change to (as
they all share the same dictionary).

This was pretty much the same functionality before (sharing
dictionaries), just that now read_directory clears/updates
an exisitng one instead creating its own brand new one.

Also, I had to add a sleep(1) call in testReload to ensure
the modification time stamp would change.

Again, either I don't have permissions to upload files, or I
just can't figure it out, so the patch is at:

http://sh5.beachead.com:8080/~stephen/patch.txt

This is my first foray into Python coding, so double
checking all of the reference counting inc/dec stuff would
be a really good idea.

I also took about 20 minutes to look at adding a reload test
to test_importhooks.py, as suggested, but couldn't get it to
work. I couldn't tell if it was because I was writing a
flawed test (which is what I am suspecting) or if the
PyImport_ReloadModule patch was not working.

Logged In: YES 
user_id=642545

Turns out there are two issues here: PEP 302 using the
loader, and then the zip import caching the contents of zip
files.

At the suggestion of Phillip Eby, the PEP 302 stuff not
using a loader has been put in a new bug that he is going to
take care of:

https://sourceforge.net/tracker/index.php?func=detail&aid=1029475&group_id=5470&atid=105470

So, now there is just the separate issue of the zip import
not checking the file modification time. The attached patch
makes sure when reload(aZipModule) is called that the file
modification time is checked and that the contents are reloaded.

More details are in my last comment.

There is a test case that passes as well.

The patch is at a more permanent location this time:

http://www.chase3000.com/stephenh/patch-zipimport.txt

Logged In: YES 
user_id=56214

I've fixed bug #1029475, so reload() will now actually
invoke the PEP 302 loader mechanism.  

However, there's another bug in zipimport: the loader
implementation in zipimport.c always creates a new module,
even when reloading.  It needs to check sys.modules first,
and reuse the existing module if present.  I'm attempting to
fix that issue now, and once I'm done, someone more familiar
with zipimport's cache mechanism can do the rest.

Logged In: YES 
user_id=56214

D'oh!  PyImport_AddModule() only creates a new module if it
doesn't already exist in sys.modules, so there's nothing to
fix on that point.

Now what's needed is someone familiar with zipimport.c
internals to review Stephen's patch at:

http://www.chase3000.com/stephenh/patch-zipimport.txt

and clean out its stderr prints.

Logged In: YES 
user_id=44345

Tossing back into the pool.