sibradzic / upp

A tool for parsing, dumping and modifying data in Radeon PowerPlay tables
GNU General Public License v3.0
153 stars 23 forks source link

Using upp requires python-registry #12

Closed alexandrakoch closed 3 years ago

alexandrakoch commented 4 years ago

As the title says, running the software will immediately crash without python-registry, even on platforms which will have no use for this library. This makes it impossible to package properly for platforms which obviously will not have easy access to a library exclusively used for win32, which I discovered while attempting to make a PKGBUILD for the Arch Linux AUR.

Reproduction is simple, after uninstalling/not installing python-registry, running upp will immediately result in the following error:

Traceback (most recent call last): File "/usr/bin/upp", line 33, in sys.exit(load_entry_point('upp==0.0.7', 'console_scripts', 'upp')()) File "/usr/bin/upp", line 25, in importlib_load_entry_point return next(matches).load() File "/usr/lib/python3.8/importlib/metadata.py", line 77, in load module = import_module(match.group('module')) File "/usr/lib/python3.8/importlib/init.py", line 127, in import_module return _bootstrap._gcd_import(name[level:], package, level) File "", line 1014, in _gcd_import File "", line 991, in _find_and_load File "", line 975, in _find_and_load_unlocked File "", line 671, in _load_unlocked File "", line 783, in exec_module File "", line 219, in _call_with_frames_removed File "/usr/lib/python3.8/site-packages/upp/upp.py", line 6, in from Registry import Registry ModuleNotFoundError: No module named 'Registry'

sibradzic commented 4 years ago

Hi @alexandrakoch AFAIK, https://github.com/williballenthin/python-registry is completely Python-native, and not related to win32 in any way (it does not depend on any library, win32 or not). It was written with not relying on anything windows-specific in mind (it is actually used for win registry forensics @ Linux). Its only dependency is unicodecsv (enum-compat is just a virtual package, see https://github.com/williballenthin/python-registry/blob/master/setup.py#L18) Which particular issue are you running into when packaging it?

alexandrakoch commented 4 years ago

Since it's almost exclusively used for Windows platforms it's not packaged for any Linux platform I can find, unlike a lot of common python libraries. And while I could make a package for python-registry as well, it feels unnecessary to add a dependency for a library which (from what I can tell) is unnecessary on the platform you're packaging for, as well as creating another package that needs to be maintained.

sibradzic commented 4 years ago

Ahhh, feelings. So you feel that python-registry is "unnecessary"? I feel that python-registry is totally useful on Linux, it's clean and is of minimal overhead. Hell, I wouldn't be adding it to the project otherwise. Regardless, I don't have strong opinion about your packaging issue, as it seems to be more of a philosophical than a technical one. As I am not going to completely remove python-registry because of this, it is totally up to you; if you feel that python-registry should be a conditional import, feel free to submit a PR or patch, I'll be happy to merge it, I'm quite busy these days do the change myself.

sibradzic commented 4 years ago

@alexandrakoch any news?

azeam commented 4 years ago

@alexandrakoch looks like you added the python-registry but there is still something up with the enum-compat package? https://www.reddit.com/r/linux_gaming/comments/hocg55/anyone_know_how_to_undervolt_amd_gpus/fxgzaq4?utm_medium=android_app&utm_source=share

sibradzic commented 3 years ago

I guess I could set python-registry as an optional dependency and add some code that warns the user to add it if she really needs the thing...