service: change custom name DSL

After some discussion, we decided to change the DSL to get rid of
the `plist_name` and `service_name` methods which aren't meaningful
for most users.

The new DSL looks like this:

```rb
service do
  name macos: "name", linux: "name"
end
```

I also updated some specs here to reflect these changes.

There was some talk about maybe deprecating `plist_name` and `service_name`
but I think that's outside of the scope of this PR so I'm leaving
them as is for now. One benefit of this is that everything here is backwards
compatible.
This commit is contained in:
apainintheneck 2023-05-13 12:35:50 -07:00
parent ed24ef4860
commit f2adbf6613
9 changed files with 63 additions and 68 deletions

View File

@ -258,15 +258,22 @@ module Formulary
if (service_hash = json_formula["service"])
service_hash = Homebrew::Service.deserialize(service_hash)
run_params = service_hash.delete(:run)
service do
T.bind(self, Homebrew::Service)
case run_params
when Hash
run(**run_params)
when Array, String
run run_params
if (run_params = service_hash.delete(:run))
case run_params
when Hash
run(**run_params)
when Array, String
run run_params
end
end
if (name_params = service_hash.delete(:name))
name(**name_params)
end
service_hash.each do |key, arg|
public_send(key, arg)
end

View File

@ -21,11 +21,8 @@ module RuboCop
share: :opt_share,
}.freeze
# These can be defined without using the `run` command.
RENAME_METHOD_CALLS = [:plist_name, :service_name].freeze
# At least one of these methods must be defined in a service block.
REQUIRED_METHOD_CALLS = [:run, *RENAME_METHOD_CALLS].freeze
REQUIRED_METHOD_CALLS = [:run, :name].freeze
def audit_formula(_node, _class_node, _parent_class_node, body_node)
service_node = find_block(body_node, :service)
@ -38,13 +35,12 @@ module RuboCop
# so we don't show both of them at the same time.
if (method_calls.keys & REQUIRED_METHOD_CALLS).empty?
offending_node(service_node)
problem "Service blocks require `run`, `plist_name` or `service_name` to be defined."
problem "Service blocks require `run` or `name` to be defined."
elsif !method_calls.key?(:run)
other_method_calls = method_calls.keys - RENAME_METHOD_CALLS
other_method_calls = method_calls.keys - [:name]
if other_method_calls.any?
offending_node(service_node)
problem "`run` must be defined to use methods other than " \
"`service_name` and `plist_name` like #{other_method_calls}."
problem "`run` must be defined to use methods other than `name` like #{other_method_calls}."
end
end

View File

