Merge pull request #14371 from dawidd6/os-specific-service

Allow defining service run command per platform

Library/Homebrew/caveats.rb

@@ -153,6 +153,7 @@ class Caveats
 
   def service_caveats
     return if !f.plist && !f.service? && !keg&.plist_installed?
+    return if f.service? && f.service.command.blank?
 
     s = []
 

Library/Homebrew/extend/on_system.rb

@@ -108,6 +108,13 @@ module OnSystem
 
       result
     end
+
+    base.define_method(:on_system_conditional) do |macos: nil, linux: nil|
+      @on_system_blocks_exist = true
+
+      return macos if OnSystem.os_condition_met?(:macos) && macos.present?
+      return linux if OnSystem.os_condition_met?(:linux) && linux.present?
+    end
   end
 
   sig { params(base: Class).void }

Library/Homebrew/formula_cellar_checks.rb

@@ -278,8 +278,7 @@ module FormulaCellarChecks
   def check_service_command(formula)
     return unless formula.prefix.directory?
     return unless formula.service?
-
+    return if formula.service.command.blank?
-    return "Service command blank" if formula.service.command.blank?
 
     "Service command does not exist" unless File.exist?(formula.service.command.first)
   end

Library/Homebrew/formula_installer.rb

@@ -1031,7 +1031,7 @@ class FormulaInstaller
       return
     end
 
-    if formula.service?
+    if formula.service? && formula.service.command.present?
       service_path = formula.systemd_service_path
       service_path.atomic_write(formula.service.to_systemd_unit)
       service_path.chmod 0644
@@ -1043,7 +1043,7 @@ class FormulaInstaller
       end
     end
 
-    service = if formula.service?
+    service = if formula.service? && formula.service.command.present?
       formula.service.to_plist
     elsif formula.plist
       formula.plist
@@ -1057,6 +1057,7 @@ class FormulaInstaller
     log = formula.var/"log"
     log.mkpath if service.include? log.to_s
   rescue Exception => e # rubocop:disable Lint/RescueException
+    puts e
     ofail "Failed to install service files"
     odebug e, e.backtrace
   end

Library/Homebrew/service.rb

@@ -1,6 +1,8 @@
 # typed: true
 # frozen_string_literal: true
 
+require "extend/on_system"
+
 module Homebrew
   # The {Service} class implements the DSL methods used in a formula's
   # `service` block and stores related instance variables. Most of these methods
@@ -8,6 +10,7 @@ module Homebrew
   class Service
     extend T::Sig
     extend Forwardable
+    include OnSystem::MacOSAndLinux
 
     RUN_TYPE_IMMEDIATE = :immediate
     RUN_TYPE_INTERVAL = :interval
@@ -33,8 +36,15 @@ module Homebrew
       @formula
     end
 
-    sig { params(command: T.nilable(T.any(T::Array[String], String, Pathname))).returns(T.nilable(Array)) }
+    sig {
-    def run(command = nil)
+      params(
+        command: T.nilable(T.any(T::Array[String], String, Pathname)),
+        macos:   T.nilable(T.any(T::Array[String], String, Pathname)),
+        linux:   T.nilable(T.any(T::Array[String], String, Pathname)),
+      ).returns(T.nilable(Array))
+    }
+    def run(command = nil, macos: nil, linux: nil)
+      command ||= on_system_conditional(macos: macos, linux: linux)
       case T.unsafe(command)
       when nil
         @run
@@ -326,10 +336,10 @@ module Homebrew
       "#{HOMEBREW_PREFIX}/bin:#{HOMEBREW_PREFIX}/sbin:/usr/bin:/bin:/usr/sbin:/sbin"
     end
 
-    sig { returns(T::Array[String]) }
+    sig { returns(T.nilable(T::Array[String])) }
     def command
       instance_eval(&@service_block)
-      @run.map(&:to_s)
+      @run&.map(&:to_s)
     end
 
     # Returns the `String` command to run manually instead of the service.
@@ -340,7 +350,8 @@ module Homebrew
       vars = @environment_variables.except(:PATH)
                                    .map { |k, v| "#{k}=\"#{v}\"" }
 
-      out = vars + command
+      cmd = command
+      out = vars + cmd if cmd.present?
       out.join(" ")
     end
 
@@ -427,7 +438,7 @@ module Homebrew
       EOS
 
       # command needs to be first because it initializes all other values
