bundle: use version env for installer service handling

Library/Homebrew/bundle/brew_installer.rb

@@ -113,12 +113,15 @@ module Homebrew
 
       def service_change_state!(verbose:)
         require "bundle/brew_services"
+
+        file = Bundle::BrewServices.versioned_service_file(@name)
+
         if restart_service_needed?
           puts "Restarting #{@name} service." if verbose
-          BrewServices.restart(@full_name, verbose:)
+          BrewServices.restart(@full_name, file:, verbose:)
         elsif start_service_needed?
           puts "Starting #{@name} service." if verbose
-          BrewServices.start(@full_name, verbose:)
+          BrewServices.start(@full_name, file:, verbose:)
         else
           true
         end

Library/Homebrew/bundle/brew_services.rb

@@ -1,43 +1,58 @@
 # typed: true # rubocop:todo Sorbet/StrictSigil
 # frozen_string_literal: true
 
+require "services/system"
+
 module Homebrew
   module Bundle
     module BrewServices
-      module_function
+      def self.reset!
-
-      def reset!
         @started_services = nil
       end
 
-      def stop(name, verbose: false)
+      def self.stop(name, keep: false, verbose: false)
         return true unless started?(name)
 
-        return unless Bundle.brew("services", "stop", name, verbose:)
+        args = ["services", "stop", name]
+        args << "--keep" if keep
+        return unless Bundle.brew(*args, verbose:)
 
         started_services.delete(name)
         true
       end
 
-      def start(name, verbose: false)
+      def self.start(name, file: nil, verbose: false)
-        return unless Bundle.brew("services", "start", name, verbose:)
+        args = ["services", "start", name]
+        args << "--file=#{file}" if file
+        return unless Bundle.brew(*args, verbose:)
 
         started_services << name
         true
       end
 
-      def restart(name, verbose: false)
+      def self.run(name, file: nil, verbose: false)
-        return unless Bundle.brew("services", "restart", name, verbose:)
+        args = ["services", "run", name]
+        args << "--file=#{file}" if file
+        return unless Bundle.brew(*args, verbose:)
 
         started_services << name
         true
       end
 
-      def started?(name)
+      def self.restart(name, file: nil, verbose: false)
+        args = ["services", "restart", name]
+        args << "--file=#{file}" if file
+        return unless Bundle.brew(*args, verbose:)
+
+        started_services << name
+        true
+      end
+
+      def self.started?(name)
         started_services.include? name
       end
 
-      def started_services
+      def self.started_services
         @started_services ||= begin
           states_to_skip = %w[stopped none]
           Utils.safe_popen_read(HOMEBREW_BREW_FILE, "services", "list").lines.filter_map do |line|
@@ -48,6 +63,23 @@ module Homebrew
           end
         end
       end
+
+      def self.versioned_service_file(name)
+        env_version = Bundle.formula_versions_from_env[name]
+        return if env_version.nil?
+
+        formula = Formula[name]
+        prefix = formula.rack/env_version
+        return unless prefix.directory?
+
+        service_file = if Homebrew::Services::System.launchctl?
+          prefix/"#{formula.plist_name}.plist"
+        else
+          prefix/"#{formula.service_name}.service"
+        end
+
+        service_file if service_file.file?
+      end
     end
   end
 end

Library/Homebrew/bundle/commands/services.rb

@@ -2,7 +2,8 @@
 # frozen_string_literal: true
 
 require "bundle/brewfile"
-require "bundle/services"
+require "bundle/brew_services"
+require "formula"
 
 module Homebrew
   module Bundle
@@ -17,13 +18,119 @@ module Homebrew
           subcommand = args.first
           case subcommand
           when "run"
-            Homebrew::Bundle::Services.run(parsed_entries)
+            run_services(parsed_entries)
           when "stop"
-            Homebrew::Bundle::Services.stop(parsed_entries)
+            stop_services(parsed_entries)
           else
             raise UsageError, "unknown bundle services subcommand: #{subcommand}"
           end
         end
