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 and make project structure more Python packaging idiomatic, making it easier for downstream packagers to package dool #80

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

Conversation

Flowdalic
Copy link

@Flowdalic Flowdalic commented Sep 27, 2024

Gentoo downstream here. Thanks for working on dool. However, the currently project structure makes it hard to package dool in Gentoo (and I believe this is true for other distributions as well).

This PR makes the project structure more Python and flit idiomatic, reducing a lot of friction when packaging dool.

Note that I did not adjust Makefile, as there are multiple ways forward.

The project structure and the pyproject.toml would only cause the
dool.py script to be installed, not the plugins.

To make it easier for downstream packages, like Gentoo in my case,
dool should use an idiomatic Python project structure. This change
moves the files accordingly, adds a __init__.py (as recommended by
flit), and adjusts pyproject.toml accordingly.
A flit installation will put the dool module into the site-packages
directory, for example/usr/lib/python3.12/site-packages/dool/plugins/.
Therefore, dool should also look there for its plugins.
@scottchiefbaker
Copy link
Owner

Wow...

At it's core dool is one script, and a bunch of optional plugins that can live in a couple of places. Moving all the plugins to dool/plugins/ is gonna be a non-starter. If you're looking to package dool I would do one of two things:

  1. Only include the core dool script (exclude the plugins)
  2. Put the plugins in /usr/share/dool

For the RPM we do #2.

@raylu
Copy link
Contributor

raylu commented Sep 28, 2024

FWIW, I'm in favor of this. this is the reason why #74 needs a whole packaging/pypi.py to rewrite the project structure

I guess I don't understand why the emphasis is on "optional". if you install dool through your package manager, there isn't really a way to install plugins unless they come with the base package or you package the plugins separately. but there isn't a reasonable way to package the plugins either

I think we should package the plugins with dool together. changing the structure here would mean I could get rid of packaging/pypi.py when packaging for pypi too

what is the issue with moving the plugins to dool/plugins/? how is that less optional than they are now? how do you envision people installing dool? why do the plugins need to be optional?

@Flowdalic
Copy link
Author

Wow...

At it's core dool is one script, and a bunch of optional plugins that can live in a couple of places. Moving all the plugins to dool/plugins/ is gonna be a non-starter. If you're looking to package dool I would do one of two things:

  1. Only include the core dool script (exclude the plugins)
  2. Put the plugins in /usr/share/dool

For the RPM we do #2.

I understand your desire to keep things as they are. But the current state causes a lot of friction, downstream and elsewhere. Switching to an idiomatic format would help. Also, it certainly would encourage to continue packing dool on Gentoo.

Furthermore, I don't understand the optional aspect of the bundled plugins. Why would it be sensible to make them optional and not simply always install them when you install the main script on Linux?

@scottchiefbaker
Copy link
Owner

I'm a big fan of the K.I.S.S principal. I did not write 90% of dool, as it was inherited from the Python 2.x codebase of dstat. There are still HUGE chunks of it I don't understand fully. I'm hesitant to make big changes to things I don't fully understand. That said I understand what you're trying to do, I just want to make sure things are clean and simple.

I say the plugins are optional because I think 95% of users (myself included) almost never use the plugins. The stuff in the plugins folder is for corner case and niche applications. If it was something used by a lot of people I would move it into core. Complicating how dool works to support plugins most people don't use isn't a big benefit for me. I really like the idea of dool being a single script with no dependencies on other files or directories.

I guess I just don't fully understand how best to package/build a Python application? I took this on a hack because I liked dstat and didn't want to lose it as a tool. Moving the version to __init__.py seems counter-intuitive to me? Is that a Python packaging thing I'm not aware of?

Again, I think for 95% of users just putting dool in their $PATH will be sufficient. For the last 5% it would be putting the pluging in /usr/share/dool.

@scottchiefbaker
Copy link
Owner

It's also worth noting that #38 has been around a while but I haven't decided how/if we want to move forward with that. Dool already checks in these directories for plugins:

os.path.expanduser('~/.dool/'),                           # home + /.dool/
os.path.dirname(os.path.abspath(__file__)) + '/plugins/', # binary path + /plugins/
'/usr/share/dool/',
'/usr/local/share/dool/',

I'm hesitant to add a bunch of directories to this as it slows down and complicates things a bit.

@Flowdalic
Copy link
Author

Moving the version to __init__.py seems counter-intuitive to me? Is that a Python packaging thing I'm not aware of?

I am sorry, that is not at all required.. I guess I did it just by habit. If you project grows a __init__.py then __version__ is probably better declared there.

I am very sympathetic towards you KISS approach.

However, and the following is written with Gentoo packaging in mind, the current state of dool makes it more cumbersome to package it. If dool would simply use flit to perform its installation, then packaging, at least from the Gentoo perspective, would as easy as it gets (because Gentoo tooling will automatically do the right thing™).

I'm hesitant to add a bunch of directories to this as it slows down and complicates things a bit.

Please consider at least merging 5c53854. Looking in the purelib path for plugins is sensible for systemd-wide installations that support multiple Python versions. Frankly, I doubt that the additional directory lookups would case a noticeable slowdown. And the additional complexity is restricted to another entry to a list.

Thanks for working on dool. I really liked dstat and was very happy to see that it got forked as dool.

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.

3 participants