Bugs item #856103, was opened at 2003-12-08 02:48 Message generated for change (Comment added) made by montanaro You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=105470&aid=856103&group_id=5470
Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: Python Interpreter Core Group: Python 2.4 Status: Open Resolution: None Priority: 5 Submitted By: Tony Meyer (anadelonbrin) >Assigned to: Nobody/Anonymous (nobody) Summary: reload() fails with modules from zips Initial Comment: 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. ---------------------------------------------------------------------- >Comment By: Skip Montanaro (montanaro) Date: 2005-05-30 19:09 Message: Logged In: YES user_id=44345 Tossing back into the pool. ---------------------------------------------------------------------- Comment By: Phillip J. Eby (pje) Date: 2004-09-23 08:41 Message: 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. ---------------------------------------------------------------------- Comment By: Phillip J. Eby (pje) Date: 2004-09-23 01:21 Message: 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. ---------------------------------------------------------------------- Comment By: Stephen Haberman (filitov) Date: 2004-09-16 16:00 Message: 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 ---------------------------------------------------------------------- Comment By: Stephen Haberman (filitov) Date: 2004-04-02 17:26 Message: 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. ---------------------------------------------------------------------- Comment By: Just van Rossum (jvr) Date: 2004-04-02 10:53 Message: 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. ---------------------------------------------------------------------- Comment By: Stephen Haberman (filitov) Date: 2004-04-02 10:35 Message: 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. ---------------------------------------------------------------------- Comment By: Stephen Haberman (filitov) Date: 2004-04-02 09:38 Message: 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. ---------------------------------------------------------------------- Comment By: Skip Montanaro (montanaro) Date: 2004-04-02 06:23 Message: 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. ---------------------------------------------------------------------- Comment By: Just van Rossum (jvr) Date: 2004-04-02 01:00 Message: 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... ---------------------------------------------------------------------- Comment By: Skip Montanaro (montanaro) Date: 2004-04-01 21:37 Message: 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. ---------------------------------------------------------------------- Comment By: Stephen Haberman (filitov) Date: 2004-04-01 18:26 Message: 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; ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=105470&aid=856103&group_id=5470 _______________________________________________ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
