| msg22460 - (view) |
Author: Stephen Haberman (filitov) |
Date: 2004-09-16 20:35 |
| This is based on bug 856103 "reload() fails with modules from zips", but only a fix for the PEP 302 loaders not being used in reload. There are other issues with reloading zip files, specifically when the size of the zip changes, new buffers are not created, but that is a separate issue and dealt with in bug 856103 instead of here. ---- This patch just modifies import.c's PyImport_ReloadModule to use a loader argument to find_module and load_module. A few things to note: 1) After my original 856103 patch was made, the path parameter to find_module now has to be non null. This path variable was only being set in the subname != NULL case off of the parent, so I copied the logic to get the path off of the existing module in the subname == NULL case. This seems to work. 2) I was fairly sure with my original 856103 patch, "reload(mod)" and then "mod.newFunction()" would work. But now the test case has to do "mod = reload(mod)" for the "newFunction()" call to work. I do not know whether this is related to something in this patch or not. |
|
|
| msg22461 - (view) |
Author: Stephen Haberman (filitov) |
Date: 2004-09-16 20:37 |
| Logged In: YES user_id=642545 Trying to upload the patch. |
|
|
| msg22462 - (view) |
Author: Stephen Haberman (filitov) |
Date: 2004-09-16 20:41 |
| Logged In: YES user_id=642545 Attached the patch with a better test case. |
|
|
| msg22463 - (view) |
Author: Stephen Haberman (filitov) |
Date: 2004-09-16 20:45 |
| Logged In: YES user_id=642545 Okay, this one is actually the patch, not the test case itself. Sheesh. |
|
|
| msg22464 - (view) |
Author: PJ Eby (pje) *  |
Date: 2004-09-16 21:00 |
| Logged In: YES user_id=56214 Hi Stephen. I don't understand why 'path' has to be non-null. Not only that, but if it *is* null and the module has no __path__, your patch is still going to pass in a null path. So I'm confused. How did you discover the problem? Similarly, I'm confused as to why 'mod=reload(mod)' would be necessary now, but wasn't before. There have been no changes that I see in test_importhooks that would cause that. What was your testing methodology for this? |
|
|
| msg22465 - (view) |
Author: Stephen Haberman (filitov) |
Date: 2004-09-16 22:38 |
| Logged In: YES user_id=642545 You're right, the path = m.__path__ stuff was all wrong. I was not initializing path to NULL in the first place. I've attached the updated patch. |
|
|
| msg22466 - (view) |
Author: PJ Eby (pje) * |
Date: 2004-09-23 05:23 |
| Logged In: YES user_id=56214 Fixed for 2.4 and the 2.3 maintenance branch, using simpler test code, and adding a missing Py_XDECREF to the C change. PEP 302 has also been updated to point out that loaders must use the existing module in sys.modules, if present, and the TestImporter in test_importhooks has been changed to do this. zipimport, however, does not do this, and that is a problem that still needs fixing (see bug #856103 for some other zipimport+reload issues). |
|
|