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.

Sign up for GitHub

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

Jump to bottom

Segregate BusKill Package Module into multiple scripts #33

Open
maltfield opened this issue Oct 2, 2022 · 5 comments
Open

Segregate BusKill Package Module into multiple scripts #33

maltfield opened this issue Oct 2, 2022 · 5 comments
Assignees
@maltfield

Comments

@maltfield
Copy link
Member

At the time of writing (2022-10-01), The buskill-app/src/packages/buskill/__init__.py file is 1,656 lines long. And growing.

This task is to track the effort to split this file up into multiple distinct files.
@maltfield
Copy link
Member Author

Functions for the following purposes could be boxed-up together into individual files:

  1. Upgrading the app
  2. Triggers (possibly one for each platform)

@maltfield maltfield self-assigned this Oct 2, 2022
@maltfield
Copy link
Member Author

See also https://www.qtrac.eu/pyclassmulti.html

maltfield added a commit that referenced this issue Oct 15, 2022
@maltfield
Attempt to split buskill/__init__.py
b57f796 
this commit is my first attempt to split-up the contents of the buskill __init__.py file into distinct files. Here I've only attempted to make upgrade() separate, following this:

 * https://www.qtrac.eu/pyclassmulti.html
 * https://stackoverflow.com/a/57562083

Note that the new builds could't find the KEYS file (which here not in the APP_DIR on Linux, but in the root-owned /tmp/ dir in which the squashfs .AppDir is extracted), so I had to do some fuckery with APP_DATA and added a new instance field RUNTIME_DIR

I've tested that this works on Linux, but I'm not sure if this works on Windows & MacOS (hence why I'm pushing this to git for the builds).

If I broke the upgrade process on any platforms, I'll revert this

 * #33
@maltfield
Copy link
Member Author

maltfield commented Oct 15, 2022 •
edited
Loading

Well, I tried to separate-out the update() function into its own file, but the resulting build had the following results:

  1. Linux Updates worked
  2. Windows Updates worked
  3. MacOS updates failed with error Error invoking gpg: 2: gpg: Fatal: libgcryptis too old (need 1.9.1, have 1.8.6)

I don't know if I caused that or not with this change, but I'm going to revert for now.

@maltfield
Copy link
Member Author

maltfield commented Oct 15, 2022 •
edited
Loading

Well, we lost all our fucking history of that work because git is stupid. Not sure why "revert everything back to X commit" is so damn hard.

Apparently if you say git revert <commit> then it doesn't do what you want. It tries to be too smart and tries to revert just that one commit. If you say git revert <commit>..HEAD then it should do what you want (revert everything to that specific commit), but it didn't actually work. The _buskill_update.py script was still present, and the __init__.py script's contents wasn't reverted!

After fighting with git for 10 minutes, I finally just did a git reset --hard, which deleted all the history. Fail.

@maltfield
Copy link
Member Author

maltfield commented Oct 16, 2022 •
edited
Loading

Rather than splitting things like 'triggers' into their own file, I think it would make more sense to split it by platform. So all the linux-specific triggers would go into '_buskill_lin.pyand all of the mac-specific triggers would go into a_buskill_mac.py` file.

I like this better because you could load less code at runtime as-needed. And because an developer probably wants to focus on one platform at at time. But if they want to view, for example, all of the "lockscreen" functions for each platform side-by-side, that's easier when it's in three distinct files. I'm writing this as I'm trying to edit the shutdown trigger on two platforms, and it's a pain to scroll up-and-down between the linux and MacOS implementations

The only problem with this idea is the *nix functions that are used for both Linux and MacOS :/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@maltfield maltfield

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@maltfield