-z, -i and -m, maybe bug in runpy?
While trying to get my -z replacement patch to work, I stumbled across a bug in the -m implementation (and in runpy). It seems that when you run the code of a -m module, it is *not* run in the __main__ module namespace! So even though __name__=='__main__', globals() is not sys.modules['__main__'].__dict__. This seems wrong to me. Does anybody know why runpy doesn't actually run the code in the target module? One consequence of this is that the -i option is much less useful when you use -m, because the script's globals have disappeared before you get to the interpreter prompt. At this point, I've successfully gotten a -z replacement patch, except that it inherits this apparent bug from -m, which for a while led me to believe my patch was broken (when in fact it works fine, apart from inheriting the -m behavior). Does anybody know if this behavior is intended, and if so, why? And what are the consequences of changing/fixing it?
Phillip J. Eby wrote:
While trying to get my -z replacement patch to work, I stumbled across a bug in the -m implementation (and in runpy). It seems that when you run the code of a -m module, it is *not* run in the __main__ module namespace!
So even though __name__=='__main__', globals() is not sys.modules['__main__'].__dict__. This seems wrong to me. Does anybody know why runpy doesn't actually run the code in the target module?
After implementing the runpy explicit relative import tests over the last couple of days, it actually occurred to me earlier today that I didn't have a test for this scenario. When I thought of the test, I was also pretty sure it would fail - it appears I was right :)
One consequence of this is that the -i option is much less useful when you use -m, because the script's globals have disappeared before you get to the interpreter prompt.
At this point, I've successfully gotten a -z replacement patch, except that it inherits this apparent bug from -m, which for a while led me to believe my patch was broken (when in fact it works fine, apart from inheriting the -m behavior).
Does anybody know if this behavior is intended, and if so, why? And what are the consequences of changing/fixing it?
I think it's a bug - when fiddling with the sys module runpy.run_module() only uses the real module name, and not the requested run_name. It should actually modify both so that the assertion "globals() is sys.modules[__name__].__dict__" is correct when running the module. So, I propose that if runpy.run_module is told it has permission to modify the sys module, and run_name is specified and exists in sys.modules, then runpy will use that module to execute the code, instead of creating a new temporary module. If run_name isn't in sys.modules, then the temporary module used to execute the code will be stored in both places. This is a semantic change so we can't really backport it, but we can at least fix it for 2.6. Cheers, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia --------------------------------------------------------------- http://www.boredomandlaziness.org
Nick Coghlan wrote:
Phillip J. Eby wrote:
While trying to get my -z replacement patch to work, I stumbled across a bug in the -m implementation (and in runpy). It seems that when you run the code of a -m module, it is *not* run in the __main__ module namespace!
So even though __name__=='__main__', globals() is not sys.modules['__main__'].__dict__. This seems wrong to me. Does anybody know why runpy doesn't actually run the code in the target module?
After implementing the runpy explicit relative import tests over the last couple of days, it actually occurred to me earlier today that I didn't have a test for this scenario. When I thought of the test, I was also pretty sure it would fail - it appears I was right :)
OK, I've now had a closer look, and the problem isn't what I initially thought when I read your message (the test which I expected to fail actually passed without changing the current implementation). It turns out that while the module is actually executing it does the right thing - the problem only arises when the run_module function attempts to clean up after itself by reverting some of the changes it makes to the sys module. The specific problem is this sentence from the run_module docs: "Both sys.argv[0] and sys.modules[__name__] are restored to their original values before the function returns." It looks like those semantics are a mistake - the changes to the sys module should persist after the function terminates, leaving it to the calling code to decide whether or not it wants to restore the original state.
One consequence of this is that the -i option is much less useful when you use -m, because the script's globals have disappeared before you get to the interpreter prompt.
See above - the problem is that the function is cleaning up after itself and deleting things that may still be of interest when -i is also specified.
At this point, I've successfully gotten a -z replacement patch, except that it inherits this apparent bug from -m, which for a while led me to believe my patch was broken (when in fact it works fine, apart from inheriting the -m behavior).
Does anybody know if this behavior is intended, and if so, why? And what are the consequences of changing/fixing it?
It was intended enough to be documented that way, but I don't recall putting any significant thought into that aspect of the implementation, and nor do I remember anyone else questioning it. The fact that it completely breaks the -i switch seems more than enough reason to consider it a bug, though. I've changed the behaviour in r56520 to simply leave the alterations to sys in place when the function terminates. While this is a definite change to the interface (and hence not a candidate for direct backporting), I think the difference is small enough for the 2.5 to 2.6 transition. If enough people prefer, I can switch the code to an approach which fixes -m while leaving the semantics of runpy.run_module alone. This would involve renaming the version of run_module I just checked into SVN have -m invoke that version directly. run_module would be changed to wrap the function used by -m in the necessary code to restore the sys module to something more closely resembling its original state. That would also be the approach to take if we decided we wanted to backport this fix to the 2.5 maintenance branch. Cheers, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia --------------------------------------------------------------- http://www.boredomandlaziness.org
At 12:16 AM 7/25/2007 +1000, Nick Coghlan wrote:
I've changed the behaviour in r56520 to simply leave the alterations to sys in place when the function terminates. While this is a definite change to the interface (and hence not a candidate for direct backporting), I think the difference is small enough for the 2.5 to 2.6 transition.
Your fix is a definite improvement for me, my "run any importable" patch is looking a lot better. There's just one problem left, which is that runpy is overwriting sys.argv[0] even if it doesn't need to. So, when running from a zipfile, sys.argv[0] ends up None, which is wrong. However, if I ask runpy not to mess with sys, it creates a new module namespace to run the code in, bringing me right back to square one (i.e., not being run in __main__). Any thoughts? My fallback at this point would be to add an option to run_module() to request that sys.argv[0] be used in place of calling _get_filename(). It seems ugly to do that, though, if only because there are already so many arguments to that function.
Phillip J. Eby wrote:
At 12:16 AM 7/25/2007 +1000, Nick Coghlan wrote:
I've changed the behaviour in r56520 to simply leave the alterations to sys in place when the function terminates. While this is a definite change to the interface (and hence not a candidate for direct backporting), I think the difference is small enough for the 2.5 to 2.6 transition.
Your fix is a definite improvement for me, my "run any importable" patch is looking a lot better. There's just one problem left, which is that runpy is overwriting sys.argv[0] even if it doesn't need to. So, when running from a zipfile, sys.argv[0] ends up None, which is wrong.
However, if I ask runpy not to mess with sys, it creates a new module namespace to run the code in, bringing me right back to square one (i.e., not being run in __main__). Any thoughts?
My fallback at this point would be to add an option to run_module() to request that sys.argv[0] be used in place of calling _get_filename(). It seems ugly to do that, though, if only because there are already so many arguments to that function.
Adding a get_filename() method to ZipImporter objects would get you something better than None in sys.argv[0] (specifically, you would see <zipfile_name>/__main__.py) For a reason I mentioned below, another idea I've had is to tweak the run_module semantics again and state that if __name__ already exists in sys.modules, then the code will be executed in the existing module, rather than in a new module (regardless of the value of the alter_sys argument). This would mean that the -m switch would always use the builtin __main__ module, instead of creating a new module the way it does now. This would not only fix your current problem, but also avoid any potential issues associated with having sys.modules["__main__"] refer to a different module while the interpreter is starting up (e.g. while running sitecustomize.py, and while doing any package imports needed to locate the module to be executed). I'm actually becoming more and more in favour of reverting run_module back to its 2.5 semantics and adding a separate function that does the right thing for the -m switch. It is really starting to look like the useful behaviour for a module namespace based execfile equivalent and the -m switch aren't as closely aligned as I thought they were back when I wrote PEP 338. Cheers, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia --------------------------------------------------------------- http://www.boredomandlaziness.org
At 08:29 PM 7/25/2007 +1000, Nick Coghlan wrote:
Phillip J. Eby wrote:
At 12:16 AM 7/25/2007 +1000, Nick Coghlan wrote:
I've changed the behaviour in r56520 to simply leave the alterations to sys in place when the function terminates. While this is a definite change to the interface (and hence not a candidate for direct backporting), I think the difference is small enough for the 2.5 to 2.6 transition.
Your fix is a definite improvement for me, my "run any importable" patch is looking a lot better. There's just one problem left, which is that runpy is overwriting sys.argv[0] even if it doesn't need to. So, when running from a zipfile, sys.argv[0] ends up None, which is wrong.
However, if I ask runpy not to mess with sys, it creates a new module namespace to run the code in, bringing me right back to square one (i.e., not being run in __main__). Any thoughts?
My fallback at this point would be to add an option to run_module() to request that sys.argv[0] be used in place of calling _get_filename(). It seems ugly to do that, though, if only because there are already so many arguments to that function.
Adding a get_filename() method to ZipImporter objects would get you something better than None in sys.argv[0] (specifically, you would see <zipfile_name>/__main__.py)
That's not the goal, actually; I want runpy to leave sys.argv[0] alone, so that we maintain the invariant that invoking sys.executable with sys.argv will re-run the same effective program. (And pointing to __main__.py inside the zipfile won't work!)
I'm actually becoming more and more in favour of reverting run_module back to its 2.5 semantics and adding a separate function that does the right thing for the -m switch.
+1; just make sure it has a way to request *not* overwriting sys.argv[0].
participants (2)
-
Nick Coghlan -
Phillip J. Eby