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

Convert to using launchctl bootstrap instead of deprecated launchctl load -w #112

Merged
merged 10 commits into from Apr 6, 2017
Merged

Convert to using launchctl bootstrap instead of deprecated launchctl load -w #112

Changes from 5 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
66ddb80
services: Document -v/--verbose
johnhawkinson Apr 6, 2017
72032ae
services: abstract out common launchctl load -w code
johnhawkinson Apr 6, 2017
13e63c1
services: Use launcchtl enable+bootstrap, not deprecated load -w
johnhawkinson Apr 6, 2017
07a2237
services: update comment, now we're boostrapping!
johnhawkinson Apr 6, 2017
ef898bb
services: Continue to support Mavericks with load -w
johnhawkinson Apr 6, 2017
0a5511c
services: trim comments
johnhawkinson Apr 6, 2017
187e3f6
services: Reorder to Current else Old
johnhawkinson Apr 6, 2017
e4637ba
services: safe_system handles nonzero exit status already
johnhawkinson Apr 6, 2017
f61a707
services: Say "successfully ran" not "started" for `brew run`
johnhawkinson Apr 6, 2017
cd4fc93
services: kill blank line per `brew style`
johnhawkinson Apr 6, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
48 changes: 32 additions & 16 deletions cmd/brew-services.rb
View file
Open in desktop
@@ -1,5 +1,6 @@
#: * `services`
#: Easily start and stop formulae via launchctl
#: * `services` [`-v`|`--verbose`] [list | run | start | stop | restart | cleanup] [...]
#: Easily start and stop formulae via launchctl.
#: With `-v` or `--verbose`, print more detail.
#:
#: Integrates Homebrew formulae with OS X's `launchctl` manager. Services can be
#: added to either `/Library/LaunchDaemons` or `~/Library/LaunchAgents`.
Expand Down Expand Up @@ -236,6 +237,33 @@ def restart(target)
end
end

# "load" a plist
Copy link
Member

Choose a reason for hiding this comment

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

Can skip this comment, pretty self-explanatory.

def launchctl_load(plist, _function, service)
Copy link
Member

Choose a reason for hiding this comment

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

Skip _function if it's never used.

Copy link
Member

Choose a reason for hiding this comment

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

But it looks like it probably wants to be called function and used in ohai("Successfully started #{service.name} (label: #{service.label})")

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That was the intention, but I was concerned that changing the message from "started" run "ran" in the brew run case might break callers. So I wanted to make the API for the function call support that concept without commiting to it. Which way should I go? Is it OK to change the output for the brew run case?

(On the other hand, maybe brew run really does start things instead of running them. In which case this should all just go away?)

Copy link
Member

Choose a reason for hiding this comment

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

Is it OK to change the output for the brew run case?

Yes.

(On the other hand, maybe brew run really does start things instead of running them. In which case this should all just go away?)

That was to preserve backwards compatibility of commands.

if root?
domain_target = "system"
else
domain_target = "gui/#{Process.uid}"
end

if MacOS.version < :yosemite
Copy link
Member

Choose a reason for hiding this comment

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

Pedantry but make this the else so the first block people see is the current version.

# This syntax was deprecated in Yosemite
safe_system launchctl, "load", "-w", plist
else
# New syntax has improved errr-handling.
Copy link
Member

Choose a reason for hiding this comment

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

Can remove this comment.

safe_system launchctl, "enable", "#{domain_target}/#{service.label}"
if $?.to_i.nonzero?
Copy link
Member

Choose a reason for hiding this comment

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

safe_system will fail with an error so this failure check will never be hit (I realise the previous code also made this mistake).

odie("Failed to enable `#{service.name}`")
end

safe_system launchctl, "bootstrap", domain_target, plist
end
if $?.to_i.nonzero?
odie("Failed to start `#{service.name}`")
Copy link
Member

Choose a reason for hiding this comment

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

safe_system will fail with an error so this failure check will never be hit (I realise the previous code also made this mistake).

else
ohai("Successfully started `#{service.name}` (label: #{service.label})")
end
end

# Run a service.
def run(target)
if target.is_a?(Service) && target.loaded?
Expand All @@ -244,13 +272,7 @@ def run(target)
end

Array(target).each do |service|
safe_system launchctl, "load", "-w", service.plist

if $?.to_i.nonzero?
odie("Failed to start `#{service.name}`")
else
ohai("Successfully started `#{service.name}` (label: #{service.label})")
end
launchctl_load(service.plist, "ran", service)
end
end

Expand Down Expand Up @@ -295,13 +317,7 @@ def start(target, custom_plist = nil)
# Clear tempfile.
temp.close

safe_system launchctl, "load", "-w", service.dest.to_s

if $?.to_i.nonzero?
odie("Failed to start `#{service.name}`")
else
ohai("Successfully started `#{service.name}` (label: #{service.label})")
end
launchctl_load(service.dest.to_s, "started", service)
end
end

Expand Down