Apply suggestions from code review

This commit is contained in:
Issy Long 2024-07-05 21:14:03 +01:00
parent cd1869437d
commit 0a18f77de4
No known key found for this signature in database
2 changed files with 19 additions and 14 deletions

View File

@ -16,7 +16,7 @@ class DevelopmentTools
# Don't call tools (cc, make, strip, etc.) directly! # Don't call tools (cc, make, strip, etc.) directly!
# Give the name of the binary you look for as a string to this method # Give the name of the binary you look for as a string to this method
# in order to get the full path back as a Pathname. # in order to get the full path back as a Pathname.
(@locate ||= T.let({}, T.nilable(T::Hash[T.untyped, T.untyped]))).fetch(tool) do |key| (@locate ||= T.let({}, T.nilable(T::Hash[T.any(String, Symbol), T.untyped]))).fetch(tool) do |key|
@locate[key] = if File.executable?((path = "/usr/bin/#{tool}")) @locate[key] = if File.executable?((path = "/usr/bin/#{tool}"))
Pathname.new path Pathname.new path
# Homebrew GCCs most frequently; much faster to check this before xcrun # Homebrew GCCs most frequently; much faster to check this before xcrun
@ -63,8 +63,9 @@ class DevelopmentTools
sig { returns(Version) } sig { returns(Version) }
def clang_version def clang_version
@clang_version ||= T.let( @clang_version ||= T.let(
if (path = locate("clang")) && (bv = `#{path} --version`[/(?:clang|LLVM) version (\d+\.\d(?:\.\d)?)/, 1]) if (path = locate("clang")) &&
Version.new(bv) (build_version = `#{path} --version`[/(?:clang|LLVM) version (\d+\.\d(?:\.\d)?)/, 1])
Version.new(build_version)
else else
Version::NULL Version::NULL
end, T.nilable(Version) end, T.nilable(Version)
@ -93,8 +94,8 @@ class DevelopmentTools
def llvm_clang_build_version def llvm_clang_build_version
@llvm_clang_build_version ||= T.let(begin @llvm_clang_build_version ||= T.let(begin
path = Formulary.factory("llvm").opt_prefix/"bin/clang" path = Formulary.factory("llvm").opt_prefix/"bin/clang"
if path.executable? && (bv = `#{path} --version`[/clang version (\d+\.\d\.\d)/, 1]) if path.executable? && (build_version = `#{path} --version`[/clang version (\d+\.\d\.\d)/, 1])
Version.new(bv) Version.new(build_version)
else else
Version::NULL Version::NULL
end end
@ -109,8 +110,9 @@ class DevelopmentTools
(@gcc_version ||= T.let({}, T.nilable(T::Hash[String, Version]))).fetch(cc) do (@gcc_version ||= T.let({}, T.nilable(T::Hash[String, Version]))).fetch(cc) do
path = HOMEBREW_PREFIX/"opt/#{CompilerSelector.preferred_gcc}/bin"/cc path = HOMEBREW_PREFIX/"opt/#{CompilerSelector.preferred_gcc}/bin"/cc
path = locate(cc) unless path.exist? path = locate(cc) unless path.exist?
version = if path && (bv = `#{path} --version`[/gcc(?:(?:-\d+(?:\.\d)?)? \(.+\))? (\d+\.\d\.\d)/, 1]) version = if path &&
Version.new(bv) (build_version = `#{path} --version`[/gcc(?:(?:-\d+(?:\.\d)?)? \(.+\))? (\d+\.\d\.\d)/, 1])
Version.new(build_version)
else else
Version::NULL Version::NULL
end end

View File

@ -7,6 +7,7 @@ require "system_command"
module UnpackStrategy module UnpackStrategy
extend T::Helpers extend T::Helpers
# FIXME: Enable cop again when https://github.com/sorbet/sorbet/issues/3532 is fixed.
AnyStrategy = T.type_alias do # rubocop:disable Style/MutableConstant AnyStrategy = T.type_alias do # rubocop:disable Style/MutableConstant
T.any( T.any(
T.class_of(UnpackStrategy::Tar), T.class_of(UnpackStrategy::Tar),
@ -101,6 +102,8 @@ module UnpackStrategy
sig { params(extension: String).returns(T.nilable(AnyStrategy)) } sig { params(extension: String).returns(T.nilable(AnyStrategy)) }
def self.from_extension(extension) def self.from_extension(extension)
return unless strategies
strategies&.sort_by { |s| s.extensions.map(&:length).max || 0 } strategies&.sort_by { |s| s.extensions.map(&:length).max || 0 }
&.reverse &.reverse
&.find { |s| s.extensions.any? { |ext| extension.end_with?(ext) } } &.find { |s| s.extensions.any? { |ext| extension.end_with?(ext) } }
@ -113,15 +116,15 @@ module UnpackStrategy
sig { sig {
params(path: Pathname, prioritize_extension: T::Boolean, type: T.nilable(Symbol), ref_type: T.nilable(String), params(path: Pathname, prioritize_extension: T::Boolean, type: T.nilable(Symbol), ref_type: T.nilable(String),
ref: T.nilable(String), merge_xattrs: T.nilable(T::Boolean)).returns(T.untyped) ref: T.nilable(String), merge_xattrs: T::Boolean).returns(T.untyped)
} }
def self.detect(path, prioritize_extension: false, type: nil, ref_type: nil, ref: nil, merge_xattrs: nil) def self.detect(path, prioritize_extension: false, type: nil, ref_type: nil, ref: nil, merge_xattrs: false)
strategy = from_type(type) if type strategy = from_type(type) if type
if prioritize_extension && path.extname.present? if prioritize_extension && path.extname.present?
strategy ||= from_extension(path.extname) strategy ||= from_extension(path.extname)
strategy ||= strategies&.select { |s| s < Directory || s == Fossil }
&.find { |s| s.can_extract?(path) } strategy ||= strategies&.find { |s| (s < Directory || s == Fossil) && s.can_extract?(path) }
else else
strategy ||= from_magic(path) strategy ||= from_magic(path)
strategy ||= from_extension(path.extname) strategy ||= from_extension(path.extname)
@ -135,14 +138,14 @@ module UnpackStrategy
sig { returns(Pathname) } sig { returns(Pathname) }
attr_reader :path attr_reader :path
sig { returns(T.nilable(T::Boolean)) } sig { returns(T::Boolean) }
attr_reader :merge_xattrs attr_reader :merge_xattrs
sig { sig {
params(path: T.any(String, Pathname), ref_type: T.nilable(String), ref: T.nilable(String), params(path: T.any(String, Pathname), ref_type: T.nilable(String), ref: T.nilable(String),
merge_xattrs: T.nilable(T::Boolean)).void merge_xattrs: T::Boolean).void
} }
def initialize(path, ref_type: nil, ref: nil, merge_xattrs: nil) def initialize(path, ref_type: nil, ref: nil, merge_xattrs: false)
@path = T.let(Pathname(path).expand_path, Pathname) @path = T.let(Pathname(path).expand_path, Pathname)
@ref_type = ref_type @ref_type = ref_type
@ref = ref @ref = ref