diff --git a/Library/Homebrew/brew.rb b/Library/Homebrew/brew.rb index 16059f829c..114f034dd3 100644 --- a/Library/Homebrew/brew.rb +++ b/Library/Homebrew/brew.rb @@ -87,6 +87,7 @@ begin if internal_cmd || Commands.external_ruby_v2_cmd_path(cmd) cmd = T.must(cmd) cmd_class = Homebrew::AbstractCommand.command(cmd) + Homebrew.running_command = cmd if cmd_class command_instance = cmd_class.new @@ -97,6 +98,7 @@ begin Homebrew.public_send Commands.method_name(cmd) end elsif (path = Commands.external_ruby_cmd_path(cmd)) + Homebrew.running_command = cmd require?(path) exit Homebrew.failed? ? 1 : 0 elsif Commands.external_cmd_path(cmd) diff --git a/Library/Homebrew/cleanup.rb b/Library/Homebrew/cleanup.rb index e28bff8d7a..da64f43ad6 100644 --- a/Library/Homebrew/cleanup.rb +++ b/Library/Homebrew/cleanup.rb @@ -411,7 +411,7 @@ module Homebrew (downloads - referenced_downloads).each do |download| if self.class.incomplete?(download) begin - LockFile.new(download.basename).with_lock do + DownloadLock.new(download).with_lock do download.unlink end rescue OperationInProgressError diff --git a/Library/Homebrew/cmd/vendor-install.sh b/Library/Homebrew/cmd/vendor-install.sh index 5a03a01629..9fa5728d9d 100644 --- a/Library/Homebrew/cmd/vendor-install.sh +++ b/Library/Homebrew/cmd/vendor-install.sh @@ -359,7 +359,7 @@ homebrew-vendor-install() { CACHED_LOCATION="${HOMEBREW_CACHE}/${VENDOR_FILENAME}" - lock "vendor-install-${VENDOR_NAME}" + lock "vendor-install ${VENDOR_NAME}" fetch install diff --git a/Library/Homebrew/download_strategy.rb b/Library/Homebrew/download_strategy.rb index 2175015d43..171c1d33cb 100644 --- a/Library/Homebrew/download_strategy.rb +++ b/Library/Homebrew/download_strategy.rb @@ -395,7 +395,7 @@ class CurlDownloadStrategy < AbstractFileDownloadStrategy def fetch(timeout: nil) end_time = Time.now + timeout if timeout - download_lock = LockFile.new(temporary_path.basename) + download_lock = DownloadLock.new(temporary_path) download_lock.lock urls = [url, *mirrors] diff --git a/Library/Homebrew/env_config.rb b/Library/Homebrew/env_config.rb index 7e22af45f9..63445e6dbe 100644 --- a/Library/Homebrew/env_config.rb +++ b/Library/Homebrew/env_config.rb @@ -295,6 +295,10 @@ module Homebrew "or `$HOME/.homebrew/livecheck_watchlist.txt` otherwise.", default: "#{ENV.fetch("HOMEBREW_USER_CONFIG_HOME")}/livecheck_watchlist.txt", }, + HOMEBREW_LOCK_CONTEXT: { + description: "If set, Homebrew will add this output as additional context for locking errors. " \ + "This is useful when running `brew` in the background.", + }, HOMEBREW_LOGS: { description: "Use this directory to store log files.", default_text: "macOS: `$HOME/Library/Logs/Homebrew`, " \ diff --git a/Library/Homebrew/exceptions.rb b/Library/Homebrew/exceptions.rb index 8671f3084e..de9b1ebbd3 100644 --- a/Library/Homebrew/exceptions.rb +++ b/Library/Homebrew/exceptions.rb @@ -359,10 +359,14 @@ end # Raised when another Homebrew operation is already in progress. class OperationInProgressError < RuntimeError - def initialize(name) + sig { params(locked_path: Pathname).void } + def initialize(locked_path) + full_command = Homebrew.running_command_with_args.presence || "brew" + lock_context = if (env_lock_context = Homebrew::EnvConfig.lock_context.presence) + "\n#{env_lock_context}" + end message = <<~EOS - Operation already in progress for #{name} - Another active Homebrew process is already using #{name}. + A `#{full_command}` process has already locked #{locked_path}.#{lock_context} Please wait for it to finish or terminate it to continue. EOS diff --git a/Library/Homebrew/global.rb b/Library/Homebrew/global.rb index 9191f7d463..9ac55d5fed 100644 --- a/Library/Homebrew/global.rb +++ b/Library/Homebrew/global.rb @@ -110,6 +110,16 @@ module Homebrew def auto_update_command? ENV.fetch("HOMEBREW_AUTO_UPDATE_COMMAND", false).present? end + + sig { params(cmd: T.nilable(String)).void } + def running_command=(cmd) + @running_command_with_args = "#{cmd} #{ARGV.join(" ")}" + end + + sig { returns String } + def running_command_with_args + "brew #{@running_command_with_args}".strip + end end end diff --git a/Library/Homebrew/lock_file.rb b/Library/Homebrew/lock_file.rb index 73adbb0c51..4df5b0466b 100644 --- a/Library/Homebrew/lock_file.rb +++ b/Library/Homebrew/lock_file.rb @@ -3,13 +3,15 @@ require "fcntl" -# A lock file. +# A lock file to prevent multiple Homebrew processes from modifying the same path. class LockFile attr_reader :path - def initialize(name) - @name = name.to_s - @path = HOMEBREW_LOCKS/"#{@name}.lock" + sig { params(type: Symbol, locked_path: Pathname).void } + def initialize(type, locked_path) + @locked_path = locked_path + lock_name = locked_path.basename.to_s + @path = HOMEBREW_LOCKS/"#{lock_name}.#{type}.lock" @lockfile = nil end @@ -18,7 +20,7 @@ class LockFile create_lockfile return if @lockfile.flock(File::LOCK_EX | File::LOCK_NB) - raise OperationInProgressError, @name + raise OperationInProgressError, @locked_path end def unlock @@ -51,14 +53,24 @@ end # A lock file for a formula. class FormulaLock < LockFile - def initialize(name) - super("#{name}.formula") + sig { params(rack_name: String).void } + def initialize(rack_name) + super(:formula, HOMEBREW_CELLAR/rack_name) end end # A lock file for a cask. class CaskLock < LockFile - def initialize(name) - super("#{name}.cask") + sig { params(cask_token: String).void } + def initialize(cask_token) + super(:cask, HOMEBREW_PREFIX/"Caskroom/#{cask_token}") + end +end + +# A lock file for a download. +class DownloadLock < LockFile + sig { params(download_path: Pathname).void } + def initialize(download_path) + super(:download, download_path) end end diff --git a/Library/Homebrew/sorbet/rbi/dsl/homebrew/env_config.rbi b/Library/Homebrew/sorbet/rbi/dsl/homebrew/env_config.rbi index 2a320a6df6..c2ca3e00e6 100644 --- a/Library/Homebrew/sorbet/rbi/dsl/homebrew/env_config.rbi +++ b/Library/Homebrew/sorbet/rbi/dsl/homebrew/env_config.rbi @@ -190,6 +190,9 @@ module Homebrew::EnvConfig sig { returns(String) } def livecheck_watchlist; end + sig { returns(String) } + def lock_context; end + sig { returns(String) } def logs; end diff --git a/Library/Homebrew/test/exceptions_spec.rb b/Library/Homebrew/test/exceptions_spec.rb index 1c2f9a0c3e..60591c3831 100644 --- a/Library/Homebrew/test/exceptions_spec.rb +++ b/Library/Homebrew/test/exceptions_spec.rb @@ -149,9 +149,9 @@ RSpec.describe "Exception" do end describe OperationInProgressError do - subject(:error) { described_class.new("foo") } + subject(:error) { described_class.new(Pathname("foo")) } - it(:to_s) { expect(error.to_s).to match(/Operation already in progress for foo/) } + it(:to_s) { expect(error.to_s).to match(/has already locked foo/) } end describe FormulaInstallationAlreadyAttemptedError do diff --git a/Library/Homebrew/test/lock_file_spec.rb b/Library/Homebrew/test/lock_file_spec.rb index a71dc2403e..cb82d3eda2 100644 --- a/Library/Homebrew/test/lock_file_spec.rb +++ b/Library/Homebrew/test/lock_file_spec.rb @@ -3,7 +3,7 @@ require "lock_file" RSpec.describe LockFile do - subject(:lock_file) { described_class.new("foo") } + subject(:lock_file) { described_class.new(:lock, Pathname("foo")) } describe "#lock" do it "does not raise an error when already locked" do @@ -16,7 +16,7 @@ RSpec.describe LockFile do lock_file.lock expect do - described_class.new("foo").lock + described_class.new(:lock, Pathname("foo")).lock end.to raise_error(OperationInProgressError) end end @@ -30,7 +30,7 @@ RSpec.describe LockFile do lock_file.lock lock_file.unlock - expect { described_class.new("foo").lock }.not_to raise_error + expect { described_class.new(:lock, Pathname("foo")).lock }.not_to raise_error end end end diff --git a/Library/Homebrew/utils/lock.sh b/Library/Homebrew/utils/lock.sh index 17e43e00e6..49ccaa1120 100644 --- a/Library/Homebrew/utils/lock.sh +++ b/Library/Homebrew/utils/lock.sh @@ -1,18 +1,20 @@ -# create a lock using `flock(2)`. A name is required as first argument. -# the lock will be automatically unlocked when the shell process quits. -# Noted due to the fixed FD, a shell process can only create one lock. +# Create a lock using `flock(2)`. A command name with arguments is required as +# first argument. The lock will be automatically unlocked when the shell process +# quits. Note due to the fixed FD, a shell process can only create one lock. # HOMEBREW_LIBRARY is by brew.sh # HOMEBREW_PREFIX is set by extend/ENV/super.rb # shellcheck disable=SC2154 lock() { - local name="$1" + local command_name_and_args="$1" + # use bash to replace spaces with dashes + local lock_filename="${command_name_and_args// /-}" local lock_dir="${HOMEBREW_PREFIX}/var/homebrew/locks" - local lock_file="${lock_dir}/${name}" + local lock_file="${lock_dir}/${lock_filename}" [[ -d "${lock_dir}" ]] || mkdir -p "${lock_dir}" if [[ ! -w "${lock_dir}" ]] then odie <&- # open the lock file to FD, so the shell process can hold the lock. exec 200>"${lock_file}" - if ! _create_lock 200 "${name}" + + if ! _create_lock 200 "${command_name_and_args}" then + local lock_context + if [[ -n "${HOMEBREW_LOCK_CONTEXT}" ]] + then + lock_context="\n${HOMEBREW_LOCK_CONTEXT}" + fi + odie <