Add python 3.12 support and drop 2.7 #73
Conversation
|
There was no much feedback on this one, is there something I can do to move forward? |
XSConsoleImporter.py
Outdated
| spec = importlib.util.spec_from_file_location( | ||
| importName, os.path.join(root, filename)) | ||
| module = importlib.util.module_from_spec(spec) | ||
| module.__loader__.exec_module(module) |
There was a problem hiding this comment.
I think it is preferred to use loader rather than __loader__ here. See https://docs.python.org/3.14/library/importlib.html#importlib.machinery.ModuleSpec.loader and the examples in that document.
Otherwise this PR looks fine to me.
There was a problem hiding this comment.
The example at https://docs.python.org/3.14/library/importlib.html#importing-a-source-file-directly is indeed useful, but it does a sys.modules[module_name] = module which when added to xsconsole causes a funky seemlingly-unrelated exception to occur:
Exception: Console size (80, 23) too small for application size (80, 24)
So I'm updating the PR with a simple change to just use spec.loader
There was a problem hiding this comment.
Exception: Console size (80, 23) too small for application size (80, 24)
I would be surprised if this exception is caused by changes to module loading since I have seen this loads of times before. XSConsole gets the current screen size by calling curses.initscr().getmaxyx() and depending on the environment it might return a size that is too small (e.g. due to running xsconsole in a small terminal window or a low resolution BMC console).
There was a problem hiding this comment.
As surprising as it is, my tests were quite consistent: always in the same shell+ssh-session+terminal or VNC client, introducing this line triggers the issue, removing it gets rid of it, 100% of the time.
I would be not surprised that messing with the global sys.modules would shadow some pre-existing one and cause a clash - or confuse curses in another way.
Nowadays it is not very clear if "2.7" is really valid, and at least uv flags it as an error. Now ">=2.7" is not strictly correct as it includes the 3.0-3.5 range, but finding a way to express that (if there is one) is likely not worth it. Signed-off-by: Yann Dirson <[email protected]>
There is an equivalent License tag already, and uv plainly errors out too on this one. Signed-off-by: Yann Dirson <[email protected]>
This switches away from deprecated `imp`, and breaks support for python 2.7, but I expect this branch to really support only python3 now, so we likely don't care about adding backward compatibility here. Signed-off-by: Yann Dirson <[email protected]>
2 participants
This switches away from deprecated
imp, and breaks support for python 2.7, but I expect this branch to really support only python3 now, so we likely don't care about adding backward compatibility here.Builds on #72