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

Support (and set) launchd user domain target #588

Merged
merged 1 commit into from
Sep 28, 2023
Merged

Conversation

MikeMcQuaid
Copy link
Member

When running brew services over ssh or sudo as a non-root user the gui domain target for launchd does not work as expected. This produces confusing failures for users.

It was tried before to detect if this was needed from the .plist or changing globally but both approaches were unsuccessful/error-prone.

Instead, use the user domain automatically on the cases we know it's needed: when running over ssh or through sudo. To make clear to users what's happening in these cases: output a warning (which can be hidden with an output environment variable).

For this to work, launchctl list is no longer sufficient. The output here, even when run as root, does not properly list user domain services. Instead, we need to use launchctl print to correctly query the status of these services. This also seems to correctly handle some launchd edge-cases where launched services cannot be detected and lets brew services now stop them.

While we're here, fix some related issues I came upon while working on this:

  • improve the brew services command documentation to note it now runs on Linux/systemd too
  • use named_args and remove the custom_plist deprecation: it's been long enough and this cleans up the code nicely.
  • if a service is status :other (an edge-case that shouldn't be possible in normal operation): actually output this rather than failing with a nil error
  • avoid running launchctl list multiple times when unnecessary
  • brew services --debug now outputs the raw output from launchctl or systemctl to aid debugging/development
  • cleanup some code style and avoid use of plist?
  • add System.systemctl_args to avoid repeating System.systemctl and System.systemctl_scope in every call site

Depends on Homebrew/brew#16041 to detect sudo or ssh correctly.

When running `brew services` over `ssh` or `sudo` as a non-`root` user
the `gui` domain target for `launchd` does not work as expected. This
produces confusing failures for users.

It was tried before to detect if this was needed from the `.plist` or
changing globally but both approaches were unsuccessful/error-prone.

Instead, use the `user` domain automatically on the cases we know it's
needed: when running over `ssh` or through `sudo`. To make clear to
users what's happening in these cases: output a warning (which can
be hidden with an output environment variable).

For this to work, `launchctl list` is no longer sufficient. The output
here, even when run as `root`, does not properly list `user` domain
services. Instead, we need to use `launchctl print` to correctly
query the status of these services. This also seems to correctly
handle some `launchd` edge-cases where launched services cannot be
detected and lets `brew services` now stop them.

While we're here, fix some related issues I came upon while working on
this:
- improve the `brew services` command documentation to note it now
  runs on Linux/`systemd` too
- use `named_args` and remove the `custom_plist` deprecation: it's
  been long enough and this cleans up the code nicely.
- if a service is status `:other` (an edge-case that shouldn't be
  possible in normal operation): actually output this rather than
  failing with a `nil` error
- avoid running `launchctl list` multiple times when unnecessary
- `brew services --debug` now outputs the raw output from `launchctl`
  or `systemctl` to aid debugging/development
- cleanup some code style and avoid use of `plist?`
- add `System.systemctl_args` to avoid repeating `System.systemctl`
  and `System.systemctl_scope` in every call site
@MikeMcQuaid MikeMcQuaid merged commit 6f5f509 into master Sep 28, 2023
5 checks passed
@MikeMcQuaid MikeMcQuaid deleted the user_domain_target branch September 28, 2023 11:31
@Uning456789

This comment was marked as off-topic.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 14, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants