Merge pull request #4651 from reitermarkus/fix-cleanup

Fix `brew cleanup`.

Library/Homebrew/cleanup.rb

@@ -1,6 +1,7 @@
 require "utils/bottles"
 require "formula"
 require "hbc/cask_loader"
+require "set"
 
 module CleanupRefinement
   LATEST_CASK_DAYS = 7
@@ -65,21 +66,29 @@ module CleanupRefinement
         end
       end
 
-      version ||= basename.to_s[/\A.*--(.*?)#{Regexp.escape(extname)}/, 1]
+      version ||= basename.to_s[/\A.*(?:\-\-.*?)*\-\-(.*?)#{Regexp.escape(extname)}\Z/, 1]
+      version ||= basename.to_s[/\A.*\-\-?(.*?)#{Regexp.escape(extname)}\Z/, 1]
 
       return false unless version
 
-      version = Version.parse(version)
+      version = Version.new(version)
 
-      return false unless (name = basename.to_s[/\A(.*?)\-\-?(?:#{Regexp.escape(version)})/, 1])
+      return false unless formula_name = basename.to_s[/\A(.*?)(?:\-\-.*?)*\-\-?(?:#{Regexp.escape(version)})/, 1]
 
       formula = begin
-        Formulary.from_rack(HOMEBREW_CELLAR/name)
+        Formulary.from_rack(HOMEBREW_CELLAR/formula_name)
       rescue FormulaUnavailableError, TapFormulaAmbiguityError, TapFormulaWithOldnameAmbiguityError
         return false
       end
 
-      if version.is_a?(PkgVersion)
+      resource_name = basename.to_s[/\A.*?\-\-(.*?)\-\-?(?:#{Regexp.escape(version)})/, 1]
+
+      if resource_name == "patch"
+        patch_hashes = formula.stable&.patches&.map(&:resource)&.map(&:version)
+        return true unless patch_hashes&.include?(Checksum.new(:sha256, version.to_s))
+      elsif resource_name && resource_version = formula.stable&.resources&.dig(resource_name)&.version
+        return true if resource_version != version
+      elsif version.is_a?(PkgVersion)
         return true if formula.pkg_version > version
       elsif formula.version > version
         return true
@@ -134,17 +143,18 @@ module Homebrew
       @scrub = scrub
       @days = days
       @cache = cache
+      @cleaned_up_paths = Set.new
     end
 
     def clean!
       if args.empty?
-        Formula.installed.each do |formula|
+        cleanup_lockfiles
+        Formula.installed.sort_by(&:name).each do |formula|
           cleanup_formula(formula)
         end
         cleanup_cache
         cleanup_logs
         return if dry_run?
-        cleanup_lockfiles
         rm_ds_store
       else
         args.each do |arg|
@@ -220,6 +230,8 @@ module Homebrew
     end
 
     def cleanup_path(path)
+      return unless @cleaned_up_paths.add?(path)
+
       disk_usage = path.disk_usage
 
       if dry_run?
@@ -242,7 +254,12 @@ module Homebrew
       lockfiles.each do |file|
         next unless file.readable?
         next unless file.open(File::RDWR).flock(File::LOCK_EX | File::LOCK_NB)
+
+        begin
           cleanup_path(file) { file.unlink }
+        ensure
+          file.open(File::RDWR).flock(File::LOCK_UN) if file.exist?
+        end
       end
     end
 

Library/Homebrew/extend/pathname.rb

@@ -213,11 +213,18 @@ class Pathname
 
   # extended to support common double extensions
   def extname(path = to_s)
-    bottle_ext = path[BOTTLE_EXTNAME_RX, 1]
+    basename = File.basename(path)
+
+    bottle_ext = basename[BOTTLE_EXTNAME_RX, 1]
     return bottle_ext if bottle_ext
-    archive_ext = path[/(\.(tar|cpio|pax)\.(gz|bz2|lz|xz|Z))$/, 1]
+
+    archive_ext = basename[/(\.(tar|cpio|pax)\.(gz|bz2|lz|xz|Z))\Z/, 1]
     return archive_ext if archive_ext
-    File.extname(path)
+
+    # Don't treat version numbers as extname.
+    return "" if basename.match?(/\b\d+\.\d+[^\.]*\Z/)
+
+    File.extname(basename)
   end
 
   # for filetypes we support, basename without extension

Library/Homebrew/test/cleanup_spec.rb

@@ -238,12 +238,14 @@ describe Homebrew::Cleanup do
     end
 
     context "cleans old files in HOMEBREW_CACHE" do
-      let(:bottle) { (HOMEBREW_CACHE/"testball--0.0.1.bottle.tar.gz") }
+      let(:bottle) { (HOMEBREW_CACHE/"testball--0.0.1.tag.bottle.tar.gz") }
       let(:testball) { (HOMEBREW_CACHE/"testball--0.0.1") }
+      let(:testball_resource) { (HOMEBREW_CACHE/"testball--rsrc--0.0.1.txt") }
 
       before do
-        FileUtils.touch(bottle)
+        FileUtils.touch bottle
-        FileUtils.touch(testball)
+        FileUtils.touch testball
+        FileUtils.touch testball_resource
         (HOMEBREW_CELLAR/"testball"/"0.0.1").mkpath
         FileUtils.touch(CoreTap.instance.formula_dir/"testball.rb")
       end
@@ -253,18 +255,21 @@ describe Homebrew::Cleanup do
         subject.cleanup_cache
         expect(bottle).not_to exist
         expect(testball).not_to exist
+        expect(testball_resource).not_to exist
       end
 
       it "cleans up file if `scrub` is true and formula not installed" do
         described_class.new(scrub: true).cleanup_cache
         expect(bottle).not_to exist
         expect(testball).not_to exist
+        expect(testball_resource).not_to exist
       end
 
       it "cleans up file if stale" do
         subject.cleanup_cache
         expect(bottle).not_to exist
         expect(testball).not_to exist
+        expect(testball_resource).not_to exist
       end
     end
   end

Library/Homebrew/test/pathname_spec.rb

@@ -133,6 +133,11 @@ describe Pathname do
       expect(described_class.new("foo-0.1.tar.gz").extname).to eq(".tar.gz")
       expect(described_class.new("foo-0.1.cpio.gz").extname).to eq(".cpio.gz")
     end
+
+    it "does not treat version numbers as extensions" do
+      expect(described_class.new("foo-0.1").extname).to eq("")
+      expect(described_class.new("foo-1.0-rc1").extname).to eq("")
+    end
   end
 
   describe "#stem" do

Library/Homebrew/test/version_spec.rb

@@ -647,8 +647,6 @@ describe Version do
         .to be_detected_from("https://codeload.github.com/gsamokovarov/jump/tar.gz/v0.7.1")
       expect(described_class.create("0.9.1234"))
         .to be_detected_from("https://my.datomic.com/downloads/free/0.9.1234")
-      expect(described_class.create("0.9"))
-        .to be_detected_from("https://my.datomic.com/downloads/free/0.9.1t34")
       expect(described_class.create("1.2.3"))
         .to be_detected_from("https://my.datomic.com/downloads/free/1.2.3")
     end