Conversation
Karlson2k
commented
Mar 6, 2025
- Fixed installation of the package
- Removed unwanted and unused files from the package
- Fixed package to work as a module: now main interface can be started by a shortcut or by python -m
- Limited to single python version only
- Other minor fixes
* Fixed installation of the package * Removed unwanted and unused files from the package * Fixed package to work as a module: now main interface can be started by a shortcut or by python -m * Limited to single python version only * Other minor fixes Signed-off-by: Evgeny Grin (Karlson2k) <k2k@drgrin.dev>
c0f5cd5 to
db3f19e
Compare
|
This PR does not drop existing ebuilds, just an additional revision is added (should be easier for users to rollback if anything goes wrong).
Any of the ways works just fine, providing additional flexibility. By using standard installation mechanics, result is automatically byte-compiled with several optimisation levels. Additionally, I removed unused files (looks like some old leftovers) and fixed image reference in one file. Run-tested on my PC. |
Given that the current version is uninstallable there's not really anything to roll-back to.
This is not actually how the python ecosystem does things. There, you install an application/library for some python version, putting all the source in a python-implementation specific directory. So, even if a application is mainly intended to be used as a program, it is still a multi-implementation python package.
This would ideally be fixed upstream, and then we package a functioning application, rather then the current situation |
OK. Should I include drop of it into this PR?
Right. It makes sense when the module is used by other python applications or modules: they must be installed for the same python version otherwise they will not work. As other python modules could be limited to some specific python versions, some modules could be kept for several versions. Also, it is problematic to use
Yep, ideally. Keeping large local patches is always bad. |
|
One more argument for single python version: DE menu icon/shortcut will start always one version only. Very unlikely that user would start manually another version.. However, if this point is fundamental, I can remove "single version" enforcement (together with shebang fix, which is not really important). |
I don't think it's that important. The biggest reason for it is because in the Gentoo python ecosystem,
The multi-implementation is pretty much completely transparent for a properly packaged python package. All impl-independent files are installed in some impl-specific like A user then can just run I haven't done that many python packaging so this is how I imagine the situation. https://projects.gentoo.org/python/guide/eclass.html's first line paragraph says using
Yes,the proper fix would be installing a separate python executable (through I did try to write a multi-impl ebuild and I basically got this dlang/net-misc/onedrivegui/onedrivegui-1.1.1-r2.ebuild Lines 34 to 51 in dfdc4bb |
Having a multi-impl package installed for 1 implementation shouldn't be that much more compared to having a single-impl package |
| DISTUTILS_USE_PEP517=setuptools | ||
| PYTHON_COMPAT=(python3_{9..13}) | ||
|
|
||
| inherit desktop distutils-r1 |
There was a problem hiding this comment.
From pkgcheck --scan:
UnusedInherits: version 1.1.1-r2: unused eclass: desktop
A little bit hard to read through everything though ;)
| dev-python/requests | ||
| dev-python/pyside[gui(+),webengine(+),widgets(+)] |
There was a problem hiding this comment.
You're missing pythong_gen_cond_dep, see https://projects.gentoo.org/python/guide/single.html#dependencies
| sed -i \ | ||
| -e 's/^from version\b/from .version/' \ | ||
| -e 's/^from ui\./from .ui./' \ | ||
| -e '/^GUI_SETTINGS_FILE =/a\'$'\n''gui_settings = None\nglobal_config = None\ntemp_global_config = None\main_window = None' \ | ||
| -e '/^if __name__ == "__main__":/a\'$'\n'' global gui_settings, global_config, temp_global_config, main_window' \ | ||
| -e 's/if __name__ == "__main__":/def main():/' \ | ||
| -e '$a\'$'\n''\nif __name__ == "__main__":\n main()' \ | ||
| "${S}/src/OneDriveGUI.py" || die |
There was a problem hiding this comment.
This is incredibly ugly and unreadable, perhaps a .patch would be better?
|
I do want to mention that I don't know what to do with the 2 PRs trying to do the same thing. I think I prefer #136 because it also fixes It would be a lot easier to decide if one of you went and fixed the build system upstream and came back with a nice and clean ebuild. It's also a possibility to merge your changes for |
I think for now lets use the ebuild from #136 (sorry if seems biases), because it uses the distutils-r1 eclass standard. Also @Karlson2k you already patching the files why not try to see if get accepted upstream. |
|
It would be a lot of help for me if you could rebase your PR against the current ebuild. Because I don't want to turn down you you changes simply because there was another PR a few days prior to yours if you address the changes I've requested against the Regardless of what you do I want to express that I am grateful for your interest and, if your changes targeted something that I had more interest in, I think I would have been more amiable :) |