@ -41,16 +41,9 @@ module Homebrew
"homebrew.mxcl.#{@formula.name}"
end
sig { params(name: T.nilable(String)).returns(String) }
def plist_name(name = nil)
case T.unsafe(name)
when nil
@plist_name ||= default_plist_name
when String
@plist_name = name
else
raise TypeError, "Service#plist_name expects a String"
end
sig { returns(String) }
def plist_name
@plist_name ||= default_plist_name
end
sig { returns(String) }
@ -58,16 +51,17 @@ module Homebrew
"homebrew.#{@formula.name}"
end
sig { params(name: T.nilable(String)).returns(String) }
def service_name(name = nil)
case T.unsafe(name)
when nil
@service_name ||= default_service_name
when String
@service_name = name
else
raise TypeError, "Service#service_name expects a String"
end
sig { returns(String) }
def service_name
@service_name ||= default_service_name
end
sig { params(macos: T.nilable(String), linux: T.nilable(String)).void }
def name(macos: nil, linux: nil)
raise TypeError, "Service#name expects at least one String" if [macos, linux].none?(String)
@plist_name = macos if macos
@service_name = linux if linux
end
sig {
@ -539,14 +533,13 @@ module Homebrew
# Prepare the service hash for inclusion in the formula API JSON.
sig { returns(Hash) }
def serialize
custom_plist_name = plist_name if plist_name != default_plist_name
custom_service_name = service_name if service_name != default_service_name
name_params = {
macos: (plist_name if plist_name != default_plist_name),
linux: (service_name if service_name != default_service_name),
}.compact
unless command?
return {
plist_name: custom_plist_name,
service_name: custom_service_name,
}.compact
return name_params.blank? ? {} : { name: name_params }
end
cron_string = if @cron.present?
@ -558,8 +551,7 @@ module Homebrew
sockets_string = "#{@sockets[:type]}://#{@sockets[:host]}:#{@sockets[:port]}" if @sockets.present?
{
plist_name: custom_plist_name,
service_name: custom_service_name,
name: name_params.presence,
run: @run_params,
run_type: @run_type,
interval: @interval,
@ -584,15 +576,10 @@ module Homebrew
sig { params(api_hash: Hash).returns(Hash) }
def self.deserialize(api_hash)
hash = {}
%w[plist_name service_name].each do |key|
next if (value = api_hash[key]).nil?
hash[key.to_sym] = value
end
hash[:name] = api_hash["name"].transform_keys(&:to_sym) if api_hash.key?("name")
# The service hash might not have a "run" command if it only documents
# an existing service file with "plist_name" or "service_name".
# an existing service file with the "name" command.
return hash unless api_hash.key?("run")
hash[:run] =

View File

@ -211,8 +211,7 @@ describe Caveats do
f = formula do
url "foo-1.0"
service do
plist_name "custom.mxcl.foo"
service_name "custom.foo"
name macos: "custom.mxcl.foo", linux: "custom.foo"
end
end
expect(Utils::Service).to receive(:installed?).with(f).once.and_return(true)

View File

@ -736,14 +736,13 @@ describe Formula do
f = formula do
url "https://brew.sh/test-1.0.tbz"
service do
plist_name "custom.macos.beanstalkd"
service_name "custom.linux.beanstalkd"
name macos: "custom.macos.beanstalkd", linux: "custom.linux.beanstalkd"
end
end
expect(f.plist_name).to eq("custom.macos.beanstalkd")
expect(f.service_name).to eq("custom.linux.beanstalkd")
expect(f.service.serialize.keys).to contain_exactly(:plist_name, :service_name)
expect(f.service.serialize.keys).to contain_exactly(:name)
end
specify "service helpers return data" do

View File

@ -272,6 +272,7 @@ describe Formulary do
"link_overwrite" => ["bin/abc"],
"caveats" => "example caveat string\n/$HOME\n$HOMEBREW_PREFIX",
"service" => {
"name" => { macos: "custom.launchd.name", linux: "custom.systemd.name" },
"run" => ["$HOMEBREW_PREFIX/opt/formula_name/bin/beanstalkd", "test"],
"run_type" => "immediate",
"working_dir" => "/$HOME",
@ -362,6 +363,8 @@ describe Formulary do
expect(formula.service.command).to eq(["#{HOMEBREW_PREFIX}/opt/formula_name/bin/beanstalkd", "test"])
expect(formula.service.run_type).to eq(:immediate)
expect(formula.service.working_dir).to eq(Dir.home)
expect(formula.plist_name).to eq("custom.launchd.name")
expect(formula.service_name).to eq("custom.systemd.name")
expect do
formula.install

View File

@ -11,7 +11,7 @@ describe RuboCop::Cop::FormulaAudit::Service do
url "https://brew.sh/foo-1.0.tgz"
service do
^^^^^^^^^^ FormulaAudit/Service: Service blocks require `run`, `plist_name` or `service_name` to be defined.
^^^^^^^^^^ FormulaAudit/Service: Service blocks require `run` or `name` to be defined.
run_type :cron
working_dir "/tmp/example"
end
@ -25,8 +25,7 @@ describe RuboCop::Cop::FormulaAudit::Service do
url "https://brew.sh/foo-1.0.tgz"
service do
plist_name "custom.mcxl.foo"
service_name "custom.foo"
name macos: "custom.mcxl.foo", linux: "custom.foo"
end
end
RUBY
@ -38,9 +37,8 @@ describe RuboCop::Cop::FormulaAudit::Service do
url "https://brew.sh/foo-1.0.tgz"
service do
^^^^^^^^^^ FormulaAudit/Service: `run` must be defined to use methods other than `service_name` and `plist_name` like [:working_dir].
plist_name "custom.mcxl.foo"
service_name "custom.foo"
^^^^^^^^^^ FormulaAudit/Service: `run` must be defined to use methods other than `name` like [:working_dir].
name macos: "custom.mcxl.foo", linux: "custom.foo"
working_dir "/tmp/example"
end
end

View File

@ -949,6 +949,10 @@ describe Homebrew::Service do
describe ".deserialize" do
let(:serialized_hash) do
{
"name" => {
"linux" => "custom.systemd.name",
"macos" => "custom.launchd.name",
},
"environment_variables" => {
"PATH" => "$HOMEBREW_PREFIX/bin:$HOMEBREW_PREFIX/sbin:/usr/bin:/bin:/usr/sbin:/sbin",
},
@ -961,6 +965,10 @@ describe Homebrew::Service do
let(:deserialized_hash) do
{
name: {
linux: "custom.systemd.name",
macos: "custom.launchd.name",
},
environment_variables: {
PATH: "#{HOMEBREW_PREFIX}/bin:#{HOMEBREW_PREFIX}/sbin:/usr/bin:/bin:/usr/sbin:/sbin",
},

View File

@ -875,14 +875,13 @@ There are two ways to add `launchd` plists and `systemd` services to a formula,
```ruby
service do
# Corresponds to the launchd script at "#{plist_name}.plist".
plist_name "custom.plist.name"
# Corresponds to the systemd script at "#{service_name}.service".
service_name "custom.service.name"
name macos: "custom.launchd.name",
linux: "custom.systemd.name"
end
```
To find the file we append `.plist` to the `launchd` service name and `.service` to the `systemd` service name internally.
2. If the formula does not provide a service file you can generate one using the following stanza:
```ruby
@ -893,7 +892,7 @@ There are two ways to add `launchd` plists and `systemd` services to a formula,
#### Service block methods
This table lists the options you can set within a `service` block. One of the `run`, `plist_name` or `service_name` fields is required inside the service block. The `run` field indicates what command to run and is required before using fields other than `plist_name` and `service_name`.
This table lists the options you can set within a `service` block. The `run` or `name` field must be defined inside the service block. The `run` field indicates what command to run and is required before using fields other than `name`.
| method | default | macOS | Linux | description |
| ----------------------- | ------------ | :---: | :---: | ----------- |
@ -914,8 +913,7 @@ This table lists the options you can set within a `service` block. One of the `r
| `process_type` | - | yes | no-op | type of process to manage: `:background`, `:standard`, `:interactive` or `:adaptive`
| `macos_legacy_timers` | - | yes | no-op | timers created by `launchd` jobs are coalesced unless this is set
| `sockets` | - | yes | no-op | socket that is created as an accesspoint to the service
| `plist_name` | `"homebrew.mxcl.#{formula.name}"` | yes | no | name of the `launchd` job
| `service_name` | `"homebrew.#{formula.name}"` | no | yes | name of the `systemd` job
| `name` | - | yes | yes | a hash with the `launchd` service name on macOS and/or the `systemd` service name on Linux
For services that are kept alive after starting you can use the default `run_type`: