From 15582dd0310aa9c84017a135beb5ff9039505fdf Mon Sep 17 00:00:00 2001 From: Douglas Eichelberger Date: Thu, 30 Mar 2023 15:04:25 -0700 Subject: [PATCH 1/6] Add Utils popen sigs, enable IO types --- Library/Homebrew/cmd/update-report.rb | 4 +- Library/Homebrew/dev-cmd/contributions.rb | 2 +- Library/Homebrew/dev-cmd/pr-pull.rb | 14 ++--- Library/Homebrew/extend/io.rb | 22 +++---- .../extend/os/linux/extend/ENV/super.rb | 7 ++- Library/Homebrew/keg_relocate.rb | 2 +- Library/Homebrew/language/node.rb | 2 +- Library/Homebrew/missing_formula.rb | 2 +- Library/Homebrew/os/linux.rb | 2 +- Library/Homebrew/os/linux/elf.rb | 2 +- Library/Homebrew/os/mac/sdk.rb | 3 +- Library/Homebrew/utils/popen.rb | 60 ++++++++++++++++--- 12 files changed, 87 insertions(+), 35 deletions(-) diff --git a/Library/Homebrew/cmd/update-report.rb b/Library/Homebrew/cmd/update-report.rb index 41e67f8c9f6e4..4df272dd3f6fe 100644 --- a/Library/Homebrew/cmd/update-report.rb +++ b/Library/Homebrew/cmd/update-report.rb @@ -130,7 +130,7 @@ def output_update_report new_tag = Utils.popen_read( "git", "-C", HOMEBREW_REPOSITORY, "tag", "--list", "--sort=-version:refname", "*.*" - ).lines.first.chomp + ).lines.fetch(0).chomp Settings.write "latesttag", new_tag if new_tag != old_tag @@ -286,6 +286,8 @@ def output_update_report puts new_major_version, new_minor_version, new_patch_version = new_tag.split(".").map(&:to_i) + raise "Invalid new tag: #{new_tag}" if !new_major_version || !new_minor_version || !new_patch_version + old_major_version, old_minor_version = (old_tag.split(".")[0, 2]).map(&:to_i) if old_tag.present? if old_tag.blank? || new_major_version > old_major_version \ || new_minor_version > old_minor_version diff --git a/Library/Homebrew/dev-cmd/contributions.rb b/Library/Homebrew/dev-cmd/contributions.rb index 3abc22f96e921..007562fb05c31 100755 --- a/Library/Homebrew/dev-cmd/contributions.rb +++ b/Library/Homebrew/dev-cmd/contributions.rb @@ -199,7 +199,7 @@ def total(results) sig { params(repo_path: Pathname, person: String, trailer: String, args: Homebrew::CLI::Args).returns(Integer) } def git_log_trailers_cmd(repo_path, person, trailer, args) - cmd = ["git", "-C", repo_path, "log", "--oneline"] + cmd = ["git", "-C", repo_path.to_s, "log", "--oneline"] cmd << "--format='%(trailers:key=#{trailer}:)'" cmd << "--before=#{args.to}" if args.to cmd << "--after=#{args.from}" if args.from diff --git a/Library/Homebrew/dev-cmd/pr-pull.rb b/Library/Homebrew/dev-cmd/pr-pull.rb index f37f993463745..2b6ceafab3f20 100644 --- a/Library/Homebrew/dev-cmd/pr-pull.rb +++ b/Library/Homebrew/dev-cmd/pr-pull.rb @@ -234,13 +234,13 @@ def self.autosquash!(original_commit, tap:, reason: "", verbose: false, resolve: "--reverse", "#{original_commit}..HEAD").lines.map(&:strip) # Generate a bidirectional mapping of commits <=> formula/cask files. - files_to_commits = {} + files_to_commits = T.let({}, T::Hash[String, T::Array[String]]) commits_to_files = commits.to_h do |commit| files = Utils.safe_popen_read("git", "-C", tap.path, "diff-tree", "--diff-filter=AMD", "-r", "--name-only", "#{commit}^", commit).lines.map(&:strip) files.each do |file| files_to_commits[file] ||= [] - files_to_commits[file] << commit + files_to_commits.fetch(file) << commit tap_file = tap.path/file if (tap_file.dirname == tap.formula_dir || tap_file.dirname == tap.cask_dir) && File.extname(file) == ".rb" @@ -266,14 +266,14 @@ def self.autosquash!(original_commit, tap:, reason: "", verbose: false, resolve: next if processed_commits.include? commit files = commits_to_files[commit] - if files.length == 1 && files_to_commits[files.first].length == 1 + if files.length == 1 && files_to_commits.fetch(files.first).length == 1 # If there's a 1:1 mapping of commits to files, just cherry pick and (maybe) reword. reword_package_commit(commit, files.first, path: tap.path, reason: reason, verbose: verbose, resolve: resolve) processed_commits << commit - elsif files.length == 1 && files_to_commits[files.first].length > 1 + elsif files.length == 1 && files_to_commits.fetch(files.first).length > 1 # If multiple commits modify a single file, squash them down into a single commit. file = files.first - commits = files_to_commits[file] + commits = files_to_commits.fetch(file) squash_package_commits(commits, file, path: tap.path, reason: reason, verbose: verbose, resolve: resolve) processed_commits += commits else @@ -336,7 +336,7 @@ def self.changed_packages(tap, original_commit) rescue FormulaUnavailableError nil end - end.compact + end casks = Utils.popen_read("git", "-C", tap.path, "diff-tree", "-r", "--name-only", "--diff-filter=AM", original_commit, "HEAD", "--", tap.cask_dir) @@ -351,7 +351,7 @@ def self.changed_packages(tap, original_commit) nil end end.compact - formulae + casks + T.must(formulae).compact + casks end def self.download_artifact(url, dir, pull_request) diff --git a/Library/Homebrew/extend/io.rb b/Library/Homebrew/extend/io.rb index f5d36dfc84f8b..383b8213c918e 100644 --- a/Library/Homebrew/extend/io.rb +++ b/Library/Homebrew/extend/io.rb @@ -1,4 +1,4 @@ -# typed: false +# typed: true # frozen_string_literal: true class IO @@ -6,17 +6,19 @@ def readline_nonblock(sep = $INPUT_RECORD_SEPARATOR) line = +"" buffer = +"" - loop do - break if buffer == sep + begin + loop do + break if buffer == sep - read_nonblock(1, buffer) - line.concat(buffer) - end + read_nonblock(1, buffer) + line.concat(buffer) + end - line.freeze - rescue IO::WaitReadable, EOFError => e - raise e if line.empty? + line.freeze + rescue IO::WaitReadable, EOFError + raise if line.empty? - line.freeze + line.freeze + end end end diff --git a/Library/Homebrew/extend/os/linux/extend/ENV/super.rb b/Library/Homebrew/extend/os/linux/extend/ENV/super.rb index c0691c0001ea7..f4288627f3053 100644 --- a/Library/Homebrew/extend/os/linux/extend/ENV/super.rb +++ b/Library/Homebrew/extend/os/linux/extend/ENV/super.rb @@ -40,8 +40,11 @@ def homebrew_extra_isystem_paths paths = [] # Add paths for GCC headers when building against glibc@2.13 because we have to use -nostdinc. if deps.any? { |d| d.name == "glibc@2.13" } - gcc_include_dir = Utils.safe_popen_read(cc, "--print-file-name=include").chomp - gcc_include_fixed_dir = Utils.safe_popen_read(cc, "--print-file-name=include-fixed").chomp + env_cc = cc + raise "No CC compiler found in ENV" if env_cc.nil? + + gcc_include_dir = Utils.safe_popen_read(env_cc, "--print-file-name=include").chomp + gcc_include_fixed_dir = Utils.safe_popen_read(env_cc, "--print-file-name=include-fixed").chomp paths << gcc_include_dir << gcc_include_fixed_dir end paths diff --git a/Library/Homebrew/keg_relocate.rb b/Library/Homebrew/keg_relocate.rb index 083529719b93d..f80d6f867f9b8 100644 --- a/Library/Homebrew/keg_relocate.rb +++ b/Library/Homebrew/keg_relocate.rb @@ -338,7 +338,7 @@ def self.text_matches_in_file(file, string, ignores, linked_libraries, formula_a # Some binaries contain strings with lists of files # e.g. `/usr/local/lib/foo:/usr/local/share/foo:/usr/lib/foo` # Each item in the list should be checked separately - match.split(":").each do |sub_match| + T.must(match).split(":").each do |sub_match| # Not all items in the list may be matches next unless sub_match.match? path_regex next if linked_libraries.include? sub_match # Don't bother reporting a string if it was found by otool diff --git a/Library/Homebrew/language/node.rb b/Library/Homebrew/language/node.rb index 4421b0f9e88ac..158a061e05a10 100644 --- a/Library/Homebrew/language/node.rb +++ b/Library/Homebrew/language/node.rb @@ -34,7 +34,7 @@ def self.pack_for_installation output = Utils.popen_read("npm", "pack", "--ignore-scripts") raise "npm failed to pack #{Dir.pwd}" if !$CHILD_STATUS.exitstatus.zero? || output.lines.empty? - output.lines.last.chomp + output.lines.fetch(-1).chomp end def self.setup_npm_environment diff --git a/Library/Homebrew/missing_formula.rb b/Library/Homebrew/missing_formula.rb index 884c3b73fcd36..1f76c805305f0 100644 --- a/Library/Homebrew/missing_formula.rb +++ b/Library/Homebrew/missing_formula.rb @@ -179,7 +179,7 @@ def deleted_reason(name, silent: false) return end - commit_message = commit_message.reject(&:empty?).join("\n ") + commit_message = T.must(commit_message).reject(&:empty?).join("\n ") commit_message.sub!(/ \(#(\d+)\)$/, " (#{tap.issues_url}/\\1)") commit_message.gsub!(/(Closes|Fixes) #(\d+)/, "\\1 #{tap.issues_url}/\\2") diff --git a/Library/Homebrew/os/linux.rb b/Library/Homebrew/os/linux.rb index 2b5df8cfc315f..f3bef19f07452 100644 --- a/Library/Homebrew/os/linux.rb +++ b/Library/Homebrew/os/linux.rb @@ -14,7 +14,7 @@ module Linux def os_version if which("lsb_release") lsb_info = Utils.popen_read("lsb_release", "-a") - description = lsb_info[/^Description:\s*(.*)$/, 1].force_encoding("UTF-8") + description = T.must(lsb_info[/^Description:\s*(.*)$/, 1]).force_encoding("UTF-8") codename = lsb_info[/^Codename:\s*(.*)$/, 1] if codename.blank? || (codename == "n/a") description diff --git a/Library/Homebrew/os/linux/elf.rb b/Library/Homebrew/os/linux/elf.rb index ca7d078b89cc9..4c3c739b31b04 100644 --- a/Library/Homebrew/os/linux/elf.rb +++ b/Library/Homebrew/os/linux/elf.rb @@ -131,7 +131,7 @@ def initialize(path) return if needed.empty? ldd = DevelopmentTools.locate "ldd" - ldd_output = Utils.popen_read(ldd, path.expand_path.to_s).split("\n") + ldd_output = Utils.popen_read(T.must(ldd).to_s, path.expand_path.to_s).split("\n") return unless $CHILD_STATUS.success? ldd_paths = ldd_output.map do |line| diff --git a/Library/Homebrew/os/mac/sdk.rb b/Library/Homebrew/os/mac/sdk.rb index 3fadf31762c60..04e4c10c88940 100644 --- a/Library/Homebrew/os/mac/sdk.rb +++ b/Library/Homebrew/os/mac/sdk.rb @@ -161,7 +161,8 @@ def sdk_prefix # Xcode.prefix is pretty smart, so let's look inside to find the sdk sdk_prefix = "#{Xcode.prefix}/Platforms/MacOSX.platform/Developer/SDKs" # Finally query Xcode itself (this is slow, so check it last) - sdk_platform_path = Utils.popen_read(DevelopmentTools.locate("xcrun"), "--show-sdk-platform-path").chomp + sdk_platform_path = Utils.popen_read(DevelopmentTools.locate("xcrun").to_s, + "--show-sdk-platform-path").chomp sdk_prefix = File.join(sdk_platform_path, "Developer", "SDKs") unless File.directory? sdk_prefix sdk_prefix diff --git a/Library/Homebrew/utils/popen.rb b/Library/Homebrew/utils/popen.rb index 733923257ded5..6a214f0b9e788 100644 --- a/Library/Homebrew/utils/popen.rb +++ b/Library/Homebrew/utils/popen.rb @@ -1,22 +1,50 @@ -# typed: true +# typed: strict # frozen_string_literal: true module Utils + extend T::Sig + IO_DEFAULT_BUFFER_SIZE = 4096 private_constant :IO_DEFAULT_BUFFER_SIZE - def self.popen_read(*args, safe: false, **options, &block) + sig { + params( + arg0: T.any(String, T::Hash[String, String]), + args: String, + safe: T::Boolean, + options: Symbol, + block: T.nilable(T.proc.params(io: IO).void), + ).returns(String) + } + def self.popen_read(arg0, *args, safe: false, **options, &block) output = popen(args, "rb", options, &block) return output if !safe || $CHILD_STATUS.success? raise ErrorDuringExecution.new(args, status: $CHILD_STATUS, output: [[:stdout, output]]) end - def self.safe_popen_read(*args, **options, &block) + sig { + params( + arg0: T.any(String, T::Hash[String, String]), + args: String, + options: Symbol, + block: T.nilable(T.proc.params(io: IO).void), + ).returns(String) + } + def self.safe_popen_read(arg0, *args, **options, &block) popen_read(*args, safe: true, **options, &block) end - def self.popen_write(*args, safe: false, **options) + sig { + params( + arg0: T.any(String, T::Hash[String, String]), + args: String, + safe: T::Boolean, + options: Symbol, + _block: T.proc.params(io: IO).void, + ).returns(String) + } + def self.popen_write(arg0, *args, safe: false, **options, &_block) output = "" popen(args, "w+b", options) do |pipe| # Before we yield to the block, capture as much output as we can @@ -31,7 +59,7 @@ def self.popen_write(*args, safe: false, **options) pipe.wait_readable # Capture the rest of the output - output += pipe.read + output += T.must(pipe.read) output.freeze end return output if !safe || $CHILD_STATUS.success? @@ -39,14 +67,30 @@ def self.popen_write(*args, safe: false, **options) raise ErrorDuringExecution.new(args, status: $CHILD_STATUS, output: [[:stdout, output]]) end - def self.safe_popen_write(*args, **options, &block) + sig { + params( + arg0: T.any(String, T::Hash[String, String]), + args: String, + options: Symbol, + block: T.nilable(T.proc.params(io: IO).void), + ).returns(String) + } + def self.safe_popen_write(arg0, *args, **options, &block) popen_write(*args, safe: true, **options, &block) end - def self.popen(args, mode, options = {}) + sig { + params( + args: T::Array[T.any(String, T::Hash[String, String])], + mode: String, + options: T::Hash[Symbol, T.untyped], + block: T.nilable(T.proc.params(io: IO).void), + ).returns(String) + } + def self.popen(args, mode, options = {}, &block) IO.popen("-", mode) do |pipe| if pipe - return pipe.read unless block_given? + return pipe.read unless block yield pipe else From a4091b4260bb6361f37d1b91ee8925935c35d081 Mon Sep 17 00:00:00 2001 From: Douglas Eichelberger Date: Thu, 30 Mar 2023 15:09:48 -0700 Subject: [PATCH 2/6] Just use a single args parameter for conciseness --- Library/Homebrew/utils/popen.rb | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/Library/Homebrew/utils/popen.rb b/Library/Homebrew/utils/popen.rb index 6a214f0b9e788..763672f3d3b65 100644 --- a/Library/Homebrew/utils/popen.rb +++ b/Library/Homebrew/utils/popen.rb @@ -9,14 +9,13 @@ module Utils sig { params( - arg0: T.any(String, T::Hash[String, String]), - args: String, + args: T.any(String, T::Hash[String, String]), safe: T::Boolean, options: Symbol, block: T.nilable(T.proc.params(io: IO).void), ).returns(String) } - def self.popen_read(arg0, *args, safe: false, **options, &block) + def self.popen_read(*args, safe: false, **options, &block) output = popen(args, "rb", options, &block) return output if !safe || $CHILD_STATUS.success? @@ -25,26 +24,24 @@ def self.popen_read(arg0, *args, safe: false, **options, &block) sig { params( - arg0: T.any(String, T::Hash[String, String]), - args: String, + args: T.any(String, T::Hash[String, String]), options: Symbol, block: T.nilable(T.proc.params(io: IO).void), ).returns(String) } - def self.safe_popen_read(arg0, *args, **options, &block) + def self.safe_popen_read(*args, **options, &block) popen_read(*args, safe: true, **options, &block) end sig { params( - arg0: T.any(String, T::Hash[String, String]), - args: String, + args: T.any(String, T::Hash[String, String]), safe: T::Boolean, options: Symbol, _block: T.proc.params(io: IO).void, ).returns(String) } - def self.popen_write(arg0, *args, safe: false, **options, &_block) + def self.popen_write(*args, safe: false, **options, &_block) output = "" popen(args, "w+b", options) do |pipe| # Before we yield to the block, capture as much output as we can @@ -69,13 +66,12 @@ def self.popen_write(arg0, *args, safe: false, **options, &_block) sig { params( - arg0: T.any(String, T::Hash[String, String]), - args: String, + args: T.any(String, T::Hash[String, String]), options: Symbol, block: T.nilable(T.proc.params(io: IO).void), ).returns(String) } - def self.safe_popen_write(arg0, *args, **options, &block) + def self.safe_popen_write(*args, **options, &block) popen_write(*args, safe: true, **options, &block) end From 80091b8a5818d7b93a44e6fda66a0fc02fbdd457 Mon Sep 17 00:00:00 2001 From: Douglas Eichelberger Date: Thu, 30 Mar 2023 15:56:18 -0700 Subject: [PATCH 3/6] Fix tests --- Library/Homebrew/dev-cmd/contributions.rb | 2 +- Library/Homebrew/extend/git_repository.rb | 2 +- Library/Homebrew/os/linux/elf.rb | 2 +- Library/Homebrew/os/mac/sdk.rb | 3 +-- Library/Homebrew/utils/popen.rb | 28 +++++++++++------------ 5 files changed, 18 insertions(+), 19 deletions(-) diff --git a/Library/Homebrew/dev-cmd/contributions.rb b/Library/Homebrew/dev-cmd/contributions.rb index 007562fb05c31..3abc22f96e921 100755 --- a/Library/Homebrew/dev-cmd/contributions.rb +++ b/Library/Homebrew/dev-cmd/contributions.rb @@ -199,7 +199,7 @@ def total(results) sig { params(repo_path: Pathname, person: String, trailer: String, args: Homebrew::CLI::Args).returns(Integer) } def git_log_trailers_cmd(repo_path, person, trailer, args) - cmd = ["git", "-C", repo_path.to_s, "log", "--oneline"] + cmd = ["git", "-C", repo_path, "log", "--oneline"] cmd << "--format='%(trailers:key=#{trailer}:)'" cmd << "--before=#{args.to}" if args.to cmd << "--after=#{args.from}" if args.from diff --git a/Library/Homebrew/extend/git_repository.rb b/Library/Homebrew/extend/git_repository.rb index a91ec91704529..6e21970e31d80 100644 --- a/Library/Homebrew/extend/git_repository.rb +++ b/Library/Homebrew/extend/git_repository.rb @@ -117,6 +117,6 @@ def popen_git(*args, safe: false, err: nil) raise "Git is unavailable" end - T.unsafe(Utils).popen_read(Utils::Git.git, *args, safe: safe, chdir: self, err: err).chomp.presence + Utils.popen_read(Utils::Git.git, *args, safe: safe, chdir: self, err: err).chomp.presence end end diff --git a/Library/Homebrew/os/linux/elf.rb b/Library/Homebrew/os/linux/elf.rb index 4c3c739b31b04..92212d0d499e8 100644 --- a/Library/Homebrew/os/linux/elf.rb +++ b/Library/Homebrew/os/linux/elf.rb @@ -131,7 +131,7 @@ def initialize(path) return if needed.empty? ldd = DevelopmentTools.locate "ldd" - ldd_output = Utils.popen_read(T.must(ldd).to_s, path.expand_path.to_s).split("\n") + ldd_output = Utils.popen_read(T.must(ldd), path.expand_path.to_s).split("\n") return unless $CHILD_STATUS.success? ldd_paths = ldd_output.map do |line| diff --git a/Library/Homebrew/os/mac/sdk.rb b/Library/Homebrew/os/mac/sdk.rb index 04e4c10c88940..9068964736a46 100644 --- a/Library/Homebrew/os/mac/sdk.rb +++ b/Library/Homebrew/os/mac/sdk.rb @@ -161,8 +161,7 @@ def sdk_prefix # Xcode.prefix is pretty smart, so let's look inside to find the sdk sdk_prefix = "#{Xcode.prefix}/Platforms/MacOSX.platform/Developer/SDKs" # Finally query Xcode itself (this is slow, so check it last) - sdk_platform_path = Utils.popen_read(DevelopmentTools.locate("xcrun").to_s, - "--show-sdk-platform-path").chomp + sdk_platform_path = Utils.popen_read(T.must(DevelopmentTools.locate("xcrun")), "--show-sdk-platform-path").chomp sdk_prefix = File.join(sdk_platform_path, "Developer", "SDKs") unless File.directory? sdk_prefix sdk_prefix diff --git a/Library/Homebrew/utils/popen.rb b/Library/Homebrew/utils/popen.rb index 763672f3d3b65..6dc2ffa920760 100644 --- a/Library/Homebrew/utils/popen.rb +++ b/Library/Homebrew/utils/popen.rb @@ -9,11 +9,11 @@ module Utils sig { params( - args: T.any(String, T::Hash[String, String]), + args: T.any(Pathname, String, T::Hash[String, String]), safe: T::Boolean, - options: Symbol, + options: T.untyped, block: T.nilable(T.proc.params(io: IO).void), - ).returns(String) + ).returns(T.nilable(String)) } def self.popen_read(*args, safe: false, **options, &block) output = popen(args, "rb", options, &block) @@ -24,10 +24,10 @@ def self.popen_read(*args, safe: false, **options, &block) sig { params( - args: T.any(String, T::Hash[String, String]), - options: Symbol, + args: T.any(Pathname, String, T::Hash[String, String]), + options: T.untyped, block: T.nilable(T.proc.params(io: IO).void), - ).returns(String) + ).returns(T.nilable(String)) } def self.safe_popen_read(*args, **options, &block) popen_read(*args, safe: true, **options, &block) @@ -35,11 +35,11 @@ def self.safe_popen_read(*args, **options, &block) sig { params( - args: T.any(String, T::Hash[String, String]), + args: T.any(Pathname, String, T::Hash[String, String]), safe: T::Boolean, - options: Symbol, + options: T.untyped, _block: T.proc.params(io: IO).void, - ).returns(String) + ).returns(T.nilable(String)) } def self.popen_write(*args, safe: false, **options, &_block) output = "" @@ -66,10 +66,10 @@ def self.popen_write(*args, safe: false, **options, &_block) sig { params( - args: T.any(String, T::Hash[String, String]), - options: Symbol, + args: T.any(Pathname, String, T::Hash[String, String]), + options: T.untyped, block: T.nilable(T.proc.params(io: IO).void), - ).returns(String) + ).returns(T.nilable(String)) } def self.safe_popen_write(*args, **options, &block) popen_write(*args, safe: true, **options, &block) @@ -77,11 +77,11 @@ def self.safe_popen_write(*args, **options, &block) sig { params( - args: T::Array[T.any(String, T::Hash[String, String])], + args: T::Array[T.any(Pathname, String, T::Hash[String, String])], mode: String, options: T::Hash[Symbol, T.untyped], block: T.nilable(T.proc.params(io: IO).void), - ).returns(String) + ).returns(T.nilable(String)) } def self.popen(args, mode, options = {}, &block) IO.popen("-", mode) do |pipe| From a20d28f343a8f42a388037cfc41712e6e468ffe1 Mon Sep 17 00:00:00 2001 From: Douglas Eichelberger Date: Thu, 30 Mar 2023 16:38:59 -0700 Subject: [PATCH 4/6] brew style --fix --- Library/Homebrew/os/mac/sdk.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Library/Homebrew/os/mac/sdk.rb b/Library/Homebrew/os/mac/sdk.rb index 9068964736a46..734c90c401245 100644 --- a/Library/Homebrew/os/mac/sdk.rb +++ b/Library/Homebrew/os/mac/sdk.rb @@ -161,7 +161,8 @@ def sdk_prefix # Xcode.prefix is pretty smart, so let's look inside to find the sdk sdk_prefix = "#{Xcode.prefix}/Platforms/MacOSX.platform/Developer/SDKs" # Finally query Xcode itself (this is slow, so check it last) - sdk_platform_path = Utils.popen_read(T.must(DevelopmentTools.locate("xcrun")), "--show-sdk-platform-path").chomp + sdk_platform_path = Utils.popen_read(T.must(DevelopmentTools.locate("xcrun")), + "--show-sdk-platform-path").chomp sdk_prefix = File.join(sdk_platform_path, "Developer", "SDKs") unless File.directory? sdk_prefix sdk_prefix From 4f5fc770f60e093c6b874190c273fa82a977b965 Mon Sep 17 00:00:00 2001 From: Douglas Eichelberger Date: Thu, 30 Mar 2023 17:14:04 -0700 Subject: [PATCH 5/6] rm popen sigs --- Library/Homebrew/cmd/update-report.rb | 4 +- Library/Homebrew/dev-cmd/pr-pull.rb | 14 +++--- .../extend/os/linux/extend/ENV/super.rb | 7 +-- Library/Homebrew/keg_relocate.rb | 2 +- Library/Homebrew/language/node.rb | 2 +- Library/Homebrew/missing_formula.rb | 2 +- Library/Homebrew/os/linux.rb | 2 +- Library/Homebrew/os/linux/elf.rb | 2 +- Library/Homebrew/os/mac/sdk.rb | 3 +- Library/Homebrew/utils/popen.rb | 50 ++----------------- 10 files changed, 21 insertions(+), 67 deletions(-) diff --git a/Library/Homebrew/cmd/update-report.rb b/Library/Homebrew/cmd/update-report.rb index 4df272dd3f6fe..41e67f8c9f6e4 100644 --- a/Library/Homebrew/cmd/update-report.rb +++ b/Library/Homebrew/cmd/update-report.rb @@ -130,7 +130,7 @@ def output_update_report new_tag = Utils.popen_read( "git", "-C", HOMEBREW_REPOSITORY, "tag", "--list", "--sort=-version:refname", "*.*" - ).lines.fetch(0).chomp + ).lines.first.chomp Settings.write "latesttag", new_tag if new_tag != old_tag @@ -286,8 +286,6 @@ def output_update_report puts new_major_version, new_minor_version, new_patch_version = new_tag.split(".").map(&:to_i) - raise "Invalid new tag: #{new_tag}" if !new_major_version || !new_minor_version || !new_patch_version - old_major_version, old_minor_version = (old_tag.split(".")[0, 2]).map(&:to_i) if old_tag.present? if old_tag.blank? || new_major_version > old_major_version \ || new_minor_version > old_minor_version diff --git a/Library/Homebrew/dev-cmd/pr-pull.rb b/Library/Homebrew/dev-cmd/pr-pull.rb index 2b6ceafab3f20..f37f993463745 100644 --- a/Library/Homebrew/dev-cmd/pr-pull.rb +++ b/Library/Homebrew/dev-cmd/pr-pull.rb @@ -234,13 +234,13 @@ def self.autosquash!(original_commit, tap:, reason: "", verbose: false, resolve: "--reverse", "#{original_commit}..HEAD").lines.map(&:strip) # Generate a bidirectional mapping of commits <=> formula/cask files. - files_to_commits = T.let({}, T::Hash[String, T::Array[String]]) + files_to_commits = {} commits_to_files = commits.to_h do |commit| files = Utils.safe_popen_read("git", "-C", tap.path, "diff-tree", "--diff-filter=AMD", "-r", "--name-only", "#{commit}^", commit).lines.map(&:strip) files.each do |file| files_to_commits[file] ||= [] - files_to_commits.fetch(file) << commit + files_to_commits[file] << commit tap_file = tap.path/file if (tap_file.dirname == tap.formula_dir || tap_file.dirname == tap.cask_dir) && File.extname(file) == ".rb" @@ -266,14 +266,14 @@ def self.autosquash!(original_commit, tap:, reason: "", verbose: false, resolve: next if processed_commits.include? commit files = commits_to_files[commit] - if files.length == 1 && files_to_commits.fetch(files.first).length == 1 + if files.length == 1 && files_to_commits[files.first].length == 1 # If there's a 1:1 mapping of commits to files, just cherry pick and (maybe) reword. reword_package_commit(commit, files.first, path: tap.path, reason: reason, verbose: verbose, resolve: resolve) processed_commits << commit - elsif files.length == 1 && files_to_commits.fetch(files.first).length > 1 + elsif files.length == 1 && files_to_commits[files.first].length > 1 # If multiple commits modify a single file, squash them down into a single commit. file = files.first - commits = files_to_commits.fetch(file) + commits = files_to_commits[file] squash_package_commits(commits, file, path: tap.path, reason: reason, verbose: verbose, resolve: resolve) processed_commits += commits else @@ -336,7 +336,7 @@ def self.changed_packages(tap, original_commit) rescue FormulaUnavailableError nil end - end + end.compact casks = Utils.popen_read("git", "-C", tap.path, "diff-tree", "-r", "--name-only", "--diff-filter=AM", original_commit, "HEAD", "--", tap.cask_dir) @@ -351,7 +351,7 @@ def self.changed_packages(tap, original_commit) nil end end.compact - T.must(formulae).compact + casks + formulae + casks end def self.download_artifact(url, dir, pull_request) diff --git a/Library/Homebrew/extend/os/linux/extend/ENV/super.rb b/Library/Homebrew/extend/os/linux/extend/ENV/super.rb index f4288627f3053..c0691c0001ea7 100644 --- a/Library/Homebrew/extend/os/linux/extend/ENV/super.rb +++ b/Library/Homebrew/extend/os/linux/extend/ENV/super.rb @@ -40,11 +40,8 @@ def homebrew_extra_isystem_paths paths = [] # Add paths for GCC headers when building against glibc@2.13 because we have to use -nostdinc. if deps.any? { |d| d.name == "glibc@2.13" } - env_cc = cc - raise "No CC compiler found in ENV" if env_cc.nil? - - gcc_include_dir = Utils.safe_popen_read(env_cc, "--print-file-name=include").chomp - gcc_include_fixed_dir = Utils.safe_popen_read(env_cc, "--print-file-name=include-fixed").chomp + gcc_include_dir = Utils.safe_popen_read(cc, "--print-file-name=include").chomp + gcc_include_fixed_dir = Utils.safe_popen_read(cc, "--print-file-name=include-fixed").chomp paths << gcc_include_dir << gcc_include_fixed_dir end paths diff --git a/Library/Homebrew/keg_relocate.rb b/Library/Homebrew/keg_relocate.rb index f80d6f867f9b8..083529719b93d 100644 --- a/Library/Homebrew/keg_relocate.rb +++ b/Library/Homebrew/keg_relocate.rb @@ -338,7 +338,7 @@ def self.text_matches_in_file(file, string, ignores, linked_libraries, formula_a # Some binaries contain strings with lists of files # e.g. `/usr/local/lib/foo:/usr/local/share/foo:/usr/lib/foo` # Each item in the list should be checked separately - T.must(match).split(":").each do |sub_match| + match.split(":").each do |sub_match| # Not all items in the list may be matches next unless sub_match.match? path_regex next if linked_libraries.include? sub_match # Don't bother reporting a string if it was found by otool diff --git a/Library/Homebrew/language/node.rb b/Library/Homebrew/language/node.rb index 158a061e05a10..4421b0f9e88ac 100644 --- a/Library/Homebrew/language/node.rb +++ b/Library/Homebrew/language/node.rb @@ -34,7 +34,7 @@ def self.pack_for_installation output = Utils.popen_read("npm", "pack", "--ignore-scripts") raise "npm failed to pack #{Dir.pwd}" if !$CHILD_STATUS.exitstatus.zero? || output.lines.empty? - output.lines.fetch(-1).chomp + output.lines.last.chomp end def self.setup_npm_environment diff --git a/Library/Homebrew/missing_formula.rb b/Library/Homebrew/missing_formula.rb index 1f76c805305f0..884c3b73fcd36 100644 --- a/Library/Homebrew/missing_formula.rb +++ b/Library/Homebrew/missing_formula.rb @@ -179,7 +179,7 @@ def deleted_reason(name, silent: false) return end - commit_message = T.must(commit_message).reject(&:empty?).join("\n ") + commit_message = commit_message.reject(&:empty?).join("\n ") commit_message.sub!(/ \(#(\d+)\)$/, " (#{tap.issues_url}/\\1)") commit_message.gsub!(/(Closes|Fixes) #(\d+)/, "\\1 #{tap.issues_url}/\\2") diff --git a/Library/Homebrew/os/linux.rb b/Library/Homebrew/os/linux.rb index f3bef19f07452..2b5df8cfc315f 100644 --- a/Library/Homebrew/os/linux.rb +++ b/Library/Homebrew/os/linux.rb @@ -14,7 +14,7 @@ module Linux def os_version if which("lsb_release") lsb_info = Utils.popen_read("lsb_release", "-a") - description = T.must(lsb_info[/^Description:\s*(.*)$/, 1]).force_encoding("UTF-8") + description = lsb_info[/^Description:\s*(.*)$/, 1].force_encoding("UTF-8") codename = lsb_info[/^Codename:\s*(.*)$/, 1] if codename.blank? || (codename == "n/a") description diff --git a/Library/Homebrew/os/linux/elf.rb b/Library/Homebrew/os/linux/elf.rb index 92212d0d499e8..ca7d078b89cc9 100644 --- a/Library/Homebrew/os/linux/elf.rb +++ b/Library/Homebrew/os/linux/elf.rb @@ -131,7 +131,7 @@ def initialize(path) return if needed.empty? ldd = DevelopmentTools.locate "ldd" - ldd_output = Utils.popen_read(T.must(ldd), path.expand_path.to_s).split("\n") + ldd_output = Utils.popen_read(ldd, path.expand_path.to_s).split("\n") return unless $CHILD_STATUS.success? ldd_paths = ldd_output.map do |line| diff --git a/Library/Homebrew/os/mac/sdk.rb b/Library/Homebrew/os/mac/sdk.rb index 734c90c401245..3fadf31762c60 100644 --- a/Library/Homebrew/os/mac/sdk.rb +++ b/Library/Homebrew/os/mac/sdk.rb @@ -161,8 +161,7 @@ def sdk_prefix # Xcode.prefix is pretty smart, so let's look inside to find the sdk sdk_prefix = "#{Xcode.prefix}/Platforms/MacOSX.platform/Developer/SDKs" # Finally query Xcode itself (this is slow, so check it last) - sdk_platform_path = Utils.popen_read(T.must(DevelopmentTools.locate("xcrun")), - "--show-sdk-platform-path").chomp + sdk_platform_path = Utils.popen_read(DevelopmentTools.locate("xcrun"), "--show-sdk-platform-path").chomp sdk_prefix = File.join(sdk_platform_path, "Developer", "SDKs") unless File.directory? sdk_prefix sdk_prefix diff --git a/Library/Homebrew/utils/popen.rb b/Library/Homebrew/utils/popen.rb index 6dc2ffa920760..733923257ded5 100644 --- a/Library/Homebrew/utils/popen.rb +++ b/Library/Homebrew/utils/popen.rb @@ -1,20 +1,10 @@ -# typed: strict +# typed: true # frozen_string_literal: true module Utils - extend T::Sig - IO_DEFAULT_BUFFER_SIZE = 4096 private_constant :IO_DEFAULT_BUFFER_SIZE - sig { - params( - args: T.any(Pathname, String, T::Hash[String, String]), - safe: T::Boolean, - options: T.untyped, - block: T.nilable(T.proc.params(io: IO).void), - ).returns(T.nilable(String)) - } def self.popen_read(*args, safe: false, **options, &block) output = popen(args, "rb", options, &block) return output if !safe || $CHILD_STATUS.success? @@ -22,26 +12,11 @@ def self.popen_read(*args, safe: false, **options, &block) raise ErrorDuringExecution.new(args, status: $CHILD_STATUS, output: [[:stdout, output]]) end - sig { - params( - args: T.any(Pathname, String, T::Hash[String, String]), - options: T.untyped, - block: T.nilable(T.proc.params(io: IO).void), - ).returns(T.nilable(String)) - } def self.safe_popen_read(*args, **options, &block) popen_read(*args, safe: true, **options, &block) end - sig { - params( - args: T.any(Pathname, String, T::Hash[String, String]), - safe: T::Boolean, - options: T.untyped, - _block: T.proc.params(io: IO).void, - ).returns(T.nilable(String)) - } - def self.popen_write(*args, safe: false, **options, &_block) + def self.popen_write(*args, safe: false, **options) output = "" popen(args, "w+b", options) do |pipe| # Before we yield to the block, capture as much output as we can @@ -56,7 +31,7 @@ def self.popen_write(*args, safe: false, **options, &_block) pipe.wait_readable # Capture the rest of the output - output += T.must(pipe.read) + output += pipe.read output.freeze end return output if !safe || $CHILD_STATUS.success? @@ -64,29 +39,14 @@ def self.popen_write(*args, safe: false, **options, &_block) raise ErrorDuringExecution.new(args, status: $CHILD_STATUS, output: [[:stdout, output]]) end - sig { - params( - args: T.any(Pathname, String, T::Hash[String, String]), - options: T.untyped, - block: T.nilable(T.proc.params(io: IO).void), - ).returns(T.nilable(String)) - } def self.safe_popen_write(*args, **options, &block) popen_write(*args, safe: true, **options, &block) end - sig { - params( - args: T::Array[T.any(Pathname, String, T::Hash[String, String])], - mode: String, - options: T::Hash[Symbol, T.untyped], - block: T.nilable(T.proc.params(io: IO).void), - ).returns(T.nilable(String)) - } - def self.popen(args, mode, options = {}, &block) + def self.popen(args, mode, options = {}) IO.popen("-", mode) do |pipe| if pipe - return pipe.read unless block + return pipe.read unless block_given? yield pipe else From f9e512bf83610299ffe6d011ed2037b8b25846b5 Mon Sep 17 00:00:00 2001 From: Douglas Eichelberger Date: Thu, 30 Mar 2023 17:31:07 -0700 Subject: [PATCH 6/6] Enable update-test types --- Library/Homebrew/dev-cmd/update-test.rb | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/Library/Homebrew/dev-cmd/update-test.rb b/Library/Homebrew/dev-cmd/update-test.rb index e6d09edf565ff..8bb643b21c3fa 100644 --- a/Library/Homebrew/dev-cmd/update-test.rb +++ b/Library/Homebrew/dev-cmd/update-test.rb @@ -1,14 +1,12 @@ -# typed: false +# typed: true # frozen_string_literal: true require "cli/parser" module Homebrew extend T::Sig - module_function - sig { returns(CLI::Parser) } - def update_test_args + def self.update_test_args Homebrew::CLI::Parser.new do description <<~EOS Run a test of `brew update` with a new repository clone. @@ -27,7 +25,7 @@ def update_test_args end end - def update_test + def self.update_test args = update_test_args.parse # Avoid `update-report.rb` tapping Homebrew/homebrew-core @@ -51,10 +49,12 @@ def update_test "master" end - start_commit = nil + # Utils.popen_read returns a String without a block argument, but that isn't easily typed. We thus label this + # as untyped for now. + start_commit = T.let("", T.untyped) end_commit = "HEAD" cd HOMEBREW_REPOSITORY do - start_commit = if (commit = args.commit) + start_commit = if (commit = T.let(args.commit, T.nilable(String))) commit elsif (date = args.before) Utils.popen_read("git", "rev-list", "-n1", "--before=#{date}", "origin/master").chomp @@ -78,7 +78,7 @@ def update_test start_commit = Utils.popen_read("git", "rev-parse", start_commit).chomp odie "Could not find start commit!" if start_commit.empty? - end_commit = Utils.popen_read("git", "rev-parse", end_commit).chomp + end_commit = T.cast(Utils.popen_read("git", "rev-parse", end_commit).chomp, String) odie "Could not find end commit!" if end_commit.empty? if Utils.popen_read("git", "branch", "--list", "master").blank? @@ -114,7 +114,7 @@ def update_test safe_system "git", "reset", "--hard", start_commit # update ENV["PATH"] - ENV["PATH"] = PATH.new(ENV.fetch("PATH")).prepend(curdir/"bin") + ENV["PATH"] = PATH.new(ENV.fetch("PATH")).prepend(curdir/"bin").to_s # run brew help to install portable-ruby (if needed) quiet_system "brew", "help" @@ -139,7 +139,7 @@ def update_test FileUtils.rm_rf "update-test" unless args.keep_tmp? end - def git_tags + def self.git_tags tags = Utils.popen_read("git", "tag", "--list", "--sort=-version:refname") if tags.blank? tags = if (HOMEBREW_REPOSITORY/".git/shallow").exist?