Revert "Use SystemCommand for curl."
				
					
				
			This commit is contained in:
		
							parent
							
								
									3c3b05d457
								
							
						
					
					
						commit
						cd2b85c40f
					
				| @ -106,10 +106,10 @@ module Hbc | ||||
|           LockFile.new(temporary_path.basename).with_lock do | ||||
|             _fetch | ||||
|           end | ||||
|         rescue ErrorDuringExecution => e | ||||
|         rescue ErrorDuringExecution | ||||
|           # 33 == range not supported | ||||
|           # try wiping the incomplete download and retrying once | ||||
|           if e.status.exitstatus == 33 && had_incomplete_download | ||||
|           if $CHILD_STATUS.exitstatus == 33 && had_incomplete_download | ||||
|             ohai "Trying a full download" | ||||
|             temporary_path.unlink | ||||
|             had_incomplete_download = false | ||||
|  | ||||
| @ -526,11 +526,7 @@ end | ||||
| 
 | ||||
| # raised by safe_system in utils.rb | ||||
| class ErrorDuringExecution < RuntimeError | ||||
|   attr_reader :status | ||||
| 
 | ||||
|   def initialize(cmd, status:, output: nil) | ||||
|     @status = status | ||||
| 
 | ||||
|     s = "Failure while executing; `#{cmd.shelljoin.gsub(/\\=/, "=")}` exited with #{status.exitstatus}." | ||||
| 
 | ||||
|     unless [*output].empty? | ||||
|  | ||||
| @ -37,13 +37,7 @@ class SystemCommand | ||||
|         puts line.chomp if print_stdout? | ||||
|         @merged_output << [:stdout, line] | ||||
|       when :stderr | ||||
|         if print_stderr? | ||||
|           if line.start_with?("\r") | ||||
|             $stderr.print line | ||||
|           else | ||||
|             $stderr.puts Formatter.error(line.chomp) | ||||
|           end | ||||
|         end | ||||
|         $stderr.puts Formatter.error(line.chomp) if print_stderr? | ||||
|         @merged_output << [:stderr, line] | ||||
|       end | ||||
|     end | ||||
| @ -82,18 +76,13 @@ class SystemCommand | ||||
|   def env_args | ||||
|     return [] if env.empty? | ||||
| 
 | ||||
|     unset_variables = env.select { |_, value| value.nil? } | ||||
|                          .flat_map { |name,| ["-u", name] } | ||||
|     variables = env.map do |name, value| | ||||
|       sanitized_name = Shellwords.escape(name) | ||||
|       sanitized_value = Shellwords.escape(value) | ||||
|       "#{sanitized_name}=#{sanitized_value}" | ||||
|     end | ||||
| 
 | ||||
|     set_variables = | ||||
|       env.reject { |_, value| value.nil? } | ||||
|          .flat_map do |name, value| | ||||
|            sanitized_name = Shellwords.escape(name) | ||||
|            sanitized_value = Shellwords.escape(value) | ||||
|            "#{sanitized_name}=#{sanitized_value}" | ||||
|          end | ||||
| 
 | ||||
|     ["env", *unset_variables, *set_variables] | ||||
|     ["env", *variables] | ||||
|   end | ||||
| 
 | ||||
|   def sudo_prefix | ||||
| @ -113,7 +102,7 @@ class SystemCommand | ||||
|     @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.to_s | ||||
|       else | ||||
|         arg.to_str | ||||
| @ -168,28 +157,23 @@ class SystemCommand | ||||
|       hash[type] << line | ||||
|     end | ||||
| 
 | ||||
|     Result.new(command, output[:stdout], output[:stderr], @status) | ||||
|     Result.new(command, output[:stdout], output[:stderr], @status.exitstatus) | ||||
|   end | ||||
| 
 | ||||
|   class Result | ||||
|     attr_accessor :command, :stdout, :stderr, :status, :exit_status | ||||
|     attr_accessor :command, :stdout, :stderr, :exit_status | ||||
| 
 | ||||
