Merge pull request #4072 from reitermarkus/cask-refactoring

Cask Refactoring

Library/Homebrew/cask/lib/hbc.rb

@@ -19,8 +19,6 @@ require "hbc/locations"
 require "hbc/config"
 require "hbc/macos"
 require "hbc/pkg"
-require "hbc/qualified_token"
-require "hbc/scopes"
 require "hbc/staged"
 require "hbc/system_command"
 require "hbc/topological_hash"
@@ -31,7 +29,6 @@ require "hbc/version"
 
 module Hbc
   include Locations
-  include Scopes
   include Utils
 
   def self.init

Library/Homebrew/cask/lib/hbc/cask.rb

@@ -3,11 +3,20 @@ require "hbc/metadata"
 
 module Hbc
   class Cask
+    extend Enumerable
     extend Forwardable
     include Metadata
 
     attr_reader :token, :sourcefile_path, :config
 
+    def self.each
+      return to_enum unless block_given?
+
+      Tap.flat_map(&:cask_files).each do |f|
+        yield CaskLoader::FromTapPathLoader.new(f).load
+      end
+    end
+
     def tap
       return super if block_given? # Object#tap
       @tap
@@ -43,11 +52,13 @@ module Hbc
     end
 
     def full_name
-      if @tap.nil? || @tap == Hbc.default_tap
+      return token if tap == Hbc.default_tap
-        token
+      qualified_token
-      else
+    end
-        "#{@tap}/#{token}"
+
-      end
+    def qualified_token
+      return token if tap.nil?
+      "#{tap.name}/#{token}"
     end
 
     def installed?

Library/Homebrew/cask/lib/hbc/caskroom.rb

@@ -15,5 +15,17 @@ module Hbc
       SystemCommand.run("/usr/sbin/chown", args: [Utils.current_user, Hbc.caskroom], sudo: sudo)
       SystemCommand.run("/usr/bin/chgrp", args: ["admin", Hbc.caskroom], sudo: sudo)
     end
+
+    def casks
+      Pathname.glob(Hbc.caskroom.join("*")).sort.select(&:directory?).map do |path|
+        token = path.basename.to_s
+
+        if tap_path = CaskLoader.tap_paths(token).first
+          next CaskLoader::FromTapPathLoader.new(tap_path).load
+        end
+
+        CaskLoader::FromPathLoader.new(Pathname.glob(path.join(".metadata/*/*/*/*.rb")).first).load
+      end
+    end
   end
 end

Library/Homebrew/cask/lib/hbc/cli/audit.rb

@@ -9,7 +9,7 @@ module Hbc
       end
 
       def run
-        failed_casks = casks(alternative: -> { Hbc.all })
+        failed_casks = casks(alternative: -> { Cask.to_a })
                        .reject { |cask| audit(cask) }
 
         return if failed_casks.empty?

Library/Homebrew/cask/lib/hbc/cli/info.rb

@@ -60,16 +60,12 @@ module Hbc
       end
 
       def self.repo_info(cask)
-        user, repo, token = QualifiedToken.parse(Hbc.all_tokens.detect { |t| t.split("/").last == cask.token })
+        return if cask.tap.nil?
 
-        return if user.nil? || repo.nil?
+        url = if cask.tap.custom_remote? && !cask.tap.remote.nil?
-
+          cask.tap.remote
-        remote_tap = Tap.fetch(user, repo)
-
-        url = if remote_tap.custom_remote? && !remote_tap.remote.nil?
-          remote_tap.remote
         else
-          "#{remote_tap.default_remote}/blob/master/Casks/#{token}.rb"
+          "#{cask.tap.default_remote}/blob/master/Casks/#{cask.token}.rb"
         end
 
         puts "From: #{Formatter.url(url)}"

Library/Homebrew/cask/lib/hbc/cli/internal_stanza.rb

@@ -58,7 +58,7 @@ module Hbc
           @stanza = :artifacts
         end
 
-        casks(alternative: -> { Hbc.all }).each do |cask|
+        casks(alternative: -> { Cask.to_a }).each do |cask|
           print "#{cask}\t" if table?
 
           begin

Library/Homebrew/cask/lib/hbc/cli/list.rb

@@ -37,7 +37,7 @@ module Hbc
       end
 
       def list_installed
-        installed_casks = Hbc.installed
+        installed_casks = Caskroom.casks
 
         if one?
           puts installed_casks.map(&:to_s)

