Skip to content

Comments

fix: stop all running darling servers on uninstall#1584

Merged
CuriousTommy merged 1 commit intodarlinghq:masterfrom
Lazerbeak12345:fix-stop-all-running-services
May 11, 2025
Merged

fix: stop all running darling servers on uninstall#1584
CuriousTommy merged 1 commit intodarlinghq:masterfrom
Lazerbeak12345:fix-stop-all-running-services

Conversation

@Lazerbeak12345
Copy link
Contributor

@Lazerbeak12345 Lazerbeak12345 commented May 8, 2025

closes #1579

Copy link
Contributor

@CuriousTommy CuriousTommy left a comment

Choose a reason for hiding this comment

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

The script is broken

Seeing if Darling is currently running
error: list of process IDs must follow -p

Usage:
 ps [options]

 Try 'ps --help <simple|list|output|threads|misc|all>'
  or 'ps --help <s|l|o|t|m|a>'
 for additional help text.

For more details see ps(1).
Darling currently running for , shutting it down...
sudo: unknown user darling
sudo: error initializing audit plugin sudoers_audit

@Lazerbeak12345
Copy link
Contributor Author

Tested much more extensively this time.

I noticed that sometimes pgrep launchd doesn't work when pgrep -f launchd does. Not sure if I want to include arguments (and capture rare, but possible false positives).

@Lazerbeak12345
Copy link
Contributor Author

Lazerbeak12345 commented May 10, 2025

The other thing: this script makes the assumption that a user can run sudo sudo -u other_user darling shutdown. Running sudo as the root user isn't possible by default on some systems (such as mine, which is actually using doas), but I'm pretty sure it's possible by default on fedora and ubuntu.

  • I may want to do a Priv. check first instead of relying on sudo capabilities

@Lazerbeak12345 Lazerbeak12345 marked this pull request as draft May 10, 2025 04:13
@Lazerbeak12345 Lazerbeak12345 marked this pull request as ready for review May 10, 2025 14:38
Copy link
Contributor

@CuriousTommy CuriousTommy left a comment

Choose a reason for hiding this comment

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

LGTM. Once you combine your commits into one, I'll merge your changes.

@Lazerbeak12345 Lazerbeak12345 marked this pull request as draft May 11, 2025 00:52
fix: quotes are needed with test UNIX util

fix: privilage checking
@Lazerbeak12345 Lazerbeak12345 force-pushed the fix-stop-all-running-services branch from 58bba75 to 9201b43 Compare May 11, 2025 00:55
@Lazerbeak12345 Lazerbeak12345 marked this pull request as ready for review May 11, 2025 01:53
@CuriousTommy CuriousTommy merged commit 2c29d48 into darlinghq:master May 11, 2025
2 checks passed
@CuriousTommy
Copy link
Contributor

Thank you for your contribution!

@Lazerbeak12345 Lazerbeak12345 deleted the fix-stop-all-running-services branch May 17, 2025 18:10
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.

tools/uninstall does not stop _all_ running darling instances.

2 participants