|     def initialize(command, stdout, stderr, status) | ||||
|     def initialize(command, stdout, stderr, exit_status) | ||||
|       @command     = command | ||||
|       @stdout      = stdout | ||||
|       @stderr      = stderr | ||||
|       @status      = status | ||||
|       @exit_status = status.exitstatus | ||||
|       @exit_status = exit_status | ||||
|     end | ||||
| 
 | ||||
|     def success? | ||||
|       @exit_status.zero? | ||||
|     end | ||||
| 
 | ||||
|     def to_ary | ||||
|       [stdout, stderr, status] | ||||
|     end | ||||
| 
 | ||||
|     def plist | ||||
|       @plist ||= begin | ||||
|         output = stdout | ||||
|  | ||||
| @ -39,7 +39,7 @@ describe "download strategies", :cask do | ||||
|       let(:url_options) { { user_agent: "Mozilla/25.0.1" } } | ||||
| 
 | ||||
|       it "adds the appropriate curl args" do | ||||
|         expect(downloader).to receive(:system_command!) { |*, args:, **| | ||||
|         expect(downloader).to receive(:safe_system) { |*args| | ||||
|           expect(args.each_cons(2)).to include(["--user-agent", "Mozilla/25.0.1"]) | ||||
|         } | ||||
| 
 | ||||
| @ -53,7 +53,7 @@ describe "download strategies", :cask do | ||||
|       let(:url_options) { { user_agent: :fake } } | ||||
| 
 | ||||
|       it "adds the appropriate curl args" do | ||||
|         expect(downloader).to receive(:system_command!) { |*, args:, **| | ||||
|         expect(downloader).to receive(:safe_system) { |*args| | ||||
|           expect(args.each_cons(2).to_a).to include(["--user-agent", a_string_matching(/Mozilla.*Mac OS X 10.*AppleWebKit/)]) | ||||
|         } | ||||
| 
 | ||||
|  | ||||
| @ -148,7 +148,7 @@ describe Hbc::Pkg, :cask do | ||||
|         "/usr/sbin/pkgutil", | ||||
|         args: ["--pkg-info-plist", pkg_id], | ||||
|       ).and_return( | ||||
|         SystemCommand::Result.new(nil, pkg_info_plist, nil, instance_double(Process::Status, exitstatus: 0)), | ||||
|         SystemCommand::Result.new(nil, pkg_info_plist, nil, 0), | ||||
|       ) | ||||
| 
 | ||||
|       info = pkg.info | ||||
|  | ||||
| @ -52,7 +52,7 @@ class FakeSystemCommand | ||||
|     if response.respond_to?(:call) | ||||
|       response.call(command_string, options) | ||||
|     else | ||||
|       SystemCommand::Result.new(command, response, "", OpenStruct.new(exitstatus: 0)) | ||||
|       SystemCommand::Result.new(command, response, "", 0) | ||||
|     end | ||||
|   end | ||||
| 
 | ||||
|  | ||||
| @ -1,22 +1,8 @@ | ||||
| require "system_command" | ||||
| 
 | ||||
| describe SystemCommand::Result do | ||||
|   describe "#to_ary" do | ||||
|     subject(:result) { | ||||
|       described_class.new([], "output", "error", instance_double(Process::Status, exitstatus: 0, success?: true)) | ||||
|     } | ||||
| 
 | ||||
|     it "can be destructed like `Open3.capture3`" do | ||||
|       out, err, status = result | ||||
| 
 | ||||
|       expect(out).to eq "output" | ||||
|       expect(err).to eq "error" | ||||
|       expect(status).to be_a_success | ||||
|     end | ||||
|   end | ||||
| 
 | ||||
|   describe "#plist" do | ||||
|     subject { described_class.new(command, stdout, "", instance_double(Process::Status, exitstatus: 0)).plist } | ||||
|     subject { described_class.new(command, stdout, "", 0).plist } | ||||
| 
 | ||||
|     let(:command) { ["true"] } | ||||
|     let(:garbage) { | ||||
|  | ||||
| @ -1,20 +1,17 @@ | ||||
| describe SystemCommand do | ||||
|   describe "#initialize" do | ||||
|     let(:env_args) { ["bash", "-c", 'printf "%s" "${A?}" "${B?}" "${C?}"'] } | ||||
|     let(:env) { { "A" => "1", "B" => "2", "C" => "3" } } | ||||
|     let(:sudo) { false } | ||||
| 
 | ||||
|     subject(:command) { | ||||
|       described_class.new( | ||||
|         "env", | ||||
|         args: env_args, | ||||
|         env: env, | ||||
|         must_succeed: true, | ||||
|         sudo: sudo, | ||||
|       ) | ||||
|     } | ||||
| 
 | ||||
|     context "when given some environment variables" do | ||||
|       subject { | ||||
|         described_class.new( | ||||
|           "env", | ||||
|           args: env_args, | ||||
|           env: { "A" => "1", "B" => "2", "C" => "3" }, | ||||
|           must_succeed: true, | ||||
|         ) | ||||
|       } | ||||
| 
 | ||||
|       its("run!.stdout") { is_expected.to eq("123") } | ||||
| 
 | ||||
|       describe "the resulting command line" do | ||||
| @ -24,23 +21,21 @@ describe SystemCommand do | ||||
|             .with(["env", "env"], "A=1", "B=2", "C=3", "env", *env_args, {}) | ||||
|             .and_call_original | ||||
| 
 | ||||
|           command.run! | ||||
|           subject.run! | ||||
|         end | ||||
|       end | ||||
|     end | ||||
| 
 | ||||
|     context "when given an environment variable which is set to nil" do | ||||
|       let(:env) { { "A" => "1", "B" => "2", "C" => nil } } | ||||
| 
 | ||||
|       it "unsets them" do | ||||
|         expect { | ||||
|           command.run! | ||||
|         }.to raise_error(/C: parameter null or not set/) | ||||
|       end | ||||
|     end | ||||
| 
 | ||||
|     context "when given some environment variables and sudo: true" do | ||||
|       let(:sudo) { true } | ||||
|       subject { | ||||
|         described_class.new( | ||||
|           "env", | ||||
|           args: env_args, | ||||
|           env: { "A" => "1", "B" => "2", "C" => "3" }, | ||||
|           must_succeed: true, | ||||
|           sudo: true, | ||||
|         ) | ||||
|       } | ||||
| 
 | ||||
|       describe "the resulting command line" do | ||||
|         it "includes the given variables explicitly" do | ||||
| @ -52,7 +47,7 @@ describe SystemCommand do | ||||
|               original_popen3.call("true", &block) | ||||
|             end | ||||
| 
 | ||||
|           command.run! | ||||
|           subject.run! | ||||
|         end | ||||
|       end | ||||
|     end | ||||
| @ -220,12 +215,5 @@ describe SystemCommand do | ||||
|         described_class.run("non_existent_executable") | ||||
|       }.not_to raise_error | ||||
|     end | ||||
| 
 | ||||
|     it 'does not format `stderr` when it starts with \r' do | ||||
|       expect { | ||||
|         system_command "bash", | ||||
|                        args: ["-c", 'printf "\r%s" "###################                                                       27.6%" 1>&2'] | ||||
|       }.to output("\r###################                                                       27.6%").to_stderr | ||||
|     end | ||||
|   end | ||||
| end | ||||
|  | ||||
| @ -4,12 +4,12 @@ describe "curl" do | ||||
|   describe "curl_args" do | ||||
|     it "returns -q as the first argument when HOMEBREW_CURLRC is not set" do | ||||
|       # -q must be the first argument according to "man curl" | ||||
|       expect(curl_args("foo").first).to eq("-q") | ||||
|       expect(curl_args("foo")[1]).to eq("-q") | ||||
|     end | ||||
| 
 | ||||
|     it "doesn't return -q as the first argument when HOMEBREW_CURLRC is set" do | ||||
|       ENV["HOMEBREW_CURLRC"] = "1" | ||||
|       expect(curl_args("foo").first).to_not eq("-q") | ||||
|       expect(curl_args("foo")[1]).to_not eq("-q") | ||||
|     end | ||||
|   end | ||||
| end | ||||
|  | ||||
| @ -11,7 +11,7 @@ def curl_executable | ||||
| end | ||||
| 
 | ||||
| def curl_args(*extra_args, show_output: false, user_agent: :default) | ||||
|   args = [] | ||||
|   args = [curl_executable.to_s] | ||||
| 
 | ||||
|   # do not load .curlrc unless requested (must be the first argument) | ||||
|   args << "-q" unless ENV["HOMEBREW_CURLRC"] | ||||
| @ -40,18 +40,18 @@ end | ||||
| def curl(*args) | ||||
|   # SSL_CERT_FILE can be incorrectly set by users or portable-ruby and screw | ||||
|   # with SSL downloads so unset it here. | ||||
|   system_command! curl_executable, | ||||
|                   args: curl_args(*args), | ||||
|                   env: { "SSL_CERT_FILE" => nil } | ||||
|   with_env SSL_CERT_FILE: nil do | ||||
|     safe_system(*curl_args(*args)) | ||||
|   end | ||||
| end | ||||
| 
 | ||||
| def curl_download(*args, to: nil, continue_at: "-", **options) | ||||
|   had_incomplete_download ||= File.exist?(to) | ||||
|   curl("--location", "--remote-time", "--continue-at", continue_at.to_s, "--output", to, *args, **options) | ||||
| rescue ErrorDuringExecution => e | ||||
| rescue ErrorDuringExecution | ||||
|   # `curl` error 33: HTTP server doesn't seem to support byte ranges. Cannot resume. | ||||
|   # HTTP status 416: Requested range not satisfiable | ||||
|   if (e.status.exitstatus == 33 || had_incomplete_download) && continue_at == "-" | ||||
|   if ($CHILD_STATUS.exitstatus == 33 || had_incomplete_download) && continue_at == "-" | ||||
|     continue_at = 0 | ||||
|     had_incomplete_download = false | ||||
|     retry | ||||
| @ -61,9 +61,7 @@ rescue ErrorDuringExecution => e | ||||
| end | ||||
| 
 | ||||
| def curl_output(*args, **options) | ||||
|   system_command(curl_executable, | ||||
|                  args: curl_args(*args, show_output: true, **options), | ||||
|                  print_stderr: false) | ||||
|   Open3.capture3(*curl_args(*args, show_output: true, **options)) | ||||
| end | ||||
| 
 | ||||
| def curl_check_http_content(url, user_agents: [:default], check_content: false, strict: false, require_http: false) | ||||
|  | ||||
| @ -131,12 +131,13 @@ module GitHub | ||||
|     # This is a no-op if the user is opting out of using the GitHub API. | ||||
|     return block_given? ? yield({}) : {} if ENV["HOMEBREW_NO_GITHUB_API"] | ||||
| 
 | ||||
|     args = ["--header", "application/vnd.github.v3+json", "--write-out", "\n%{http_code}"] | ||||
|     args = %W[--header application/vnd.github.v3+json --write-out \n%{http_code}] # rubocop:disable Lint/NestedPercentLiteral | ||||
|     args += curl_args | ||||
| 
 | ||||
|     token, username = api_credentials | ||||
|     case api_credentials_type | ||||
|     when :keychain | ||||
|       args += ["--user", "#{username}:#{token}"] | ||||
|       args += %W[--user #{username}:#{token}] | ||||
|     when :environment | ||||
|       args += ["--header", "Authorization: token #{token}"] | ||||
|     end | ||||
| @ -161,7 +162,7 @@ module GitHub | ||||
| 
 | ||||
|       args += ["--dump-header", headers_tmpfile.path] | ||||
| 
 | ||||
|       output, errors, status = curl_output("--location", url.to_s, *args) | ||||
|       output, errors, status = curl_output(url.to_s, "--location", *args) | ||||
|       output, _, http_code = output.rpartition("\n") | ||||
|       output, _, http_code = output.rpartition("\n") if http_code == "000" | ||||
|       headers = headers_tmpfile.read | ||||
|  | ||||
		Loading…
	
	
			
			x
			
			
		
	
		Reference in New Issue
	
	Block a user
	 ilovezfs
						ilovezfs