admin/brew
1
0
Fork 0
Code Issues Packages Projects Releases 6 Wiki Activity

Merge pull request #20613 from Homebrew/dug/typed-system-command

Browse Source 
Enable strict typing in SystemCommand
This commit is contained in:
Mike McQuaid 2025-09-08 07:32:58 +00:00 committed by GitHub
commit efc036f75a
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
19 changed files with 188 additions and 88 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
Library/Homebrew/cask/artifact/abstract_uninstall.rb
View File

@ -172,7 +172,7 @@ module Cask
.stdout.lines.drop(1) # skip stdout column headers
.filter_map do |line|
pid, _state, id = line.chomp.split(/\s+/)
id if pid.to_i.nonzero? && id.match?(regex)
id if pid.to_i.nonzero? && T.must(id).match?(regex)
end
end
@ -460,9 +460,9 @@ module Cask
def trash_paths(*paths, command: nil, **_)
return if paths.empty?
stdout, = system_command HOMEBREW_LIBRARY_PATH/"cask/utils/trash.swift",
args: paths,
print_stderr: Homebrew::EnvConfig.developer?
stdout = system_command(HOMEBREW_LIBRARY_PATH/"cask/utils/trash.swift",
args: paths,
print_stderr: Homebrew::EnvConfig.developer?).stdout
trashed, _, untrashable = stdout.partition("\n")
trashed = trashed.split(":")

3
Library/Homebrew/cask/audit.rb
View File

@ -543,6 +543,7 @@ module Cask
print_stderr: false)
else
add_error "Unknown artifact type: #{artifact.class}", location: url.location
next
end
next false if result.success?
@ -697,7 +698,7 @@ module Cask
add_error "No binaries in App: #{artifact.source}", location: url.location if files.empty?
main_binary = get_plist_main_binary(path)
main_binary ||= files.first
main_binary ||= files.fetch(0)
system_command("lipo", args: ["-archs", main_binary], print_stderr: false)
when Artifact::Binary

2
Library/Homebrew/cask/caskroom.rb
View File

@ -49,7 +49,7 @@ module Cask
SystemCommand.run("/bin/mkdir", args: ["-p", path], sudo:)
SystemCommand.run("/bin/chmod", args: ["g+rwx", path], sudo:)
SystemCommand.run("/usr/sbin/chown", args: [User.current, path], sudo:)
SystemCommand.run("/usr/sbin/chown", args: [User.current.to_s, path], sudo:)
SystemCommand.run("/usr/bin/chgrp", args: ["admin", path], sudo:)
end

2
Library/Homebrew/extend/os/linux/system_config.rb
View File

@ -36,7 +36,7 @@ module OS
end
def host_ruby_version
out, _, status = system_command(HOST_RUBY_PATH, args: ["-e", "puts RUBY_VERSION"], print_stderr: false)
out, _, status = system_command(HOST_RUBY_PATH, args: ["-e", "puts RUBY_VERSION"], print_stderr: false).to_a
return "N/A" unless status.success?
out

12
Library/Homebrew/extend/pathname.rb
View File

@ -422,11 +422,13 @@ class Pathname
sig { returns(T::Array[String]) }
def zipinfo
@zipinfo ||= T.let(nil, T.nilable(String))
@zipinfo ||= system_command("zipinfo", args: ["-1", self], print_stderr: false)
.stdout
.encode(Encoding::UTF_8, invalid: :replace)
.split("\n")
@zipinfo ||= T.let(
system_command("zipinfo", args: ["-1", self], print_stderr: false)
.stdout
.encode(Encoding::UTF_8, invalid: :replace)
.split("\n"),
T.nilable(T::Array[String]),
)
end
private

4
Library/Homebrew/livecheck/strategy/git.rb
View File

@ -129,9 +129,9 @@ module Homebrew
print_stderr: false,
debug: false,
verbose: false,
)
).to_a
tags_data = { tags: [] }
tags_data = { tags: T.let([], T::Array[String]) }
tags_data[:messages] = stderr.split("\n") if stderr.present?
return tags_data if stdout.blank?

