From 16acd08d0a16168a840b14f09732c32208c7c0a2 Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Thu, 10 Sep 2020 23:39:19 +0200 Subject: [PATCH 1/2] Don't override global `system_command` methods in download strategies. --- Library/Homebrew/download_strategy.rb | 146 +++++++++++++------------- Library/Homebrew/system_command.rb | 1 - Library/Homebrew/utils/curl.rb | 4 +- 3 files changed, 75 insertions(+), 76 deletions(-) diff --git a/Library/Homebrew/download_strategy.rb b/Library/Homebrew/download_strategy.rb index f4305e429d..811a33a0bc 100644 --- a/Library/Homebrew/download_strategy.rb +++ b/Library/Homebrew/download_strategy.rb @@ -113,12 +113,12 @@ class AbstractDownloadStrategy super(*args) unless quiet? end - def system_command(*args, **options) - super(*args, print_stderr: false, env: env, **options) + def silent_command(*args, **options) + system_command(*args, print_stderr: false, env: env, **options) end - def system_command!(*args, **options) - super( + def command!(*args, **options) + system_command!( *args, print_stdout: !quiet?, print_stderr: !quiet?, @@ -575,7 +575,7 @@ class SubversionDownloadStrategy < VCSDownloadStrategy # # @api public def fetch - if @url.chomp("/") != repo_url || !system_command("svn", args: ["switch", @url, cached_location]).success? + if @url.chomp("/") != repo_url || !silent_command("svn", args: ["switch", @url, cached_location]).success? clear_cache end super @@ -584,10 +584,10 @@ class SubversionDownloadStrategy < VCSDownloadStrategy # (see AbstractDownloadStrategy#source_modified_time) def source_modified_time time = if Version.create(Utils::Svn.version) >= Version.create("1.9") - out, = system_command("svn", args: ["info", "--show-item", "last-changed-date"], chdir: cached_location) + out, = silent_command("svn", args: ["info", "--show-item", "last-changed-date"], chdir: cached_location) out else - out, = system_command("svn", args: ["info"], chdir: cached_location) + out, = silent_command("svn", args: ["info"], chdir: cached_location) out[/^Last Changed Date: (.+)$/, 1] end Time.parse time @@ -595,19 +595,19 @@ class SubversionDownloadStrategy < VCSDownloadStrategy # (see VCSDownloadStrategy#source_modified_time) def last_commit - out, = system_command("svn", args: ["info", "--show-item", "revision"], chdir: cached_location) + out, = silent_command("svn", args: ["info", "--show-item", "revision"], chdir: cached_location) out.strip end private def repo_url - out, = system_command("svn", args: ["info"], chdir: cached_location) + out, = silent_command("svn", args: ["info"], chdir: cached_location) out.strip[/^URL: (.+)$/, 1] end def externals - out, = system_command("svn", args: ["propget", "svn:externals", @url]) + out, = silent_command("svn", args: ["propget", "svn:externals", @url]) out.chomp.split("\n").each do |line| name, url = line.split(/\s+/) yield name, url @@ -634,9 +634,9 @@ class SubversionDownloadStrategy < VCSDownloadStrategy end if target.directory? - system_command!("svn", args: ["update", *args], chdir: target.to_s) + command!("svn", args: ["update", *args], chdir: target.to_s) else - system_command!("svn", args: ["checkout", url, target, *args]) + command!("svn", args: ["checkout", url, target, *args]) end end @@ -687,13 +687,13 @@ class GitDownloadStrategy < VCSDownloadStrategy # (see AbstractDownloadStrategy#source_modified_time) def source_modified_time - out, = system_command("git", args: ["--git-dir", git_dir, "show", "-s", "--format=%cD"]) + out, = silent_command("git", args: ["--git-dir", git_dir, "show", "-s", "--format=%cD"]) Time.parse(out) end # (see VCSDownloadStrategy#source_modified_time) def last_commit - out, = system_command("git", args: ["--git-dir", git_dir, "rev-parse", "--short=7", "HEAD"]) + out, = silent_command("git", args: ["--git-dir", git_dir, "rev-parse", "--short=7", "HEAD"]) out.chomp end @@ -732,18 +732,18 @@ class GitDownloadStrategy < VCSDownloadStrategy end def ref? - system_command("git", + silent_command("git", args: ["--git-dir", git_dir, "rev-parse", "-q", "--verify", "#{@ref}^{commit}"]) .success? end def current_revision - out, = system_command("git", args: ["--git-dir", git_dir, "rev-parse", "-q", "--verify", "HEAD"]) + out, = silent_command("git", args: ["--git-dir", git_dir, "rev-parse", "-q", "--verify", "HEAD"]) out.strip end def repo_valid? - system_command("git", args: ["--git-dir", git_dir, "status", "-s"]).success? + silent_command("git", args: ["--git-dir", git_dir, "status", "-s"]).success? end def submodules? @@ -772,44 +772,44 @@ class GitDownloadStrategy < VCSDownloadStrategy end def config_repo - system_command! "git", - args: ["config", "remote.origin.url", @url], - chdir: cached_location - system_command! "git", - args: ["config", "remote.origin.fetch", refspec], - chdir: cached_location - system_command! "git", - args: ["config", "remote.origin.tagOpt", "--no-tags"], - chdir: cached_location + command! "git", + args: ["config", "remote.origin.url", @url], + chdir: cached_location + command! "git", + args: ["config", "remote.origin.fetch", refspec], + chdir: cached_location + command! "git", + args: ["config", "remote.origin.tagOpt", "--no-tags"], + chdir: cached_location end def update_repo return unless @ref_type == :branch || !ref? if !shallow_clone? && shallow_dir? - system_command! "git", - args: ["fetch", "origin", "--unshallow"], - chdir: cached_location + command! "git", + args: ["fetch", "origin", "--unshallow"], + chdir: cached_location else - system_command! "git", - args: ["fetch", "origin"], - chdir: cached_location + command! "git", + args: ["fetch", "origin"], + chdir: cached_location end end def clone_repo - system_command! "git", args: clone_args + command! "git", args: clone_args - system_command! "git", - args: ["config", "homebrew.cacheversion", cache_version], - chdir: cached_location + command! "git", + args: ["config", "homebrew.cacheversion", cache_version], + chdir: cached_location checkout update_submodules if submodules? end def checkout ohai "Checking out #{@ref_type} #{@ref}" if @ref_type && @ref - system_command! "git", args: ["checkout", "-f", @ref, "--"], chdir: cached_location + command! "git", args: ["checkout", "-f", @ref, "--"], chdir: cached_location end def reset @@ -820,18 +820,18 @@ class GitDownloadStrategy < VCSDownloadStrategy @ref end - system_command! "git", - args: ["reset", "--hard", *ref, "--"], - chdir: cached_location + command! "git", + args: ["reset", "--hard", *ref, "--"], + chdir: cached_location end def update_submodules - system_command! "git", - args: ["submodule", "foreach", "--recursive", "git submodule sync"], - chdir: cached_location - system_command! "git", - args: ["submodule", "update", "--init", "--recursive"], - chdir: cached_location + command! "git", + args: ["submodule", "foreach", "--recursive", "git submodule sync"], + chdir: cached_location + command! "git", + args: ["submodule", "update", "--init", "--recursive"], + chdir: cached_location fix_absolute_submodule_gitdir_references! end @@ -843,9 +843,9 @@ class GitDownloadStrategy < VCSDownloadStrategy # in 2.8.3. Clones created with affected version remain broken.) # See https://github.com/Homebrew/homebrew-core/pull/1520 for an example. def fix_absolute_submodule_gitdir_references! - submodule_dirs = system_command!("git", - args: ["submodule", "--quiet", "foreach", "--recursive", "pwd"], - chdir: cached_location).stdout + submodule_dirs = command!("git", + args: ["submodule", "--quiet", "foreach", "--recursive", "pwd"], + chdir: cached_location).stdout submodule_dirs.lines.map(&:chomp).each do |submodule_dir| work_dir = Pathname.new(submodule_dir) @@ -981,17 +981,17 @@ class CVSDownloadStrategy < VCSDownloadStrategy def clone_repo # Login is only needed (and allowed) with pserver; skip for anoncvs. - system_command! "cvs", args: [*quiet_flag, "-d", @url, "login"] if @url.include? "pserver" + command! "cvs", args: [*quiet_flag, "-d", @url, "login"] if @url.include? "pserver" - system_command! "cvs", - args: [*quiet_flag, "-d", @url, "checkout", "-d", cached_location.basename, @module], - chdir: cached_location.dirname + command! "cvs", + args: [*quiet_flag, "-d", @url, "checkout", "-d", cached_location.basename, @module], + chdir: cached_location.dirname end def update - system_command! "cvs", - args: [*quiet_flag, "update"], - chdir: cached_location + command! "cvs", + args: [*quiet_flag, "update"], + chdir: cached_location end def split_url(in_url) @@ -1013,7 +1013,7 @@ class MercurialDownloadStrategy < VCSDownloadStrategy # (see AbstractDownloadStrategy#source_modified_time) def source_modified_time - out, = system_command("hg", + out, = silent_command("hg", args: ["tip", "--template", "{date|isodate}", "-R", cached_location]) Time.parse(out) @@ -1021,7 +1021,7 @@ class MercurialDownloadStrategy < VCSDownloadStrategy # (see VCSDownloadStrategy#source_modified_time) def last_commit - out, = system_command("hg", args: ["parent", "--template", "{node|short}", "-R", cached_location]) + out, = silent_command("hg", args: ["parent", "--template", "{node|short}", "-R", cached_location]) out.chomp end @@ -1040,11 +1040,11 @@ class MercurialDownloadStrategy < VCSDownloadStrategy end def clone_repo - system_command! "hg", args: ["clone", @url, cached_location] + command! "hg", args: ["clone", @url, cached_location] end def update - system_command! "hg", args: ["--cwd", cached_location, "pull", "--update"] + command! "hg", args: ["--cwd", cached_location, "pull", "--update"] update_args = if @ref_type && @ref ohai "Checking out #{@ref_type} #{@ref}" @@ -1053,7 +1053,7 @@ class MercurialDownloadStrategy < VCSDownloadStrategy ["--clean"] end - system_command! "hg", args: ["--cwd", cached_location, "update", *update_args] + command! "hg", args: ["--cwd", cached_location, "update", *update_args] end end @@ -1068,7 +1068,7 @@ class BazaarDownloadStrategy < VCSDownloadStrategy # (see AbstractDownloadStrategy#source_modified_time) def source_modified_time - out, = system_command("bzr", args: ["log", "-l", "1", "--timezone=utc", cached_location]) + out, = silent_command("bzr", args: ["log", "-l", "1", "--timezone=utc", cached_location]) timestamp = out.chomp raise "Could not get any timestamps from bzr!" if timestamp.blank? @@ -1077,7 +1077,7 @@ class BazaarDownloadStrategy < VCSDownloadStrategy # (see VCSDownloadStrategy#source_modified_time) def last_commit - out, = system_command("bzr", args: ["revno", cached_location]) + out, = silent_command("bzr", args: ["revno", cached_location]) out.chomp end @@ -1100,14 +1100,14 @@ class BazaarDownloadStrategy < VCSDownloadStrategy def clone_repo # "lightweight" means history-less - system_command! "bzr", - args: ["checkout", "--lightweight", @url, cached_location] + command! "bzr", + args: ["checkout", "--lightweight", @url, cached_location] end def update - system_command! "bzr", - args: ["update"], - chdir: cached_location + command! "bzr", + args: ["update"], + chdir: cached_location end end @@ -1122,18 +1122,18 @@ class FossilDownloadStrategy < VCSDownloadStrategy # (see AbstractDownloadStrategy#source_modified_time) def source_modified_time - out, = system_command("fossil", args: ["info", "tip", "-R", cached_location]) + out, = silent_command("fossil", args: ["info", "tip", "-R", cached_location]) Time.parse(out[/^uuid: +\h+ (.+)$/, 1]) end # (see VCSDownloadStrategy#source_modified_time) def last_commit - out, = system_command("fossil", args: ["info", "tip", "-R", cached_location]) + out, = silent_command("fossil", args: ["info", "tip", "-R", cached_location]) out[/^uuid: +(\h+) .+$/, 1] end def repo_valid? - system_command("fossil", args: ["branch", "-R", cached_location]).success? + silent_command("fossil", args: ["branch", "-R", cached_location]).success? end private @@ -1147,11 +1147,11 @@ class FossilDownloadStrategy < VCSDownloadStrategy end def clone_repo - system_command!("fossil", args: ["clone", @url, cached_location]) + silent_command!("fossil", args: ["clone", @url, cached_location]) end def update - system_command!("fossil", args: ["pull", "-R", cached_location]) + silent_command!("fossil", args: ["pull", "-R", cached_location]) end end diff --git a/Library/Homebrew/system_command.rb b/Library/Homebrew/system_command.rb index 05c4cb86c2..89307f7c33 100644 --- a/Library/Homebrew/system_command.rb +++ b/Library/Homebrew/system_command.rb @@ -62,7 +62,6 @@ class SystemCommand def initialize(executable, args: [], sudo: false, env: {}, input: [], must_succeed: false, print_stdout: false, print_stderr: true, verbose: false, secrets: [], **options) - require "extend/ENV" @executable = executable @args = args diff --git a/Library/Homebrew/utils/curl.rb b/Library/Homebrew/utils/curl.rb index 274dc6f201..d1f37dcede 100644 --- a/Library/Homebrew/utils/curl.rb +++ b/Library/Homebrew/utils/curl.rb @@ -44,7 +44,7 @@ def curl_args(*extra_args, show_output: false, user_agent: :default) args + extra_args end -def curl_with_workarounds(*args, secrets: nil, print_stdout: nil, print_stderr: nil, **options) +def curl_with_workarounds(*args, secrets: nil, print_stdout: nil, print_stderr: nil, env: {}, **options) command_options = { secrets: secrets, print_stdout: print_stdout, @@ -55,7 +55,7 @@ def curl_with_workarounds(*args, secrets: nil, print_stdout: nil, print_stderr: # with SSL downloads so unset it here. result = system_command curl_executable, args: curl_args(*args, **options), - env: { "SSL_CERT_FILE" => nil }, + env: env.merge({ "SSL_CERT_FILE" => nil }), **command_options if !result.success? && !args.include?("--http1.1") From 172bf76d74d637ac840e41a9caa807f8e8f75618 Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Thu, 10 Sep 2020 23:50:24 +0200 Subject: [PATCH 2/2] Pass verbosity options to `curl` in download strategies. --- Library/Homebrew/download_strategy.rb | 14 ++++++++++---- Library/Homebrew/utils/curl.rb | 5 +++-- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/Library/Homebrew/download_strategy.rb b/Library/Homebrew/download_strategy.rb index 811a33a0bc..c6f18f7038 100644 --- a/Library/Homebrew/download_strategy.rb +++ b/Library/Homebrew/download_strategy.rb @@ -120,12 +120,18 @@ class AbstractDownloadStrategy def command!(*args, **options) system_command!( *args, + env: env.merge(options.fetch(:env, {})), + **command_output_options, + **options, + ) + end + + def command_output_options + { print_stdout: !quiet?, print_stderr: !quiet?, verbose: verbose? && !quiet?, - env: env, - **options, - ) + } end def env @@ -484,7 +490,7 @@ class CurlDownloadStrategy < AbstractFileDownloadStrategy def curl(*args, **options) args << "--connect-timeout" << "15" unless mirrors.empty? - super(*_curl_args, *args, **_curl_opts, **options) + super(*_curl_args, *args, **_curl_opts, **command_output_options, **options) end end diff --git a/Library/Homebrew/utils/curl.rb b/Library/Homebrew/utils/curl.rb index d1f37dcede..4b0992b89b 100644 --- a/Library/Homebrew/utils/curl.rb +++ b/Library/Homebrew/utils/curl.rb @@ -44,18 +44,19 @@ def curl_args(*extra_args, show_output: false, user_agent: :default) args + extra_args end -def curl_with_workarounds(*args, secrets: nil, print_stdout: nil, print_stderr: nil, env: {}, **options) +def curl_with_workarounds(*args, secrets: nil, print_stdout: nil, print_stderr: nil, verbose: nil, env: {}, **options) command_options = { secrets: secrets, print_stdout: print_stdout, print_stderr: print_stderr, + verbose: verbose, }.compact # SSL_CERT_FILE can be incorrectly set by users or portable-ruby and screw # with SSL downloads so unset it here. result = system_command curl_executable, args: curl_args(*args, **options), - env: env.merge({ "SSL_CERT_FILE" => nil }), + env: { "SSL_CERT_FILE" => nil }.merge(env), **command_options if !result.success? && !args.include?("--http1.1")