Added support for running services fully backgrounded on macOS

[GottemHams]

This PR is the product of a discussion on the main Homebrew repo. In short: it aims to improve working with brew services on multi-user systems when sharing a global HB instance, although there is also some benefit for single-user systems (see the next section).

Besides changing the gui/* domain to user/* stuff, HB now also modifies every plist it installs (or checks if it has to, at least). I've explained that last part in comments next to the relevant code, so I'm not going to repeat it here. It's not a very large patch anyway. :> The user domain seems to always be present as long as some form of login occurred, it can also co-exist with gui. And since homebrew-services is specifically meant for background services it also makes the most sense to just use user domains.

The session types aren't explained there, but I found them here:

• Aqua: a GUI agent; has access to all the GUI services
• LoginWindow: pre-login agent; runs in the login window context
• Background: runs in the parent context of the user
• StandardIO: runs only in non-GUI login session (e.g. SSH sessions)

I could guess about System, but then so could you.

Just for reference, if you wanted to check the current session type, just run launchctl managername and it'll print any of those names.

---

Some important usage/background information

Although it requires a specific setup, many parts are already recommended for/relevant to multi-user systems even without this patch:

1. You need a dedicated user account for managing HB, which is already recommended in the HB FAQ.
2. If you want to be able to just run brew as a non-HB user, you'll need an alias/shell function, which you'd already need without the patch too. In my case I also want to be able to run brew without having to type the other account's password every time. An example:

brew() {
	sudo su -l mybrew -c "source \$HOME/.bash_profile; brew $*"
}

Then /etc/sudoers.d/homebrew contains simply myuser ALL=NOPASSWD: /usr/bin/su -l mybrew -c *, meaning my own account can run any commands as mybrew no prob.

Note how I'm doing sudo su instead of just sudo or su, the latter of which doesn't even pass through sudoers config so using it stand-alone would always require typing a password. Furthermore, there's a major problem with just using sudo because it either doesn't set up the environment for the target user, or (and this is the important part relevant for this patch) launchctl detects the wrong login session type and the service won't even start. brew will report it did but it's actually hanging on the bootstrap command and you have to launchctl bootout user/<UID> <service name> to get it unstuck. Like I'm doing in the example you can't actually pass multiple commands directly to sudo without using e.g. sudo -u mybrew bash -c "multiple; commands; here". Using sudo with any shell like that will cause launchctl to detect an Aqua session, although I've only checked with bash and sh there might be shell-specific overrides. Finally, I'm using su -l to discard the current environment, which again is needed to get it detected as a Background session. That also means I have to manually source the target user's shell config, since I have some HB-related environment variables in there and I need them exported for the brew command. Pretty sure sudo simply overrides the "interactivity" of su's shell and causes it to no longer read profiles automatically, but instead goes looking for an environment variable BASH_ENV (or ENV for sh). May as well just source the file directly then. =]

With this setup you can run brew services commands as any user and the service will be able to start regardless of whether you're in an actual GUI/Aqua session or doing some su(do) magic. It also doesn't break things for anyone with a single-user setup; if anything it also improves it for them, e.g. starting a service purely through SSH should also be possible now.

And a final note: keep in mind that although HB installs a LaunchAgent that would normally autostart the service when rebooting, it won't actually trigger if you're not logging on to an Aqua session for the dedicated user. So you'll always have to manually run a start command, or write a LaunchAgent for your own user that runs the necessary brew commands. It probably could autostart if you were to put the plist in the system /Library and have it run in LoginWindow or even System context. But then brew itself needs to sudo up and that goes against Homebrew's ideas. :D

[SMillerDev]

I think this addition should happen when generating the file, not here.

[GottemHams]

You mean def generate_plist? I tried that initially but it doesn't even seem to get called (at least for Apache). :>

[SMillerDev]

I mean in brew itself, in the Service class.

[GottemHams]

I only see 1 candidate in /opt/homebrew/Library/Homebrew/service.rb (def to_plist), but that too doesn't seem to get called. I even made sure to stop and start instead of restart, or simply remove the plist beforehand. The place where it currently makes the changes gets called every time, even when doing restart.

[SMillerDev]

It gets called when you install a formula, there's no real benefits in rewriting the plist at every start.

[GottemHams]

But wouldn't that mean everyone who wants to use the new feature would have to reinstall all their services? I'm not entirely sure if it might even remove some config files.

[SMillerDev]

It would slowly propagate with software updates.

[GottemHams]

Alright that works, plus reinstalling doesn't seem to remove configs so that should be fine too. I checked with redis because I noticed that your GitHub CI reported an error about it:

==>Successfully stopped `redis` (label: homebrew.mxcl.redis)
Bootstrap failed: 5: Input/output error
Error: Failure while executing; `/bin/launchctl bootstrap user/501 /usr/local/opt/redis/homebrew.mxcl.redis.plist` exited with 5.
Error: Process completed with exit code 1.

However, I can start/stop/restart without errors and even the exact commands being used in the test work fine:

brew services stop redis
sleep 5
brew services run redis
brew services restart redis

Anyways, the only thing this PR now changes is gui/$UID => user/$UID. :>

[MikeMcQuaid]

You need a dedicated user account for managing HB, which is already recommended in the HB FAQ.

I thing "recommended" is a bit strong here given it says "if you need".

With this setup you can run brew services commands as any user and the service will be able to start regardless of whether you're in an actual GUI/Aqua session or doing some su(do) magic. It also doesn't break things for anyone with a single-user setup; if anything it also improves it for them, e.g. starting a service purely through SSH should also be possible now.

I think this behaviour should be opt-in --user and/or detected based on the lack of DISPLAY. I don't think we should change the (very long-running/old) defaults for everyone based on a multiuser case requested by a single user. This seems very likely to break things for a large number of people.

[RandomDSdevel]

Finally, I'm using su -l to discard the current environment, which again is needed to get it detected as a Background session. That also means I have to manually source the target user's shell config, since I have some HB-related environment variables in there and I need them exported for the brew command. Pretty sure sudo simply overrides the "interactivity" of su's shell and causes it to no longer read profiles automatically, but instead goes looking for an environment variable BASH_ENV (or ENV for sh). May as well just source the file directly then. =]

     Did you try:

brew() {
  sudo su -l mybrew -c "bash -l -c \"brew $*;\""
}

by any chance?

[GottemHams]

I thing "recommended" is a bit strong here given it says "if you need".

I'm speaking from the perspective of a multi-user system there though.

I think this behaviour should be opt-in --user and/or detected based on the lack of DISPLAY. I don't think we should change the (very long-running/old) defaults for everyone based on a multiuser case requested by a single user. This seems very likely to break things for a large number of people.

The problem I noticed with doing it detection-based is that a plist with an outdated session type might linger around. E.g. you use SSH to initially start the service, so Homebrew will generate a plist with either the Background or StandardIO session type. Then after a certain while you reboot (which would keep the current plist as-is) and log (back) in to a regular GUI session. Launchctl has been told to automatically start the service but since the specified session type is not Aqua it won't start. I think launchctl's default is to only allow Aqua unless it's a LaunchDaemon, which will run only in the System context. Global LaunchAgents can run at the login screen (so LoginWindow context) as well as a user's Aqua session.

@RandomDSdevel I just tried it and although the session type is properly Background, it still doesn't read neither .bashrc nor .bash_profile.

[RandomDSdevel]

@RandomDSdevel I just tried it and although the session type is properly Background, it still doesn't read neither .bashrc nor .bash_profile.

     Hmm, interesting/surprising. Passing '-l' to 'bash' should have it start a login session. One or both of '~/.bashrc' and/or '~/.bash_profile' exist for the 'mybrew' user, presumably. If not that, then I'm out of ideas.

[GottemHams]

Yeah both exist just to be sure. =] I think the reason is that su is invoked by a non-shell (sudo) so the actual shell falls back to reading environment variables, despite still starting a login session.

[MikeMcQuaid]

The problem I noticed with doing it detection-based is that a plist with an outdated session type might linger around. E.g. you use SSH to initially start the service, so Homebrew will generate a plist with either the Background or StandardIO session type. Then after a certain while you reboot (which would keep the current plist as-is) and log (back) in to a regular GUI session. Launchctl has been told to automatically start the service but since the specified session type is not Aqua it won't start. I think launchctl's default is to only allow Aqua unless it's a LaunchDaemon, which will run only in the System context. Global LaunchAgents can run at the login screen (so LoginWindow context) as well as a user's Aqua session.

Ok, thanks for explaining. Given this and your other explanations above: I'm cautiously in favour.

[MikeMcQuaid]

CI is failing on macOS I'm afraid.

[GottemHams]

CI is failing on macOS I'm afraid.

Yeah that's what I mentioned in the comment thread a bit above. I can't reproduce the error on my end so it might be specific to how the tests are run.

[MikeMcQuaid]

Yeah that's what I mentioned in the #485 (comment). I can't reproduce the error on my end so it might be specific to how the tests are run.

@GottemHams Not sure what to say here beyond:

1. this will need addressed before this PR can be merged
2. I'm suspicious that if this breaks things on GitHub Actions it's also likely to break things for some user configurations

Sorry!

[GottemHams]

@MikeMcQuaid So I added some stuff to the GitHub CI, such as printing the service plist file. It doesn't even contain the LimitLoadToSessionType key. Which of course makes sense, since the plist change is done by Homebrew/brew and not Homebrew/homebrew-services. The CI here isn't aware of the unmerged changes made over in my fork of brew, so of course it's going to fail.

[MikeMcQuaid]

@GottemHams So does Homebrew/brew#13510 need merged before this PR can have green CI? If so, is there a way to scope this PR down so it can be merged first?

[GottemHams]

If so, is there a way to scope this PR down so it can be merged first?

Actually no, this PR literally only changes gui to user where needed, which causes the actual functionality change. :>

The PR over at brew should be able to be safely merged though. I found an actual Apple dev's comment on the subject:

If you don’t specify a session type then the system assumes a default of Aqua and your agent should only run in GUI contexts

Since that PR only changes the plist and it also/still includes the Aqua session type, there should be no change yet. Only when you merge this PR is the functionality switch complete.

[MikeMcQuaid]

Have restarted jobs post-Homebrew/brew PR merge. Let's see if they pass now.

[github-actions]

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

[MikeMcQuaid]

Thanks again @GottemHams, let's see how this goes 🤞🏻!

[Eric-Guo]

Fail to work on macOS Monterey 12.5, after revert this PR, it return to normal.

─▪ brew services start percona-server
Bootstrap failed: 5: Input/output error
Try re-running the command as root for richer errors.
Error: Failure while executing; `/bin/launchctl bootstrap user/502 /Users/guochunzhong/Library/LaunchAgents/homebrew.mxcl.percona-server.plist` exited with 5.

[MikeMcQuaid]

Reverting in #498

[MikeMcQuaid]

3 reports within 30m of merging. Yup, this was pretty broken I'm afraid @GottemHams.

[GottemHams]

@MikeMcQuaid Nothing is broken, it seems nobody has been paying attention.

It doesn't even contain the LimitLoadToSessionType key. Which of course makes sense, since the plist change is done by Homebrew/brew and not Homebrew/homebrew-services. The CI here isn't aware of the unmerged changes made over in my fork of brew, so of course it's going to fail.

So what's going to happen if someone (like @Eric-Guo) has a pre-existing plist, which also doesn't get rewritten? The exact same thing. ;] I figured you kept this PR open for a while to deliberate (internally) how to deal with this, since @SMillerDev previously said he didn't really want the plist to be rewritten on every start.

[MikeMcQuaid]

@MikeMcQuaid Nothing is broken, it seems nobody has been paying attention.

@GottemHams I really don't appreciate your tone here. Please re-read our Code of Conduct: https://github.com/Homebrew/.github/blob/HEAD/CODE_OF_CONDUCT.md#code-of-conduct

So what's going to happen if someone (like @Eric-Guo) has a pre-existing plist, which also doesn't get rewritten? The exact same thing. ;] I figured you kept this PR open for a while to deliberate (internally) how to deal with this, since @SMillerDev previously said he didn't really want the plist to be rewritten on every start.

Your communication did not at all make it clear that you knew this PR would break things once it was merged.

If you'd still like to: please reopen a PR with a CI test that fails in this known-broken case. If you'd rather leave it: that's fine and we can revert the Homebrew/brew change.

[GottemHams]

I dunno, I thought it was pretty clear; doesn't contain LimitLoadToSessionType (un-rewritten plist) => error. Anyways, you want me to write just a test that fails when you try to start a service with an "old" plist? Or do you mean to reopen/redo this PR and include that test alongside the current changes?

[MikeMcQuaid]

I dunno, I thought it was pretty clear

Not to me, sorry.

doesn't contain LimitLoadToSessionType (un-rewritten plist) => error

We'll need a way to ensure that plists are rewritten as expected in this case.

Or do you mean to reopen/redo this PR and include that test alongside the current changes?

Both, please. Need to have a fix for this issue above and a test for this before we can merge again.

Thanks!

[GottemHams]

Sure, will take some time though since I have a tonne of other things to do at the moment. :> The change already merged for brew doesn't do anything on its own so there should be no problems in the meantime.

We'll need a way to ensure that plists are rewritten as expected in this case.

Well my original proposal was to rewrite the plist on brew services start instead of brew install, if it doesn't contain the LimitLoadToSessionType key yet. I think that would be the best solution, so you have only one place where the change happens instead of doing the same stuff in multiple places (e.g. having both on-install and on-start checks). The downside would be that any tests probably can no longer include that key.

Alternatively, we could keep only the current on-install change and use the default/previous gui domain if the plist hasn't been rewritten yet. But then you still have to reinstall all services or wait for updates before it finally does get rewritten. Also at a quick glance I don't see a way to access the plist from around there so this may not even be an option?

[MikeMcQuaid]

Alternatively, we could keep only the current on-install change and use the default/previous gui domain if the plist hasn't been rewritten yet. But then you still have to reinstall all services or wait for updates before it finally does get rewritten.

This seems like a good idea 👍🏻.

@SMillerDev any thoughts?

[SMillerDev]

Sounds good to me.