brew icon indicating copy to clipboard operation
brew copied to clipboard

Allow systemctl stop on keep_alive services - Restart=on-failure

Open lethedata opened this issue 1 year ago • 5 comments

Verification

  • [X] This issue's title and/or description do not reference a single formula e.g. brew install wget. If they do, open an issue at https://github.com/Homebrew/homebrew-core/issues/new/choose instead.

Provide a detailed description of the proposed feature

When brew generates a systemd unit file with keep_alive true in the formula, it generates with Restart=always which prevents manually killing the service with systemctl directly. By switching to Restart=on-failure Linux users will be able to kill services with systemctl while still retaining a proper keep-alive functionality[^1]. This would also limit the need for brew to handle generating multiple exit codes (I think it was done this way due to a limitation on the MacOS side).

I think this change only requires a single line be changed in the service.rb file but I'm not familiar enough the code to be sure. https://github.com/Homebrew/brew/blob/8c4c7319fc6ba3a69b1ba65659b03d418ebbfb2f/Library/Homebrew/service.rb#L475

[^1]: systemd.service manual

What is the motivation for the feature?

Being unable to kill a service with systemctl as a workaround to the brew services kill keep-alive limitation. The service only needed to be temporarily killed, not completely disabled, and being unable to manually kill it seemed a bit odd.

How will the feature be relevant to at least 90% of Homebrew users?

It will allow Linux users manually kill services with systemctl stop --user homebrew.SERVICENAME.service without needing to fully disable the service.

What alternatives to the feature have been considered?

I wasn't able to find any previous discussion but but an alternative would be to have brew handle multiple exit states within the keep-alive but I think this would diverge the Linux and MacOS side requiring quite a bit of change.

lethedata avatar Jul 06 '24 01:07 lethedata

I don't think we should change the default behaviour at this point but I can see an argument for having a different DSL to accomplish what you'd want.

CC @SMillerDev for thoughts.

MikeMcQuaid avatar Jul 08 '24 07:07 MikeMcQuaid

IIRC there is a different keepalive parameter for this, where it only keeps the service alive on failures.

I'm also not that excited about optimising something in brew service so that you can manage them outside of brew, since that's not really a goal for brew service in my opinion.

SMillerDev avatar Jul 08 '24 07:07 SMillerDev

IIRC there is a different keepalive parameter for this, where it only keeps the service alive on failures.

Well now I'm a bit confused. I went back and looked and found reference to different states under the Formula Cookbook but it looks like other states aren't implemented on a systemd generation? I'm like 99% sure I'm missing something here. I'll try testing it when I have sometime later today.

https://github.com/Homebrew/brew/blob/07b6b713403c2e957dab20ab517a259a93760494/docs/Formula-Cookbook.md?plain=1#L1064

I'm also not that excited about optimising something in brew service so that you can manage them outside of brew, since that's not really a goal for brew service in my opinion.

I can agree with that. Figured I'd ask as I struggled to think of a use case where always restart makes since on desktop and now that you mention it I can see wanting to avoid management drift.

lethedata avatar Jul 08 '24 08:07 lethedata

Well now I'm a bit confused. I went back and looked and found reference to different states under the Formula Cookbook but it looks like other states aren't implemented on a systemd generation? I'm like 99% sure I'm missing something here. I'll try testing it when I have sometime later today.

It is possible that I never implemented all options for systemd.

SMillerDev avatar Jul 08 '24 09:07 SMillerDev

@lethedata Thanks for the link! In that case: yes, this should definitely be implemented.

MikeMcQuaid avatar Jul 08 '24 11:07 MikeMcQuaid

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

github-actions[bot] avatar Aug 15 '25 00:08 github-actions[bot]

Any way to mark this issue for it doesn't get autoclosed by the bot? This is more of a potential improvement/feature-request so lack of discussion is kinda expected

lethedata avatar Aug 15 '25 00:08 lethedata

@lethedata No, sorry. That's by design. If it seems like only one person cares: we'll probably let it get closed out and that person can create a PR if they desire.

MikeMcQuaid avatar Aug 15 '25 06:08 MikeMcQuaid

@MikeMcQuaid gotcha gotcha.

This never really came up enough for me to worry about and I could just work around it by disabling the service through brew. So no worries here

lethedata avatar Aug 15 '25 16:08 lethedata

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

github-actions[bot] avatar Sep 06 '25 00:09 github-actions[bot]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

github-actions[bot] avatar Oct 01 '25 00:10 github-actions[bot]