From 15582dd0310aa9c84017a135beb5ff9039505fdf Mon Sep 17 00:00:00 2001 From: Douglas Eichelberger Date: Thu, 30 Mar 2023 15:04:25 -0700 Subject: [PATCH] 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 | 24 ++++---- .../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, 88 insertions(+), 36 deletions(-) diff --git a/Library/Homebrew/cmd/update-report.rb b/Library/Homebrew/cmd/update-report.rb index 41e67f8c9f..4df272dd3f 100644 --- a/Library/Homebrew/cmd/update-report.rb +++ b/Library/Homebrew/cmd/update-report.rb @@ -130,7 +130,7 @@ module Homebrew 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 @@ module Homebrew 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 3abc22f96e..007562fb05 100755 --- a/Library/Homebrew/dev-cmd/contributions.rb +++ b/Library/Homebrew/dev-cmd/contributions.rb @@ -199,7 +199,7 @@ module Homebrew 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 f37f993463..2b6ceafab3 100644 --- a/Library/Homebrew/dev-cmd/pr-pull.rb +++ b/Library/Homebrew/dev-cmd/pr-pull.rb @@ -234,13 +234,13 @@ module Homebrew "--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 @@ module Homebrew 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 @@ module Homebrew 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 @@ module Homebrew 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 f5d36dfc84..383b8213c9 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 @@ class IO line = +"" buffer = +"" - loop do - break if buffer == sep + begin + loop do + break if buffer == sep - read_nonblock(1, buffer) - line.concat(buffer) + read_nonblock(1, buffer) + line.concat(buffer) + end + + line.freeze + rescue IO::WaitReadable, EOFError + raise if line.empty? + + line.freeze end - - line.freeze - rescue IO::WaitReadable, EOFError => e - raise e if line.empty? - - line.freeze 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 c0691c0001..f4288627f3 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 @@ module Superenv 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 083529719b..f80d6f867f 100644 --- a/Library/Homebrew/keg_relocate.rb +++ b/Library/Homebrew/keg_relocate.rb @@ -338,7 +338,7 @@ class Keg # 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 4421b0f9e8..158a061e05 100644 --- a/Library/Homebrew/language/node.rb +++ b/Library/Homebrew/language/node.rb @@ -34,7 +34,7 @@ module Language 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 884c3b73fc..1f76c80530 100644 --- a/Library/Homebrew/missing_formula.rb +++ b/Library/Homebrew/missing_formula.rb @@ -179,7 +179,7 @@ module Homebrew 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 2b5df8cfc3..f3bef19f07 100644 --- a/Library/Homebrew/os/linux.rb +++ b/Library/Homebrew/os/linux.rb @@ -14,7 +14,7 @@ module OS 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 ca7d078b89..4c3c739b31 100644 --- a/Library/Homebrew/os/linux/elf.rb +++ b/Library/Homebrew/os/linux/elf.rb @@ -131,7 +131,7 @@ module ELFShim 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 3fadf31762..04e4c10c88 100644 --- a/Library/Homebrew/os/mac/sdk.rb +++ b/Library/Homebrew/os/mac/sdk.rb @@ -161,7 +161,8 @@ module OS # 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 733923257d..6a214f0b9e 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 @@ module Utils 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 @@ module Utils 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