From 55cde526a819a1f78fd4d8afd4b8917dbaef60a6 Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Thu, 9 Aug 2018 15:00:19 +0200 Subject: [PATCH] Deprecate `brew cask cleanup`. --- .../lib/hbc/artifact/abstract_artifact.rb | 2 +- .../lib/hbc/artifact/abstract_uninstall.rb | 2 +- Library/Homebrew/cask/lib/hbc/cache.rb | 2 +- Library/Homebrew/cask/lib/hbc/cli.rb | 1 - Library/Homebrew/cask/lib/hbc/cli/cleanup.rb | 96 ------------------- Library/Homebrew/cask/lib/hbc/cli/doctor.rb | 12 --- Library/Homebrew/cmd/cleanup.rb | 13 +-- Library/Homebrew/compat/hbc.rb | 1 + Library/Homebrew/compat/hbc/cli/cleanup.rb | 43 +++++++++ Library/Homebrew/dev-cmd/tests.rb | 2 +- Library/Homebrew/manpages/brew-cask.1.md | 2 +- .../Homebrew/test/cask/cli/cleanup_spec.rb | 83 ---------------- Library/Homebrew/test/cleanup_spec.rb | 55 +++++++++++ .../spec/shared_context/homebrew_cask.rb | 2 +- completions/bash/brew | 13 --- completions/zsh/_brew_cask | 5 - docs/Manpage.md | 13 +-- manpages/brew-cask.1 | 2 +- manpages/brew.1 | 4 +- 19 files changed, 122 insertions(+), 231 deletions(-) delete mode 100644 Library/Homebrew/cask/lib/hbc/cli/cleanup.rb create mode 100644 Library/Homebrew/compat/hbc/cli/cleanup.rb delete mode 100644 Library/Homebrew/test/cask/cli/cleanup_spec.rb diff --git a/Library/Homebrew/cask/lib/hbc/artifact/abstract_artifact.rb b/Library/Homebrew/cask/lib/hbc/artifact/abstract_artifact.rb index 86643b90d4..0f3ceec626 100644 --- a/Library/Homebrew/cask/lib/hbc/artifact/abstract_artifact.rb +++ b/Library/Homebrew/cask/lib/hbc/artifact/abstract_artifact.rb @@ -72,7 +72,7 @@ module Hbc permitted_keys = [:args, :input, :executable, :must_succeed, :sudo, :print_stdout, :print_stderr] unknown_keys = arguments.keys - permitted_keys unless unknown_keys.empty? - opoo %Q{Unknown arguments to #{description} -- #{unknown_keys.inspect} (ignored). Running "brew update; brew cleanup; brew cask cleanup" will likely fix it.} + opoo %Q{Unknown arguments to #{description} -- #{unknown_keys.inspect} (ignored). Running "brew update; brew cleanup" will likely fix it.} end arguments.select! { |k| permitted_keys.include?(k) } diff --git a/Library/Homebrew/cask/lib/hbc/artifact/abstract_uninstall.rb b/Library/Homebrew/cask/lib/hbc/artifact/abstract_uninstall.rb index 735300f084..c8abee5205 100644 --- a/Library/Homebrew/cask/lib/hbc/artifact/abstract_uninstall.rb +++ b/Library/Homebrew/cask/lib/hbc/artifact/abstract_uninstall.rb @@ -66,7 +66,7 @@ module Hbc def warn_for_unknown_directives(directives) unknown_keys = directives.keys - ORDERED_DIRECTIVES return if unknown_keys.empty? - opoo %Q(Unknown arguments to #{stanza} -- #{unknown_keys.inspect}. Running "brew update; brew cleanup; brew cask cleanup" will likely fix it.) + opoo %Q(Unknown arguments to #{stanza} -- #{unknown_keys.inspect}. Running "brew update; brew cleanup" will likely fix it.) end # Preserve prior functionality of script which runs first. Should rarely be needed. diff --git a/Library/Homebrew/cask/lib/hbc/cache.rb b/Library/Homebrew/cask/lib/hbc/cache.rb index 96260b83ff..69bcefb68b 100644 --- a/Library/Homebrew/cask/lib/hbc/cache.rb +++ b/Library/Homebrew/cask/lib/hbc/cache.rb @@ -3,7 +3,7 @@ module Hbc module_function def path - @path ||= HOMEBREW_CACHE.join("Cask") + @path ||= HOMEBREW_CACHE/"Cask" end end end diff --git a/Library/Homebrew/cask/lib/hbc/cli.rb b/Library/Homebrew/cask/lib/hbc/cli.rb index fb2f16376c..5569b9f5a8 100644 --- a/Library/Homebrew/cask/lib/hbc/cli.rb +++ b/Library/Homebrew/cask/lib/hbc/cli.rb @@ -10,7 +10,6 @@ require "hbc/cli/options" require "hbc/cli/abstract_command" require "hbc/cli/audit" require "hbc/cli/cat" -require "hbc/cli/cleanup" require "hbc/cli/create" require "hbc/cli/doctor" require "hbc/cli/edit" diff --git a/Library/Homebrew/cask/lib/hbc/cli/cleanup.rb b/Library/Homebrew/cask/lib/hbc/cli/cleanup.rb deleted file mode 100644 index 71ee17bb4e..0000000000 --- a/Library/Homebrew/cask/lib/hbc/cli/cleanup.rb +++ /dev/null @@ -1,96 +0,0 @@ -require "cleanup" - -using CleanupRefinement - -module Hbc - class CLI - class Cleanup < AbstractCommand - OUTDATED_DAYS = 10 - OUTDATED_TIMESTAMP = Time.now - (60 * 60 * 24 * OUTDATED_DAYS) - - def self.help - "cleans up cached downloads and tracker symlinks" - end - - attr_reader :cache_location - - def initialize(*args, cache_location: Cache.path) - super(*args) - @cache_location = Pathname.new(cache_location) - end - - def run - remove_cache_files(*args) - end - - def cache_files - return [] unless cache_location.directory? - cache_location.children - .map(&method(:Pathname)) - .reject(&method(:outdated?)) - end - - def outdated?(file) - outdated_only? && file && file.stat.mtime > OUTDATED_TIMESTAMP - end - - def cache_incompletes - cache_files.select(&:incomplete?) - end - - def cache_completes - cache_files.reject(&:incomplete?) - end - - def disk_cleanup_size - cache_files.map(&:disk_usage).inject(:+) - end - - def remove_cache_files(*tokens) - message = "Removing cached downloads" - message.concat " for #{tokens.join(", ")}" unless tokens.empty? - message.concat " older than #{OUTDATED_DAYS} days old" if outdated_only? - ohai message - - deletable_cache_files = if tokens.empty? - cache_files - else - start_withs = tokens.map { |token| "#{token}--" } - - cache_files.select do |path| - path.basename.to_s.start_with?(*start_withs) - end - end - - delete_paths(deletable_cache_files) - end - - def delete_paths(paths) - cleanup_size = 0 - processed_files = 0 - paths.each do |item| - next unless item.exist? - processed_files += 1 - - begin - LockFile.new(item.basename).with_lock do - puts item - cleanup_size += File.size(item) - item.rmtree - end - rescue OperationInProgressError - puts "skipping: #{item} is locked" - next - end - end - - if processed_files.zero? - puts "Nothing to do" - else - disk_space = disk_usage_readable(cleanup_size) - ohai "This operation has freed approximately #{disk_space} of disk space." - end - end - end - end -end diff --git a/Library/Homebrew/cask/lib/hbc/cli/doctor.rb b/Library/Homebrew/cask/lib/hbc/cli/doctor.rb index 20824f3474..921d96d7de 100644 --- a/Library/Homebrew/cask/lib/hbc/cli/doctor.rb +++ b/Library/Homebrew/cask/lib/hbc/cli/doctor.rb @@ -24,7 +24,6 @@ module Hbc check_software_versions check_install_location check_staging_location - check_cached_downloads check_taps check_load_path check_environment_variables @@ -69,17 +68,6 @@ module Hbc puts user_tilde(path.to_s) end - def check_cached_downloads - ohai "Homebrew-Cask Cached Downloads" - - cleanup = CLI::Cleanup.new - count = cleanup.cache_files.count - size = cleanup.disk_cleanup_size - msg = user_tilde(Cache.path.to_s) - msg << " (#{number_readable(count)} files, #{disk_usage_readable(size)})" unless count.zero? - puts msg - end - def check_taps default_tap = Tap.default_cask_tap alt_taps = Tap.select { |t| t.cask_dir.exist? && t != default_tap } diff --git a/Library/Homebrew/cmd/cleanup.rb b/Library/Homebrew/cmd/cleanup.rb index 7707de6316..0cf4478937 100644 --- a/Library/Homebrew/cmd/cleanup.rb +++ b/Library/Homebrew/cmd/cleanup.rb @@ -1,15 +1,16 @@ -#: * `cleanup` [`--prune=`] [`--dry-run`] [`-s`] []: -#: For all installed or specific formulae, remove any older versions from the -#: cellar. In addition, old downloads from the Homebrew download-cache are deleted. +#: * `cleanup` [`--prune=`] [`--dry-run`] [`-s`] [ ...]: +#: Remove stale lock files and outdated downloads for formulae and casks, +#: and remove old versions of installed formulae. If arguments are specified, +#: only do this for the specified formulae and casks. #: #: If `--prune=` is specified, remove all cache files older than . #: #: If `--dry-run` or `-n` is passed, show what would be removed, but do not #: actually remove anything. #: -#: If `-s` is passed, scrub the cache, removing downloads for even the latest -#: versions of formulae. Note downloads for any installed formulae will still not be -#: deleted. If you want to delete those too: `rm -rf $(brew --cache)` +#: If `-s` is passed, scrub the cache, including downloads for even the latest +#: versions. Note downloads for any installed formula or cask will still not +#: be deleted. If you want to delete those too: `rm -rf $(brew --cache)` require "cleanup" require "cli_parser" diff --git a/Library/Homebrew/compat/hbc.rb b/Library/Homebrew/compat/hbc.rb index 0ebb67e15a..f254a27360 100644 --- a/Library/Homebrew/compat/hbc.rb +++ b/Library/Homebrew/compat/hbc.rb @@ -1,4 +1,5 @@ require "compat/hbc/cask_loader" +require "compat/hbc/cli/cleanup" require "compat/hbc/cli/search" require "compat/hbc/cache" require "compat/hbc/caskroom" diff --git a/Library/Homebrew/compat/hbc/cli/cleanup.rb b/Library/Homebrew/compat/hbc/cli/cleanup.rb new file mode 100644 index 0000000000..4a1104838c --- /dev/null +++ b/Library/Homebrew/compat/hbc/cli/cleanup.rb @@ -0,0 +1,43 @@ +require "hbc/cli/abstract_command" +require "cleanup" + +using CleanupRefinement + +module Hbc + class CLI + class Cleanup < AbstractCommand + OUTDATED_DAYS = 10 + OUTDATED_TIMESTAMP = Time.now - (60 * 60 * 24 * OUTDATED_DAYS) + + def self.help + "cleans up cached downloads and tracker symlinks" + end + + def self.visible + false + end + + attr_reader :cache_location + + def initialize(*args, cache_location: Cache.path) + super(*args) + @cache_location = Pathname.new(cache_location) + end + + def run + odeprecated "`brew cask cleanup`", "´brew cleanup`", disable_on: Time.new(2018, 9, 30) + + cleanup = Homebrew::Cleanup.new + + casks(alternative: -> { Cask.to_a }).each do |cask| + cleanup.cleanup_cask(cask) + end + + return if cleanup.disk_cleanup_size.zero? + + disk_space = disk_usage_readable(cleanup.disk_cleanup_size) + ohai "This operation has freed approximately #{disk_space} of disk space." + end + end + end +end diff --git a/Library/Homebrew/dev-cmd/tests.rb b/Library/Homebrew/dev-cmd/tests.rb index f869f43afd..f2310202bb 100644 --- a/Library/Homebrew/dev-cmd/tests.rb +++ b/Library/Homebrew/dev-cmd/tests.rb @@ -107,7 +107,7 @@ module Homebrew ] unless OS.mac? - bundle_args << "--tag" << "~needs_macos" + bundle_args << "--tag" << "~needs_macos" << "--tag" << "~cask" files = files.reject { |p| p =~ %r{^test/(os/mac|cask)(/.*|_spec\.rb)$} } end diff --git a/Library/Homebrew/manpages/brew-cask.1.md b/Library/Homebrew/manpages/brew-cask.1.md index 8897635edf..cb579cfb95 100644 --- a/Library/Homebrew/manpages/brew-cask.1.md +++ b/Library/Homebrew/manpages/brew-cask.1.md @@ -219,7 +219,7 @@ Homebrew-Cask is implemented as a external command for Homebrew. That means this project is entirely built upon the Homebrew infrastructure. For example, upgrades to the Homebrew-Cask tool are received through Homebrew: - brew update; brew cask upgrade; brew cleanup; brew cask cleanup + brew update; brew cask upgrade; brew cleanup And updates to individual Cask definitions are received whenever you issue the Homebrew command: diff --git a/Library/Homebrew/test/cask/cli/cleanup_spec.rb b/Library/Homebrew/test/cask/cli/cleanup_spec.rb deleted file mode 100644 index 9fd21dd103..0000000000 --- a/Library/Homebrew/test/cask/cli/cleanup_spec.rb +++ /dev/null @@ -1,83 +0,0 @@ -require_relative "shared_examples/invalid_option" - -describe Hbc::CLI::Cleanup, :cask do - subject { described_class.new(*cask_tokens, cache_location: cache_location) } - - let(:cache_location) { Pathname.new(Dir.mktmpdir).realpath } - let(:outdated_only) { false } - - before do - allow_any_instance_of(described_class).to receive(:outdated_only?).and_return(outdated_only) - end - - after do - cache_location.rmtree - end - - it_behaves_like "a command that handles invalid options" - - describe "cleanup" do - let(:cask_token) { "caffeine" } - let(:cask_tokens) { [cask_token] } - - it "removes cached downloads of given casks" do - cached_downloads = [ - cache_location.join("#{cask_token}--latest.zip"), - cache_location.join("transmission--2.61.dmg"), - ] - - cached_downloads.each(&FileUtils.method(:touch)) - - cleanup_size = cached_downloads[0].disk_usage - - expect { - subject.run - }.to output(<<~EOS).to_stdout - ==> Removing cached downloads for #{cask_token} - #{cached_downloads[0]} - ==> This operation has freed approximately #{disk_usage_readable(cleanup_size)} of disk space. - EOS - - expect(cached_downloads[0].exist?).to eq(false) - expect(cached_downloads[1].exist?).to eq(true) - end - - context "when no argument is given" do - let(:cask_tokens) { [] } - - it "removes all cached downloads" do - cached_download = cache_location.join("SomeDownload.dmg") - FileUtils.touch(cached_download) - cleanup_size = subject.disk_cleanup_size - - expect { - subject.run - }.to output(<<~EOS).to_stdout - ==> Removing cached downloads - #{cached_download} - ==> This operation has freed approximately #{disk_usage_readable(cleanup_size)} of disk space. - EOS - - expect(cached_download.exist?).to eq(false) - end - - context "and :outdated_only is specified" do - let(:outdated_only) { true } - - it "does not remove cache files newer than 10 days old" do - cached_download = cache_location.join("SomeNewDownload.dmg") - FileUtils.touch(cached_download) - - expect { - subject.run - }.to output(<<~EOS).to_stdout - ==> Removing cached downloads older than 10 days old - Nothing to do - EOS - - expect(cached_download.exist?).to eq(true) - end - end - end - end -end diff --git a/Library/Homebrew/test/cleanup_spec.rb b/Library/Homebrew/test/cleanup_spec.rb index 4b86fe01e9..70351eeee2 100644 --- a/Library/Homebrew/test/cleanup_spec.rb +++ b/Library/Homebrew/test/cleanup_spec.rb @@ -1,5 +1,6 @@ require "test/support/fixtures/testball" require "cleanup" +require "hbc/cache" require "fileutils" using CleanupRefinement @@ -135,6 +136,60 @@ describe Homebrew::Cleanup do expect(f4).to be_installed end + describe "#cleanup_cask", :cask do + before(:each) do + Hbc::Cache.path.mkpath + end + + context "when given a versioned cask" do + let(:cask) { Hbc::CaskLoader.load("local-transmission") } + + it "removes the download if it is not for the latest version" do + download = Hbc::Cache.path/"#{cask.token}--7.8.9" + + FileUtils.touch download + + subject.cleanup_cask(cask) + + expect(download).not_to exist + end + + it "does not remove downloads for the latest version" do + download = Hbc::Cache.path/"#{cask.token}--#{cask.version}" + + FileUtils.touch download + + subject.cleanup_cask(cask) + + expect(download).to exist + end + end + + context "when given a `:latest` cask" do + let(:cask) { Hbc::CaskLoader.load("latest-with-appcast") } + + it "does not remove the download for the latest version" do + download = Hbc::Cache.path/"#{cask.token}--#{cask.version}" + + FileUtils.touch download + + subject.cleanup_cask(cask) + + expect(download).to exist + end + + it "removes the download for the latest version after a week" do + download = Hbc::Cache.path/"#{cask.token}--#{cask.version}" + + FileUtils.touch download, mtime: Time.now - 7 * 60 * 60 * 24 + + subject.cleanup_cask(cask) + + expect(download).not_to exist + end + end + end + describe "::cleanup_logs" do let(:path) { (HOMEBREW_LOGS/"delete_me") } diff --git a/Library/Homebrew/test/support/helper/spec/shared_context/homebrew_cask.rb b/Library/Homebrew/test/support/helper/spec/shared_context/homebrew_cask.rb index 657d462a95..3155bf0de3 100644 --- a/Library/Homebrew/test/support/helper/spec/shared_context/homebrew_cask.rb +++ b/Library/Homebrew/test/support/helper/spec/shared_context/homebrew_cask.rb @@ -11,7 +11,7 @@ HOMEBREW_CASK_DIRS = { :servicedir => Pathname.new(TEST_TMPDIR).join("cask-servicedir"), }.freeze -RSpec.shared_context "Homebrew-Cask" do +RSpec.shared_context "Homebrew-Cask", :needs_macos do before do HOMEBREW_CASK_DIRS.each do |method, path| allow(Hbc::Config.global).to receive(method).and_return(path) diff --git a/completions/bash/brew b/completions/bash/brew index 681a6ff091..e6292f5de6 100644 --- a/completions/bash/brew +++ b/completions/bash/brew @@ -600,18 +600,6 @@ __brew_cask_complete_outdated () COMPREPLY=($(compgen -W "$outdated" -- "$cur")) } -_brew_cask_cleanup () -{ - local cur="${COMP_WORDS[COMP_CWORD]}" - case "$cur" in - -*) - __brew_caskcomp "--force" - return - ;; - esac - __brew_cask_complete_installed -} - _brew_cask_fetch () { local cur="${COMP_WORDS[COMP_CWORD]}" @@ -733,7 +721,6 @@ _brew_cask () --version) __brewcomp_null ;; audit) __brew_cask_complete_formulae ;; cat) __brew_cask_complete_formulae ;; - cleanup) _brew_cask_cleanup ;; create) ;; doctor) __brewcomp_null ;; edit) __brew_cask_complete_formulae ;; diff --git a/completions/zsh/_brew_cask b/completions/zsh/_brew_cask index ee24e2f545..b9f7ef81e9 100644 --- a/completions/zsh/_brew_cask +++ b/completions/zsh/_brew_cask @@ -35,7 +35,6 @@ __brew_cask_commands() { commands=( 'audit:verifies installability of Casks' 'cat:dump raw source of the given Cask to the standard output' - 'cleanup:cleans up cached downloads and tracker symlinks' 'create:creates the given Cask and opens it in an editor' 'doctor:checks for configuration issues' 'edit:edits the given Cask' @@ -90,10 +89,6 @@ _brew_cask_cat() { __brew_all_casks } -_brew_cask_cleanup() { - _arguments '--outdated:only clean up cached downloads older than 10 days old' -} - _brew_cask_create() { _arguments '*::token:' } diff --git a/docs/Manpage.md b/docs/Manpage.md index 4f0bd4247f..71271e1bc2 100644 --- a/docs/Manpage.md +++ b/docs/Manpage.md @@ -50,18 +50,19 @@ With `--verbose` or `-v`, many commands print extra debugging information. Note * `cat` `formula`: Display the source to `formula`. - * `cleanup` [`--prune=``days`] [`--dry-run`] [`-s`] [`formulae`]: - For all installed or specific formulae, remove any older versions from the - cellar. In addition, old downloads from the Homebrew download-cache are deleted. + * `cleanup` [`--prune=``days`] [`--dry-run`] [`-s`] [ ...]: + Remove stale lock files and outdated downloads for formulae and casks, + and remove old versions of installed formulae. If arguments are specified, + only do this for the specified formulae and casks. If `--prune=``days` is specified, remove all cache files older than `days`. If `--dry-run` or `-n` is passed, show what would be removed, but do not actually remove anything. - If `-s` is passed, scrub the cache, removing downloads for even the latest - versions of formulae. Note downloads for any installed formulae will still not be - deleted. If you want to delete those too: `rm -rf $(brew --cache)` + If `-s` is passed, scrub the cache, including downloads for even the latest + versions. Note downloads for any installed formula or cask will still not + be deleted. If you want to delete those too: `rm -rf $(brew --cache)` * `command` `cmd`: Display the path to the file which is used when invoking `brew` `cmd`. diff --git a/manpages/brew-cask.1 b/manpages/brew-cask.1 index 92ccc6ed30..8404f0ce57 100644 --- a/manpages/brew-cask.1 +++ b/manpages/brew-cask.1 @@ -232,7 +232,7 @@ Homebrew\-Cask is implemented as a external command for Homebrew\. That means th . .nf -brew update; brew cask upgrade; brew cleanup; brew cask cleanup +brew update; brew cask upgrade; brew cleanup . .fi . diff --git a/manpages/brew.1 b/manpages/brew.1 index b5f1392f80..9ace50a435 100644 --- a/manpages/brew.1 +++ b/manpages/brew.1 @@ -56,7 +56,7 @@ Perform a substring search of formula names for \fItext\fR\. If \fItext\fR is su \fBcat\fR \fIformula\fR: Display the source to \fIformula\fR\. . .IP "\(bu" 4 -\fBcleanup\fR [\fB\-\-prune=\fR\fIdays\fR] [\fB\-\-dry\-run\fR] [\fB\-s\fR] [\fIformulae\fR]: For all installed or specific formulae, remove any older versions from the cellar\. In addition, old downloads from the Homebrew download\-cache are deleted\. +\fBcleanup\fR [\fB\-\-prune=\fR\fIdays\fR] [\fB\-\-dry\-run\fR] [\fB\-s\fR] [ \.\.\.]: Remove stale lock files and outdated downloads for formulae and casks, and remove old versions of installed formulae\. If arguments are specified, only do this for the specified formulae and casks\. . .IP If \fB\-\-prune=\fR\fIdays\fR is specified, remove all cache files older than \fIdays\fR\. @@ -65,7 +65,7 @@ If \fB\-\-prune=\fR\fIdays\fR is specified, remove all cache files older than \f If \fB\-\-dry\-run\fR or \fB\-n\fR is passed, show what would be removed, but do not actually remove anything\. . .IP -If \fB\-s\fR is passed, scrub the cache, removing downloads for even the latest versions of formulae\. Note downloads for any installed formulae will still not be deleted\. If you want to delete those too: \fBrm \-rf $(brew \-\-cache)\fR +If \fB\-s\fR is passed, scrub the cache, including downloads for even the latest versions\. Note downloads for any installed formula or cask will still not be deleted\. If you want to delete those too: \fBrm \-rf $(brew \-\-cache)\fR . .IP "\(bu" 4 \fBcommand\fR \fIcmd\fR: Display the path to the file which is used when invoking \fBbrew\fR \fIcmd\fR\.