Merge pull request #14820 from apainintheneck/delay-loading-from-cask-source-api

Delay loading from cask source api

Library/Homebrew/cask/cask.rb

@@ -83,15 +83,14 @@ module Cask
       @tap
     end
 
-    def initialize(token, sourcefile_path: nil, source: nil, source_checksum: nil, tap: nil,
+    def initialize(token, sourcefile_path: nil, source: nil, tap: nil,
-                   config: nil, allow_reassignment: false, loaded_from_api: false, loader: nil, &block)
+                   config: nil, allow_reassignment: false, loader: nil, &block)
       @token = token
       @sourcefile_path = sourcefile_path
       @source = source
-      @ruby_source_checksum = source_checksum
       @tap = tap
       @allow_reassignment = allow_reassignment
-      @loaded_from_api = loaded_from_api
+      @loaded_from_api = false
       @loader = loader
       @block = block
 
@@ -166,6 +165,12 @@ module Cask
       !versions.empty?
     end
 
+    # The caskfile is needed during installation when there are
+    # `*flight` blocks or the cask has multiple languages
+    def caskfile_only?
+      languages.any? || artifacts.any?(Artifact::AbstractFlightBlock)
+    end
+
     sig { returns(T.nilable(Time)) }
     def install_time
       _, time = timestamped_versions.last
@@ -258,6 +263,27 @@ module Cask
       end
     end
 
+    def ruby_source_checksum
+      @ruby_source_checksum ||= {
+        "sha256" => Digest::SHA256.file(sourcefile_path).hexdigest,
+      }.freeze
+    end
+
+    def languages
+      @languages ||= @dsl.languages
+    end
+
+    def tap_git_head
+      @tap_git_head ||= tap&.git_head
+    end
+
+    def populate_from_api!(json_cask)
+      @loaded_from_api = true
+      @languages = json_cask[:languages]
+      @tap_git_head = json_cask[:tap_git_head]
+      @ruby_source_checksum = json_cask[:ruby_source_checksum].freeze
+    end
+
     def to_s
       @token
     end
@@ -306,7 +332,7 @@ module Cask
         "conflicts_with"       => conflicts_with,
         "container"            => container&.pairs,
         "auto_updates"         => auto_updates,
-        "tap_git_head"         => tap&.git_head,
+        "tap_git_head"         => tap_git_head,
         "languages"            => languages,
         "ruby_source_checksum" => ruby_source_checksum,
       }
@@ -360,12 +386,6 @@ module Cask
       hash
     end
 
-    def ruby_source_checksum
-      @ruby_source_checksum ||= {
-        "sha256" => Digest::SHA256.file(sourcefile_path).hexdigest,
-      }
-    end
-
     def artifacts_list
       artifacts.map do |artifact|
         case artifact

Library/Homebrew/cask/cask_loader.rb

@@ -46,11 +46,7 @@ module Cask
       private
 
       def cask(header_token, **options, &block)
-        checksum = {
+        Cask.new(header_token, source: content, tap: tap, **options, config: @config, &block)
-          "sha256" => Digest::SHA256.hexdigest(content),
-        }
-        Cask.new(header_token, source: content, source_checksum: checksum, tap: tap, **options,
-                 config: @config, &block)
       end
     end
 
@@ -150,18 +146,10 @@ module Cask
         super && !Tap.from_path(ref).nil?
       end
 
-      attr_reader :tap
-
       def initialize(path)
         @tap = Tap.from_path(path)
         super(path)
       end
-
-      private
-
-      def cask(*args, &block)
-        super(*args, tap: tap, &block)
-      end
     end
 
     # Loads a cask from a specific tap.
@@ -214,8 +202,6 @@ module Cask
     class FromAPILoader
       attr_reader :token, :path
 
-      FLIGHT_STANZAS = [:preflight, :postflight, :uninstall_preflight, :uninstall_postflight].freeze
-
       def self.can_load?(ref)
         return false if Homebrew::EnvConfig.no_install_from_api?
         return false unless ref.is_a?(String)
