From 060af0a26ab7219e46b500fd1c7f420b6cc74cbb Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Mon, 22 May 2017 03:23:50 +0200 Subject: [PATCH 1/2] Rename `FormulaLock` to `LockFile`. --- Library/Homebrew/formula.rb | 2 +- Library/Homebrew/keg.rb | 2 +- .../Homebrew/{formula_lock.rb => lock_file.rb} | 16 ++++++++++++++-- Library/Homebrew/migrator.rb | 2 +- .../{formula_lock_spec.rb => lock_file_spec.rb} | 6 +++--- 5 files changed, 20 insertions(+), 8 deletions(-) rename Library/Homebrew/{formula_lock.rb => lock_file.rb} (72%) rename Library/Homebrew/test/{formula_lock_spec.rb => lock_file_spec.rb} (88%) diff --git a/Library/Homebrew/formula.rb b/Library/Homebrew/formula.rb index 0b3e87bb18..d42332c645 100644 --- a/Library/Homebrew/formula.rb +++ b/Library/Homebrew/formula.rb @@ -1,5 +1,5 @@ require "formula_support" -require "formula_lock" +require "lock_file" require "formula_pin" require "hardware" require "utils/bottles" diff --git a/Library/Homebrew/keg.rb b/Library/Homebrew/keg.rb index 8733def270..07d4dd9cd8 100644 --- a/Library/Homebrew/keg.rb +++ b/Library/Homebrew/keg.rb @@ -1,6 +1,6 @@ require "extend/pathname" require "keg_relocate" -require "formula_lock" +require "lock_file" require "ostruct" class Keg diff --git a/Library/Homebrew/formula_lock.rb b/Library/Homebrew/lock_file.rb similarity index 72% rename from Library/Homebrew/formula_lock.rb rename to Library/Homebrew/lock_file.rb index bf747fea22..83743b7441 100644 --- a/Library/Homebrew/formula_lock.rb +++ b/Library/Homebrew/lock_file.rb @@ -1,9 +1,9 @@ require "fcntl" -class FormulaLock +class LockFile def initialize(name) @name = name - @path = HOMEBREW_LOCK_DIR/"#{@name}.brewing" + @path = HOMEBREW_LOCK_DIR/"#{@name}.lock" @lockfile = nil end @@ -35,3 +35,15 @@ class FormulaLock @lockfile.fcntl(Fcntl::F_SETFD, Fcntl::FD_CLOEXEC) end end + +class FormulaLock < LockFile + def initialize(name) + super("#{name}.formula") + end +end + +class CaskLock < LockFile + def initialize(name) + super("#{name}.cask") + end +end diff --git a/Library/Homebrew/migrator.rb b/Library/Homebrew/migrator.rb index 3cb6c51787..f975962cf1 100644 --- a/Library/Homebrew/migrator.rb +++ b/Library/Homebrew/migrator.rb @@ -1,5 +1,5 @@ require "formula" -require "formula_lock" +require "lock_file" require "keg" require "tab" diff --git a/Library/Homebrew/test/formula_lock_spec.rb b/Library/Homebrew/test/lock_file_spec.rb similarity index 88% rename from Library/Homebrew/test/formula_lock_spec.rb rename to Library/Homebrew/test/lock_file_spec.rb index 9b5ece8131..82c47a70a7 100644 --- a/Library/Homebrew/test/formula_lock_spec.rb +++ b/Library/Homebrew/test/lock_file_spec.rb @@ -1,6 +1,6 @@ -require "formula_lock" +require "lock_file" -describe FormulaLock do +describe LockFile do subject { described_class.new("foo") } describe "#lock" do @@ -24,7 +24,7 @@ describe FormulaLock do expect { subject.unlock }.not_to raise_error end - it "unlocks a locked Formula" do + it "unlocks when locked" do subject.lock subject.unlock From fd97e88b990b4804e9cfec90e3e5fe60fa54437a Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Mon, 22 May 2017 04:15:11 +0200 Subject: [PATCH 2/2] Use `LockFile` instead of `Hbc::Utils::file_locked?`. --- Library/Homebrew/cask/lib/hbc/cli/cleanup.rb | 14 +++++++++----- .../Homebrew/cask/lib/hbc/download_strategy.rb | 18 +++++++++++------- Library/Homebrew/cask/lib/hbc/utils.rb | 2 -- Library/Homebrew/cask/lib/hbc/utils/file.rb | 14 -------------- Library/Homebrew/lock_file.rb | 2 +- 5 files changed, 21 insertions(+), 29 deletions(-) delete mode 100644 Library/Homebrew/cask/lib/hbc/utils/file.rb diff --git a/Library/Homebrew/cask/lib/hbc/cli/cleanup.rb b/Library/Homebrew/cask/lib/hbc/cli/cleanup.rb index e253932307..40b37dd5dd 100644 --- a/Library/Homebrew/cask/lib/hbc/cli/cleanup.rb +++ b/Library/Homebrew/cask/lib/hbc/cli/cleanup.rb @@ -75,14 +75,18 @@ module Hbc paths.each do |item| next unless item.exist? processed_files += 1 - if Utils.file_locked?(item) + + begin + LockFile.new(item.basename).with_lock do + puts item + item_size = File.size?(item) + cleanup_size += item_size unless item_size.nil? + item.unlink + end + rescue OperationInProgressError puts "skipping: #{item} is locked" next end - puts item - item_size = File.size?(item) - cleanup_size += item_size unless item_size.nil? - item.unlink end if processed_files.zero? diff --git a/Library/Homebrew/cask/lib/hbc/download_strategy.rb b/Library/Homebrew/cask/lib/hbc/download_strategy.rb index 137af319a4..935391558a 100644 --- a/Library/Homebrew/cask/lib/hbc/download_strategy.rb +++ b/Library/Homebrew/cask/lib/hbc/download_strategy.rb @@ -82,10 +82,16 @@ module Hbc end def clear_cache - [cached_location, temporary_path].each do |f| - next unless f.exist? - raise CurlDownloadStrategyError, "#{f} is in use by another process" if Utils.file_locked?(f) - f.unlink + [cached_location, temporary_path].each do |path| + next unless path.exist? + + begin + LockFile.new(path.basename).with_lock do + path.unlink + end + rescue OperationInProgressError + raise CurlDownloadStrategyError, "#{path} is in use by another process" + end end end @@ -105,10 +111,8 @@ module Hbc else had_incomplete_download = temporary_path.exist? begin - File.open(temporary_path, "a+") do |f| - f.flock(File::LOCK_EX) + LockFile.new(temporary_path.basename).with_lock do _fetch - f.flock(File::LOCK_UN) end rescue ErrorDuringExecution # 33 == range not supported diff --git a/Library/Homebrew/cask/lib/hbc/utils.rb b/Library/Homebrew/cask/lib/hbc/utils.rb index 4562fc4689..59e85aaeb3 100644 --- a/Library/Homebrew/cask/lib/hbc/utils.rb +++ b/Library/Homebrew/cask/lib/hbc/utils.rb @@ -2,8 +2,6 @@ require "yaml" require "open3" require "stringio" -require "hbc/utils/file" - BUG_REPORTS_URL = "https://github.com/caskroom/homebrew-cask#reporting-bugs".freeze # monkeypatch Object - not a great idea diff --git a/Library/Homebrew/cask/lib/hbc/utils/file.rb b/Library/Homebrew/cask/lib/hbc/utils/file.rb deleted file mode 100644 index 6b80f33ce6..0000000000 --- a/Library/Homebrew/cask/lib/hbc/utils/file.rb +++ /dev/null @@ -1,14 +0,0 @@ -module Hbc - module Utils - module_function - - def file_locked?(file) - unlocked = File.open(file).flock(File::LOCK_EX | File::LOCK_NB) - # revert lock if file was unlocked before check - File.open(file).flock(File::LOCK_UN) if unlocked - !unlocked - rescue - true - end - end -end diff --git a/Library/Homebrew/lock_file.rb b/Library/Homebrew/lock_file.rb index 83743b7441..ffd29f8172 100644 --- a/Library/Homebrew/lock_file.rb +++ b/Library/Homebrew/lock_file.rb @@ -2,7 +2,7 @@ require "fcntl" class LockFile def initialize(name) - @name = name + @name = name.to_s @path = HOMEBREW_LOCK_DIR/"#{@name}.lock" @lockfile = nil end