diff --git a/Library/Homebrew/context.rb b/Library/Homebrew/context.rb index 60c354af1e..7655c8eaa7 100644 --- a/Library/Homebrew/context.rb +++ b/Library/Homebrew/context.rb @@ -34,15 +34,15 @@ module Context end def debug? - @debug + @debug == true end def quiet? - @quiet + @quiet == true end def verbose? - @verbose + @verbose == true end end diff --git a/Library/Homebrew/sorbet/plugins/unpack_strategy_magic.rb b/Library/Homebrew/sorbet/plugins/using.rb similarity index 57% rename from Library/Homebrew/sorbet/plugins/unpack_strategy_magic.rb rename to Library/Homebrew/sorbet/plugins/using.rb index d5c83fa8b5..e1bf761b29 100644 --- a/Library/Homebrew/sorbet/plugins/unpack_strategy_magic.rb +++ b/Library/Homebrew/sorbet/plugins/using.rb @@ -3,7 +3,8 @@ source = ARGV[5] -/\busing +Magic\b/.match(source) do |_| +case source[/\Ausing\s+(.*)\Z/, 1] +when "Magic" puts <<-RUBY # typed: strict @@ -18,4 +19,13 @@ source = ARGV[5] def zipinfo; end end RUBY +when "HashValidator" + puts <<-RUBY + # typed: strict + + class ::Hash + sig { params(valid_keys: T.untyped).void } + def assert_valid_keys!(*valid_keys); end + end + RUBY end diff --git a/Library/Homebrew/sorbet/rbi/hidden-definitions/hidden.rbi b/Library/Homebrew/sorbet/rbi/hidden-definitions/hidden.rbi index 6d06779583..a9699d5831 100644 --- a/Library/Homebrew/sorbet/rbi/hidden-definitions/hidden.rbi +++ b/Library/Homebrew/sorbet/rbi/hidden-definitions/hidden.rbi @@ -28559,6 +28559,7 @@ class Socket IPV6_PATHMTU = ::T.let(nil, ::T.untyped) IPV6_RECVPATHMTU = ::T.let(nil, ::T.untyped) IPV6_USE_MIN_MTU = ::T.let(nil, ::T.untyped) + IP_DONTFRAG = ::T.let(nil, ::T.untyped) IP_PORTRANGE = ::T.let(nil, ::T.untyped) IP_RECVDSTADDR = ::T.let(nil, ::T.untyped) IP_RECVIF = ::T.let(nil, ::T.untyped) @@ -28650,6 +28651,7 @@ module Socket::Constants IPV6_PATHMTU = ::T.let(nil, ::T.untyped) IPV6_RECVPATHMTU = ::T.let(nil, ::T.untyped) IPV6_USE_MIN_MTU = ::T.let(nil, ::T.untyped) + IP_DONTFRAG = ::T.let(nil, ::T.untyped) IP_PORTRANGE = ::T.let(nil, ::T.untyped) IP_RECVDSTADDR = ::T.let(nil, ::T.untyped) IP_RECVIF = ::T.let(nil, ::T.untyped) @@ -29210,6 +29212,11 @@ end class SynchronizedDelegator end +class SystemCommand::Result + extend ::T::Private::Methods::MethodHooks + extend ::T::Private::Methods::SingletonMethodHooks +end + class SystemCommand extend ::T::Private::Methods::MethodHooks extend ::T::Private::Methods::SingletonMethodHooks diff --git a/Library/Homebrew/sorbet/triggers.yml b/Library/Homebrew/sorbet/triggers.yml index 115f740ea8..747e297c2e 100644 --- a/Library/Homebrew/sorbet/triggers.yml +++ b/Library/Homebrew/sorbet/triggers.yml @@ -2,6 +2,6 @@ ruby_extra_args: - --disable-gems triggers: - using: sorbet/plugins/unpack_strategy_magic.rb + using: sorbet/plugins/using.rb attr_predicate: sorbet/plugins/attr_predicate.rb delegate: sorbet/plugins/delegate.rb diff --git a/Library/Homebrew/system_command.rb b/Library/Homebrew/system_command.rb index a9ef6cf6b5..33ce5069bc 100644 --- a/Library/Homebrew/system_command.rb +++ b/Library/Homebrew/system_command.rb @@ -9,7 +9,6 @@ require "shellwords" require "extend/io" require "extend/predicable" require "extend/hash_validator" -using HashValidator # Class for running sub-processes and capturing their output and exit status. # @@ -17,14 +16,16 @@ using HashValidator class SystemCommand extend T::Sig + using HashValidator + # Helper functions for calling {SystemCommand.run}. module Mixin def system_command(*args) - SystemCommand.run(*args) + T.unsafe(SystemCommand).run(*args) end def system_command!(*args) - SystemCommand.run!(*args) + T.unsafe(SystemCommand).run!(*args) end end @@ -34,11 +35,11 @@ class SystemCommand attr_reader :pid def self.run(executable, **options) - new(executable, **options).run! + T.unsafe(self).new(executable, **options).run! end def self.run!(command, **options) - run(command, **options, must_succeed: true) + T.unsafe(self).run(command, **options, must_succeed: true) end sig { returns(SystemCommand::Result) } @@ -63,45 +64,61 @@ class SystemCommand result end + sig do + params( + executable: T.any(String, Pathname), + args: T::Array[T.any(String, Integer, Float, URI::Generic)], + sudo: T::Boolean, + env: T::Hash[String, String], + input: T.any(String, T::Array[String]), + must_succeed: T::Boolean, + print_stdout: T::Boolean, + print_stderr: T::Boolean, + verbose: T::Boolean, + secrets: T::Array[String], + chdir: T.any(String, Pathname), + ).void + end def initialize(executable, args: [], sudo: false, env: {}, input: [], must_succeed: false, - print_stdout: false, print_stderr: true, verbose: false, secrets: [], **options) + print_stdout: false, print_stderr: true, verbose: false, secrets: [], chdir: T.unsafe(nil)) require "extend/ENV" @executable = executable @args = args @sudo = sudo - @input = Array(input) - @print_stdout = print_stdout - @print_stderr = print_stderr - @verbose = verbose - @secrets = (Array(secrets) + ENV.sensitive_environment.values).uniq - @must_succeed = must_succeed - options.assert_valid_keys!(:chdir) - @options = options - @env = env - - @env.each_key do |name| + env.each_key do |name| next if /^[\w&&\D]\w*$/.match?(name) raise ArgumentError, "Invalid variable name: '#{name}'" end + @env = env + @input = Array(input) + @must_succeed = must_succeed + @print_stdout = print_stdout + @print_stderr = print_stderr + @verbose = verbose + @secrets = (Array(secrets) + ENV.sensitive_environment.values).uniq + @chdir = chdir end + sig { returns(T::Array[String]) } def command [*sudo_prefix, *env_args, executable.to_s, *expanded_args] end private - attr_reader :executable, :args, :input, :options, :env + attr_reader :executable, :args, :input, :chdir, :env attr_predicate :sudo?, :print_stdout?, :print_stderr?, :must_succeed? + sig { returns(T::Boolean) } def verbose? return super if @verbose.nil? @verbose end + sig { returns(T::Array[String]) } def env_args set_variables = env.compact.map do |name, value| sanitized_name = Shellwords.escape(name) @@ -114,6 +131,7 @@ class SystemCommand ["/usr/bin/env", *set_variables] end + sig { returns(T::Array[String]) } def sudo_prefix return [] unless sudo? @@ -121,11 +139,12 @@ class SystemCommand ["/usr/bin/sudo", *askpass_flags, "-E", "--"] end + sig { returns(T::Array[String]) } def expanded_args @expanded_args ||= args.map do |arg| if arg.respond_to?(:to_path) File.absolute_path(arg) - elsif arg.is_a?(Integer) || arg.is_a?(Float) || arg.is_a?(URI) + elsif arg.is_a?(Integer) || arg.is_a?(Float) || arg.is_a?(URI::Generic) arg.to_s else arg.to_str @@ -137,7 +156,7 @@ class SystemCommand executable, *args = command raw_stdin, raw_stdout, raw_stderr, raw_wait_thr = - Open3.popen3(env, [executable, executable], *args, **options) + T.unsafe(Open3).popen3(env, [executable, executable], *args, **{ chdir: chdir }.compact) @pid = raw_wait_thr.pid write_input_to(raw_stdin) @@ -158,7 +177,7 @@ class SystemCommand loop do readable_sources, = IO.select(sources) - readable_sources = readable_sources.reject(&:eof?) + readable_sources = T.must(readable_sources).reject(&:eof?) break if readable_sources.empty? @@ -176,10 +195,20 @@ class SystemCommand # Result containing the output and exit status of a finished sub-process. class Result + extend T::Sig + include Context attr_accessor :command, :status, :exit_status + sig do + params( + command: T::Array[String], + output: T::Array[[Symbol, String]], + status: Process::Status, + secrets: T::Array[String], + ).void + end def initialize(command, output, status, secrets:) @command = command @output = output @@ -188,57 +217,65 @@ class SystemCommand @secrets = secrets end + sig { void } def assert_success! return if @status.success? raise ErrorDuringExecution.new(command, status: @status, output: @output, secrets: @secrets) end + sig { returns(String) } def stdout @stdout ||= @output.select { |type,| type == :stdout } .map { |_, line| line } .join end + sig { returns(String) } def stderr @stderr ||= @output.select { |type,| type == :stderr } .map { |_, line| line } .join end + sig { returns(String) } def merged_output @merged_output ||= @output.map { |_, line| line } .join end + sig { returns(T::Boolean) } def success? return false if @exit_status.nil? @exit_status.zero? end + sig { returns([String, String, Process::Status]) } def to_ary [stdout, stderr, status] end + sig { returns(T.nilable(T.any(Array, Hash))) } def plist @plist ||= begin output = stdout - if /\A(?.*?)<\?\s*xml/m =~ output - output = output.sub(/\A#{Regexp.escape(garbage)}/m, "") - warn_plist_garbage(garbage) + output = output.sub(/\A(.*?)(\s*<\?\s*xml)/m) do + warn_plist_garbage(T.must(Regexp.last_match(1))) + Regexp.last_match(2) end - if %r{<\s*/\s*plist\s*>(?.*?)\Z}m =~ output - output = output.sub(/#{Regexp.escape(garbage)}\Z/, "") - warn_plist_garbage(garbage) + output = output.sub(%r{(<\s*/\s*plist\s*>\s*)(.*?)\Z}m) do + warn_plist_garbage(T.must(Regexp.last_match(2))) + Regexp.last_match(1) end Plist.parse_xml(output) end end + sig { params(garbage: String).void } def warn_plist_garbage(garbage) return unless verbose? return unless garbage.match?(/\S/) diff --git a/Library/Homebrew/test/cask/artifact/shared_examples/uninstall_zap.rb b/Library/Homebrew/test/cask/artifact/shared_examples/uninstall_zap.rb index fc91356cfb..3d97672cc4 100644 --- a/Library/Homebrew/test/cask/artifact/shared_examples/uninstall_zap.rb +++ b/Library/Homebrew/test/cask/artifact/shared_examples/uninstall_zap.rb @@ -8,7 +8,7 @@ shared_examples "#uninstall_phase or #zap_phase" do let(:artifact_dsl_key) { described_class.dsl_key } let(:artifact) { cask.artifacts.find { |a| a.is_a?(described_class) } } - let(:fake_system_command) { FakeSystemCommand } + let(:fake_system_command) { class_double(SystemCommand) } context "using :launchctl" do let(:cask) { Cask::CaskLoader.load(cask_path("with-#{artifact_dsl_key}-launchctl")) } @@ -31,41 +31,37 @@ shared_examples "#uninstall_phase or #zap_phase" do end it "works when job is owned by user" do - FakeSystemCommand.stubs_command( - launchctl_list_cmd, - service_info, - ) + allow(fake_system_command).to receive(:run) + .with("/bin/launchctl", args: ["list", "my.fancy.package.service"], print_stderr: false, sudo: false) + .and_return(instance_double(SystemCommand::Result, stdout: service_info)) + allow(fake_system_command).to receive(:run) + .with("/bin/launchctl", args: ["list", "my.fancy.package.service"], print_stderr: false, sudo: true) + .and_return(instance_double(SystemCommand::Result, stdout: unknown_response)) - FakeSystemCommand.stubs_command( - sudo(launchctl_list_cmd), - unknown_response, - ) - - FakeSystemCommand.expects_command(launchctl_remove_cmd) + expect(fake_system_command).to receive(:run!) + .with("/bin/launchctl", args: ["remove", "my.fancy.package.service"], sudo: false) + .and_return(instance_double(SystemCommand::Result)) subject.public_send(:"#{artifact_dsl_key}_phase", command: fake_system_command) end it "works when job is owned by system" do - FakeSystemCommand.stubs_command( - launchctl_list_cmd, - unknown_response, - ) + allow(fake_system_command).to receive(:run) + .with("/bin/launchctl", args: ["list", "my.fancy.package.service"], print_stderr: false, sudo: false) + .and_return(instance_double(SystemCommand::Result, stdout: unknown_response)) + allow(fake_system_command).to receive(:run) + .with("/bin/launchctl", args: ["list", "my.fancy.package.service"], print_stderr: false, sudo: true) + .and_return(instance_double(SystemCommand::Result, stdout: service_info)) - FakeSystemCommand.stubs_command( - sudo(launchctl_list_cmd), - service_info, - ) - - FakeSystemCommand.expects_command(sudo(launchctl_remove_cmd)) + expect(fake_system_command).to receive(:run!) + .with("/bin/launchctl", args: ["remove", "my.fancy.package.service"], sudo: true) + .and_return(instance_double(SystemCommand::Result)) subject.public_send(:"#{artifact_dsl_key}_phase", command: fake_system_command) end end context "using :pkgutil" do - let(:fake_system_command) { class_double(SystemCommand) } - let(:cask) { Cask::CaskLoader.load(cask_path("with-#{artifact_dsl_key}-pkgutil")) } let(:main_pkg_id) { "my.fancy.package.main" } diff --git a/Library/Homebrew/test/cask/dsl/postflight_spec.rb b/Library/Homebrew/test/cask/dsl/postflight_spec.rb index 259921fceb..bc36ff4367 100644 --- a/Library/Homebrew/test/cask/dsl/postflight_spec.rb +++ b/Library/Homebrew/test/cask/dsl/postflight_spec.rb @@ -6,7 +6,8 @@ require "test/cask/dsl/shared_examples/staged" describe Cask::DSL::Postflight, :cask do let(:cask) { Cask::CaskLoader.load(cask_path("basic-cask")) } - let(:dsl) { described_class.new(cask, FakeSystemCommand) } + let(:fake_system_command) { class_double(SystemCommand) } + let(:dsl) { described_class.new(cask, fake_system_command) } it_behaves_like Cask::DSL::Base diff --git a/Library/Homebrew/test/cask/dsl/preflight_spec.rb b/Library/Homebrew/test/cask/dsl/preflight_spec.rb index 93f1a60174..12cbf50b17 100644 --- a/Library/Homebrew/test/cask/dsl/preflight_spec.rb +++ b/Library/Homebrew/test/cask/dsl/preflight_spec.rb @@ -6,7 +6,8 @@ require "test/cask/dsl/shared_examples/staged" describe Cask::DSL::Preflight, :cask do let(:cask) { Cask::CaskLoader.load(cask_path("basic-cask")) } - let(:dsl) { described_class.new(cask, FakeSystemCommand) } + let(:fake_system_command) { class_double(SystemCommand) } + let(:dsl) { described_class.new(cask, fake_system_command) } it_behaves_like Cask::DSL::Base diff --git a/Library/Homebrew/test/cask/dsl/shared_examples/staged.rb b/Library/Homebrew/test/cask/dsl/shared_examples/staged.rb index b6490e825d..0036751501 100644 --- a/Library/Homebrew/test/cask/dsl/shared_examples/staged.rb +++ b/Library/Homebrew/test/cask/dsl/shared_examples/staged.rb @@ -4,8 +4,8 @@ require "cask/staged" shared_examples Cask::Staged do - let(:existing_path) { Pathname.new("/path/to/file/that/exists") } - let(:non_existent_path) { Pathname.new("/path/to/file/that/does/not/exist") } + let(:existing_path) { Pathname("/path/to/file/that/exists") } + let(:non_existent_path) { Pathname("/path/to/file/that/does/not/exist") } before do allow(existing_path).to receive(:exist?).and_return(true) @@ -17,9 +17,8 @@ shared_examples Cask::Staged do end it "can run system commands with list-form arguments" do - FakeSystemCommand.expects_command( - ["echo", "homebrew-cask", "rocks!"], - ) + expect(fake_system_command).to receive(:run!) + .with("echo", args: ["homebrew-cask", "rocks!"]) staged.system_command("echo", args: ["homebrew-cask", "rocks!"]) end @@ -28,9 +27,8 @@ shared_examples Cask::Staged do fake_pathname = existing_path allow(staged).to receive(:Pathname).and_return(fake_pathname) - FakeSystemCommand.expects_command( - ["/bin/chmod", "-R", "--", "777", fake_pathname], - ) + expect(fake_system_command).to receive(:run!) + .with("/bin/chmod", args: ["-R", "--", "777", fake_pathname], sudo: false) staged.set_permissions(fake_pathname.to_s, "777") end @@ -39,9 +37,8 @@ shared_examples Cask::Staged do fake_pathname = existing_path allow(staged).to receive(:Pathname).and_return(fake_pathname) - FakeSystemCommand.expects_command( - ["/bin/chmod", "-R", "--", "777", fake_pathname, fake_pathname], - ) + expect(fake_system_command).to receive(:run!) + .with("/bin/chmod", args: ["-R", "--", "777", fake_pathname, fake_pathname], sudo: false) staged.set_permissions([fake_pathname.to_s, fake_pathname.to_s], "777") end @@ -58,9 +55,8 @@ shared_examples Cask::Staged do allow(User).to receive(:current).and_return(User.new("fake_user")) allow(staged).to receive(:Pathname).and_return(fake_pathname) - FakeSystemCommand.expects_command( - sudo("/usr/sbin/chown", "-R", "--", "fake_user:staff", fake_pathname), - ) + expect(fake_system_command).to receive(:run!) + .with("/usr/sbin/chown", args: ["-R", "--", "fake_user:staff", fake_pathname], sudo: true) staged.set_ownership(fake_pathname.to_s) end @@ -71,9 +67,8 @@ shared_examples Cask::Staged do allow(User).to receive(:current).and_return(User.new("fake_user")) allow(staged).to receive(:Pathname).and_return(fake_pathname) - FakeSystemCommand.expects_command( - sudo("/usr/sbin/chown", "-R", "--", "fake_user:staff", fake_pathname, fake_pathname), - ) + expect(fake_system_command).to receive(:run!) + .with("/usr/sbin/chown", args: ["-R", "--", "fake_user:staff", fake_pathname, fake_pathname], sudo: true) staged.set_ownership([fake_pathname.to_s, fake_pathname.to_s]) end @@ -83,9 +78,8 @@ shared_examples Cask::Staged do allow(staged).to receive(:Pathname).and_return(fake_pathname) - FakeSystemCommand.expects_command( - sudo("/usr/sbin/chown", "-R", "--", "other_user:other_group", fake_pathname), - ) + expect(fake_system_command).to receive(:run!) + .with("/usr/sbin/chown", args: ["-R", "--", "other_user:other_group", fake_pathname], sudo: true) staged.set_ownership(fake_pathname.to_s, user: "other_user", group: "other_group") end diff --git a/Library/Homebrew/test/cask/dsl/uninstall_postflight_spec.rb b/Library/Homebrew/test/cask/dsl/uninstall_postflight_spec.rb index 48c7653d5f..c188142015 100644 --- a/Library/Homebrew/test/cask/dsl/uninstall_postflight_spec.rb +++ b/Library/Homebrew/test/cask/dsl/uninstall_postflight_spec.rb @@ -5,7 +5,7 @@ require "test/cask/dsl/shared_examples/base" describe Cask::DSL::UninstallPostflight, :cask do let(:cask) { Cask::CaskLoader.load(cask_path("basic-cask")) } - let(:dsl) { described_class.new(cask, FakeSystemCommand) } + let(:dsl) { described_class.new(cask, class_double(SystemCommand)) } it_behaves_like Cask::DSL::Base end diff --git a/Library/Homebrew/test/cask/dsl/uninstall_preflight_spec.rb b/Library/Homebrew/test/cask/dsl/uninstall_preflight_spec.rb index 635c35c030..e8d7d5c4ff 100644 --- a/Library/Homebrew/test/cask/dsl/uninstall_preflight_spec.rb +++ b/Library/Homebrew/test/cask/dsl/uninstall_preflight_spec.rb @@ -6,7 +6,8 @@ require "test/cask/dsl/shared_examples/staged" describe Cask::DSL::UninstallPreflight, :cask do let(:cask) { Cask::CaskLoader.load(cask_path("basic-cask")) } - let(:dsl) { described_class.new(cask, FakeSystemCommand) } + let(:fake_system_command) { class_double(SystemCommand) } + let(:dsl) { described_class.new(cask, fake_system_command) } it_behaves_like Cask::DSL::Base diff --git a/Library/Homebrew/test/cask/pkg_spec.rb b/Library/Homebrew/test/cask/pkg_spec.rb index e98face38a..cd45d57cf7 100644 --- a/Library/Homebrew/test/cask/pkg_spec.rb +++ b/Library/Homebrew/test/cask/pkg_spec.rb @@ -153,8 +153,12 @@ describe Cask::Pkg, :cask do "/usr/sbin/pkgutil", args: ["--pkg-info-plist", pkg_id], ).and_return( - SystemCommand::Result.new(nil, [[:stdout, pkg_info_plist]], instance_double(Process::Status, exitstatus: 0), - secrets: []), + SystemCommand::Result.new( + ["/usr/sbin/pkgutil", "--pkg-info-plist", pkg_id], + [[:stdout, pkg_info_plist]], + instance_double(Process::Status, exitstatus: 0), + secrets: [], + ), ) info = pkg.info diff --git a/Library/Homebrew/test/support/helper/cask/fake_system_command.rb b/Library/Homebrew/test/support/helper/cask/fake_system_command.rb deleted file mode 100644 index 2587e7bc52..0000000000 --- a/Library/Homebrew/test/support/helper/cask/fake_system_command.rb +++ /dev/null @@ -1,74 +0,0 @@ -# typed: true -# frozen_string_literal: true - -def sudo(*args) - ["/usr/bin/sudo", "-E", "--"] + args.flatten -end - -class FakeSystemCommand - def self.responses - @responses ||= {} - end - - def self.expectations - @expectations ||= {} - end - - def self.system_calls - @system_calls ||= Hash.new(0) - end - - def self.clear - @responses = nil - @expectations = nil - @system_calls = nil - end - - def self.stubs_command(command, response = "") - command = command.map(&:to_s) - responses[command] = response - end - - def self.expects_command(command, response = "", times = 1) - command = command.map(&:to_s) - stubs_command(command, response) - expectations[command] = times - end - - def self.verify_expectations! - expectations.each do |command, times| - unless system_calls[command] == times - raise("expected #{command.inspect} to be run #{times} times, but got #{system_calls[command]}") - end - end - end - - def self.run(command_string, options = {}) - command = SystemCommand.new(command_string, options).command - puts command - unless responses.key?(command) - raise("no response faked for #{command.inspect}, faked responses are: #{responses.inspect}") - end - - system_calls[command] += 1 - - response = responses[command] - if response.respond_to?(:call) - response.call(command_string, options) - else - SystemCommand::Result.new(command, [[:stdout, response]], OpenStruct.new(exitstatus: 0), secrets: []) - end - end - - def self.run!(command, options = {}) - run(command, options.merge(must_succeed: true)) - end -end - -RSpec.configure do |config| - config.after do - FakeSystemCommand.verify_expectations! - ensure - FakeSystemCommand.clear - end -end diff --git a/Library/Homebrew/test/support/helper/spec/shared_context/homebrew_cask.rb b/Library/Homebrew/test/support/helper/spec/shared_context/homebrew_cask.rb index 760bd80147..1682091b97 100644 --- a/Library/Homebrew/test/support/helper/spec/shared_context/homebrew_cask.rb +++ b/Library/Homebrew/test/support/helper/spec/shared_context/homebrew_cask.rb @@ -4,7 +4,6 @@ require "cask/config" require "cask/cache" -require "test/support/helper/cask/fake_system_command" require "test/support/helper/cask/install_helper" require "test/support/helper/cask/never_sudo_system_command"