@@ -235,17 +221,9 @@ module Cask
         json_cask = @from_json || Homebrew::API::Cask.all_casks[token]
         cask_source = JSON.pretty_generate(json_cask)
 
-        json_cask = Homebrew::API.merge_variations(json_cask).deep_symbolize_keys
+        json_cask = Homebrew::API.merge_variations(json_cask).deep_symbolize_keys.freeze
-        tap = Tap.fetch(json_cask[:tap]) if json_cask[:tap].to_s.include?("/")
 
-        # Use the cask-source API if there are any `*flight` blocks or the cask has multiple languages
+        tap = Tap.fetch(json_cask[:tap]) if json_cask[:tap].to_s.include?("/")
-        if json_cask[:artifacts].any? { |artifact| FLIGHT_STANZAS.include?(artifact.keys.first) } ||
-           json_cask[:languages].any?
-          cask_source = Homebrew::API::Cask.fetch_source(token,
-                                                         git_head: json_cask[:tap_git_head],
-                                                         sha256:   json_cask.dig(:ruby_source_checksum, :sha256))
-          return FromContentLoader.new(cask_source, tap: tap).load(config: config)
-        end
 
         user_agent = json_cask.dig(:url_specs, :user_agent)
         json_cask[:url_specs][:user_agent] = user_agent[1..].to_sym if user_agent && user_agent[0] == ":"
@@ -253,13 +231,7 @@ module Cask
           json_cask[:url_specs][:using] = using.to_sym
         end
 
-        Cask.new(token,
+        api_cask = Cask.new(token, tap: tap, source: cask_source, config: config, loader: self) do
-                 tap:             tap,
-                 source:          cask_source,
-                 source_checksum: json_cask[:ruby_source_checksum],
-                 config:          config,
-                 loaded_from_api: true,
-                 loader:          self) do
           version json_cask[:version]
 
           if json_cask[:sha256] == "no_check"
@@ -319,15 +291,21 @@ module Cask
             # convert generic string replacements into actual ones
             artifact = cask.loader.from_h_hash_gsubs(artifact, appdir)
             key = artifact.keys.first
+            if artifact[key].nil?
+              # for artifacts with blocks that can't be loaded from the API
+              send(key) {} # empty on purpose
+            else
               send(key, *artifact[key])
             end
+          end
 
           if json_cask[:caveats].present?
             # convert generic string replacements into actual ones
-            json_cask[:caveats] = cask.loader.from_h_string_gsubs(json_cask[:caveats], appdir)
+            caveats cask.loader.from_h_string_gsubs(json_cask[:caveats], appdir)
-            caveats json_cask[:caveats]
           end
         end
+        api_cask.populate_from_api!(json_cask)
+        api_cask
       end
 
       def from_h_string_gsubs(string, appdir)

Library/Homebrew/cask/dsl.rb

@@ -77,7 +77,6 @@ module Cask
       :depends_on,
       :homepage,
       :language,
-      :languages,
       :name,
       :sha256,
       :staged_path,

Library/Homebrew/cask/installer.rb

@@ -64,6 +64,8 @@ module Cask
     def fetch(quiet: nil, timeout: nil)
       odebug "Cask::Installer#fetch"
 
+      load_cask_from_source_api! if @cask.loaded_from_api && @cask.caskfile_only?
+
       verify_has_sha if require_sha? && !force?
 
       download(quiet: quiet, timeout: timeout)
@@ -149,16 +151,8 @@ module Cask
     def uninstall_existing_cask
       return unless @cask.installed?
 
-      # use the same cask file that was used for installation, if possible
-      installed_caskfile = @cask.installed_caskfile
-      installed_cask = begin
-        installed_caskfile.exist? ? CaskLoader.load(installed_caskfile) : @cask
-      rescue CaskInvalidError # could be thrown by call to CaskLoader#load with outdated caskfile
-        @cask # default
-      end
-
       # Always force uninstallation, ignore method parameter
