Merge pull request #2663 from reitermarkus/lockfile

Convert `FormulaLock` to more generic `LockFile`.

9 files changed, 41 insertions, 37 deletions

diff --git a/Library/Homebrew/cask/lib/hbc/cli/cleanup.rb b/Library/Homebrew/cask/lib/hbc/cli/cleanup.rb
index e25393230..40b37dd5d 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 137af319a..935391558 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 4562fc468..59e85aaeb 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 6b80f33ce..000000000
--- 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/formula.rb b/Library/Homebrew/formula.rb
index 700645a6c..52702eaca 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 8733def27..07d4dd9cd 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
index bf747fea2..ffd29f817 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"
+    @name = name.to_s
+    @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 3cb6c5178..f975962cf 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
index 9b5ece813..82c47a70a 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