From 0f9bad0052ed6ef225dbf5dce8c1e292d1c10fbc Mon Sep 17 00:00:00 2001 From: Mike McQuaid Date: Wed, 9 Dec 2020 13:53:10 +0000 Subject: [PATCH] Fix bottle prefix and repository handling We were previously only looking at the `cellar` value when pouring bottles and ignoring the `prefix` and (implicit) `repository`. Actually look at these values and set the defaults for each platform. Also, when we're relocating to create or pour bottles when `prefix` and `repository` are equal then skip relocating the `repository` and always use references to the `prefix` instead. Fixes #9453 --- Library/Homebrew/fetch.rb | 2 +- Library/Homebrew/formula.rb | 1 + Library/Homebrew/formula_installer.rb | 12 ++++--- Library/Homebrew/global.rb | 5 ++- Library/Homebrew/keg_relocate.rb | 8 +++-- Library/Homebrew/os/linux/global.rb | 10 +++--- Library/Homebrew/os/mac/global.rb | 10 +++--- Library/Homebrew/software_spec.rb | 36 ++++++++++++++----- .../sorbet/rbi/hidden-definitions/hidden.rbi | 4 +++ .../Homebrew/test/os/linux/pathname_spec.rb | 1 + Library/Homebrew/test/support/lib/config.rb | 4 +++ 11 files changed, 66 insertions(+), 27 deletions(-) diff --git a/Library/Homebrew/fetch.rb b/Library/Homebrew/fetch.rb index f7bb7517e0..49d1e22a0d 100644 --- a/Library/Homebrew/fetch.rb +++ b/Library/Homebrew/fetch.rb @@ -13,7 +13,7 @@ module Homebrew return true if args.force_bottle? && bottle return false unless bottle && f.pour_bottle? return false if args.build_from_source_formulae.include?(f.full_name) - return false unless bottle.compatible_cellar? + return false unless bottle.compatible_locations? true end diff --git a/Library/Homebrew/formula.rb b/Library/Homebrew/formula.rb index 4d4f94b67c..08ec8a21b4 100644 --- a/Library/Homebrew/formula.rb +++ b/Library/Homebrew/formula.rb @@ -343,6 +343,7 @@ class Formula :bottle_disabled?, :bottle_disable_reason, :bottle_defined?, + :bottle_tag?, :bottled?, :bottle_specification, :downloader, diff --git a/Library/Homebrew/formula_installer.rb b/Library/Homebrew/formula_installer.rb index 908287cf5e..679f7500ca 100644 --- a/Library/Homebrew/formula_installer.rb +++ b/Library/Homebrew/formula_installer.rb @@ -156,7 +156,7 @@ class FormulaInstaller def pour_bottle?(install_bottle_options = { warn: false }) return false if @pour_failed - return false if !formula.bottled? && !formula.local_bottle_path + return false if !formula.bottle_tag? && !formula.local_bottle_path return true if force_bottle? return false if build_from_source? || build_bottle? || interactive? return false if @cc @@ -174,11 +174,13 @@ class FormulaInstaller end bottle = formula.bottle_specification - unless bottle.compatible_cellar? + unless bottle.compatible_locations? if install_bottle_options[:warn] opoo <<~EOS - Building #{formula.full_name} from source: - The bottle needs a #{bottle.cellar} Cellar (yours is #{HOMEBREW_CELLAR}). + Building #{formula.full_name} from source as the bottle needs: + - HOMEBREW_CELLAR: #{bottle.cellar} (yours is #{HOMEBREW_CELLAR}) + - HOMEBREW_PREFIX: #{bottle.prefix} (yours is #{HOMEBREW_PREFIX}) + - HOMEBREW_REPOSITORY: #{bottle.repository} (yours is #{HOMEBREW_REPOSITORY}) EOS end return false @@ -193,7 +195,7 @@ class FormulaInstaller return false if @build_from_source_formulae.include?(dep.full_name) return false unless dep.bottle && dep.pour_bottle? return false unless build.used_options.empty? - return false unless dep.bottle&.compatible_cellar? + return false unless dep.bottle&.compatible_locations? true end diff --git a/Library/Homebrew/global.rb b/Library/Homebrew/global.rb index 218ed1baf1..949d8c8caf 100644 --- a/Library/Homebrew/global.rb +++ b/Library/Homebrew/global.rb @@ -61,8 +61,11 @@ HOMEBREW_USER_AGENT_FAKE_SAFARI = "(KHTML, like Gecko) Version/10.0.3 Safari/602.4.8" HOMEBREW_DEFAULT_PREFIX = "/usr/local" +HOMEBREW_DEFAULT_REPOSITORY = "#{HOMEBREW_DEFAULT_PREFIX}/Homebrew" HOMEBREW_MACOS_ARM_DEFAULT_PREFIX = "/opt/homebrew" +HOMEBREW_MACOS_ARM_DEFAULT_REPOSITORY = HOMEBREW_MACOS_ARM_DEFAULT_PREFIX HOMEBREW_LINUX_DEFAULT_PREFIX = "/home/linuxbrew/.linuxbrew" +HOMEBREW_LINUX_DEFAULT_REPOSITORY = "#{HOMEBREW_LINUX_DEFAULT_PREFIX}/Homebrew" HOMEBREW_PULL_API_REGEX = %r{https://api\.github\.com/repos/([\w-]+)/([\w-]+)?/pulls/(\d+)}.freeze @@ -81,11 +84,11 @@ module Homebrew extend FileUtils DEFAULT_PREFIX ||= HOMEBREW_DEFAULT_PREFIX + DEFAULT_REPOSITORY ||= HOMEBREW_DEFAULT_REPOSITORY DEFAULT_CELLAR = "#{DEFAULT_PREFIX}/Cellar" DEFAULT_MACOS_CELLAR = "#{HOMEBREW_DEFAULT_PREFIX}/Cellar" DEFAULT_MACOS_ARM_CELLAR = "#{HOMEBREW_MACOS_ARM_DEFAULT_PREFIX}/Cellar" DEFAULT_LINUX_CELLAR = "#{HOMEBREW_LINUX_DEFAULT_PREFIX}/Cellar" - DEFAULT_REPOSITORY = "#{DEFAULT_PREFIX}/Homebrew" class << self attr_writer :failed, :raise_deprecation_exceptions, :auditing diff --git a/Library/Homebrew/keg_relocate.rb b/Library/Homebrew/keg_relocate.rb index 0ab2e24079..0ea9544b33 100644 --- a/Library/Homebrew/keg_relocate.rb +++ b/Library/Homebrew/keg_relocate.rb @@ -69,10 +69,12 @@ class Keg s = first.open("rb", &:read) replacements = { - relocation.old_prefix => relocation.new_prefix, - relocation.old_cellar => relocation.new_cellar, - relocation.old_repository => relocation.new_repository, + relocation.old_prefix => relocation.new_prefix, + relocation.old_cellar => relocation.new_cellar, } + # when HOMEBREW_PREFIX == HOMEBREW_REPOSITORY we should use HOMEBREW_PREFIX for all relocations to avoid + # being unable to differentiate between them. + replacements[relocation.old_repository] = relocation.new_repository if HOMEBREW_PREFIX != HOMEBREW_REPOSITORY changed = s.gsub!(Regexp.union(replacements.keys.sort_by(&:length).reverse), replacements) next unless changed diff --git a/Library/Homebrew/os/linux/global.rb b/Library/Homebrew/os/linux/global.rb index b32374004a..ea552cc9cd 100644 --- a/Library/Homebrew/os/linux/global.rb +++ b/Library/Homebrew/os/linux/global.rb @@ -10,9 +10,11 @@ HOMEBREW_PATCHELF_RB_WRITE = ( ).freeze module Homebrew - DEFAULT_PREFIX ||= if EnvConfig.force_homebrew_on_linux? - HOMEBREW_DEFAULT_PREFIX + if EnvConfig.force_homebrew_on_linux? + DEFAULT_PREFIX ||= HOMEBREW_DEFAULT_PREFIX.freeze + DEFAULT_REPOSITORY ||= HOMEBREW_DEFAULT_REPOSITORY.freeze else - HOMEBREW_LINUX_DEFAULT_PREFIX - end.freeze + DEFAULT_PREFIX ||= HOMEBREW_LINUX_DEFAULT_PREFIX.freeze + DEFAULT_REPOSITORY ||= HOMEBREW_LINUX_DEFAULT_REPOSITORY.freeze + end end diff --git a/Library/Homebrew/os/mac/global.rb b/Library/Homebrew/os/mac/global.rb index 90cc213d62..5b66c6347f 100644 --- a/Library/Homebrew/os/mac/global.rb +++ b/Library/Homebrew/os/mac/global.rb @@ -2,9 +2,11 @@ # frozen_string_literal: true module Homebrew - DEFAULT_PREFIX ||= if Hardware::CPU.arm? - HOMEBREW_MACOS_ARM_DEFAULT_PREFIX + if Hardware::CPU.arm? + DEFAULT_PREFIX ||= HOMEBREW_MACOS_ARM_DEFAULT_PREFIX.freeze + DEFAULT_REPOSITORY ||= HOMEBREW_MACOS_ARM_DEFAULT_REPOSITORY.freeze else - HOMEBREW_DEFAULT_PREFIX - end.freeze + DEFAULT_PREFIX ||= HOMEBREW_DEFAULT_PREFIX.freeze + DEFAULT_REPOSITORY ||= HOMEBREW_DEFAULT_REPOSITORY.freeze + end end diff --git a/Library/Homebrew/software_spec.rb b/Library/Homebrew/software_spec.rb index d34314e8bc..cd762675a9 100644 --- a/Library/Homebrew/software_spec.rb +++ b/Library/Homebrew/software_spec.rb @@ -93,9 +93,13 @@ class SoftwareSpec !bottle_specification.collector.keys.empty? end + def bottle_tag? + bottle_specification.tag?(Utils::Bottles.tag) + end + def bottled? - bottle_specification.tag?(Utils::Bottles.tag) && \ - (bottle_specification.compatible_cellar? || owner.force_bottle) + bottle_tag? && \ + (bottle_specification.compatible_locations? || owner.force_bottle) end def bottle(disable_type = nil, disable_reason = nil, &block) @@ -317,8 +321,8 @@ class Bottle @rebuild = spec.rebuild end - def compatible_cellar? - @spec.compatible_cellar? + def compatible_locations? + @spec.compatible_locations? end # Does the bottle need to be relocated? @@ -341,17 +345,16 @@ end class BottleSpecification extend T::Sig - DEFAULT_PREFIX = Homebrew::DEFAULT_PREFIX - attr_rw :prefix, :cellar, :rebuild attr_accessor :tap - attr_reader :checksum, :collector, :root_url_specs + attr_reader :checksum, :collector, :root_url_specs, :repository sig { void } def initialize @rebuild = 0 @prefix = Homebrew::DEFAULT_PREFIX @cellar = Homebrew::DEFAULT_CELLAR + @repository = Homebrew::DEFAULT_REPOSITORY @collector = Utils::Bottles::Collector.new @root_url_specs = {} end @@ -365,8 +368,23 @@ class BottleSpecification end end - def compatible_cellar? - cellar == :any || cellar == :any_skip_relocation || cellar == HOMEBREW_CELLAR.to_s + def compatible_locations? + compatible_cellar = cellar == :any || + cellar == :any_skip_relocation || + cellar == HOMEBREW_CELLAR.to_s + + compatible_prefix = prefix == HOMEBREW_PREFIX.to_s + + # Only check the repository matches if the prefix is the default. + # This is because the bottle DSL does not allow setting a custom repository + # but does allow setting a custom prefix. + compatible_repository = if prefix == Homebrew::DEFAULT_PREFIX + repository == HOMEBREW_REPOSITORY.to_s + else + true + end + + compatible_cellar && compatible_prefix && compatible_repository end # Does the {Bottle} this {BottleSpecification} belongs to need to be relocated? diff --git a/Library/Homebrew/sorbet/rbi/hidden-definitions/hidden.rbi b/Library/Homebrew/sorbet/rbi/hidden-definitions/hidden.rbi index 0374298cfb..361422d896 100644 --- a/Library/Homebrew/sorbet/rbi/hidden-definitions/hidden.rbi +++ b/Library/Homebrew/sorbet/rbi/hidden-definitions/hidden.rbi @@ -8124,6 +8124,7 @@ class HeadVersion end module Homebrew + DEFAULT_REPOSITORY = ::T.let(nil, ::T.untyped) MAX_PORT = ::T.let(nil, ::T.untyped) MIN_PORT = ::T.let(nil, ::T.untyped) end @@ -13538,14 +13539,17 @@ class Object HOMEBREW_DEFAULT_CACHE = ::T.let(nil, ::T.untyped) HOMEBREW_DEFAULT_LOGS = ::T.let(nil, ::T.untyped) HOMEBREW_DEFAULT_PREFIX = ::T.let(nil, ::T.untyped) + HOMEBREW_DEFAULT_REPOSITORY = ::T.let(nil, ::T.untyped) HOMEBREW_DEFAULT_TEMP = ::T.let(nil, ::T.untyped) HOMEBREW_LIBRARY = ::T.let(nil, ::T.untyped) HOMEBREW_LIBRARY_PATH = ::T.let(nil, ::T.untyped) HOMEBREW_LINKED_KEGS = ::T.let(nil, ::T.untyped) HOMEBREW_LINUX_DEFAULT_PREFIX = ::T.let(nil, ::T.untyped) + HOMEBREW_LINUX_DEFAULT_REPOSITORY = ::T.let(nil, ::T.untyped) HOMEBREW_LOCKS = ::T.let(nil, ::T.untyped) HOMEBREW_LOGS = ::T.let(nil, ::T.untyped) HOMEBREW_MACOS_ARM_DEFAULT_PREFIX = ::T.let(nil, ::T.untyped) + HOMEBREW_MACOS_ARM_DEFAULT_REPOSITORY = ::T.let(nil, ::T.untyped) HOMEBREW_OFFICIAL_REPO_PREFIXES_REGEX = ::T.let(nil, ::T.untyped) HOMEBREW_PATCHELF_RB_WRITE = ::T.let(nil, ::T.untyped) HOMEBREW_PINNED_KEGS = ::T.let(nil, ::T.untyped) diff --git a/Library/Homebrew/test/os/linux/pathname_spec.rb b/Library/Homebrew/test/os/linux/pathname_spec.rb index cae19a846e..be6734473e 100644 --- a/Library/Homebrew/test/os/linux/pathname_spec.rb +++ b/Library/Homebrew/test/os/linux/pathname_spec.rb @@ -45,6 +45,7 @@ describe Pathname do describe "#patch!" do # testing only patchelf.rb as HOMEBREW_PREFIX is different for tests, # and DevelopmentTools.locate fails to locate patchelf + # TODO: use stub_const("HOMEBREW_PATCHELF_RB_WRITE", true) in tests instead. HOMEBREW_PATCHELF_RB_WRITE = true let(:placeholder_prefix) { "@@HOMEBREW_PREFIX@@" } diff --git a/Library/Homebrew/test/support/lib/config.rb b/Library/Homebrew/test/support/lib/config.rb index b68490628b..1dede23e84 100644 --- a/Library/Homebrew/test/support/lib/config.rb +++ b/Library/Homebrew/test/support/lib/config.rb @@ -50,5 +50,9 @@ TEST_SHA256 = "deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef" # For testing's sake always assume the default prefix module Homebrew + remove_const :DEFAULT_PREFIX if defined?(DEFAULT_PREFIX) DEFAULT_PREFIX = HOMEBREW_PREFIX.to_s.freeze + + remove_const :DEFAULT_REPOSITORY if defined?(DEFAULT_REPOSITORY) + DEFAULT_REPOSITORY = HOMEBREW_REPOSITORY.to_s.freeze end