From 921c6a33ddab75a17af16029df64c8a5c6212b75 Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Wed, 8 Aug 2018 09:43:38 +0200 Subject: [PATCH] Refactor `Cleanup`. --- Library/Homebrew/cask/lib/hbc/cli/cleanup.rb | 12 +- Library/Homebrew/cleanup.rb | 140 ++++++++++--------- Library/Homebrew/test/cleanup_spec.rb | 49 ++++--- 3 files changed, 108 insertions(+), 93 deletions(-) diff --git a/Library/Homebrew/cask/lib/hbc/cli/cleanup.rb b/Library/Homebrew/cask/lib/hbc/cli/cleanup.rb index 1553d8e85d..71ee17bb4e 100644 --- a/Library/Homebrew/cask/lib/hbc/cli/cleanup.rb +++ b/Library/Homebrew/cask/lib/hbc/cli/cleanup.rb @@ -1,3 +1,7 @@ +require "cleanup" + +using CleanupRefinement + module Hbc class CLI class Cleanup < AbstractCommand @@ -30,16 +34,12 @@ module Hbc outdated_only? && file && file.stat.mtime > OUTDATED_TIMESTAMP end - def incomplete?(file) - file.extname == ".incomplete" - end - def cache_incompletes - cache_files.select(&method(:incomplete?)) + cache_files.select(&:incomplete?) end def cache_completes - cache_files.reject(&method(:incomplete?)) + cache_files.reject(&:incomplete?) end def disk_cleanup_size diff --git a/Library/Homebrew/cleanup.rb b/Library/Homebrew/cleanup.rb index d08c586718..0b3f623432 100644 --- a/Library/Homebrew/cleanup.rb +++ b/Library/Homebrew/cleanup.rb @@ -1,6 +1,71 @@ require "utils/bottles" require "formula" +module CleanupRefinement + refine Pathname do + def incomplete? + extname.end_with?(".incomplete") + end + + def nested_cache? + directory? && ["glide_home", "java_cache", "npm_cache"].include?(basename.to_s) + end + + def prune?(days) + return false unless days + return true if days.zero? + + # TODO: Replace with ActiveSupport's `.days.ago`. + mtime < ((@time ||= Time.now) - days * 60 * 60 * 24) + end + + def stale?(scrub = false) + return false unless file? + + stale_formula?(scrub) + end + + private + + def stale_formula?(scrub) + return false unless HOMEBREW_CELLAR.directory? + + version = if to_s.match?(Pathname::BOTTLE_EXTNAME_RX) + begin + Utils::Bottles.resolve_version(self) + rescue + self.version + end + else + self.version + end + + return false unless version + return false unless (name = basename.to_s[/\A(.*?)\-\-?(?:#{Regexp.escape(version)})/, 1]) + + formula = begin + Formulary.from_rack(HOMEBREW_CELLAR/name) + rescue FormulaUnavailableError, TapFormulaAmbiguityError, TapFormulaWithOldnameAmbiguityError + return false + end + + if version.is_a?(PkgVersion) + return true if formula.pkg_version > version + elsif formula.version > version + return true + end + + return true if scrub && !formula.installed? + + return true if Utils::Bottles.file_outdated?(formula, self) + + false + end + end +end + +using CleanupRefinement + module Homebrew module Cleanup @disk_cleanup_size = 0 @@ -43,25 +108,22 @@ module Homebrew unremovable_kegs << keg end + DEFAULT_LOG_DAYS = 14 + def cleanup_logs return unless HOMEBREW_LOGS.directory? HOMEBREW_LOGS.subdirs.each do |dir| - cleanup_path(dir) { dir.rmtree } if prune?(dir, days_default: 14) + cleanup_path(dir) { dir.rmtree } if dir.prune?(ARGV.value("prune")&.to_i || DEFAULT_LOG_DAYS) end end def cleanup_cache(cache = HOMEBREW_CACHE) return unless cache.directory? cache.children.each do |path| - if path.to_s.end_with? ".incomplete" - cleanup_path(path) { path.unlink } - next - end - if %w[glide_home java_cache npm_cache].include?(path.basename.to_s) && path.directory? - cleanup_path(path) { FileUtils.rm_rf path } - next - end - if prune?(path) + next cleanup_path(path) { path.unlink } if path.incomplete? + next cleanup_path(path) { FileUtils.rm_rf path } if path.nested_cache? + + if path.prune?(ARGV.value("prune")&.to_i) if path.file? cleanup_path(path) { path.unlink } elsif path.directory? && path.to_s.include?("--") @@ -70,42 +132,13 @@ module Homebrew next end - next unless path.file? - file = path - - if file.to_s =~ Pathname::BOTTLE_EXTNAME_RX - version = begin - Utils::Bottles.resolve_version(file) - rescue - file.version - end - else - version = file.version - end - next unless version - next unless (name = file.basename.to_s[/\A(.*?)\-\-?(?:#{Regexp.escape(version)})/, 1]) - - next unless HOMEBREW_CELLAR.directory? - - begin - f = Formulary.from_rack(HOMEBREW_CELLAR/name) - rescue FormulaUnavailableError, TapFormulaAmbiguityError, TapFormulaWithOldnameAmbiguityError - next - end - - file_is_stale = if version.is_a?(PkgVersion) - f.pkg_version > version - else - f.version > version - end - - if file_is_stale || ARGV.switch?("s") && !f.installed? || Utils::Bottles.file_outdated?(f, file) - cleanup_path(file) { file.unlink } - end + next cleanup_path(path) { path.unlink } if path.stale?(ARGV.switch?("s")) end end def cleanup_path(path) + disk_usage = path.disk_usage + if ARGV.dry_run? puts "Would remove: #{path} (#{path.abv})" else @@ -113,7 +146,7 @@ module Homebrew yield end - update_disk_cleanup_size(path.disk_usage) + update_disk_cleanup_size(disk_usage) end def cleanup_lockfiles @@ -143,26 +176,5 @@ module Homebrew end workers.map(&:join) end - - def prune?(path, options = {}) - @time ||= Time.now - - path_modified_time = path.mtime - days_default = options[:days_default] - - prune = ARGV.value "prune" - - return true if prune == "all" - - prune_time = if prune - @time - 60 * 60 * 24 * prune.to_i - elsif days_default - @time - 60 * 60 * 24 * days_default.to_i - end - - return false unless prune_time - - path_modified_time < prune_time - end end end diff --git a/Library/Homebrew/test/cleanup_spec.rb b/Library/Homebrew/test/cleanup_spec.rb index 2ad1858da3..7eabdb82c7 100644 --- a/Library/Homebrew/test/cleanup_spec.rb +++ b/Library/Homebrew/test/cleanup_spec.rb @@ -2,10 +2,32 @@ require "test/support/fixtures/testball" require "cleanup" require "fileutils" +using CleanupRefinement + +describe CleanupRefinement do + describe "::prune?" do + alias_matcher :be_pruned, :be_prune + + subject(:path) { HOMEBREW_CACHE/"foo" } + + before do + path.mkpath + end + + it "returns true when path_modified_time < days_default" do + allow_any_instance_of(Pathname).to receive(:mtime).and_return(Time.now - 2 * 60 * 60 * 24) + expect(path.prune?(1)).to be true + end + + it "returns false when path_modified_time >= days_default" do + expect(path.prune?(2)).to be false + end + end +end + describe Homebrew::Cleanup do let(:ds_store) { Pathname.new("#{HOMEBREW_PREFIX}/Library/.DS_Store") } let(:lock_file) { Pathname.new("#{HOMEBREW_LOCK_DIR}/foo") } - let(:sec_in_a_day) { 60 * 60 * 24 } around do |example| begin @@ -131,13 +153,13 @@ describe Homebrew::Cleanup do end it "cleans up logs if older than 14 days" do - allow_any_instance_of(Pathname).to receive(:mtime).and_return(Time.now - sec_in_a_day * 15) + allow_any_instance_of(Pathname).to receive(:mtime).and_return(Time.now - 15 * 60 * 60 * 24) described_class.cleanup_logs expect(path).not_to exist end it "does not clean up logs less than 14 days old" do - allow_any_instance_of(Pathname).to receive(:mtime).and_return(Time.now - sec_in_a_day * 2) + allow_any_instance_of(Pathname).to receive(:mtime).and_return(Time.now - 2 * 60 * 60 * 24) described_class.cleanup_logs expect(path).to exist end @@ -212,7 +234,7 @@ describe Homebrew::Cleanup do foo = (HOMEBREW_CACHE/"--foo") foo.mkpath allow(ARGV).to receive(:value).with("prune").and_return("1") - allow_any_instance_of(Pathname).to receive(:mtime).and_return(Time.now - sec_in_a_day * 2) + allow_any_instance_of(Pathname).to receive(:mtime).and_return(Time.now - 2 * 60 * 60 * 24) described_class.cleanup_cache expect(foo).not_to exist end @@ -257,23 +279,4 @@ describe Homebrew::Cleanup do end end end - - describe "::prune?" do - alias_matcher :be_pruned, :be_prune - - before do - foo.mkpath - end - - let(:foo) { HOMEBREW_CACHE/"foo" } - - it "returns true when path_modified_time < days_default" do - allow_any_instance_of(Pathname).to receive(:mtime).and_return(Time.now - sec_in_a_day * 2) - expect(described_class).to be_pruned(foo, days_default: "1") - end - - it "returns false when path_modified_time >= days_default" do - expect(described_class).not_to be_pruned(foo, days_default: "2") - end - end end