diff --git a/Library/Homebrew/caveats.rb b/Library/Homebrew/caveats.rb index ebd3ff5736..db7d9b870a 100644 --- a/Library/Homebrew/caveats.rb +++ b/Library/Homebrew/caveats.rb @@ -2,6 +2,7 @@ # frozen_string_literal: true require "language/python" +require "utils/service" # A formula's caveats. # @@ -153,33 +154,32 @@ class Caveats end def service_caveats - return if !formula.plist && !formula.service? && !keg&.plist_installed? - return if formula.service? && formula.service.command.blank? + return if !formula.plist && !formula.service? && !Utils::Service.installed?(formula) && !keg&.plist_installed? + return if formula.service? && !formula.service.command? && !Utils::Service.installed?(formula) s = [] - command = if formula.service? + command = if formula.service.command? formula.service.manual_command else formula.plist_manual end - return <<~EOS if !which("launchctl") && formula.plist + return <<~EOS if !Utils::Service.launchctl? && formula.plist #{Formatter.warning("Warning:")} #{formula.name} provides a launchd plist which can only be used on macOS! You can manually execute the service instead with: #{command} EOS # Brew services only works with these two tools - return <<~EOS if !which("systemctl") && !which("launchctl") && formula.service? + return <<~EOS if !Utils::Service.systemctl? && !Utils::Service.launchctl? && formula.service.command? #{Formatter.warning("Warning:")} #{formula.name} provides a service which can only be used on macOS or systemd! You can manually execute the service instead with: #{command} EOS - is_running_service = formula.service? && quiet_system("ps aux | grep #{formula.service.command&.first}") - startup = formula.service&.requires_root? || formula.plist_startup - if is_running_service || (formula.plist && quiet_system("/bin/launchctl list #{formula.plist_name} &>/dev/null")) + startup = formula.service.requires_root? || formula.plist_startup + if Utils::Service.running?(formula) s << "To restart #{formula.full_name} after an upgrade:" s << " #{startup ? "sudo " : ""}brew services restart #{formula.full_name}" elsif startup @@ -190,7 +190,7 @@ class Caveats s << " brew services start #{formula.full_name}" end - if formula.plist_manual || formula.service? + if formula.plist_manual || formula.service.command? s << "Or, if you don't want/need a background service you can just run:" s << " #{command}" end diff --git a/Library/Homebrew/formula.rb b/Library/Homebrew/formula.rb index f61f106a62..f2de1d29d2 100644 --- a/Library/Homebrew/formula.rb +++ b/Library/Homebrew/formula.rb @@ -1015,13 +1015,13 @@ class Formula # The generated launchd {.plist} service name. sig { returns(String) } def plist_name - "homebrew.mxcl.#{name}" + service.plist_name end # The generated service name. sig { returns(String) } def service_name - "homebrew.#{name}" + service.service_name end # The generated launchd {.plist} file path. @@ -1051,8 +1051,6 @@ class Formula # The service specification of the software. def service - return unless service? - @service ||= Homebrew::Service.new(self, &self.class.service) end @@ -2169,7 +2167,7 @@ class Formula "disabled" => disabled?, "disable_date" => disable_date, "disable_reason" => disable_reason, - "service" => service&.serialize, + "service" => (service.serialize if service?), "tap_git_head" => tap_git_head, "ruby_source_path" => ruby_source_path, "ruby_source_checksum" => {}, diff --git a/Library/Homebrew/formula_cellar_checks.rb b/Library/Homebrew/formula_cellar_checks.rb index aabb25cafd..78321b7e27 100644 --- a/Library/Homebrew/formula_cellar_checks.rb +++ b/Library/Homebrew/formula_cellar_checks.rb @@ -288,7 +288,7 @@ module FormulaCellarChecks def check_service_command(formula) return unless formula.prefix.directory? return unless formula.service? - return if formula.service.command.blank? + return unless formula.service.command? "Service command does not exist" unless File.exist?(formula.service.command.first) end diff --git a/Library/Homebrew/formula_installer.rb b/Library/Homebrew/formula_installer.rb index 490be08d4c..d509eb2c73 100644 --- a/Library/Homebrew/formula_installer.rb +++ b/Library/Homebrew/formula_installer.rb @@ -1040,7 +1040,7 @@ on_request: installed_on_request?, options: options) return end - if formula.service? && formula.service.command.present? + if formula.service? && formula.service.command? service_path = formula.systemd_service_path service_path.atomic_write(formula.service.to_systemd_unit) service_path.chmod 0644 @@ -1052,7 +1052,7 @@ on_request: installed_on_request?, options: options) end end - service = if formula.service? && formula.service.command.present? + service = if formula.service? && formula.service.command? formula.service.to_plist elsif formula.plist formula.plist diff --git a/Library/Homebrew/formulary.rb b/Library/Homebrew/formulary.rb index d3514a050c..f077b6a41a 100644 --- a/Library/Homebrew/formulary.rb +++ b/Library/Homebrew/formulary.rb @@ -261,9 +261,10 @@ module Formulary run_params = service_hash.delete(:run) service do T.bind(self, Homebrew::Service) - if run_params.is_a?(Hash) + case run_params + when Hash run(**run_params) - else + when Array, String run run_params end service_hash.each do |key, arg| diff --git a/Library/Homebrew/service.rb b/Library/Homebrew/service.rb index 4fafa2c80e..81b2c2c0e0 100644 --- a/Library/Homebrew/service.rb +++ b/Library/Homebrew/service.rb @@ -28,7 +28,7 @@ module Homebrew @run_type = RUN_TYPE_IMMEDIATE @run_at_load = true @environment_variables = {} - @service_block = block + instance_eval(&block) if block end sig { returns(Formula) } @@ -36,6 +36,40 @@ module Homebrew @formula end + sig { returns(String) } + def default_plist_name + "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 + end + + sig { returns(String) } + def default_service_name + "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 + end + sig { params( command: T.nilable(T.any(T::Array[String], String, Pathname)), @@ -162,7 +196,6 @@ module Homebrew # @return [Boolean] sig { returns(T::Boolean) } def requires_root? - eval_service_block @require_root.present? && @require_root == true end @@ -198,7 +231,6 @@ module Homebrew # @return [Boolean] sig { returns(T::Boolean) } def keep_alive? - eval_service_block @keep_alive.present? && @keep_alive[:always] != false end @@ -357,20 +389,22 @@ module Homebrew sig { returns(T.nilable(T::Array[String])) } def command - eval_service_block @run&.map(&:to_s) end + sig { returns(T::Boolean) } + def command? + @run.present? + end + # Returns the `String` command to run manually instead of the service. # @return [String] sig { returns(String) } def manual_command - eval_service_block vars = @environment_variables.except(:PATH) .map { |k, v| "#{k}=\"#{v}\"" } - cmd = command - out = vars + cmd if cmd.present? + out = vars + command if command? out.join(" ") end @@ -378,7 +412,6 @@ module Homebrew # @return [Boolean] sig { returns(T::Boolean) } def timed? - eval_service_block @run_type == RUN_TYPE_CRON || @run_type == RUN_TYPE_INTERVAL end @@ -388,7 +421,7 @@ module Homebrew def to_plist # command needs to be first because it initializes all other values base = { - Label: @formula.plist_name, + Label: plist_name, ProgramArguments: command, RunAtLoad: @run_at_load == true, } @@ -487,10 +520,9 @@ module Homebrew WantedBy=timers.target [Timer] - Unit=#{@formula.service_name} + Unit=#{service_name} EOS - eval_service_block options = [] options << "Persistent=true" if @run_type == RUN_TYPE_CRON options << "OnUnitActiveSec=#{@interval}" if @run_type == RUN_TYPE_INTERVAL @@ -504,19 +536,18 @@ module Homebrew timer + options.join("\n") end - # Only evaluate the service block once. - sig { void } - def eval_service_block - return if @eval_service_block - - instance_eval(&@service_block) - @eval_service_block = true - end - # Prepare the service hash for inclusion in the formula API JSON. sig { returns(Hash) } def serialize - eval_service_block + custom_plist_name = plist_name if plist_name != default_plist_name + custom_service_name = service_name if service_name != default_service_name + + unless command? + return { + plist_name: custom_plist_name, + service_name: custom_service_name, + }.compact + end cron_string = if @cron.present? [:Minute, :Hour, :Day, :Month, :Weekday] @@ -527,6 +558,8 @@ module Homebrew sockets_string = "#{@sockets[:type]}://#{@sockets[:host]}:#{@sockets[:port]}" if @sockets.present? { + plist_name: custom_plist_name, + service_name: custom_service_name, run: @run_params, run_type: @run_type, interval: @interval, @@ -551,6 +584,17 @@ 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 + + # The service hash might not have a "run" command if it only documents + # an existing service file with "plist_name" or "service_name". + return hash unless api_hash.key?("run") + hash[:run] = case api_hash["run"] when String diff --git a/Library/Homebrew/test/caveats_spec.rb b/Library/Homebrew/test/caveats_spec.rb index 8dd70569a0..e1ee5a3e8c 100644 --- a/Library/Homebrew/test/caveats_spec.rb +++ b/Library/Homebrew/test/caveats_spec.rb @@ -32,6 +32,10 @@ describe Caveats do describe "#caveats" do context "when f.plist is not nil", :needs_macos do + before do + allow(Utils::Service).to receive(:launchctl?).and_return(true) + end + it "prints error when no launchd is present" do f = formula do url "foo-1.0" @@ -39,7 +43,7 @@ describe Caveats do "plist_test.plist" end end - allow_any_instance_of(Object).to receive(:which).with("launchctl").and_return(nil) + expect(Utils::Service).to receive(:launchctl?).once.and_return(false) expect(described_class.new(f).caveats).to include("provides a launchd plist which can only be used on macOS!") end @@ -50,7 +54,7 @@ describe Caveats do "plist_test.plist" end end - expect(described_class.new(f).caveats).to include("login") + expect(described_class.new(f).caveats).to include("restart at login") end it "gives information about service" do @@ -82,12 +86,25 @@ describe Caveats do expect(caveats).to include("WARNING:") expect(caveats).to include("tmux") end + + # @todo This should get deprecated and the service block `plist_name` method should get used instead. + it "prints info when there are custom service files" do + f = formula do + url "foo-1.0" + def plist_name + "custom.mxcl.foo" + end + end + expect(Utils::Service).to receive(:installed?).with(f).once.and_return(true) + expect(Utils::Service).to receive(:running?).with(f).once.and_return(false) + expect(described_class.new(f).caveats).to include("restart at login") + end end - context "when f.service is not nil" do + context "when service block is defined" do before do - allow_any_instance_of(Object).to receive(:which).with("launchctl").and_return(true) - allow_any_instance_of(Object).to receive(:which).with("systemctl").and_return(true) + allow(Utils::Service).to receive(:launchctl?).and_return(true) + allow(Utils::Service).to receive(:systemctl?).and_return(true) end it "prints warning when no service deamon is found" do @@ -97,9 +114,8 @@ describe Caveats do run [bin/"cmd"] end end - - allow_any_instance_of(Object).to receive(:which).with("launchctl").and_return(nil) - allow_any_instance_of(Object).to receive(:which).with("systemctl").and_return(nil) + expect(Utils::Service).to receive(:launchctl?).twice.and_return(false) + expect(Utils::Service).to receive(:systemctl?).once.and_return(false) expect(described_class.new(f).caveats).to include("service which can only be used on macOS or systemd!") end @@ -111,9 +127,7 @@ describe Caveats do require_root true end end - cmd = "#{HOMEBREW_CELLAR}/formula_name/1.0/bin/cmd" - allow(Homebrew).to receive(:_system).and_return(true) - allow(Homebrew).to receive(:_system).with("ps aux | grep #{cmd}").and_return(false) + expect(Utils::Service).to receive(:running?).with(f).once.and_return(false) expect(described_class.new(f).caveats).to include("startup") end @@ -124,10 +138,8 @@ describe Caveats do run [bin/"cmd"] end end - cmd = "#{HOMEBREW_CELLAR}/formula_name/1.0/bin/cmd" - allow(Homebrew).to receive(:_system).and_return(true) - expect(Homebrew).to receive(:_system).with("ps aux | grep #{cmd}").and_return(false) - expect(described_class.new(f).caveats).to include("login") + expect(Utils::Service).to receive(:running?).with(f).once.and_return(false) + expect(described_class.new(f).caveats).to include("restart at login") end it "gives information about require_root restarting services after upgrade" do @@ -138,10 +150,8 @@ describe Caveats do require_root true end end - cmd = "#{HOMEBREW_CELLAR}/formula_name/1.0/bin/cmd" f_obj = described_class.new(f) - allow(Homebrew).to receive(:_system).and_return(true) - expect(Homebrew).to receive(:_system).with("ps aux | grep #{cmd}").and_return(true) + expect(Utils::Service).to receive(:running?).with(f).once.and_return(true) expect(f_obj.caveats).to include(" sudo brew services restart #{f.full_name}") end @@ -152,10 +162,8 @@ describe Caveats do run [bin/"cmd"] end end - cmd = "#{HOMEBREW_CELLAR}/formula_name/1.0/bin/cmd" f_obj = described_class.new(f) - allow(Homebrew).to receive(:_system).and_return(true) - expect(Homebrew).to receive(:_system).with("ps aux | grep #{cmd}").and_return(true) + expect(Utils::Service).to receive(:running?).with(f).once.and_return(true) expect(f_obj.caveats).to include(" brew services restart #{f.full_name}") end @@ -167,10 +175,8 @@ describe Caveats do require_root true end end - cmd = "#{HOMEBREW_CELLAR}/formula_name/1.0/bin/cmd" f_obj = described_class.new(f) - allow(Homebrew).to receive(:_system).and_return(true) - allow(Homebrew).to receive(:_system).with("ps aux | grep #{cmd}").and_return(false) + expect(Utils::Service).to receive(:running?).with(f).once.and_return(false) expect(f_obj.caveats).to include(" sudo brew services start #{f.full_name}") end @@ -181,10 +187,8 @@ describe Caveats do run [bin/"cmd"] end end - cmd = "#{HOMEBREW_CELLAR}/formula_name/1.0/bin/cmd" f_obj = described_class.new(f) - allow(Homebrew).to receive(:_system).and_return(true) - allow(Homebrew).to receive(:_system).with("ps aux | grep #{cmd}").and_return(false) + expect(Utils::Service).to receive(:running?).with(f).once.and_return(false) expect(f_obj.caveats).to include(" brew services start #{f.full_name}") end @@ -202,6 +206,19 @@ describe Caveats do expect(caveats).to include("if you don't want/need a background service") expect(caveats).to include("VAR=\"foo\" #{cmd} start") end + + it "prints info when there are custom service files" do + f = formula do + url "foo-1.0" + service do + plist_name "custom.mxcl.foo" + service_name "custom.foo" + end + end + expect(Utils::Service).to receive(:installed?).with(f).once.and_return(true) + expect(Utils::Service).to receive(:running?).with(f).once.and_return(false) + expect(described_class.new(f).caveats).to include("restart at login") + end end context "when f.keg_only is not nil" do diff --git a/Library/Homebrew/test/formula_installer_spec.rb b/Library/Homebrew/test/formula_installer_spec.rb index 812e0741a4..49404f8208 100644 --- a/Library/Homebrew/test/formula_installer_spec.rb +++ b/Library/Homebrew/test/formula_installer_spec.rb @@ -218,21 +218,21 @@ describe FormulaInstaller do it "works if service is set" do formula = Testball.new + service = Homebrew::Service.new(formula) launchd_service_path = formula.launchd_service_path service_path = formula.systemd_service_path - service = Homebrew::Service.new(formula) formula.opt_prefix.mkpath expect(formula).to receive(:plist).and_return(nil) expect(formula).to receive(:service?).exactly(3).and_return(true) - expect(formula).to receive(:service).exactly(5).and_return(service) + expect(formula).to receive(:service).exactly(7).and_return(service) expect(formula).to receive(:launchd_service_path).and_call_original expect(formula).to receive(:systemd_service_path).and_call_original expect(service).to receive(:timed?).and_return(false) + expect(service).to receive(:command?).exactly(2).and_return(true) expect(service).to receive(:to_plist).and_return("plist") expect(service).to receive(:to_systemd_unit).and_return("unit") - expect(service).to receive(:command).exactly(2).and_return("/bin/sh") installer = described_class.new(formula) expect do @@ -245,24 +245,24 @@ describe FormulaInstaller do it "works if timed service is set" do formula = Testball.new + service = Homebrew::Service.new(formula) launchd_service_path = formula.launchd_service_path service_path = formula.systemd_service_path timer_path = formula.systemd_timer_path - service = Homebrew::Service.new(formula) formula.opt_prefix.mkpath expect(formula).to receive(:plist).and_return(nil) expect(formula).to receive(:service?).exactly(3).and_return(true) - expect(formula).to receive(:service).exactly(6).and_return(service) + expect(formula).to receive(:service).exactly(9).and_return(service) expect(formula).to receive(:launchd_service_path).and_call_original expect(formula).to receive(:systemd_service_path).and_call_original expect(formula).to receive(:systemd_timer_path).and_call_original - expect(service).to receive(:to_plist).and_return("plist") expect(service).to receive(:timed?).and_return(true) + expect(service).to receive(:command?).exactly(2).and_return(true) + expect(service).to receive(:to_plist).and_return("plist") expect(service).to receive(:to_systemd_unit).and_return("unit") expect(service).to receive(:to_systemd_timer).and_return("timer") - expect(service).to receive(:command).exactly(2).and_return("/bin/sh") installer = described_class.new(formula) expect do diff --git a/Library/Homebrew/test/formula_spec.rb b/Library/Homebrew/test/formula_spec.rb index 0cb2279d80..817f73ca8f 100644 --- a/Library/Homebrew/test/formula_spec.rb +++ b/Library/Homebrew/test/formula_spec.rb @@ -701,7 +701,7 @@ describe Formula do url "https://brew.sh/test-1.0.tbz" end - expect(f.service).to be_nil + expect(f.service.serialize).to eq({}) end specify "service complicated" do @@ -717,7 +717,8 @@ describe Formula do keep_alive true end end - expect(f.service).not_to be_nil + expect(f.service.serialize.keys) + .to contain_exactly(:run, :run_type, :error_log_path, :log_path, :working_dir, :keep_alive) end specify "service uses simple run" do @@ -728,7 +729,21 @@ describe Formula do end end - expect(f.service).not_to be_nil + expect(f.service.serialize.keys).to contain_exactly(:run, :run_type) + end + + specify "service with only custom names" do + f = formula do + url "https://brew.sh/test-1.0.tbz" + service do + plist_name "custom.macos.beanstalkd" + service_name "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) end specify "service helpers return data" do diff --git a/Library/Homebrew/utils/service.rb b/Library/Homebrew/utils/service.rb new file mode 100644 index 0000000000..edd6deb524 --- /dev/null +++ b/Library/Homebrew/utils/service.rb @@ -0,0 +1,50 @@ +# typed: true +# frozen_string_literal: true + +module Utils + # Helpers for `brew services` related code. + module Service + # Check if a service is running for a specified formula. + sig { params(formula: Formula).returns(T::Boolean) } + def self.running?(formula) + if launchctl? + quiet_system(launchctl, "list", formula.plist_name) + elsif systemctl? + quiet_system(systemctl, "is-active", "--quiet", formula.service_name) + end + end + + # Check if a service file is installed in the expected location. + sig { params(formula: Formula).returns(T::Boolean) } + def self.installed?(formula) + (launchctl? && formula.launchd_service_path.exist?) || + (systemctl? && formula.systemd_service_path.exist?) + end + + # Path to launchctl binary. + sig { returns(T.nilable(Pathname)) } + def self.launchctl + return @launchctl if defined? @launchctl + + @launchctl = which("launchctl") + end + + # Path to systemctl binary. + sig { returns(T.nilable(Pathname)) } + def self.systemctl + return @systemctl if defined? @systemctl + + @systemctl = which("systemctl") + end + + sig { returns(T::Boolean) } + def self.launchctl? + !launchctl.nil? + end + + sig { returns(T::Boolean) } + def self.systemctl? + !systemctl.nil? + end + end +end diff --git a/Library/Homebrew/utils/service.rbi b/Library/Homebrew/utils/service.rbi new file mode 100644 index 0000000000..9211be719a --- /dev/null +++ b/Library/Homebrew/utils/service.rbi @@ -0,0 +1,7 @@ +# typed: strict + +module Utils + module Service + include Kernel + end +end