2
Library/Homebrew/readall.rb
View File

@ -125,7 +125,7 @@ module Readall
sig { params(filename: Pathname).returns(T::Boolean) }
private_class_method def self.syntax_errors_or_warnings?(filename)
# Retrieve messages about syntax errors/warnings printed to `$stderr`.
_, err, status = system_command(RUBY_PATH, args: ["-c", "-w", filename], print_stderr: false)
_, err, status = system_command(RUBY_PATH, args: ["-c", "-w", filename], print_stderr: false).to_a
# Ignore unnecessary warning about named capture conflicts.
# See https://bugs.ruby-lang.org/issues/12359.

2
Library/Homebrew/services/system/systemctl.rb
View File

@ -34,7 +34,7 @@ module Homebrew
private_class_method def self._run(*args, mode:)
require "system_command"
result = SystemCommand.run(executable,
result = SystemCommand.run(T.must(executable),
args: [scope, *args.map(&:to_s)],
print_stdout: mode == :default,
print_stderr: mode == :default,

196
Library/Homebrew/system_command.rb
View File

@ -1,4 +1,4 @@
# typed: true # rubocop:todo Sorbet/StrictSigil
# typed: strict
# frozen_string_literal: true
require "plist"
@ -21,33 +21,119 @@ class SystemCommand
# Run a fallible system command.
#
# @api internal
def system_command(executable, **options)
SystemCommand.run(executable, **options)
sig {
params(
executable: T.any(String, Pathname),
args: T::Array[T.any(String, Integer, Float, Pathname, URI::Generic)],
sudo: T::Boolean,
sudo_as_root: T::Boolean,
env: T::Hash[String, String],
input: T.any(String, T::Array[String]),
must_succeed: T::Boolean,
print_stdout: T.any(T::Boolean, Symbol),
print_stderr: T.any(T::Boolean, Symbol),
debug: T.nilable(T::Boolean),
verbose: T.nilable(T::Boolean),
secrets: T.any(String, T::Array[String]),
chdir: T.any(String, Pathname),
reset_uid: T::Boolean,
timeout: T.nilable(T.any(Integer, Float)),
).returns(SystemCommand::Result)
}
def system_command(executable, args: [], sudo: false, sudo_as_root: false, env: {}, input: [],
must_succeed: false, print_stdout: false, print_stderr: true, debug: nil, verbose: nil,
secrets: [], chdir: T.unsafe(nil), reset_uid: false, timeout: nil)
SystemCommand.run(executable, args:, sudo:, sudo_as_root:, env:, input:, must_succeed:, print_stdout:,
print_stderr:, debug:, verbose:, secrets:, chdir:, reset_uid:, timeout:)
end
# Run an infallible system command.
#
# @api internal
def system_command!(command, **options)
SystemCommand.run!(command, **options)
sig {
params(
executable: T.any(String, Pathname),
args: T::Array[T.any(String, Integer, Float, Pathname, URI::Generic)],
sudo: T::Boolean,
sudo_as_root: T::Boolean,
env: T::Hash[String, String],
input: T.any(String, T::Array[String]),
print_stdout: T.any(T::Boolean, Symbol),
print_stderr: T.any(T::Boolean, Symbol),
debug: T.nilable(T::Boolean),
verbose: T.nilable(T::Boolean),
secrets: T.any(String, T::Array[String]),
chdir: T.any(String, Pathname),
reset_uid: T::Boolean,
timeout: T.nilable(T.any(Integer, Float)),
).returns(SystemCommand::Result)
}
def system_command!(executable, args: [], sudo: false, sudo_as_root: false, env: {}, input: [],
print_stdout: false, print_stderr: true, debug: nil, verbose: nil, secrets: [],
chdir: T.unsafe(nil), reset_uid: false, timeout: nil)
SystemCommand.run!(executable, args:, sudo:, sudo_as_root:, env:, input:, print_stdout:,
print_stderr:, debug:, verbose:, secrets:, chdir:, reset_uid:, timeout:)
end
end
include Context
def self.run(executable, **options)
new(executable, **options).run!
sig {
params(
executable: T.any(String, Pathname),
args: T::Array[T.any(String, Integer, Float, Pathname, URI::Generic)],
sudo: T::Boolean,
sudo_as_root: T::Boolean,
env: T::Hash[String, String],
input: T.any(String, T::Array[String]),
must_succeed: T::Boolean,
print_stdout: T.any(T::Boolean, Symbol),
print_stderr: T.any(T::Boolean, Symbol),
debug: T.nilable(T::Boolean),
verbose: T.nilable(T::Boolean),
secrets: T.any(String, T::Array[String]),
chdir: T.any(NilClass, String, Pathname),
reset_uid: T::Boolean,
timeout: T.nilable(T.any(Integer, Float)),
).returns(SystemCommand::Result)
}
def self.run(executable, args: [], sudo: false, sudo_as_root: false, env: {}, input: [], must_succeed: false,
print_stdout: false, print_stderr: true, debug: nil, verbose: nil, secrets: [], chdir: nil,
reset_uid: false, timeout: nil)
new(executable, args:, sudo:, sudo_as_root:, env:, input:, must_succeed:, print_stdout:, print_stderr:, debug:,
verbose:, secrets:, chdir:, reset_uid:, timeout:).run!
end
def self.run!(command, **options)
run(command, **options, must_succeed: true)
sig {
params(
executable: T.any(String, Pathname),
args: T::Array[T.any(String, Integer, Float, Pathname, URI::Generic)],
sudo: T::Boolean,
sudo_as_root: T::Boolean,
env: T::Hash[String, String],
input: T.any(String, T::Array[String]),
print_stdout: T.any(T::Boolean, Symbol),
print_stderr: T.any(T::Boolean, Symbol),
debug: T.nilable(T::Boolean),
verbose: T.nilable(T::Boolean),
secrets: T.any(String, T::Array[String]),
chdir: T.any(NilClass, String, Pathname),
reset_uid: T::Boolean,
timeout: T.nilable(T.any(Integer, Float)),
).returns(SystemCommand::Result)
}
def self.run!(executable, args: [], sudo: false, sudo_as_root: false, env: {}, input: [], print_stdout: false,
print_stderr: true, debug: nil, verbose: nil, secrets: [], chdir: nil, reset_uid: false, timeout: nil)
run(executable, args:, sudo:, sudo_as_root:, env:, input:, must_succeed: true, print_stdout:, print_stderr:,
debug:, verbose:, secrets:, chdir:, reset_uid:, timeout:)
end
sig { returns(SystemCommand::Result) }
def run!
$stderr.puts redact_secrets(command.shelljoin.gsub('\=', "="), @secrets) if verbose? && debug?
@output = []
@output = T.let([], T.nilable(T::Array[[Symbol, String]]))
@output = T.must(@output)
each_output_line do |type, line|
case type
@ -70,7 +156,7 @@ class SystemCommand
end
end
result = Result.new(command, @output, @status, secrets: @secrets)
result = Result.new(command, @output, T.must(@status), secrets: @secrets)
result.assert_success! if must_succeed?
result
end
@ -78,7 +164,7 @@ class SystemCommand
sig {
params(
executable: T.any(String, Pathname),
args: T::Array[T.any(String, Integer, Float, URI::Generic)],
args: T::Array[T.any(String, Integer, Float, Pathname, URI::Generic)],
sudo: T::Boolean,
sudo_as_root: T::Boolean,
env: T::Hash[String, String],
@ -89,28 +175,14 @@ class SystemCommand
debug: T.nilable(T::Boolean),
verbose: T.nilable(T::Boolean),
secrets: T.any(String, T::Array[String]),
chdir: T.any(String, Pathname),
chdir: T.any(NilClass, String, Pathname),
reset_uid: T::Boolean,
timeout: T.nilable(T.any(Integer, Float)),
).void
}
def initialize(
executable,
args: [],
sudo: false,
sudo_as_root: false,
env: {},
input: [],
must_succeed: false,
print_stdout: false,
print_stderr: true,
debug: nil,
verbose: T.unsafe(nil),
secrets: [],
chdir: T.unsafe(nil),
reset_uid: false,
timeout: nil
)
def initialize(executable, args: [], sudo: false, sudo_as_root: false, env: {}, input: [], must_succeed: false,
print_stdout: false, print_stderr: true, debug: nil, verbose: nil, secrets: [], chdir: nil,
reset_uid: false, timeout: nil)
require "extend/ENV"
@executable = executable
@args = args
@ -132,13 +204,13 @@ class SystemCommand
raise ArgumentError, "Invalid variable name: #{name}"
end
@env = env
@input = Array(input)
@input = T.let(Array(input), T::Array[String])
@must_succeed = must_succeed
@print_stdout = print_stdout
@print_stderr = print_stderr
@debug = debug
@verbose = verbose
@secrets = (Array(secrets) + ENV.sensitive_environment.values).uniq
@secrets = T.let((Array(secrets) + ENV.sensitive_environment.values).uniq, T::Array[String])
@chdir = chdir
@reset_uid = reset_uid
@timeout = timeout
@ -151,7 +223,20 @@ class SystemCommand
private
attr_reader :executable, :args, :input, :chdir, :env
sig { returns(T.any(Pathname, String)) }
attr_reader :executable
sig { returns(T::Array[T.any(String, Integer, Float, Pathname, URI::Generic)]) }
attr_reader :args
sig { returns(T::Array[String]) }
attr_reader :input
sig { returns(T.any(NilClass, String, Pathname)) }
attr_reader :chdir
sig { returns(T::Hash[String, String]) }
attr_reader :env
sig { returns(T::Boolean) }
def must_succeed? = @must_succeed
@ -227,15 +312,13 @@ class SystemCommand
sig { returns(T::Array[String]) }
def expanded_args
@expanded_args ||= args.map do |arg|
if arg.respond_to?(:to_path)
@expanded_args ||= T.let(args.map do |arg|
if arg.is_a?(Pathname)
File.absolute_path(arg)
elsif arg.is_a?(Integer) || arg.is_a?(Float) || arg.is_a?(URI::Generic)
arg.to_s
else
arg.to_str
arg.to_s
end
end
end, T.nilable(T::Array[String]))
end
class ProcessTerminatedInterrupt < StandardError; end
@ -275,7 +358,7 @@ class SystemCommand
end_time = Time.now + @timeout if @timeout
raise Timeout::Error if raw_wait_thr.join(Utils::Timer.remaining(end_time)).nil?
@status = raw_wait_thr.value
@status = T.let(raw_wait_thr.value, T.nilable(Process::Status))
rescue Interrupt
Process.kill("INT", raw_wait_thr.pid) if raw_wait_thr && !sudo?
raise Interrupt
@ -383,7 +466,14 @@ class SystemCommand
include Context
include Utils::Output::Mixin
attr_accessor :command, :status, :exit_status
sig { returns(T::Array[String]) }
attr_accessor :command
sig { returns(Process::Status) }
attr_accessor :status
sig { returns(T.nilable(Integer)) }
attr_accessor :exit_status
sig {
params(
@ -397,7 +487,7 @@ class SystemCommand
@command = command
@output = output
@status = status
@exit_status = status.exitstatus
@exit_status = T.let(status.exitstatus, T.nilable(Integer))
@secrets = secrets
end
@ -410,22 +500,21 @@ class SystemCommand
sig { returns(String) }
def stdout
@stdout ||= @output.select { |type,| type == :stdout }
.map { |_, line| line }
.join
@stdout ||= T.let(@output.select { |type,| type == :stdout }
.map { |_, line| line }
.join, T.nilable(String))
end
sig { returns(String) }
def stderr
@stderr ||= @output.select { |type,| type == :stderr }
.map { |_, line| line }
.join
@stderr ||= T.let(@output.select { |type,| type == :stderr }
.map { |_, line| line }
.join, T.nilable(String))
end
sig { returns(String) }
def merged_output
@merged_output ||= @output.map { |_, line| line }
.join
@merged_output ||= T.let(@output.map { |_, line| line }.join, T.nilable(String))
end
sig { returns(T::Boolean) }
@ -439,10 +528,11 @@ class SystemCommand
def to_ary
[stdout, stderr, status]
end
alias to_a to_ary
sig { returns(T.nilable(T.any(Array, Hash))) }
sig { returns(T.untyped) }
def plist
@plist ||= begin
@plist ||= T.let(begin
output = stdout
output = output.sub(/\A(.*?)(\s*<\?\s*xml)/m) do
@ -456,7 +546,7 @@ class SystemCommand
end
Plist.parse_xml(output, marshal: false)
end
end, T.untyped)
end
sig { params(garbage: String).void }

2
Library/Homebrew/system_config.rb
View File

@ -102,7 +102,7 @@ module SystemConfig
sig { returns(String) }
def describe_curl
out, = system_command(Utils::Curl.curl_executable, args: ["--version"], verbose: false)
out = system_command(Utils::Curl.curl_executable, args: ["--version"], verbose: false).stdout
match_data = /^curl (?<curl_version>[\d.]+)/.match(out)
if match_data

3
Library/Homebrew/test/utils/user_spec.rb
View File

@ -11,7 +11,8 @@ RSpec.describe User do
before do
allow(SystemCommand).to receive(:run)
.with("who", any_args)
.and_return([who_output, "", instance_double(Process::Status, success?: true)])
.and_return(instance_double(SystemCommand::Result,
to_a: [who_output, "", instance_double(Process::Status, success?: true)]))
end
context "when the current user is in a console session" do

6
Library/Homebrew/unpack_strategy/dmg.rb
View File

@ -155,7 +155,7 @@ module UnpackStrategy
filelist.close
system_command! "mkbom",
args: ["-s", "-i", filelist.path, "--", bomfile.path],
args: ["-s", "-i", T.must(filelist.path), "--", T.must(bomfile.path)],
verbose:
end
@ -179,8 +179,8 @@ module UnpackStrategy
sig { override.params(path: Pathname).returns(T::Boolean) }
def self.can_extract?(path)
stdout, _, status = system_command("hdiutil", args: ["imageinfo", "-format", path], print_stderr: false)
status.success? && !stdout.empty?
stdout, _, status = system_command("hdiutil", args: ["imageinfo", "-format", path], print_stderr: false).to_a
(status.success? && !stdout.empty?) || false
end
private

4
Library/Homebrew/unpack_strategy/tar.rb
View File

@ -29,8 +29,8 @@ module UnpackStrategy
return false unless [Bzip2, Gzip, Lzip, Xz, Zstd].any? { |s| s.can_extract?(path) }
# Check if `tar` can list the contents, then it can also extract it.
stdout, _, status = system_command("tar", args: ["--list", "--file", path], print_stderr: false)
status.success? && !stdout.empty?
stdout, _, status = system_command("tar", args: ["--list", "--file", path], print_stderr: false).to_a
(status.success? && !stdout.empty?) || false
end
private

2
Library/Homebrew/utils/curl.rb
View File

@ -183,7 +183,7 @@ module Utils
return result if result.success? || args.include?("--http1.1")
raise Timeout::Error, result.stderr.lines.last.chomp if timeout && result.status.exitstatus == 28
raise Timeout::Error, result.stderr.lines.fetch(-1).chomp if timeout && result.status.exitstatus == 28
# Error in the HTTP2 framing layer
if result.exit_status == 16

2
Library/Homebrew/utils/git.rb
View File

@ -17,7 +17,7 @@ module Utils
def self.version
return @version if defined?(@version)
stdout, _, status = system_command(git, args: ["--version"], verbose: false, print_stderr: false)
stdout, _, status = system_command(git, args: ["--version"], verbose: false, print_stderr: false).to_a
@version = status.success? ? stdout.chomp[/git version (\d+(?:\.\d+)*)/, 1] : nil
end

8
Library/Homebrew/utils/github/api.rb
View File

@ -163,10 +163,10 @@ module GitHub
"PATH" => PATH.new(HOMEBREW_PREFIX/"opt/gh/bin", ENV.fetch("PATH")),
"HOME" => Utils::UID.uid_home,
}.compact
gh_out, _, result = system_command "gh",
gh_out, _, result = system_command("gh",
args: ["auth", "token", "--hostname", "github.com"],
env:,
print_stderr: false
print_stderr: false).to_a
return unless result.success?
gh_out.chomp.presence
@ -179,11 +179,11 @@ module GitHub
def self.keychain_username_password
require "utils/uid"
Utils::UID.drop_euid do
git_credential_out, _, result = system_command "git",
git_credential_out, _, result = system_command("git",
args: ["credential-osxkeychain", "get"],
input: ["protocol=https\n", "host=github.com\n"],
env: { "HOME" => Utils::UID.uid_home }.compact,
print_stderr: false
print_stderr: false).to_a
return unless result.success?
git_credential_out.force_encoding("ASCII-8BIT")

7
Library/Homebrew/utils/svn.rb
View File

@ -20,7 +20,8 @@ module Utils
def version
return @version if defined?(@version)
stdout, _, status = system_command(HOMEBREW_SHIMS_PATH/"shared/svn", args: ["--version"], print_stderr: false)
stdout, _, status = system_command(HOMEBREW_SHIMS_PATH/"shared/svn", args: ["--version"],
print_stderr: false).to_a
@version = T.let(status.success? ? stdout.chomp[/svn, version (\d+(?:\.\d+)*)/, 1] : nil, T.nilable(String))
end
@ -29,8 +30,8 @@ module Utils
return true unless available?
args = ["ls", url, "--depth", "empty"]
_, stderr, status = system_command("svn", args:, print_stderr: false)
return status.success? unless stderr.include?("certificate verification failed")
_, stderr, status = system_command("svn", args:, print_stderr: false).to_a
return !!status.success? unless stderr.include?("certificate verification failed")
# OK to unconditionally trust here because we're just checking if a URL exists.
system_command("svn", args: args.concat(invalid_cert_flags), print_stderr: false).success?

3
Library/Homebrew/utils/tar.rb
View File

@ -34,7 +34,8 @@ module Utils
path = Pathname.new(path)
return unless TAR_FILE_EXTENSIONS.include? path.extname
stdout, _, status = system_command(executable, args: ["--list", "--file", path], print_stderr: false)
stdout, _, status = system_command(T.must(executable), args: ["--list", "--file", path],
print_stderr: false).to_a
odie "#{path} is not a valid tar file!" if !status.success? || stdout.blank?
end

8
Library/Homebrew/utils/user.rb
View File

@ -13,12 +13,12 @@ class User < SimpleDelegator
# Return whether the user has an active GUI session.
sig { returns(T::Boolean) }
def gui?
out, _, status = system_command "who"
out, _, status = system_command("who").to_a
return false unless status.success?
out.lines
.map(&:split)
.any? { |user, type,| user == self && type == "console" }
.any? { |user, type,| to_s == user && type == "console" }
end
# Return the current user.
@ -31,4 +31,8 @@ class User < SimpleDelegator
@current = T.let(new(pwuid.name), T.nilable(T.attached_class))
end
# This explicit delegator exists to make to_s visible to sorbet.
sig { returns(String) }
def to_s = __getobj__.to_s
end