-      cask_installer = Installer.new(installed_cask, verbose: verbose?, force: true, upgrade: upgrade?)
+      cask_installer = Installer.new(@cask, verbose: verbose?, force: true, upgrade: upgrade?)
       zap? ? cask_installer.zap : cask_installer.uninstall
     end
 
@@ -403,6 +397,7 @@ module Cask
     end
 
     def uninstall
+      load_installed_caskfile!
       oh1 "Uninstalling Cask #{Formatter.identifier(@cask)}"
       uninstall_artifacts(clear: true)
       if !reinstall? && !upgrade?
@@ -480,6 +475,7 @@ module Cask
     end
 
     def zap
+      load_installed_caskfile!
       ohai "Implied `brew uninstall --cask #{@cask}`"
       uninstall_artifacts
       if (zap_stanzas = @cask.artifacts.select { |a| a.is_a?(Artifact::Zap) }).empty?
@@ -547,5 +543,30 @@ module Cask
       odebug "Purging all staged versions of Cask #{@cask}"
       gain_permissions_remove(@cask.caskroom_path)
     end
+
+    private
+
+    # load the same cask file that was used for installation, if possible
+    def load_installed_caskfile!
+      installed_caskfile = @cask.installed_caskfile
+
+      if installed_caskfile.exist?
+        begin
+          @cask = CaskLoader.load(installed_caskfile)
+          return
+        rescue CaskInvalidError
+          # could be caused by trying to load outdated caskfile
+        end
+      end
+
+      load_cask_from_source_api! if @cask.loaded_from_api && @cask.caskfile_only?
+      # otherwise we default to the current cask
+    end
+
+    def load_cask_from_source_api!
+      options = { git_head: @cask.tap_git_head, sha256: @cask.ruby_source_checksum["sha256"] }
+      cask_source = Homebrew::API::Cask.fetch_source(@cask.token, **options)
+      @cask = CaskLoader::FromContentLoader.new(cask_source, tap: @cask.tap).load(config: @cask.config)
+    end
   end
 end

Library/Homebrew/test/cask/cask_loader/from_api_loader_spec.rb

@@ -59,80 +59,64 @@ describe Cask::CaskLoader::FromAPILoader, :cask do
   end
 
   describe "#load" do
-    shared_examples "loads from fetched source" do |cask_token|
+    shared_examples "loads from API" do |cask_token, caskfile_only|
-      include_context "with API setup", cask_token
-      let(:content_loader) { instance_double(Cask::CaskLoader::FromContentLoader) }
-
-      it "fetches cask source from API" do
-        expect(Homebrew::API::Cask).to receive(:fetch_source).once
-        expect(Cask::CaskLoader::FromContentLoader)
-          .to receive(:new).once
-          .and_return(content_loader)
-        expect(content_loader).to receive(:load).once
-
-        api_loader.load(config: nil)
-      end
-    end
-
-    context "with a preflight stanza" do
-      include_examples "loads from fetched source", "with-preflight"
-    end
-
-    context "with an uninstall-preflight stanza" do
-      include_examples "loads from fetched source", "with-uninstall-preflight"
-    end
-
-    context "with a postflight stanza" do
-      include_examples "loads from fetched source", "with-postflight"
-    end
-
-    context "with an uninstall-postflight stanza" do
-      include_examples "loads from fetched source", "with-uninstall-postflight"
-    end
-
-    context "with a language stanza" do
-      include_examples "loads from fetched source", "with-languages"
-    end
-
-    shared_examples "loads from API" do |cask_token|
       include_context "with API setup", cask_token
       let(:cask_from_api) { api_loader.load(config: nil) }
 
       it "loads from JSON API" do
-        expect(Homebrew::API::Cask).not_to receive(:fetch_source)
-        expect(Cask::CaskLoader::FromContentLoader).not_to receive(:new)
-
         expect(cask_from_api).to be_a(Cask::Cask)
         expect(cask_from_api.token).to eq(cask_token)
+        expect(cask_from_api.loaded_from_api).to be(true)
+        expect(cask_from_api.caskfile_only?).to be(caskfile_only)
       end
     end
 
     context "with a binary stanza" do