Library/Homebrew/cask/lib/hbc/cli/outdated.rb

@@ -10,7 +10,7 @@ module Hbc
       end
 
       def run
-        casks(alternative: -> { Hbc.installed }).each do |cask|
+        casks(alternative: -> { Caskroom.casks }).each do |cask|
           odebug "Checking update info of Cask #{cask}"
           self.class.list_if_outdated(cask, greedy?, verbose?)
         end

Library/Homebrew/cask/lib/hbc/cli/search.rb

@@ -3,7 +3,7 @@ module Hbc
     class Search < AbstractCommand
       def run
         if args.empty?
-          puts Formatter.columns(CLI.nice_listing(Hbc.all_tokens))
+          puts Formatter.columns(CLI.nice_listing(Cask.map(&:qualified_token)))
         else
           results = self.class.search(*args)
           self.class.render_results(*results)
@@ -43,7 +43,7 @@ module Hbc
         partial_matches = []
         search_term = arguments.join(" ")
         search_regexp = extract_regexp arguments.first
-        all_tokens = CLI.nice_listing(Hbc.all_tokens)
+        all_tokens = CLI.nice_listing(Cask.map(&:qualified_token))
         if search_regexp
           search_term = arguments.first
           partial_matches = all_tokens.grep(/#{search_regexp}/i)

Library/Homebrew/cask/lib/hbc/cli/style.rb

@@ -27,7 +27,7 @@ module Hbc
 
       def cask_paths
         @cask_paths ||= if args.empty?
-          Hbc.all_tapped_cask_dirs
+          Tap.map(&:cask_dir).select(&:directory?)
         elsif args.any? { |file| File.exist?(file) }
           args
         else

Library/Homebrew/cask/lib/hbc/cli/upgrade.rb

@@ -13,7 +13,7 @@ module Hbc
 
       def run
         outdated_casks = casks(alternative: lambda {
-          Hbc.installed.select do |cask|
+          Caskroom.casks.select do |cask|
             cask.outdated?(greedy?)
           end
         }).select { |cask| cask.outdated?(true) }

Library/Homebrew/cask/lib/hbc/macos.rb

@@ -238,26 +238,35 @@ module OS
 
     # TODO: There should be a way to specify a containing
     #       directory under which nothing can be deleted.
-    UNDELETABLE_DIRS = [
+    UNDELETABLE_PATHS = [
       "~/",
       "~/Applications",
+      "~/Applications/.localized",
       "~/Desktop",
+      "~/Desktop/.localized",
       "~/Documents",
+      "~/Documents/.localized",
       "~/Downloads",
+      "~/Downloads/.localized",
       "~/Mail",
       "~/Movies",
+      "~/Movies/.localized",
       "~/Music",
+      "~/Music/.localized",
       "~/Music/iTunes",
       "~/Music/iTunes/iTunes Music",
       "~/Music/iTunes/Album Artwork",
       "~/News",
       "~/Pictures",
+      "~/Pictures/.localized",
       "~/Pictures/Desktops",
       "~/Pictures/Photo Booth",
       "~/Pictures/iChat Icons",
       "~/Pictures/iPhoto Library",
       "~/Public",
+      "~/Public/.localized",
       "~/Sites",
+      "~/Sites/.localized",
       "~/Library",
       "~/Library/.localized",
       "~/Library/Accessibility",
@@ -364,17 +373,17 @@ module OS
       "~/Library/Widgets",
       "~/Library/Workflows",
     ]
-                       .map { |x| Pathname(x.sub(%r{^~(?=(/|$))}, Dir.home)).expand_path }
+                        .map { |path| Pathname(path.sub(%r{^~(?=(/|$))}, Dir.home)).expand_path }
-                       .to_set
+                        .to_set
-                       .union(SYSTEM_DIRS)
+                        .union(SYSTEM_DIRS)
-                       .freeze
+                        .freeze
 
     def system_dir?(dir)
       SYSTEM_DIRS.include?(Pathname.new(dir).expand_path)
     end
 
-    def undeletable?(dir)
+    def undeletable?(path)
-      UNDELETABLE_DIRS.include?(Pathname.new(dir).expand_path)
+      UNDELETABLE_PATHS.include?(Pathname.new(path).expand_path)
     end
   end
 end

Library/Homebrew/cask/lib/hbc/qualified_token.rb

@@ -1,11 +0,0 @@
-module Hbc
-  module QualifiedToken
-    def self.parse(arg)
-      return nil unless arg.is_a?(String)
-      return nil unless match = arg.downcase.match(HOMEBREW_TAP_CASK_REGEX)
-      user, repo, token = match.captures
-      odebug "[user, repo, token] might be [#{user}, #{repo}, #{token}]"
-      [user, repo, token]
-    end
-  end
-end

Library/Homebrew/cask/lib/hbc/scopes.rb

@@ -1,47 +0,0 @@
-module Hbc
-  module Scopes
-    def self.included(base)
-      base.extend(ClassMethods)
-    end
-
-    module ClassMethods
-      def all
-        all_tokens.map(&CaskLoader.public_method(:load))
-      end
-
-      def all_tapped_cask_dirs
-        Tap.map(&:cask_dir).select(&:directory?)
-      end
-
-      def all_tokens
-        Tap.flat_map do |t|
-          t.cask_files.map do |p|
-            "#{t.name}/#{File.basename(p, ".rb")}"
-          end
-        end
-      end
-
-      def installed
-        # CaskLoader.load has some DWIM which is slow.  Optimize here
-        # by spoon-feeding CaskLoader.load fully-qualified paths.
-        # TODO: speed up Hbc::Source::Tapped (main perf drag is calling Hbc.all_tokens repeatedly)
-        # TODO: ability to specify expected source when calling CaskLoader.load (minor perf benefit)
-        Pathname.glob(caskroom.join("*"))
-                .sort
-                .map do |caskroom_path|
-                  token = caskroom_path.basename.to_s
-
-                  path_to_cask = all_tapped_cask_dirs.find do |tap_dir|
-                    tap_dir.join("#{token}.rb").exist?
-                  end
-
-                  if path_to_cask
-                    CaskLoader.load(path_to_cask.join("#{token}.rb"))
-                  else
-                    CaskLoader.load(token)
-                  end
-                end
-      end
-    end
-  end
-end

Library/Homebrew/cask/lib/hbc/utils.rb

@@ -4,17 +4,6 @@ require "stringio"
 
 BUG_REPORTS_URL = "https://github.com/caskroom/homebrew-cask#reporting-bugs".freeze
 
-class Buffer < StringIO
-  extend Predicable
-
-  attr_predicate :tty?
-
-  def initialize(tty = false)
-    super()
-    @tty = tty
-  end
-end
-
 # global methods
 
 def odebug(title, *sput)

Library/Homebrew/test/cask/artifact/generic_artifact_spec.rb

@@ -19,7 +19,7 @@ describe Hbc::Artifact::Artifact, :cask do
   context "without target" do
     it "fails to load" do
       expect {
-        Hbc::CaskLoader.load(cask_path("with-generic-artifact-no-target"))
+        Hbc::CaskLoader.load(cask_path("invalid/invalid-generic-artifact-no-target"))
       }.to raise_error(Hbc::CaskInvalidError, /target required for Generic Artifact/)
     end
   end

Library/Homebrew/test/cask/artifact/two_apps_incorrect_spec.rb

@@ -1,9 +0,0 @@
-describe Hbc::Artifact::App, :cask do
-  # FIXME: Doesn't actually raise because the `app` stanza is not evaluated on load.
-  # it "must raise" do
-  #   lambda {
-  #     Hbc::CaskLoader.load(cask_path("with-two-apps-incorrect"))
-  #   }.must_raise
-  #   # TODO: later give the user a nice exception for this case and check for it here
-  # end
-end

Library/Homebrew/test/cask/cask_spec.rb

@@ -70,14 +70,6 @@ describe Hbc::Cask, :cask do
     end
   end
 
-  describe "all_tokens" do
-    it "returns a token for every Cask" do
-      all_cask_tokens = Hbc.all_tokens
-      expect(all_cask_tokens.count).to be > 20
-      all_cask_tokens.each { |token| expect(token).to be_kind_of(String) }
-    end
-  end
-
   describe "metadata" do
     it "proposes a versioned metadata directory name for each instance" do
       cask_token = "local-caffeine"

Library/Homebrew/test/cask/cli/audit_spec.rb

@@ -7,9 +7,7 @@ describe Hbc::CLI::Audit, :cask do
 
   describe "selection of Casks to audit" do
     it "audits all Casks if no tokens are given" do
-      expect(cask).to be_a Hbc::Cask
+      allow(Hbc::Cask).to receive(:to_a).and_return([cask, cask])
-
-      allow(Hbc).to receive(:all).and_return([cask, cask])
 
       expect(Hbc::Auditor).to receive(:audit).twice.and_return(true)
 

Library/Homebrew/test/cask/cli/style_spec.rb

@@ -83,11 +83,13 @@ describe Hbc::CLI::Style, :cask do
     context "when no cask tokens are given" do
       let(:tokens) { [] }
 
-      before do
+      matcher :a_path_ending_with do |end_string|
-        allow(Hbc).to receive(:all_tapped_cask_dirs).and_return(%w[Casks MoreCasks])
+        match do |actual|
+          expect(actual.to_s).to end_with(end_string)
+        end
       end
 
-      it { is_expected.to eq(%w[Casks MoreCasks]) }
+      it { is_expected.to contain_exactly(a_path_ending_with("/caskroom/homebrew-spec/Casks"), a_path_ending_with("/third-party/homebrew-tap/Casks")) }
     end
 
     context "when at least one cask token is a path that exists" do

Library/Homebrew/test/cask/dsl_spec.rb

@@ -72,7 +72,7 @@ describe Hbc::DSL, :cask do
     end
 
     context "when it contains a deprecated DSL version", :needs_compat do
-      let(:token) { "with-dsl-version" }
+      let(:token) { "compat/with-dsl-version" }
 
       it "may use deprecated DSL version hash syntax" do
         allow(ENV).to receive(:[]).with("HOMEBREW_DEVELOPER").and_return(nil)

Library/Homebrew/test/cask/scopes_spec.rb

@@ -1,37 +0,0 @@
-describe Hbc::Scopes, :cask do
-  describe "installed" do
-    it "returns a list installed Casks by loading Casks for all the dirs that exist in the caskroom" do
-      allow(Hbc::CaskLoader).to receive(:load) { |token| "loaded-#{token}" }
-
-      Hbc.caskroom.join("cask-bar").mkpath
-      Hbc.caskroom.join("cask-foo").mkpath
-
-      installed_casks = Hbc.installed
-
-      expect(Hbc::CaskLoader).to have_received(:load).with("cask-bar")
-      expect(Hbc::CaskLoader).to have_received(:load).with("cask-foo")
-      expect(installed_casks).to eq(
-        %w[
-          loaded-cask-bar
-          loaded-cask-foo
-        ],
-      )
-    end
-
-    it "optimizes performance by resolving to a fully qualified path before calling Hbc::CaskLoader.load" do
-      fake_tapped_cask_dir = Pathname.new(Dir.mktmpdir).join("Casks")
-      absolute_path_to_cask = fake_tapped_cask_dir.join("some-cask.rb")
-
-      allow(Hbc::CaskLoader).to receive(:load)
-      allow(Hbc).to receive(:all_tapped_cask_dirs) { [fake_tapped_cask_dir] }
-
-      Hbc.caskroom.join("some-cask").mkdir
-      fake_tapped_cask_dir.mkdir
-      FileUtils.touch(absolute_path_to_cask)
-
-      Hbc.installed
-
-      expect(Hbc::CaskLoader).to have_received(:load).with(absolute_path_to_cask)
-    end
-  end
-end

Library/Homebrew/test/support/fixtures/cask/Casks/generic-artifact-no-target.rb

@@ -1,3 +0,0 @@
-cask 'generic-artifact-no-target' do
-  artifact 'Caffeine.app'
-end

Library/Homebrew/test/support/fixtures/cask/Casks/invalid/invalid-generic-artifact-no-target.rb

@@ -1,4 +1,4 @@
-cask 'with-generic-artifact-no-target' do
+cask 'invalid-generic-artifact-no-target' do
   version '1.2.3'
   sha256 '67cdb8a02803ef37fdbf7e0be205863172e41a561ca446cd84f0d7ab35a99d94'
 

Library/Homebrew/test/support/fixtures/cask/Casks/with-two-apps-incorrect.rb

@@ -1,9 +0,0 @@
-cask 'with-two-apps-incorrect' do
-  version '1.2.3'
-  sha256 '67cdb8a02803ef37fdbf7e0be205863172e41a561ca446cd84f0d7ab35a99d94'
-
-  url "file://#{TEST_FIXTURE_DIR}/cask/caffeine.zip"
-  homepage 'http://example.com/local-caffeine'
-
-  app 'Caffeine.app', 'Caffeine.app/Contents/MacOS/Caffeine'
-end