-      cmd = command.join(" ")
+      cmd = command&.join(" ")
 
       options = []
       options << "Type=#{(@launch_only_once == true) ? "oneshot" : "simple"}"

Library/Homebrew/service.rbi

@@ -0,0 +1,7 @@
+# typed: strict
+
+module Homebrew
+  class Service
+    def on_system_conditional(macos: nil, linux: nil); end
+  end
+end

Library/Homebrew/test/dev-cmd/audit_spec.rb

@@ -564,7 +564,7 @@ module Homebrew
         RUBY
 
         mkdir_p fa.formula.prefix
-        expect(fa.check_service_command(fa.formula)).to match "Service command blank"
+        expect(fa.check_service_command(fa.formula)).to match nil
       end
 
       specify "Invalid command" do

Library/Homebrew/test/formula_installer_spec.rb

@@ -226,13 +226,14 @@ describe FormulaInstaller do
 
       expect(formula).to receive(:plist).and_return(nil)
       expect(formula).to receive(:service?).exactly(3).and_return(true)
-      expect(formula).to receive(:service).exactly(3).and_return(service)
+      expect(formula).to receive(:service).exactly(5).and_return(service)
       expect(formula).to receive(:plist_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(: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 {
@@ -253,7 +254,7 @@ describe FormulaInstaller do
 
       expect(formula).to receive(:plist).and_return(nil)
       expect(formula).to receive(:service?).exactly(3).and_return(true)
-      expect(formula).to receive(:service).exactly(4).and_return(service)
+      expect(formula).to receive(:service).exactly(6).and_return(service)
       expect(formula).to receive(:plist_path).and_call_original
       expect(formula).to receive(:systemd_service_path).and_call_original
       expect(formula).to receive(:systemd_timer_path).and_call_original
@@ -262,6 +263,7 @@ describe FormulaInstaller do
       expect(service).to receive(:timed?).and_return(true)
       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 {

Library/Homebrew/test/service_spec.rb

@@ -803,5 +803,77 @@ describe Homebrew::Service do
       command = f.service.command
       expect(command).to eq(["#{HOMEBREW_PREFIX}/opt/#{name}/bin/beanstalkd", "test"])
     end
+
+    it "returns @run data on Linux", :needs_linux do
+      f = stub_formula do
+        service do
+          run linux: [opt_bin/"beanstalkd", "test"]
+          run_type :immediate
+        end
+      end
+
+      command = f.service.command
+      expect(command).to eq(["#{HOMEBREW_PREFIX}/opt/#{name}/bin/beanstalkd", "test"])
+    end
+
+    it "returns nil on Linux", :needs_linux do
+      f = stub_formula do
+        service do
+          run macos: [opt_bin/"beanstalkd", "test"]
+          run_type :immediate
+        end
+      end
+
+      command = f.service.command
+      expect(command).to be_nil
+    end
+
+    it "returns @run data on macOS", :needs_macos do
+      f = stub_formula do
+        service do
+          run macos: [opt_bin/"beanstalkd", "test"]
+          run_type :immediate
+        end
+      end
+
+      command = f.service.command
+      expect(command).to eq(["#{HOMEBREW_PREFIX}/opt/#{name}/bin/beanstalkd", "test"])
+    end
+
+    it "returns nil on macOS", :needs_macos do
+      f = stub_formula do
+        service do
+          run linux: [opt_bin/"beanstalkd", "test"]
+          run_type :immediate
+        end
+      end
+
+      command = f.service.command
+      expect(command).to be_nil
+    end
+
+    it "returns appropriate @run data on Linux", :needs_linux do
+      f = stub_formula do
+        service do
+          run macos: [opt_bin/"beanstalkd", "test", "macos"], linux: [opt_bin/"beanstalkd", "test", "linux"]
+          run_type :immediate
+        end
+      end
+
+      command = f.service.command
+      expect(command).to eq(["#{HOMEBREW_PREFIX}/opt/#{name}/bin/beanstalkd", "test", "linux"])
+    end
+
+    it "returns appropriate @run data on macOS", :needs_macos do
+      f = stub_formula do
+        service do
+          run macos: [opt_bin/"beanstalkd", "test", "macos"], linux: [opt_bin/"beanstalkd", "test", "linux"]
+          run_type :immediate
+        end
+      end
+
+      command = f.service.command
+      expect(command).to eq(["#{HOMEBREW_PREFIX}/opt/#{name}/bin/beanstalkd", "test", "macos"])
+    end
   end
 end