-      include_examples "loads from API", "with-binary"
+      include_examples "loads from API", "with-binary", false
     end
 
     context "with cask dependencies" do
-      include_examples "loads from API", "with-depends-on-cask-multiple"
+      include_examples "loads from API", "with-depends-on-cask-multiple", false
     end
 
     context "with formula dependencies" do
-      include_examples "loads from API", "with-depends-on-formula-multiple"
+      include_examples "loads from API", "with-depends-on-formula-multiple", false
     end
 
     context "with macos dependencies" do
-      include_examples "loads from API", "with-depends-on-macos-array"
+      include_examples "loads from API", "with-depends-on-macos-array", false
     end
 
     context "with an installer stanza" do
-      include_examples "loads from API", "with-installer-script"
+      include_examples "loads from API", "with-installer-script", false
     end
 
     context "with uninstall stanzas" do
-      include_examples "loads from API", "with-uninstall-multi"
+      include_examples "loads from API", "with-uninstall-multi", false
     end
 
     context "with a zap stanza" do
-      include_examples "loads from API", "with-zap"
+      include_examples "loads from API", "with-zap", false
+    end
+
+    context "with a preflight stanza" do
+      include_examples "loads from API", "with-preflight", true
+    end
+
+    context "with an uninstall-preflight stanza" do
+      include_examples "loads from API", "with-uninstall-preflight", true
+    end
+
+    context "with a postflight stanza" do
+      include_examples "loads from API", "with-postflight", true
+    end
+
+    context "with an uninstall-postflight stanza" do
+      include_examples "loads from API", "with-uninstall-postflight", true
+    end
+
+    context "with a language stanza" do
+      include_examples "loads from API", "with-languages", true
     end
   end
 end

Library/Homebrew/test/cask/installer_spec.rb

@@ -235,6 +235,22 @@ describe Cask::Installer, :cask do
 
       expect(latest_cask.download_sha_path).to be_a_file
     end
+
+    context "when loaded from the api and caskfile is required" do
+      let(:path) { cask_path("local-caffeine") }
+      let(:content) { File.read(path) }
+
+      it "installs cask" do
+        expect(Homebrew::API::Cask).to receive(:fetch_source).once.and_return(content)
+
+        caffeine = Cask::CaskLoader.load(path)
+        expect(caffeine).to receive(:loaded_from_api).once.and_return(true)
+        expect(caffeine).to receive(:caskfile_only?).once.and_return(true)
+
+        described_class.new(caffeine).install
+        expect(Cask::CaskLoader.load(path)).to be_installed
+      end
+    end
   end
 
   describe "uninstall" do
@@ -269,6 +285,31 @@ describe Cask::Installer, :cask do
       expect(Cask::Caskroom.path.join("local-caffeine", mutated_version)).not_to be_a_directory
       expect(Cask::Caskroom.path.join("local-caffeine")).not_to be_a_directory
     end
+
+    context "when loaded from the api, caskfile is required and installed caskfile is invalid" do
+      let(:path) { cask_path("local-caffeine") }
+      let(:content) { File.read(path) }
+      let(:invalid_path) { instance_double(Pathname) }
+
+      before do
+        allow(invalid_path).to receive(:exist?).and_return(false)
+      end
+
+      it "uninstalls cask" do
+        expect(Homebrew::API::Cask).to receive(:fetch_source).twice.and_return(content)
+
+        caffeine = Cask::CaskLoader.load(path)
+        expect(caffeine).to receive(:loaded_from_api).twice.and_return(true)
+        expect(caffeine).to receive(:caskfile_only?).twice.and_return(true)
+        expect(caffeine).to receive(:installed_caskfile).once.and_return(invalid_path)
+
+        described_class.new(caffeine).install
+        expect(Cask::CaskLoader.load(path)).to be_installed
+
+        described_class.new(caffeine).uninstall
+        expect(Cask::CaskLoader.load(path)).not_to be_installed
+      end
+    end
   end
 
   describe "uninstall_existing_cask" do