-
Notifications
You must be signed in to change notification settings - Fork 198
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ImportError: Master does not have 'termios' #121
Comments
I knew overriding the whitelist semantics was a bad idea anyway, but didn't realize it was quite so broken :( Sorry about this, I was in a rush.. Thinking through the obvious fixes, this is kinda can-of-wormy and I'm a little tired :) Will sort it first thing tomorrow. One fix might simply be to add yet another kind of list passed through to the children, but this feels like tacking more mess on top of mess (the need for those lists at all is IMHO already a design disaster) |
This is a fun one :) Simply removing that code unveils another issue: " Having a second try at this later today |
This actually addresses multiple problems: * Single-file programs were broken, since the fix introduced in 6931cc1 caused builtin_find_module() to start indicating __main__ can always be loaded locally. That's broken, and there might be more cases where the same problem will crop up. Since it was indicated __main__ could be loaded locally, the built-in import machinery was allowed to attempt that (since we remove __main__ from sys.modules during bootstrap), which caused a safety check to fire in the bowels of Python: "Cannot re-init internal module %.200s" * The check for presence of the whitelist was totally broken, since the whitelist is never an empty list. Therefore 'self' was being returned for every module, including extension modules like 'termios'. I have hand-verified this does not break the fix for issue #113. I looked at writing a test for that, but it requires a Docker container (or similar) with an ancient version of Ansible installed. Will open a separate ticket tracking this.
A first small mea culpa to all my testing sins of late :)
Hi Alex, I'm going to make an explicit push to get better at testing now this has users, and get away from "lol just a hobby project" mentality. I've defined a new issue label, NeedsRegressionTest, please feel free to apply it liberally to any open and closed bugs. Also if there are similar labels for tracking test quality and whatnot, let me know and I'm happy to add more. |
This actually addresses multiple problems: * Single-file programs were broken, since the fix introduced in 6931cc1 caused builtin_find_module() to start indicating __main__ can always be loaded locally. That's broken, and there might be more cases where the same problem will crop up. Since it was indicated __main__ could be loaded locally, the built-in import machinery was allowed to attempt that (since we remove __main__ from sys.modules during bootstrap), which caused a safety check to fire in the bowels of Python: "Cannot re-init internal module %.200s" * The check for presence of the whitelist was totally broken, since the whitelist is never an empty list. Therefore 'self' was being returned for every module, including extension modules like 'termios'. I have hand-verified this does not break the fix for issue #113. I looked at writing a test for that, but it requires a Docker container (or similar) with an ancient version of Ansible installed. Will open a separate ticket tracking this.
A first small mea culpa to all my testing sins of late :)
china-mobile sip provider
Commit a9c6c13 (importer has priority over system packages when whitelisting is enabled) breaks a large portion of the test suite for me. e.g.
I identified the commit with bisect. I presume it's something to do with using a virtualenv, but I haven't confirmed that.
The text was updated successfully, but these errors were encountered: