From 71d81a8611011d7683787b7670c2c5f474010086 Mon Sep 17 00:00:00 2001 From: Bo Anderson Date: Tue, 9 Feb 2021 18:34:20 +0000 Subject: [PATCH 1/5] version: handle subclasses in major_minor and major_minor_patch --- Library/Homebrew/version.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Library/Homebrew/version.rb b/Library/Homebrew/version.rb index 7eb556a3bc..ccde6b58c1 100644 --- a/Library/Homebrew/version.rb +++ b/Library/Homebrew/version.rb @@ -589,15 +589,15 @@ class Version end # @api public - sig { returns(Version) } + sig { returns(T.self_type) } def major_minor - Version.new([major, minor].compact.join(".")) + self.class.new([major, minor].compact.join(".")) end # @api public - sig { returns(Version) } + sig { returns(T.self_type) } def major_minor_patch - Version.new([major, minor, patch].compact.join(".")) + self.class.new([major, minor, patch].compact.join(".")) end sig { returns(T::Boolean) } From 61cda1465f6d5db241801f8e5a4ec66453711835 Mon Sep 17 00:00:00 2001 From: Bo Anderson Date: Tue, 9 Feb 2021 18:34:47 +0000 Subject: [PATCH 2/5] os/mac/version: add strip_patch method --- Library/Homebrew/os/mac/version.rb | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/Library/Homebrew/os/mac/version.rb b/Library/Homebrew/os/mac/version.rb index 0244607383..389b3d739d 100644 --- a/Library/Homebrew/os/mac/version.rb +++ b/Library/Homebrew/os/mac/version.rb @@ -78,17 +78,19 @@ module OS end end + sig { returns(T.self_type) } + def strip_patch + # Big Sur is 11.x but Catalina is 10.15.x. + if major >= 11 + self.class.new(major.to_s) + else + major_minor + end + end + sig { returns(Symbol) } def to_sym - @to_sym ||= begin - # Big Sur is 11.x but Catalina is 10.15. - major_macos = if major >= 11 - major - else - major_minor - end.to_s - SYMBOLS.invert.fetch(major_macos, :dunno) - end + @to_sym ||= SYMBOLS.invert.fetch(strip_patch.to_s, :dunno) end sig { returns(String) } From 8c81a2822cb1a2af4794d5d08ee38c8d376f43e2 Mon Sep 17 00:00:00 2001 From: Bo Anderson Date: Sun, 7 Feb 2021 04:16:36 +0000 Subject: [PATCH 3/5] os/mac/sdk: read version from SDKSettings --- Library/Homebrew/os/mac/sdk.rb | 94 ++++++++++++++++++++-------------- 1 file changed, 56 insertions(+), 38 deletions(-) diff --git a/Library/Homebrew/os/mac/sdk.rb b/Library/Homebrew/os/mac/sdk.rb index 4b6f893efb..f39d9e527d 100644 --- a/Library/Homebrew/os/mac/sdk.rb +++ b/Library/Homebrew/os/mac/sdk.rb @@ -9,6 +9,8 @@ module OS # # @api private class SDK + VERSIONED_SDK_REGEX = /MacOSX(\d+\.\d+)\.sdk$/.freeze + attr_reader :version, :path, :source def initialize(version, path, source) @@ -25,14 +27,39 @@ module OS class NoSDKError < StandardError; end def sdk_for(v) - path = sdk_paths[v] - raise NoSDKError if path.nil? + sdk = all_sdks.find { |s| s.version == v } + raise NoSDKError if sdk.nil? - SDK.new v, path, source + sdk end def all_sdks - sdk_paths.map { |v, p| SDK.new v, p, source } + return @all_sdks if @all_sdks + + @all_sdks = [] + + # Bail out if there is no SDK prefix at all + return @all_sdks unless File.directory? sdk_prefix + + # Use unversioned SDK path on Big Sur to avoid issues such as: + # https://github.com/Homebrew/homebrew-core/issues/67075 + unversioned_sdk_path = Pathname.new("#{sdk_prefix}/MacOSX.sdk") + version = read_sdk_version(unversioned_sdk_path) + if version && version >= :big_sur + unversioned_sdk_version = version + @all_sdks << SDK.new(unversioned_sdk_version, unversioned_sdk_path, source) + end + + Dir["#{sdk_prefix}/MacOSX*.sdk"].each do |sdk_path| + next unless sdk_path.match?(SDK::VERSIONED_SDK_REGEX) + + version = read_sdk_version(Pathname.new(sdk_path)) + next if version.nil? || version == unversioned_sdk_version + + @all_sdks << SDK.new(version, sdk_path, source) + end + + @all_sdks end def sdk_if_applicable(v = nil) @@ -64,43 +91,34 @@ module OS "" end - def sdk_paths - @sdk_paths ||= begin - # Bail out if there is no SDK prefix at all - if File.directory? sdk_prefix - paths = {} - - Dir[File.join(sdk_prefix, "MacOSX*.sdk")].each do |sdk_path| - version = sdk_path[/MacOSX(\d+\.\d+)u?\.sdk$/, 1] - paths[OS::Mac::Version.new(version)] = sdk_path if version.present? - end - - # Use unversioned SDK path on Big Sur to avoid issues such as: - # https://github.com/Homebrew/homebrew-core/issues/67075 - # This creates an entry in `paths` whose key is the OS major version - sdk_path = Pathname.new("#{sdk_prefix}/MacOSX.sdk") - sdk_settings = sdk_path/"SDKSettings.json" - if sdk_settings.exist? && - (sdk_settings_string = sdk_settings.read.presence) && - (sdk_settings_json = JSON.parse(sdk_settings_string).presence) && - (version_string = sdk_settings_json.fetch("Version", nil).presence) && - (version = version_string[/(\d+)\./, 1].presence) - paths[OS::Mac::Version.new(version)] = sdk_path - end - - paths - else - {} - end - end + def latest_sdk + all_sdks.max_by(&:version) end - # NOTE: This returns a versioned SDK path, even on Big Sur - def latest_sdk - return if sdk_paths.empty? + def read_sdk_version(sdk_path) + sdk_settings = sdk_path/"SDKSettings.json" + sdk_settings_string = sdk_settings.read if sdk_settings.exist? - v, path = sdk_paths.max { |(v1, _), (v2, _)| v1 <=> v2 } - SDK.new v, path, source + # Pre-10.14 SDKs + sdk_settings = sdk_path/"SDKSettings.plist" + if sdk_settings_string.blank? && sdk_settings.exist? + result = system_command("plutil", args: ["-convert", "json", "-o", "-", sdk_settings]) + sdk_settings_string = result.stdout if result.success? + end + + return if sdk_settings_string.blank? + + sdk_settings_json = JSON.parse(sdk_settings_string) + return if sdk_settings_json.blank? + + version_string = sdk_settings_json.fetch("Version", nil) + return if version_string.blank? + + begin + OS::Mac::Version.new(version_string).strip_patch + rescue MacOSVersionError + nil + end end end private_constant :BaseSDKLocator From fe52b3a40231cfb7d4462ae6b32b5a4b2c84ec6b Mon Sep 17 00:00:00 2001 From: Bo Anderson Date: Sun, 7 Feb 2021 04:21:59 +0000 Subject: [PATCH 4/5] os/mac/diagnostic: check SDK version matches the path --- .../extend/os/mac/development_tools.rb | 5 +- Library/Homebrew/extend/os/mac/diagnostic.rb | 37 ++++++++- Library/Homebrew/os/mac/xcode.rb | 28 +++++++ .../Homebrew/test/os/mac/diagnostic_spec.rb | 75 +++++++++++++++++++ 4 files changed, 140 insertions(+), 5 deletions(-) diff --git a/Library/Homebrew/extend/os/mac/development_tools.rb b/Library/Homebrew/extend/os/mac/development_tools.rb index f101a31056..6f25836ae3 100644 --- a/Library/Homebrew/extend/os/mac/development_tools.rb +++ b/Library/Homebrew/extend/os/mac/development_tools.rb @@ -51,10 +51,7 @@ class DevelopmentTools sig { returns(String) } def installation_instructions - <<~EOS - Install the Command Line Tools: - xcode-select --install - EOS + MacOS::CLT.installation_instructions end sig { returns(String) } diff --git a/Library/Homebrew/extend/os/mac/diagnostic.rb b/Library/Homebrew/extend/os/mac/diagnostic.rb index 71fe9f3c4a..69aa335b23 100644 --- a/Library/Homebrew/extend/os/mac/diagnostic.rb +++ b/Library/Homebrew/extend/os/mac/diagnostic.rb @@ -64,6 +64,7 @@ module Homebrew check_clt_minimum_version check_if_xcode_needs_clt_installed check_if_supported_sdk_available + check_broken_sdks ].freeze end @@ -425,7 +426,7 @@ module Homebrew source = if locator.source == :clt update_instructions = MacOS::CLT.update_instructions - "CLT" + "Command Line Tools (CLT)" else update_instructions = MacOS::Xcode.update_instructions "Xcode" @@ -438,6 +439,40 @@ module Homebrew #{update_instructions} EOS end + + # The CLT 10.x -> 11.x upgrade process on 10.14 contained a bug which broke the SDKs. + # Notably, MacOSX10.14.sdk would indirectly symlink to MacOSX10.15.sdk. + # This diagnostic was introduced to check for this and recommend a full reinstall. + def check_broken_sdks + locator = MacOS.sdk_locator + + return if locator.all_sdks.all? do |sdk| + path_version = sdk.path.basename.to_s[MacOS::SDK::VERSIONED_SDK_REGEX, 1] + next true if path_version.blank? + + sdk.version == MacOS::Version.new(path_version).strip_patch + end + + if locator.source == :clt + source = "Command Line Tools (CLT)" + path_to_remove = MacOS::CLT::PKG_PATH + installation_instructions = MacOS::CLT.installation_instructions + else + source = "Xcode" + path_to_remove = MacOS::Xcode.bundle_path + installation_instructions = MacOS::Xcode.installation_instructions + end + + <<~EOS + The contents of the SDKs in your #{source} installation do not match the SDK folder names. + A clean reinstall of #{source} should fix this. + + Remove the broken installation before reinstalling: + sudo rm -rf #{path_to_remove} + + #{installation_instructions} + EOS + end end end end diff --git a/Library/Homebrew/os/mac/xcode.rb b/Library/Homebrew/os/mac/xcode.rb index 9a5f765800..7de72b63e3 100644 --- a/Library/Homebrew/os/mac/xcode.rb +++ b/Library/Homebrew/os/mac/xcode.rb @@ -134,6 +134,19 @@ module OS sdk(v)&.path end + def installation_instructions + if OS::Mac.prerelease? + <<~EOS + Xcode can be installed from: + #{Formatter.url("https://developer.apple.com/download/more/")} + EOS + else + <<~EOS + Xcode can be installed from the App Store. + EOS + end + end + sig { returns(String) } def update_instructions if OS::Mac.prerelease? @@ -254,6 +267,21 @@ module OS sdk(v)&.path end + def installation_instructions + if MacOS.version == "10.14" + # This is not available from `xcode-select` + <<~EOS + Install the Command Line Tools for Xcode 11.3.1 from: + #{Formatter.url("https://developer.apple.com/download/more/")} + EOS + else + <<~EOS + Install the Command Line Tools: + xcode-select --install + EOS + end + end + sig { returns(String) } def update_instructions software_update_location = if MacOS.version >= "10.14" diff --git a/Library/Homebrew/test/os/mac/diagnostic_spec.rb b/Library/Homebrew/test/os/mac/diagnostic_spec.rb index f58aef8cff..d8c1436355 100644 --- a/Library/Homebrew/test/os/mac/diagnostic_spec.rb +++ b/Library/Homebrew/test/os/mac/diagnostic_spec.rb @@ -39,4 +39,79 @@ describe Homebrew::Diagnostic::Checks do expect(checks.check_ruby_version) .to match "Ruby version 1.8.6 is unsupported on 10.12" end + + describe "#check_if_supported_sdk_available" do + let(:macos_version) { OS::Mac::Version.new("11") } + + before do + allow(DevelopmentTools).to receive(:installed?).and_return(true) + allow(OS::Mac).to receive(:version).and_return(macos_version) + end + + it "doesn't trigger when SDK root is not needed" do + allow(OS::Mac).to receive(:sdk_root_needed?).and_return(false) + allow(OS::Mac).to receive(:sdk).and_return(nil) + + expect(checks.check_if_supported_sdk_available).to be_nil + end + + it "doesn't trigger when a valid SDK is present" do + allow(OS::Mac).to receive(:sdk_root_needed?).and_return(true) + allow(OS::Mac).to receive(:sdk).and_return(OS::Mac::SDK.new(macos_version, "/some/path/MacOSX.sdk", :clt)) + + expect(checks.check_if_supported_sdk_available).to be_nil + end + + it "triggers when a valid SDK is not present on CLT systems" do + allow(OS::Mac).to receive(:sdk_root_needed?).and_return(true) + allow(OS::Mac).to receive(:sdk).and_return(nil) + allow(OS::Mac).to receive(:sdk_locator).and_return(OS::Mac::CLT.sdk_locator) + + expect(checks.check_if_supported_sdk_available) + .to include("Your Command Line Tools (CLT) does not support macOS #{macos_version}") + end + + it "triggers when a valid SDK is not present on Xcode systems" do + allow(OS::Mac).to receive(:sdk_root_needed?).and_return(true) + allow(OS::Mac).to receive(:sdk).and_return(nil) + allow(OS::Mac).to receive(:sdk_locator).and_return(OS::Mac::Xcode.sdk_locator) + + expect(checks.check_if_supported_sdk_available) + .to include("Your Xcode does not support macOS #{macos_version}") + end + end + + describe "#check_broken_sdks" do + it "doesn't trigger when SDK versions are as expected" do + allow(OS::Mac).to receive(:sdk_locator).and_return(OS::Mac::CLT.sdk_locator) + allow_any_instance_of(OS::Mac::CLTSDKLocator).to receive(:all_sdks) + .and_return([ + OS::Mac::SDK.new(OS::Mac::Version.new("11"), "/some/path/MacOSX.sdk", :clt), + OS::Mac::SDK.new(OS::Mac::Version.new("10.15"), "/some/path/MacOSX10.15.sdk", :clt), + ]) + + expect(checks.check_broken_sdks).to be_nil + end + + it "triggers when the CLT SDK version doesn't match the folder name" do + allow_any_instance_of(OS::Mac::CLTSDKLocator).to receive(:all_sdks) + .and_return([ + OS::Mac::SDK.new(OS::Mac::Version.new("10.14"), "/some/path/MacOSX10.15.sdk", :clt), + ]) + + expect(checks.check_broken_sdks) + .to include("SDKs in your Command Line Tools (CLT) installation do not match the SDK folder names") + end + + it "triggers when the Xcode SDK version doesn't match the folder name" do + allow(OS::Mac).to receive(:sdk_locator).and_return(OS::Mac::Xcode.sdk_locator) + allow_any_instance_of(OS::Mac::XcodeSDKLocator).to receive(:all_sdks) + .and_return([ + OS::Mac::SDK.new(OS::Mac::Version.new("10.14"), "/some/path/MacOSX10.15.sdk", :xcode), + ]) + + expect(checks.check_broken_sdks) + .to include("The contents of the SDKs in your Xcode installation do not match the SDK folder names") + end + end end From 87b6fe8f8422d84a450b4f829b10363b8d1ce1e3 Mon Sep 17 00:00:00 2001 From: Bo Anderson Date: Thu, 11 Feb 2021 00:17:13 +0000 Subject: [PATCH 5/5] test/os/mac/sdk: add tests --- Library/Homebrew/test/os/mac/sdk_spec.rb | 97 +++++++++++++++++++ .../sdks/big_sur/MacOSX.sdk/SDKSettings.json | 1 + .../MacOSX10.13.sdk/SDKSettings.plist | 8 ++ .../MacOSX10.15.sdk/SDKSettings.json | 1 + .../MacOSX10.14.sdk/SDKSettings.json | 1 + 5 files changed, 108 insertions(+) create mode 100644 Library/Homebrew/test/os/mac/sdk_spec.rb create mode 100644 Library/Homebrew/test/support/fixtures/sdks/big_sur/MacOSX.sdk/SDKSettings.json create mode 100644 Library/Homebrew/test/support/fixtures/sdks/high_sierra/MacOSX10.13.sdk/SDKSettings.plist create mode 100644 Library/Homebrew/test/support/fixtures/sdks/malformed/MacOSX10.15.sdk/SDKSettings.json create mode 100644 Library/Homebrew/test/support/fixtures/sdks/mojave_broken/MacOSX10.14.sdk/SDKSettings.json diff --git a/Library/Homebrew/test/os/mac/sdk_spec.rb b/Library/Homebrew/test/os/mac/sdk_spec.rb new file mode 100644 index 0000000000..55c18aa221 --- /dev/null +++ b/Library/Homebrew/test/os/mac/sdk_spec.rb @@ -0,0 +1,97 @@ +# typed: false +# frozen_string_literal: true + +describe OS::Mac::CLTSDKLocator do + subject(:locator) { described_class.new } + + let(:big_sur_sdk) { OS::Mac::SDK.new(OS::Mac::Version.new("11"), "/some/path/MacOSX.sdk", :clt) } + let(:catalina_sdk) { OS::Mac::SDK.new(OS::Mac::Version.new("10.15"), "/some/path/MacOSX10.15.sdk", :clt) } + + specify "#sdk_for" do + allow(locator).to receive(:all_sdks).and_return([big_sur_sdk, catalina_sdk]) + + expect(locator.sdk_for(OS::Mac::Version.new("11"))).to eq(big_sur_sdk) + expect(locator.sdk_for(OS::Mac::Version.new("10.15"))).to eq(catalina_sdk) + expect { locator.sdk_for(OS::Mac::Version.new("10.14")) }.to raise_error(described_class::NoSDKError) + end + + describe "#sdk_if_applicable" do + before do + allow(locator).to receive(:all_sdks).and_return([big_sur_sdk, catalina_sdk]) + end + + it "returns the requested SDK" do + expect(locator.sdk_if_applicable(OS::Mac::Version.new("11"))).to eq(big_sur_sdk) + expect(locator.sdk_if_applicable(OS::Mac::Version.new("10.15"))).to eq(catalina_sdk) + end + + it "returns the latest SDK if the requested version is not found" do + expect(locator.sdk_if_applicable(OS::Mac::Version.new("10.14"))).to eq(big_sur_sdk) + expect(locator.sdk_if_applicable(OS::Mac::Version.new("12"))).to eq(big_sur_sdk) + end + + it "returns the SDK matching the OS version if no version is specified" do + allow(OS::Mac).to receive(:version).and_return(OS::Mac::Version.new("10.15")) + expect(locator.sdk_if_applicable).to eq(catalina_sdk) + end + + it "returns the latest SDK on older OS versions when there's no matching SDK" do + allow(OS::Mac).to receive(:version).and_return(OS::Mac::Version.new("10.14")) + expect(locator.sdk_if_applicable).to eq(big_sur_sdk) + end + + it "returns nil if the OS is newer than all SDKs" do + allow(OS::Mac).to receive(:version).and_return(OS::Mac::Version.new("12")) + expect(locator.sdk_if_applicable).to be_nil + end + end + + describe "#all_sdks" do + let(:big_sur_sdk_prefix) { TEST_FIXTURE_DIR/"sdks/big_sur" } + let(:mojave_broken_sdk_prefix) { TEST_FIXTURE_DIR/"sdks/mojave_broken" } + let(:high_sierra_sdk_prefix) { TEST_FIXTURE_DIR/"sdks/high_sierra" } + let(:malformed_sdk_prefix) { TEST_FIXTURE_DIR/"sdks/malformed" } + + it "reads the SDKSettings.json version of unversioned SDKs folders" do + allow(locator).to receive(:sdk_prefix).and_return(big_sur_sdk_prefix.to_s) + + sdks = locator.all_sdks + expect(sdks.count).to eq(1) + + sdk = sdks.first + expect(sdk.path).to eq(big_sur_sdk_prefix/"MacOSX.sdk") + expect(sdk.version).to eq(OS::Mac::Version.new("11")) + expect(sdk.source).to eq(:clt) + end + + it "reads the SDKSettings.json version of versioned SDKs folders" do + allow(locator).to receive(:sdk_prefix).and_return(mojave_broken_sdk_prefix.to_s) + + sdks = locator.all_sdks + expect(sdks.count).to eq(1) + + sdk = sdks.first + expect(sdk.path).to eq(mojave_broken_sdk_prefix/"MacOSX10.14.sdk") + expect(sdk.version).to eq(OS::Mac::Version.new("10.15")) + expect(sdk.source).to eq(:clt) + end + + it "reads the SDKSettings.plist version" do + allow(locator).to receive(:sdk_prefix).and_return(high_sierra_sdk_prefix.to_s) + + sdks = locator.all_sdks + expect(sdks.count).to eq(1) + + sdk = sdks.first + expect(sdk.path).to eq(high_sierra_sdk_prefix/"MacOSX10.13.sdk") + expect(sdk.version).to eq(OS::Mac::Version.new("10.13")) + expect(sdk.source).to eq(:clt) + end + + it "rejects malformed sdks" do + allow(locator).to receive(:sdk_prefix).and_return(malformed_sdk_prefix.to_s) + + expect(locator.all_sdks).to be_empty + end + end +end diff --git a/Library/Homebrew/test/support/fixtures/sdks/big_sur/MacOSX.sdk/SDKSettings.json b/Library/Homebrew/test/support/fixtures/sdks/big_sur/MacOSX.sdk/SDKSettings.json new file mode 100644 index 0000000000..3e9fb27234 --- /dev/null +++ b/Library/Homebrew/test/support/fixtures/sdks/big_sur/MacOSX.sdk/SDKSettings.json @@ -0,0 +1 @@ +{"Version":"11.1"} diff --git a/Library/Homebrew/test/support/fixtures/sdks/high_sierra/MacOSX10.13.sdk/SDKSettings.plist b/Library/Homebrew/test/support/fixtures/sdks/high_sierra/MacOSX10.13.sdk/SDKSettings.plist new file mode 100644 index 0000000000..7e57e7b081 --- /dev/null +++ b/Library/Homebrew/test/support/fixtures/sdks/high_sierra/MacOSX10.13.sdk/SDKSettings.plist @@ -0,0 +1,8 @@ + + + + + Version + 10.13 + + diff --git a/Library/Homebrew/test/support/fixtures/sdks/malformed/MacOSX10.15.sdk/SDKSettings.json b/Library/Homebrew/test/support/fixtures/sdks/malformed/MacOSX10.15.sdk/SDKSettings.json new file mode 100644 index 0000000000..8613898c19 --- /dev/null +++ b/Library/Homebrew/test/support/fixtures/sdks/malformed/MacOSX10.15.sdk/SDKSettings.json @@ -0,0 +1 @@ +{"Version":"broken"} diff --git a/Library/Homebrew/test/support/fixtures/sdks/mojave_broken/MacOSX10.14.sdk/SDKSettings.json b/Library/Homebrew/test/support/fixtures/sdks/mojave_broken/MacOSX10.14.sdk/SDKSettings.json new file mode 100644 index 0000000000..62938d457a --- /dev/null +++ b/Library/Homebrew/test/support/fixtures/sdks/mojave_broken/MacOSX10.14.sdk/SDKSettings.json @@ -0,0 +1 @@ +{"Version":"10.15.6"}