From 73bc934c85dd6fabf44076482affe267cd21c5a6 Mon Sep 17 00:00:00 2001 From: Sean Molenaar Date: Mon, 23 Aug 2021 22:52:36 +0200 Subject: [PATCH] Caveats: suggest brew services for linux --- Library/Homebrew/caveats.rb | 50 ++++++--- Library/Homebrew/extend/os/caveats.rb | 4 - Library/Homebrew/extend/os/mac/caveats.rb | 63 ----------- Library/Homebrew/test/caveats_spec.rb | 131 +++++++++++++++------- 4 files changed, 120 insertions(+), 128 deletions(-) delete mode 100644 Library/Homebrew/extend/os/caveats.rb delete mode 100644 Library/Homebrew/extend/os/mac/caveats.rb diff --git a/Library/Homebrew/caveats.rb b/Library/Homebrew/caveats.rb index 397ba9bb06..690cda4959 100644 --- a/Library/Homebrew/caveats.rb +++ b/Library/Homebrew/caveats.rb @@ -40,7 +40,7 @@ class Caveats caveats << function_completion_caveats(shell) end - caveats << plist_caveats + caveats << service_caveats caveats << elisp_caveats caveats.compact.join("\n") end @@ -151,8 +151,10 @@ class Caveats EOS end - def plist_caveats - return if !f.plist_manual && !f.service? + def service_caveats + return if !f.plist && !f.service? && !keg&.plist_installed? + + s = [] command = if f.service? f.service.manual_command @@ -160,30 +162,42 @@ class Caveats f.plist_manual end - # Default to brew services not being supported. macOS overrides this behavior. - <<~EOS + return <<~EOS if !which("launchctl") && f.plist #{Formatter.warning("Warning:")} #{f.name} provides a launchd plist which can only be used on macOS! You can manually execute the service instead with: #{command} EOS - end - def plist_path - destination = if f.plist_startup - "/Library/LaunchDaemons" + # Brew services only works with these two tools + return <<~EOS if !which("systemctl") && !which("launchctl") && f.service? + #{Formatter.warning("Warning:")} #{f.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 = f.service? && quiet_system("ps aux | grep #{f.service.command&.first}") + if is_running_service || (f.plist && quiet_system("/bin/launchctl list #{f.plist_name} &>/dev/null")) + s << "To restart #{f.full_name} after an upgrade:" + s << " #{f.plist_startup ? "sudo " : ""}brew services restart #{f.full_name}" + elsif f.plist_startup + s << "To start #{f.full_name} now and restart at startup:" + s << " sudo brew services start #{f.full_name}" else - "~/Library/LaunchAgents" + s << "To start #{f.full_name} now and restart at login:" + s << " brew services start #{f.full_name}" end - plist_filename = if f.plist - f.plist_path.basename - else - File.basename Dir["#{keg}/*.plist"].first + if f.plist_manual || f.service? + s << "Or, if you don't want/need a background service you can just run:" + s << " #{command}" end - destination_path = Pathname.new(File.expand_path(destination)) - destination_path/plist_filename + # pbpaste is the system clipboard tool on macOS and fails with `tmux` by default + # check if this is being run under `tmux` to avoid failing + if ENV["HOMEBREW_TMUX"] && !quiet_system("/usr/bin/pbpaste") + s << "" << "WARNING: brew services will fail when run under tmux." + end + + "#{s.join("\n")}\n" unless s.empty? end end - -require "extend/os/caveats" diff --git a/Library/Homebrew/extend/os/caveats.rb b/Library/Homebrew/extend/os/caveats.rb deleted file mode 100644 index 75bd37fed5..0000000000 --- a/Library/Homebrew/extend/os/caveats.rb +++ /dev/null @@ -1,4 +0,0 @@ -# typed: strict -# frozen_string_literal: true - -require "extend/os/mac/caveats" if OS.mac? diff --git a/Library/Homebrew/extend/os/mac/caveats.rb b/Library/Homebrew/extend/os/mac/caveats.rb deleted file mode 100644 index 7abc7655ad..0000000000 --- a/Library/Homebrew/extend/os/mac/caveats.rb +++ /dev/null @@ -1,63 +0,0 @@ -# typed: true -# frozen_string_literal: true - -class Caveats - undef plist_caveats - - def plist_caveats - s = [] - return if !f.plist && !f.service? && !keg&.plist_installed? - - plist_domain = f.plist_path.basename(".plist") - - # we readlink because this path probably doesn't exist since caveats - # occurs before the link step of installation - # Yosemite security measures mildly tighter rules: - # https://github.com/Homebrew/legacy-homebrew/issues/33815 - if f.plist && (!plist_path.file? || !plist_path.symlink?) - if f.plist_startup - s << "To have launchd start #{f.full_name} now and restart at startup:" - s << " sudo brew services start #{f.full_name}" - else - s << "To have launchd start #{f.full_name} now and restart at login:" - s << " brew services start #{f.full_name}" - end - # For startup plists, we cannot tell whether it's running on launchd, - # as it requires for `sudo launchctl list` to get real result. - elsif f.plist_startup - s << "To restart #{f.full_name} after an upgrade:" - s << " sudo brew services restart #{f.full_name}" - elsif Kernel.system "/bin/launchctl list #{plist_domain} &>/dev/null" - s << "To restart #{f.full_name} after an upgrade:" - s << " brew services restart #{f.full_name}" - else - s << "To start #{f.full_name}:" - s << " brew services start #{f.full_name}" - end - - if f.plist_manual || f.service? - command = if f.service? - f.service - .command - .map do |arg| - next arg unless arg.match?(/\s/) - - # quote multi-word arguments - "'#{arg}'" - end.join(" ") - else - f.plist_manual - end - - s << "Or, if you don't want/need a background service you can just run:" - s << " #{command}" - end - - # pbpaste is the system clipboard tool on macOS and fails with `tmux` by default - # check if this is being run under `tmux` to avoid failing - if ENV["HOMEBREW_TMUX"] && !quiet_system("/usr/bin/pbpaste") - s << "" << "WARNING: brew services will fail when run under tmux." - end - "#{s.join("\n")}\n" unless s.empty? - end -end diff --git a/Library/Homebrew/test/caveats_spec.rb b/Library/Homebrew/test/caveats_spec.rb index 571e471016..a8b35661ba 100644 --- a/Library/Homebrew/test/caveats_spec.rb +++ b/Library/Homebrew/test/caveats_spec.rb @@ -33,6 +33,17 @@ describe Caveats do describe "#caveats" do context "when f.plist is not nil", :needs_macos do + it "prints error when no launchd is present" do + f = formula do + url "foo-1.0" + def plist + "plist_test.plist" + end + end + allow_any_instance_of(Object).to receive(:which).with("launchctl").and_return(nil) + expect(described_class.new(f).caveats).to include("provides a launchd plist which can only be used on macOS!") + end + it "prints plist startup information when f.plist_startup is not nil" do f = formula do url "foo-1.0" @@ -66,40 +77,13 @@ describe Caveats do plist_path = mktmpdir/"plist" FileUtils.touch plist_path allow(f_obj).to receive(:plist_path).and_return(plist_path) + allow(Homebrew).to receive(:_system).and_return(true) + allow(Homebrew).to receive(:_system).with("/bin/launchctl list #{f.plist_name} &>/dev/null").and_return(true) allow(plist_path).to receive(:symlink?).and_return(true) expect(f_obj.caveats).to include("restart #{f.full_name}") expect(f_obj.caveats).to include("sudo") end - context "when plist_path is not a file nor symlinked and plist_startup is false" do - let(:f) { - formula do - url "foo-1.0" - def plist - "plist_test.plist" - end - end - } - let(:f_obj) { described_class.new(f) } - let(:caveats) { f_obj.caveats } - let(:plist_path) { mktmpdir/"plist" } - - before do - FileUtils.touch plist_path - allow(f_obj).to receive(:plist_path).and_return(plist_path) - allow(plist_path).to receive(:symlink?).and_return(true) - end - - it "tells command to run after upgrade" do - allow(Kernel).to receive(:system).with(any_args).and_return(true) - expect(caveats).to include("restart #{f.full_name} after an upgrade") - end - - it "tells command to run to start formula" do - expect(caveats).to include("To start #{f.full_name}:") - end - end - it "gives information about plist_manual" do f = formula do url "foo-1.0" @@ -128,20 +112,6 @@ describe Caveats do expect(caveats).to include("background service") end - it "wraps multi-word service parameters" do - f = formula do - url "foo-1.0" - service do - run [bin/"nginx", "-g", "daemon off;"] - end - end - caveats = described_class.new(f).caveats - - expect(f.service?).to eq(true) - expect(caveats).to include("#{f.bin}/nginx -g 'daemon off;'") - expect(caveats).to include("background service") - end - it "warns about brew failing under tmux" do f = formula do url "foo-1.0" @@ -150,6 +120,7 @@ describe Caveats do end end ENV["HOMEBREW_TMUX"] = "1" + allow(Homebrew).to receive(:_system).and_return(true) allow(Homebrew).to receive(:_system).with("/usr/bin/pbpaste").and_return(false) caveats = described_class.new(f).caveats @@ -158,6 +129,80 @@ describe Caveats do end end + context "when f.service is not nil" do + it "prints warning when no service deamon is found" do + f = formula do + url "foo-1.0" + service do + run [bin/"cmd"] + end + plist_options startup: true + 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(described_class.new(f).caveats).to include("service which can only be used on macOS or systemd!") + end + + it "prints service startup information when f.plist_startup is not nil" do + f = formula do + url "foo-1.0" + service do + run [bin/"cmd"] + end + plist_options startup: true + 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(described_class.new(f).caveats).to include("startup") + end + + it "prints service login information when f.plist_startup is nil" do + f = formula do + url "foo-1.0" + service do + run [bin/"cmd"] + 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(described_class.new(f).caveats).to include("login") + end + + it "gives information about restarting services after upgrade" do + f = formula do + url "foo-1.0" + service do + run [bin/"cmd"] + end + plist_options startup: true + 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(true) + expect(f_obj.caveats).to include("restart #{f.full_name}") + expect(f_obj.caveats).to include("sudo") + end + + it "gives information about service manual command" do + f = formula do + url "foo-1.0" + service do + run [bin/"cmd", "start"] + environment_variables VAR: "foo" + end + end + cmd = "#{HOMEBREW_CELLAR}/formula_name/1.0/bin/cmd" + caveats = described_class.new(f).caveats + + expect(caveats).to include("background service") + expect(caveats).to include("VAR=\"foo\" #{cmd} start") + end + end + context "when f.keg_only is not nil" do let(:f) { formula do