+
+        sig {
+          params(
+            entries: T::Array[Homebrew::Bundle::Dsl::Entry],
+            _block:  T.proc.params(
+              info:                 T::Hash[String, T.anything],
+              service_file:         Pathname,
+              conflicting_services: T::Array[T::Hash[String, T.anything]],
+            ).void,
+          ).void
+        }
+        private_class_method def self.map_entries(entries, &_block)
+          entries_formulae = entries.filter_map do |entry|
+            next if entry.type != :brew
+
+            formula = Formula[entry.name]
+            next unless formula.any_version_installed?
+
+            [entry, formula]
+          end.to_h
+
+          # The formula + everything that could possible conflict with the service
+          names_to_query = entries_formulae.flat_map do |entry, formula|
+            [
+              formula.name,
+              *formula.versioned_formulae_names,
+              *formula.conflicts.map(&:name),
+              *entry.options[:conflicts_with],
+            ]
+          end
+
+          # We parse from a command invocation so that brew wrappers can invoke special actions
+          # for the elevated nature of `brew services`
+          services_info = JSON.parse(
+            Utils.safe_popen_read(HOMEBREW_BREW_FILE, "services", "info", "--json", *names_to_query),
+          )
+
+          entries_formulae.filter_map do |entry, formula|
+            service_file = Bundle::BrewServices.versioned_service_file(entry.name)
+
+            unless service_file&.file?
+              prefix = formula.any_installed_prefix
+              next if prefix.nil?
+
+              service_file = if Homebrew::Services::System.launchctl?
+                prefix/"#{formula.plist_name}.plist"
+              else
+                prefix/"#{formula.service_name}.service"
+              end
+            end
+
+            next unless service_file.file?
+
+            info = services_info.find { |candidate| candidate["name"] == formula.name }
+            conflicting_services = services_info.select do |candidate|
+              next unless candidate["running"]
+
+              formula.versioned_formulae_names.include?(candidate["name"])
+            end
+
+            raise "Failed to get service info for #{entry.name}" if info.nil?
+
+            yield info, service_file, conflicting_services
+          end
+        end
+
+        sig { params(entries: T::Array[Homebrew::Bundle::Dsl::Entry], _block: T.nilable(T.proc.void)).void }
+        def self.run_services(entries, &_block)
+          map_entries(entries) do |info, service_file, conflicting_services|
+            if info["running"] && !Bundle::BrewServices.stop(info["name"], keep: true)
+              opoo "Failed to stop #{info["name"]} service"
+            end
+
+            conflicting_services.each do |conflict|
+              if conflict["running"] && !Bundle::BrewServices.stop(conflict["name"], keep: true)
+                opoo "Failed to stop #{conflict["name"]} service"
+              end
+            end
+
+            unless Bundle::BrewServices.run(info["name"], file: service_file)
+              opoo "Failed to start #{info["name"]} service"
+            end
+
+            return unless block_given?
+
+            begin
+              yield
+            ensure
+              stop_services(entries)
+            end
+          end
+        end
+
+        sig { params(entries: T::Array[Homebrew::Bundle::Dsl::Entry]).void }
+        def self.stop_services(entries)
+          map_entries(entries) do |info, _, _|
+            next unless info["loaded"]
+
+            # Try avoid services not started by `brew bundle services`
+            next if Homebrew::Services::System.launchctl? && info["registered"]
+
+            if info["running"] && !Bundle::BrewServices.stop(info["name"], keep: true)
+              opoo "Failed to stop #{info["name"]} service"
+            end
+          end
+        end
       end
     end
   end

Library/Homebrew/bundle/services.rb

@@ -1,112 +0,0 @@
-# typed: strict
-# frozen_string_literal: true
-
-require "bundle/dsl"
-require "formula"
-require "services/system"
-
-module Homebrew
-  module Bundle
-    module Services
-      sig {
-        params(
-          entries: T::Array[Homebrew::Bundle::Dsl::Entry],
-          _block:  T.proc.params(
-            info:                 T::Hash[String, T.anything],
-            service_file:         Pathname,
-            conflicting_services: T::Array[T::Hash[String, T.anything]],
-          ).void,
-        ).void
-      }
-      private_class_method def self.map_entries(entries, &_block)
-        formula_versions = Bundle.formula_versions_from_env
-
-        entries_formulae = entries.filter_map do |entry|
-          next if entry.type != :brew
-
-          formula = Formula[entry.name]
-          next unless formula.any_version_installed?
-
-          [entry, formula]
-        end.to_h
-
-        # The formula + everything that could possible conflict with the service
-        names_to_query = entries_formulae.flat_map do |entry, formula|
-          [
-            formula.name,
-            *formula.versioned_formulae_names,
-            *formula.conflicts.map(&:name),
-            *entry.options[:conflicts_with],
-          ]
-        end
-
-        # We parse from a command invocation so that brew wrappers can invoke special actions
-        # for the elevated nature of `brew services`
-        services_info = JSON.parse(
-          Utils.safe_popen_read(HOMEBREW_BREW_FILE, "services", "info", "--json", *names_to_query),
-        )
-
-        entries_formulae.filter_map do |entry, formula|
-          version = formula_versions[entry.name.downcase]
-          prefix = formula.rack/version if version
-
-          service_file = if prefix&.directory?
-            if Homebrew::Services::System.launchctl?
-              prefix/"#{formula.plist_name}.plist"
-            else
-              prefix/"#{formula.service_name}.service"
-            end
-          end
-
-          unless service_file&.file?
-            prefix = formula.any_installed_prefix
-            next if prefix.nil?
-
-            service_file = if Homebrew::Services::System.launchctl?
-              prefix/"#{formula.plist_name}.plist"
-            else
-              prefix/"#{formula.service_name}.service"
-            end
-          end
-
-          next unless service_file.file?
-
-          info = services_info.find { |candidate| candidate["name"] == formula.name }
-          conflicting_services = services_info.select do |candidate|
-            next unless candidate["running"]
-
-            formula.versioned_formulae_names.include?(candidate["name"])
-          end
-
-          raise "Failed to get service info for #{entry.name}" if info.nil?
-
-          yield info, service_file, conflicting_services
-        end
-      end
-
-      sig { params(entries: T::Array[Homebrew::Bundle::Dsl::Entry]).void }
-      def self.run(entries)
-        map_entries(entries) do |info, service_file, conflicting_services|
-          safe_system HOMEBREW_BREW_FILE, "services", "stop", "--keep", info["name"] if info["running"]
-          conflicting_services.each do |conflicting_service|
-            safe_system HOMEBREW_BREW_FILE, "services", "stop", "--keep", conflicting_service["name"]
-          end
-
-          safe_system HOMEBREW_BREW_FILE, "services", "run", "--file=#{service_file}", info["name"]
-        end
-      end
-
-      sig { params(entries: T::Array[Homebrew::Bundle::Dsl::Entry]).void }
-      def self.stop(entries)
-        map_entries(entries) do |info, _, _|
-          next unless info["loaded"]
-
-          # Try avoid services not started by `brew bundle services`
-          next if Homebrew::Services::System.launchctl? && info["registered"]
-
-          safe_system HOMEBREW_BREW_FILE, "services", "stop", info["name"]
-        end
-      end
-    end
-  end
-end

Library/Homebrew/test/bundle/brew_installer_spec.rb

@@ -58,7 +58,7 @@ RSpec.describe Homebrew::Bundle::BrewInstaller do
         context "with a successful installation" do
           it "start service" do
             expect(Homebrew::Bundle::BrewServices).to \
-              receive(:start).with(formula_name, verbose: false).and_return(true)
+              receive(:start).with(formula_name, file: nil, verbose: false).and_return(true)
             described_class.preinstall(formula_name, start_service: true)
             described_class.install(formula_name, start_service: true)
           end
@@ -67,7 +67,7 @@ RSpec.describe Homebrew::Bundle::BrewInstaller do
         context "with a skipped installation" do
           it "start service" do
             expect(Homebrew::Bundle::BrewServices).to \
-              receive(:start).with(formula_name, verbose: false).and_return(true)
+              receive(:start).with(formula_name, file: nil, verbose: false).and_return(true)
             described_class.install(formula_name, preinstall: false, start_service: true)
           end
         end
@@ -83,7 +83,7 @@ RSpec.describe Homebrew::Bundle::BrewInstaller do
       context "with a successful installation" do
         it "restart service" do
           expect(Homebrew::Bundle::BrewServices).to \
-            receive(:restart).with(formula_name, verbose: false).and_return(true)
+            receive(:restart).with(formula_name, file: nil, verbose: false).and_return(true)
           described_class.preinstall(formula_name, restart_service: :always)
           described_class.install(formula_name, restart_service: :always)
         end
@@ -92,7 +92,7 @@ RSpec.describe Homebrew::Bundle::BrewInstaller do
       context "with a skipped installation" do
         it "restart service" do
           expect(Homebrew::Bundle::BrewServices).to \
-            receive(:restart).with(formula_name, verbose: false).and_return(true)
+            receive(:restart).with(formula_name, file: nil, verbose: false).and_return(true)
           described_class.install(formula_name, preinstall: false, restart_service: :always)
         end
       end
@@ -201,7 +201,8 @@ RSpec.describe Homebrew::Bundle::BrewInstaller do
                                                           verbose:).and_return(true)
         expect(Homebrew::Bundle::BrewServices).to receive(:stop).with("mysql55", verbose:).and_return(true)
         expect(Homebrew::Bundle::BrewServices).to receive(:stop).with("mysql56", verbose:).and_return(true)
-        expect(Homebrew::Bundle::BrewServices).to receive(:restart).with(formula_name, verbose:).and_return(true)
+        expect(Homebrew::Bundle::BrewServices).to receive(:restart).with(formula_name, file:    nil,
+                                                                                       verbose:).and_return(true)
         described_class.preinstall(formula_name, restart_service: :always, conflicts_with: ["mysql56"])
         described_class.install(formula_name, restart_service: :always, conflicts_with: ["mysql56"])
       end
@@ -216,7 +217,8 @@ RSpec.describe Homebrew::Bundle::BrewInstaller do
                                                           verbose:).and_return(true)
         expect(Homebrew::Bundle::BrewServices).to receive(:stop).with("mysql55", verbose:).and_return(true)
         expect(Homebrew::Bundle::BrewServices).to receive(:stop).with("mysql56", verbose:).and_return(true)
-        expect(Homebrew::Bundle::BrewServices).to receive(:restart).with(formula_name, verbose:).and_return(true)
+        expect(Homebrew::Bundle::BrewServices).to receive(:restart).with(formula_name, file:    nil,
+                                                                                       verbose:).and_return(true)
         described_class.preinstall(formula_name, restart_service: :always, conflicts_with: ["mysql56"], verbose: true)
         described_class.install(formula_name, restart_service: :always, conflicts_with: ["mysql56"], verbose: true)
       end