From 3eea1434cc0096907580494692c9a71befc5b9e9 Mon Sep 17 00:00:00 2001
From: Michka Popoff <michkapopoff@gmail.com>
Date: Sun, 29 Nov 2020 17:51:47 +0100
Subject: [PATCH] bottle: tag specific cellars

---
 Library/Homebrew/dev-cmd/bottle.rb            |  2 +-
 Library/Homebrew/formula.rb                   |  2 +-
 Library/Homebrew/software_spec.rb             | 46 +++++++++++++------
 .../test/formula_installer_bottle_spec.rb     | 44 ++++++++++++------
 Library/Homebrew/test/software_spec_spec.rb   | 44 +++++++++++++-----
 .../fixtures/testball_bottle_cellar.rb        | 24 ++++++++++
 .../test/utils/bottles/collector_spec.rb      |  6 +--
 Library/Homebrew/utils/bottles.rb             |  5 +-
 8 files changed, 128 insertions(+), 45 deletions(-)
 create mode 100644 Library/Homebrew/test/support/fixtures/testball_bottle_cellar.rb

diff --git a/Library/Homebrew/dev-cmd/bottle.rb b/Library/Homebrew/dev-cmd/bottle.rb
index 1d239eee3a..f9af1d2ff2 100644
--- a/Library/Homebrew/dev-cmd/bottle.rb
+++ b/Library/Homebrew/dev-cmd/bottle.rb
@@ -553,7 +553,7 @@ module Homebrew
     return [mismatches, checksums] if old_keys.exclude? :sha256
 
     old_bottle_spec.collector.each_key do |tag|
-      old_value = old_bottle_spec.collector[tag].hexdigest
+      old_value = old_bottle_spec.collector[tag][:checksum].hexdigest
       new_value = new_bottle_hash.dig("tags", tag.to_s)
       if new_value.present?
         mismatches << "sha256 => #{tag}"
diff --git a/Library/Homebrew/formula.rb b/Library/Homebrew/formula.rb
index 37c08fb136..c719f5fe69 100644
--- a/Library/Homebrew/formula.rb
+++ b/Library/Homebrew/formula.rb
@@ -1831,7 +1831,7 @@ class Formula
         bottle_info["files"] = {}
         bottle_spec.collector.each_key do |os|
           bottle_url = "#{bottle_spec.root_url}/#{Bottle::Filename.create(self, os, bottle_spec.rebuild).bintray}"
-          checksum = bottle_spec.collector[os]
+          checksum = bottle_spec.collector[os][:checksum]
           bottle_info["files"][os] = {
             "url"    => bottle_url,
             "sha256" => checksum.hexdigest,
diff --git a/Library/Homebrew/software_spec.rb b/Library/Homebrew/software_spec.rb
index b8af355ccc..bbaa116def 100644
--- a/Library/Homebrew/software_spec.rb
+++ b/Library/Homebrew/software_spec.rb
@@ -302,7 +302,7 @@ class Bottle
     @resource.specs[:bottle] = true
     @spec = spec
 
-    checksum, tag = spec.checksum_for(Utils::Bottles.tag)
+    checksum, tag, cellar = spec.checksum_for(Utils::Bottles.tag)
 
     filename = Filename.create(formula, tag, spec.rebuild)
     @resource.url("#{spec.root_url}/#{filename.bintray}",
@@ -310,7 +310,7 @@ class Bottle
     @resource.version = formula.pkg_version
     @resource.checksum = checksum
     @prefix = spec.prefix
-    @cellar = spec.cellar
+    @cellar = cellar
     @rebuild = spec.rebuild
   end
 
@@ -338,15 +338,15 @@ end
 class BottleSpecification
   extend T::Sig
 
-  attr_rw :prefix, :cellar, :rebuild
+  attr_rw :prefix, :rebuild
   attr_accessor :tap
-  attr_reader :checksum, :collector, :root_url_specs, :repository
+  attr_reader :all_tags_cellar, :checksum, :collector, :root_url_specs, :repository
 
   sig { void }
   def initialize
     @rebuild = 0
     @prefix = Homebrew::DEFAULT_PREFIX
-    @cellar = Homebrew::DEFAULT_CELLAR
+    @all_tags_cellar = Homebrew::DEFAULT_CELLAR
     @repository = Homebrew::DEFAULT_REPOSITORY
     @collector = Utils::Bottles::Collector.new
     @root_url_specs = {}
@@ -370,6 +370,12 @@ class BottleSpecification
     end
   end
 
+  def cellar(val = nil)
+    return collector.dig(Utils::Bottles.tag, :cellar) || @all_tags_cellar if val.nil?
+
+    @all_tags_cellar = val
+  end
+
   def compatible_locations?
     # this looks like it should check prefix and repository too but to be
     # `cellar :any` actually requires no references to the cellar, prefix or
@@ -398,17 +404,30 @@ class BottleSpecification
     cellar == :any_skip_relocation
   end
 
+  sig { params(tag: Symbol).returns(T::Boolean) }
   def tag?(tag)
     checksum_for(tag) ? true : false
   end
 
-  # Checksum methods in the DSL's bottle block optionally take
+  # Checksum methods in the DSL's bottle block take
   # a Hash, which indicates the platform the checksum applies on.
-  def sha256(val)
-    digest, tag = val.shift
-    collector[tag] = Checksum.new(digest)
+  # Example bottle block syntax:
+  # bottle do
+  #  sha256 "69489ae397e4645..." => :big_sur, :cellar => :any_skip_relocation
+  #  sha256 "449de5ea35d0e94..." => :catalina, :cellar => :any
+  # end
+  # Example args:
+  # {"69489ae397e4645..."=> :big_sur, :cellar=>:any_skip_relocation}
+  def sha256(hash)
+    sha256_regex = /^[a-f0-9]{64}$/i
+    digest, tag = hash.find do |key, value|
+      key.is_a?(String) && value.is_a?(Symbol) && key.match?(sha256_regex)
+    end
+    cellar = hash[:cellar] || all_tags_cellar
+    collector[tag] = { checksum: Checksum.new(digest), cellar: cellar }
   end
 
+  sig { params(tag: Symbol).returns(T.nilable([Checksum, Symbol, T.any(Symbol, String)])) }
   def checksum_for(tag)
     collector.fetch_checksum_for(tag)
   end
@@ -420,10 +439,11 @@ class BottleSpecification
       # Sort non-MacOS tags below MacOS tags.
       "0.#{tag}"
     end
-    sha256s = []
-    tags.reverse_each do |tag|
-      checksum = collector[tag]
-      sha256s << { checksum => tag }
+    sha256s = tags.reverse.map do |tag|
+      {
+        collector[tag][:checksum] => tag,
+        cellar: collector[tag][:cellar],
+      }
     end
     { sha256: sha256s }
   end
diff --git a/Library/Homebrew/test/formula_installer_bottle_spec.rb b/Library/Homebrew/test/formula_installer_bottle_spec.rb
index d307e7a607..c399404410 100644
--- a/Library/Homebrew/test/formula_installer_bottle_spec.rb
+++ b/Library/Homebrew/test/formula_installer_bottle_spec.rb
@@ -8,6 +8,7 @@ require "tab"
 require "cmd/install"
 require "test/support/fixtures/testball"
 require "test/support/fixtures/testball_bottle"
+require "test/support/fixtures/testball_bottle_cellar"
 
 describe FormulaInstaller do
   alias_matcher :pour_bottle, :be_pour_bottle
@@ -49,26 +50,43 @@ describe FormulaInstaller do
     expect(formula).not_to be_latest_version_installed
   end
 
+  def test_basic_formula_setup(f)
+    # Test that things made it into the Keg
+    expect(f.bin).to be_a_directory
+
+    expect(f.libexec).to be_a_directory
+
+    expect(f.prefix/"main.c").not_to exist
+
+    # Test that things made it into the Cellar
+    keg = Keg.new f.prefix
+    keg.link
+
+    bin = HOMEBREW_PREFIX/"bin"
+    expect(bin).to be_a_directory
+
+    expect(f.libexec).to be_a_directory
+  end
+
   specify "basic bottle install" do
     allow(DevelopmentTools).to receive(:installed?).and_return(false)
     Homebrew.install_args.parse(["testball_bottle"])
     temporarily_install_bottle(TestballBottle.new) do |f|
-      # Copied directly from formula_installer_spec.rb
-      # as we expect the same behavior.
-
-      # Test that things made it into the Keg
-      expect(f.bin).to be_a_directory
-
-      expect(f.libexec).to be_a_directory
+      test_basic_formula_setup(f)
+    end
+  end
 
-      expect(f.prefix/"main.c").not_to exist
+  specify "basic bottle install with cellar information on sha256 line" do
+    allow(DevelopmentTools).to receive(:installed?).and_return(false)
+    Homebrew.install_args.parse(["testball_bottle_cellar"])
+    temporarily_install_bottle(TestballBottleCellar.new) do |f|
+      test_basic_formula_setup(f)
 
-      # Test that things made it into the Cellar
-      keg = Keg.new f.prefix
-      keg.link
+      # skip_relocation is always false on Linux but can be true on macOS.
+      # see: extend/os/linux/software_spec.rb
+      skip_relocation = !OS.linux?
 
-      bin = HOMEBREW_PREFIX/"bin"
-      expect(bin).to be_a_directory
+      expect(f.bottle_specification.skip_relocation?).to eq(skip_relocation)
     end
   end
 
diff --git a/Library/Homebrew/test/software_spec_spec.rb b/Library/Homebrew/test/software_spec_spec.rb
index 0bf63e70cc..088fa2ff6a 100644
--- a/Library/Homebrew/test/software_spec_spec.rb
+++ b/Library/Homebrew/test/software_spec_spec.rb
@@ -183,18 +183,38 @@ describe HeadSoftwareSpec do
 end
 
 describe BottleSpecification do
-  specify "#sha256" do
-    checksums = {
-      snow_leopard_32: "deadbeef" * 8,
-      snow_leopard:    "faceb00c" * 8,
-      lion:            "baadf00d" * 8,
-      mountain_lion:   "8badf00d" * 8,
-    }
-
-    checksums.each_pair do |cat, digest|
-      subject.sha256(digest => cat)
-      checksum, = subject.checksum_for(cat)
-      expect(Checksum.new(digest)).to eq(checksum)
+  describe "#sha256" do
+    it "works without cellar" do
+      checksums = {
+        snow_leopard_32: "deadbeef" * 8,
+        snow_leopard:    "faceb00c" * 8,
+        lion:            "baadf00d" * 8,
+        mountain_lion:   "8badf00d" * 8,
+      }
+
+      checksums.each_pair do |cat, digest|
+        subject.sha256(digest => cat)
+        checksum, = subject.checksum_for(cat)
+        expect(Checksum.new(digest)).to eq(checksum)
+      end
+    end
+
+    it "works with cellar" do
+      checksums = [
+        { digest: "deadbeef" * 8, tag: :snow_leopard_32, cellar: :any_skip_relocation },
+        { digest: "faceb00c" * 8, tag: :snow_leopard, cellar: :any },
+        { digest: "baadf00d" * 8, tag: :lion, cellar: "/usr/local/Cellar" },
+        { digest: "8badf00d" * 8, tag: :mountain_lion, cellar: Homebrew::DEFAULT_CELLAR },
+      ]
+
+      checksums.each do |checksum|
+        subject.sha256(checksum[:digest] => checksum[:tag], cellar: checksum[:cellar])
+        digest, tag, cellar = subject.checksum_for(checksum[:tag])
+        expect(Checksum.new(checksum[:digest])).to eq(digest)
+        expect(checksum[:tag]).to eq(tag)
+        checksum[:cellar] ||= Homebrew::DEFAULT_CELLAR
+        expect(checksum[:cellar]).to eq(cellar)
+      end
     end
   end
 
diff --git a/Library/Homebrew/test/support/fixtures/testball_bottle_cellar.rb b/Library/Homebrew/test/support/fixtures/testball_bottle_cellar.rb
new file mode 100644
index 0000000000..feae788169
--- /dev/null
+++ b/Library/Homebrew/test/support/fixtures/testball_bottle_cellar.rb
@@ -0,0 +1,24 @@
+# typed: true
+# frozen_string_literal: true
+
+class TestballBottleCellar < Formula
+  def initialize(name = "testball_bottle", path = Pathname.new(__FILE__).expand_path, spec = :stable,
+                 alias_path: nil, force_bottle: false)
+    self.class.instance_eval do
+      stable.url "file://#{TEST_FIXTURE_DIR}/tarballs/testball-0.1.tbz"
+      stable.sha256 TESTBALL_SHA256
+      hexdigest = "8f9aecd233463da6a4ea55f5f88fc5841718c013f3e2a7941350d6130f1dc149"
+      stable.bottle do
+        root_url "file://#{TEST_FIXTURE_DIR}/bottles"
+        sha256 hexdigest => Utils::Bottles.tag, :cellar => :any_skip_relocation
+      end
+      cxxstdlib_check :skip
+    end
+    super
+  end
+
+  def install
+    prefix.install "bin"
+    prefix.install "libexec"
+  end
+end
diff --git a/Library/Homebrew/test/utils/bottles/collector_spec.rb b/Library/Homebrew/test/utils/bottles/collector_spec.rb
index 831f978321..aa9d0aa065 100644
--- a/Library/Homebrew/test/utils/bottles/collector_spec.rb
+++ b/Library/Homebrew/test/utils/bottles/collector_spec.rb
@@ -8,9 +8,9 @@ describe Utils::Bottles::Collector do
 
   describe "#fetch_checksum_for" do
     it "returns passed tags" do
-      collector[:mojave] = "foo"
-      collector[:catalina] = "bar"
-      expect(collector.fetch_checksum_for(:catalina)).to eq(["bar", :catalina])
+      collector[:mojave] = { checksum: Checksum.new("foo_checksum"), cellar: "foo_cellar" }
+      collector[:catalina] = { checksum: Checksum.new("bar_checksum"), cellar: "bar_cellar" }
+      expect(collector.fetch_checksum_for(:catalina)).to eq(["bar_checksum", :catalina, "bar_cellar"])
     end
 
     it "returns nil if empty" do
diff --git a/Library/Homebrew/utils/bottles.rb b/Library/Homebrew/utils/bottles.rb
index a237af9110..1898f68517 100644
--- a/Library/Homebrew/utils/bottles.rb
+++ b/Library/Homebrew/utils/bottles.rb
@@ -100,16 +100,17 @@ module Utils
 
       extend Forwardable
 
-      def_delegators :@checksums, :keys, :[], :[]=, :key?, :each_key
+      def_delegators :@checksums, :keys, :[], :[]=, :key?, :each_key, :dig
 
       sig { void }
       def initialize
         @checksums = {}
       end
 
+      sig { params(tag: Symbol).returns(T.nilable([Checksum, Symbol, T.any(Symbol, String)])) }
       def fetch_checksum_for(tag)
         tag = find_matching_tag(tag)
-        return self[tag], tag if tag
+        return self[tag][:checksum], tag, self[tag][:cellar] if tag
       end
 
       private
-- 
GitLab