Skip to content
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

Fix pacgraph-tk regression for Python 3.12 #13

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ekorchmar
Copy link

Fixes issue #12.

Backstory

CPython's imp module was considered deprecated since 3.4, and was completely removed at 3.12; importlib should be used instead. As of right now, Python 3.12 is the default Arch package, so pacgraph-tk is de facto broken out of the box.

Relevant links

Changes

Added a conditional fork to import necessary package using importlib rather than imp for Python versions >= 3.12. Code was written in a way to as not to introduce syntax that would not be understood by earlier Python versions (e.g. installed manually or from AUR).

Testing

Manually tested on ArchLinux with default Python installation.

References

ekorchmar added 2 commits May 9, 2024 18:37
Introduces a fork in import logic depending on version, choosing
different manual import mechanic
Just in case it will be important in the future
@@ -8,9 +8,21 @@ from multiprocessing import Process
import sys, threading, queue, time

if not dev:
import imp
imp.load_source('pacgraph', '/usr/bin/pacgraph')
import pacgraph
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If dev = True is set to enable developement mode, import pacgraph from local folder should work. This MR moves all the importing of pacgraph into the if not dev: branch. Which renders the development mode broken (with no import).

Instead of hard coding the dev mode, we could first try to import pacgraph locally and catch the ImportError and only then attempt to do the import magic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants