From 644ae115f56de90a72ab0fac68d5bdac906cf82e Mon Sep 17 00:00:00 2001 From: Frank Lam Date: Thu, 25 Jun 2020 22:29:34 +0800 Subject: [PATCH 01/32] Prepare inline patch specs --- .../Homebrew/test/rubocops/patches_spec.rb | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/Library/Homebrew/test/rubocops/patches_spec.rb b/Library/Homebrew/test/rubocops/patches_spec.rb index f2a0fc25a7..6b0fc2a1a5 100644 --- a/Library/Homebrew/test/rubocops/patches_spec.rb +++ b/Library/Homebrew/test/rubocops/patches_spec.rb @@ -163,6 +163,40 @@ describe RuboCop::Cop::FormulaAudit::Patches do end end + context "When auditing inline patches" do + it "reports no offenses for valid inline patches" do + expect_no_offenses(<<~RUBY) + class Foo < Formula + url 'https://brew.sh/foo-1.0.tgz' + patch :DATA + end + __END__ + patch content here + RUBY + end + + it "reports an offense when DATA is found with no __END__" do + expect_offense(<<~RUBY) + class Foo < Formula + url 'https://brew.sh/foo-1.0.tgz' + patch :DATA + ^^^^^^^^^^^ patch is missing '__END__' + end + RUBY + end + + it "reports an offense when __END__ is found with no DATA" do + expect_offense(<<~RUBY) + class Foo < Formula + url 'https://brew.sh/foo-1.0.tgz' + end + __END__ + ^^^^^^^ patch is missing 'DATA' + patch content here + RUBY + end + end + context "When auditing external patches" do it "Patch URLs" do patch_urls = [ From 35328ed5351a7df7f3fe09fb23d2d23bb088017a Mon Sep 17 00:00:00 2001 From: rmnull Date: Tue, 23 Jun 2020 03:18:29 +0530 Subject: [PATCH 02/32] [draft] refactoring keg_relocate to use ELFShim#interpreter instead of .with_interpreter? --- .../Homebrew/extend/os/linux/keg_relocate.rb | 17 +++++----- Library/Homebrew/os/linux/elf.rb | 34 ++++++++----------- 2 files changed, 24 insertions(+), 27 deletions(-) diff --git a/Library/Homebrew/extend/os/linux/keg_relocate.rb b/Library/Homebrew/extend/os/linux/keg_relocate.rb index 5371e2ab1e..a707898c03 100644 --- a/Library/Homebrew/extend/os/linux/keg_relocate.rb +++ b/Library/Homebrew/extend/os/linux/keg_relocate.rb @@ -27,6 +27,7 @@ class Keg # patchelf requires that the ELF file have a .dynstr section. # Skip ELF files that do not have a .dynstr section. return if ["cannot find section .dynstr", "strange: no string table"].include?(old_rpath) + unless $CHILD_STATUS.success? raise ErrorDuringExecution.new(cmd_rpath, status: $CHILD_STATUS, output: [[:stderr, old_rpath]]) end @@ -41,15 +42,15 @@ class Keg new_rpath = rpath.join(":") cmd = [patchelf, "--force-rpath", "--set-rpath", new_rpath] - if file.with_interpreter? - old_interpreter = Utils.safe_popen_read(patchelf, "--print-interpreter", file).strip - new_interpreter = if File.readable? "#{new_prefix}/lib/ld.so" - "#{new_prefix}/lib/ld.so" - else - old_interpreter.sub old_prefix, new_prefix - end - cmd << "--set-interpreter" << new_interpreter if old_interpreter != new_interpreter + old_interpreter = file.interpreter + new_interpreter = if old_interpreter.nil? + nil + elsif File.readable? "#{new_prefix}/lib/ld.so" + "#{new_prefix}/lib/ld.so" + else + old_interpreter.sub old_prefix, new_prefix end + cmd << "--set-interpreter" << new_interpreter if old_interpreter != new_interpreter return if old_rpath == new_rpath && old_interpreter == new_interpreter diff --git a/Library/Homebrew/os/linux/elf.rb b/Library/Homebrew/os/linux/elf.rb index 83cd6e8982..51a1240207 100644 --- a/Library/Homebrew/os/linux/elf.rb +++ b/Library/Homebrew/os/linux/elf.rb @@ -68,28 +68,24 @@ module ELFShim elf_type == :executable end - def with_interpreter? - return @with_interpreter if defined? @with_interpreter + def interpreter + return @interpreter if defined? @interpreter - @with_interpreter = if binary_executable? - true - elsif dylib? - if HOMEBREW_PATCHELF_RB - begin - patchelf_patcher.interpreter.present? - rescue PatchELF::PatchError => e - opoo e unless e.to_s.start_with? "No interpreter found" - false - end - elsif which "readelf" - Utils.popen_read("readelf", "-l", to_path).include?(" INTERP ") - elsif which "file" - Utils.popen_read("file", "-L", "-b", to_path).include?(" interpreter ") - else - raise "Please install either readelf (from binutils) or file." + @interpreter = if HOMEBREW_PATCHELF_RB + begin + patchelf_patcher.interpreter + rescue PatchELF::PatchError => e + opoo e unless e.to_s.start_with? "No interpreter found" + nil end + elsif (patchelf = DevelopmentTools.locate "patchelf") + interp = Utils.popen_read(patchelf, "--print-interpreter", to_s, err: :out).strip + $CHILD_STATUS.success? ? interp : nil + elsif (file = DevelopmentTools.locate("file")) + output = Utils.popen_read(file, "-L", "-b", to_s, err: :out).strip + output[/^ELF.*, interpreter (.+?), /, 1] else - false + raise "Please install either patchelf or file." end end From 33a86ba24770e7ecdc6de42eb6d2889b814c83aa Mon Sep 17 00:00:00 2001 From: "dependabot-preview[bot]" <27856297+dependabot-preview[bot]@users.noreply.github.com> Date: Fri, 26 Jun 2020 05:20:09 +0000 Subject: [PATCH 03/32] build(deps): bump rubocop-ast from 0.0.3 to 0.1.0 in /Library/Homebrew Bumps [rubocop-ast](https://github.com/rubocop-hq/rubocop-ast) from 0.0.3 to 0.1.0. - [Release notes](https://github.com/rubocop-hq/rubocop-ast/releases) - [Changelog](https://github.com/rubocop-hq/rubocop-ast/blob/master/CHANGELOG.md) - [Commits](https://github.com/rubocop-hq/rubocop-ast/compare/v0.0.3...v0.1.0) Signed-off-by: dependabot-preview[bot] --- Library/Homebrew/Gemfile.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Library/Homebrew/Gemfile.lock b/Library/Homebrew/Gemfile.lock index c3630a38b0..0e0057f50d 100644 --- a/Library/Homebrew/Gemfile.lock +++ b/Library/Homebrew/Gemfile.lock @@ -91,7 +91,7 @@ GEM rubocop-ast (>= 0.0.3, < 1.0) ruby-progressbar (~> 1.7) unicode-display_width (>= 1.4.0, < 2.0) - rubocop-ast (0.0.3) + rubocop-ast (0.1.0) parser (>= 2.7.0.1) rubocop-performance (1.6.1) rubocop (>= 0.71.0) From b558eb342ff3aff4a022ac3e23c848c869f021b3 Mon Sep 17 00:00:00 2001 From: Marlon Richert Date: Wed, 17 Jun 2020 23:02:46 +0300 Subject: [PATCH 04/32] Improve zsh completion performance * Move main completion cache handling to `brew update`. * Enable completion caching by default. --- completions/zsh/_brew | 48 ++++++++++++++++++-------------------- completions/zsh/_brew_cask | 2 +- 2 files changed, 24 insertions(+), 26 deletions(-) diff --git a/completions/zsh/_brew b/completions/zsh/_brew index e8402fb475..ff75d1ab09 100644 --- a/completions/zsh/_brew +++ b/completions/zsh/_brew @@ -45,20 +45,19 @@ __brew_completion_caching_policy() { tmp=( $1(mw-2N) ) (( $#tmp )) || return 0 - # otherwise, invalidate if latest tap index file is missing or newer than - # cache file + # otherwise, invalidate if latest tap index file is missing or newer than cache file tmp=( ${HOMEBREW_REPOSITORY:-/usr/local/Homebrew}/Library/Taps/*/*/.git/index(om[1]N) ) [[ -z $tmp || $tmp -nt $1 ]] } __brew_formulae() { - local -a formulae + local -a list local comp_cachename=brew_formulae - if _cache_invalid $comp_cachename || ! _retrieve_cache $comp_cachename; then - formulae=($(brew search)) - _store_cache $comp_cachename formulae + if ! _retrieve_cache $comp_cachename; then + list=( $(brew search) ) + _store_cache $comp_cachename list fi - _describe -t formulae 'all formulae' formulae + _describe -t formulae 'all formulae' list } __brew_installed_formulae() { @@ -145,18 +144,17 @@ __brew_common_commands() { } __brew_all_commands() { - local -a commands + local -a list local comp_cachename=brew_all_commands - if _cache_invalid $comp_cachename || ! _retrieve_cache $comp_cachename; then - HOMEBREW_CACHE=$(brew --cache) - HOMEBREW_REPOSITORY=$(brew --repo) - [[ -f "$HOMEBREW_CACHE/all_commands_list.txt" ]] && - commands=($(cat "$HOMEBREW_CACHE/all_commands_list.txt")) || - commands=($(cat "$HOMEBREW_REPOSITORY/completions/internal_commands_list.txt")) - commands=(${commands:#*instal}) # Exclude instal, uninstal, etc. - _store_cache $comp_cachename commands + if ! _retrieve_cache $comp_cachename; then + local cache_dir=$(brew --cache) + [[ -f $cache_dir/all_commands_list.txt ]] && + list=( $(<$cache_dir/all_commands_list.txt) ) || + list=( $(<$(brew --repo)/completions/internal_commands_list.txt) ) + list=( ${list:#*instal} ) # Exclude instal, uninstal, etc. + _store_cache $comp_cachename list fi - _describe -t all-commands 'all commands' commands + _describe -t all-commands 'all commands' list } __brew_commands() { @@ -857,10 +855,10 @@ _brew() { case "$state" in command) # set default cache policy - zstyle -s ":completion:${curcontext%:*}:*" cache-policy tmp - [[ -n $tmp ]] || - zstyle ":completion:${curcontext%:*}:*" cache-policy \ - __brew_completion_caching_policy + zstyle -s ":completion:${curcontext%:*}:*" cache-policy tmp || + zstyle ":completion:${curcontext%:*}:*" cache-policy __brew_completion_caching_policy + zstyle -s ":completion:${curcontext%:*}:*" use-cache tmp || + zstyle ":completion:${curcontext%:*}:*" use-cache true __brew_commands && return 0 ;; @@ -878,10 +876,10 @@ _brew() { # set default cache policy (we repeat this dance because the context # service differs from above) - zstyle -s ":completion:${curcontext%:*}:*" cache-policy tmp - [[ -n $tmp ]] || - zstyle ":completion:${curcontext%:*}:*" cache-policy \ - __brew_completion_caching_policy + zstyle -s ":completion:${curcontext%:*}:*" cache-policy tmp || + zstyle ":completion:${curcontext%:*}:*" cache-policy __brew_completion_caching_policy + zstyle -s ":completion:${curcontext%:*}:*" use-cache tmp || + zstyle ":completion:${curcontext%:*}:*" use-cache true # call completion for named command e.g. _brew_list local completion_func="_brew_${command//-/_}" diff --git a/completions/zsh/_brew_cask b/completions/zsh/_brew_cask index c841b006c3..1a62163294 100644 --- a/completions/zsh/_brew_cask +++ b/completions/zsh/_brew_cask @@ -21,7 +21,7 @@ __brew_all_casks() { local expl local comp_cachename=brew_casks - if _cache_invalid $comp_cachename || ! _retrieve_cache $comp_cachename; then + if ! _retrieve_cache $comp_cachename; then list=( $(brew search --casks) ) _store_cache $comp_cachename list fi From 9520eabc357687aed7cfdfad9648bbb195bb3174 Mon Sep 17 00:00:00 2001 From: Rylan Polster Date: Thu, 25 Jun 2020 20:10:37 -0400 Subject: [PATCH 05/32] style: enforce ISO-8601 in deprecation date --- Library/Homebrew/rubocops.rb | 1 + Library/Homebrew/rubocops/deprecate.rb | 35 ++++++++++++ .../Homebrew/test/rubocops/deprecate_spec.rb | 56 +++++++++++++++++++ 3 files changed, 92 insertions(+) create mode 100644 Library/Homebrew/rubocops/deprecate.rb create mode 100644 Library/Homebrew/test/rubocops/deprecate_spec.rb diff --git a/Library/Homebrew/rubocops.rb b/Library/Homebrew/rubocops.rb index 7a0f2c70e8..50f01a9cf8 100644 --- a/Library/Homebrew/rubocops.rb +++ b/Library/Homebrew/rubocops.rb @@ -22,5 +22,6 @@ require "rubocops/uses_from_macos" require "rubocops/files" require "rubocops/keg_only" require "rubocops/version" +require "rubocops/deprecate" require "rubocops/rubocop-cask" diff --git a/Library/Homebrew/rubocops/deprecate.rb b/Library/Homebrew/rubocops/deprecate.rb new file mode 100644 index 0000000000..15e936bebc --- /dev/null +++ b/Library/Homebrew/rubocops/deprecate.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +require "rubocops/extend/formula" + +module RuboCop + module Cop + module FormulaAudit + # This cop audits deprecate! + class Deprecate < FormulaCop + def audit_formula(_node, _class_node, _parent_class_node, body_node) + deprecate_node = find_node_method_by_name(body_node, :deprecate!) + + return if deprecate_node.nil? || deprecate_node.children.length < 3 + + date_node = find_strings(deprecate_node).first + + begin + Date.iso8601(string_content(date_node)) + rescue ArgumentError + fixed_date_string = Date.parse(string_content(date_node)).iso8601 + offending_node(date_node) + problem "Use `#{fixed_date_string}` to comply with ISO 8601" + end + end + + def autocorrect(node) + lambda do |corrector| + fixed_fixed_date_string = Date.parse(string_content(node)).iso8601 + corrector.replace(node.source_range, "\"#{fixed_fixed_date_string}\"") + end + end + end + end + end +end diff --git a/Library/Homebrew/test/rubocops/deprecate_spec.rb b/Library/Homebrew/test/rubocops/deprecate_spec.rb new file mode 100644 index 0000000000..49176069f9 --- /dev/null +++ b/Library/Homebrew/test/rubocops/deprecate_spec.rb @@ -0,0 +1,56 @@ +# frozen_string_literal: true + +require "rubocops/deprecate" + +describe RuboCop::Cop::FormulaAudit::Deprecate do + subject(:cop) { described_class.new } + + context "When auditing formula for deprecate!" do + it "deprecation date is not ISO-8601 compliant" do + expect_offense(<<~RUBY) + class Foo < Formula + url 'https://brew.sh/foo-1.0.tgz' + deprecate! :date => "June 25, 2020" + ^^^^^^^^^^^^^^^ Use `2020-06-25` to comply with ISO 8601 + end + RUBY + end + + it "deprecation date is ISO-8601 compliant" do + expect_no_offenses(<<~RUBY) + class Foo < Formula + url 'https://brew.sh/foo-1.0.tgz' + deprecate! :date => "2020-06-25" + end + RUBY + end + + it "no deprecation date" do + expect_no_offenses(<<~RUBY) + class Foo < Formula + url 'https://brew.sh/foo-1.0.tgz' + deprecate! + end + RUBY + end + + it "auto corrects to ISO-8601 format" do + source = <<~RUBY + class Foo < Formula + url 'https://brew.sh/foo-1.0.tgz' + deprecate! :date => "June 25, 2020" + end + RUBY + + corrected_source = <<~RUBY + class Foo < Formula + url 'https://brew.sh/foo-1.0.tgz' + deprecate! :date => "2020-06-25" + end + RUBY + + new_source = autocorrect_source(source) + expect(new_source).to eq(corrected_source) + end + end +end From afb844538048055d3a5ce42ce051a21fb3ff8cf2 Mon Sep 17 00:00:00 2001 From: Frank Lam Date: Sat, 27 Jun 2020 01:10:52 +0800 Subject: [PATCH 06/32] Add inline patch checks to patches cop --- Library/Homebrew/rubocops/patches.rb | 36 +++++++++++++++++++++++++++- 1 file changed, 35 insertions(+), 1 deletion(-) diff --git a/Library/Homebrew/rubocops/patches.rb b/Library/Homebrew/rubocops/patches.rb index eeae550780..5a0e0effae 100644 --- a/Library/Homebrew/rubocops/patches.rb +++ b/Library/Homebrew/rubocops/patches.rb @@ -8,7 +8,9 @@ module RuboCop module FormulaAudit # This cop audits patches in Formulae. class Patches < FormulaCop - def audit_formula(_node, _class_node, _parent_class_node, body) + def audit_formula(node, _class_node, _parent_class_node, body) + @full_source_content = source_buffer(node).source + external_patches = find_all_blocks(body, :patch) external_patches.each do |patch_block| url_node = find_every_method_call_by_name(patch_block, :url).first @@ -16,6 +18,14 @@ module RuboCop patch_problems(url_string) end + inline_patches = find_method_calls_by_name(body, :patch) + inline_patches.each { |patch| inline_patch_problems(patch) } + + if inline_patches.empty? && patch_end? + offending_patch_end_node(node) + problem "patch is missing 'DATA'" + end + patches_node = find_method_def(body, :patches) return if patches_node.nil? @@ -84,6 +94,30 @@ module RuboCop #{patch_url} EOS end + + def inline_patch_problems(patch) + return unless patch_data?(patch) && !patch_end? + + offending_node(patch) + problem "patch is missing '__END__'" + end + + def_node_search :patch_data?, <<~AST + (send nil? :patch (:sym :DATA)) + AST + + def patch_end? + /^__END__$/.match?(@full_source_content) + end + + def offending_patch_end_node(node) + @offensive_node = node + @source_buf = source_buffer(node) + @line_no = node.loc.last_line + 1 + @column = 0 + @length = 7 # "__END__".size + @offense_source_range = source_range(@source_buf, @line_no, @column, @length) + end end end end From b2eafdf11d1bec99479e9a097f68cf5be31c2146 Mon Sep 17 00:00:00 2001 From: Frank Lam Date: Sat, 27 Jun 2020 02:37:57 +0800 Subject: [PATCH 07/32] Remove patch checks from audit --- Library/Homebrew/dev-cmd/audit.rb | 14 ------- Library/Homebrew/test/dev-cmd/audit_spec.rb | 41 --------------------- 2 files changed, 55 deletions(-) diff --git a/Library/Homebrew/dev-cmd/audit.rb b/Library/Homebrew/dev-cmd/audit.rb index aade8065e9..9210c975f9 100644 --- a/Library/Homebrew/dev-cmd/audit.rb +++ b/Library/Homebrew/dev-cmd/audit.rb @@ -168,14 +168,6 @@ module Homebrew @text.split("\n__END__").first end - def data? - /^[^#]*\bDATA\b/ =~ @text - end - - def end? - /^__END__$/ =~ @text - end - def trailing_newline? /\Z\n/ =~ @text end @@ -234,12 +226,6 @@ module Homebrew end def audit_file - # TODO: check could be in RuboCop - problem "'DATA' was found, but no '__END__'" if text.data? && !text.end? - - # TODO: check could be in RuboCop - problem "'__END__' was found, but 'DATA' is not used" if text.end? && !text.data? - # TODO: check could be in RuboCop if text.to_s.match?(/inreplace [^\n]* do [^\n]*\n[^\n]*\.gsub![^\n]*\n\ *end/m) problem "'inreplace ... do' was used for a single substitution (use the non-block form instead)." diff --git a/Library/Homebrew/test/dev-cmd/audit_spec.rb b/Library/Homebrew/test/dev-cmd/audit_spec.rb index 0a437b20d6..992281d5c1 100644 --- a/Library/Homebrew/test/dev-cmd/audit_spec.rb +++ b/Library/Homebrew/test/dev-cmd/audit_spec.rb @@ -41,8 +41,6 @@ module Homebrew url "https://www.brew.sh/valid-1.0.tar.gz" RUBY - expect(ft).not_to have_data - expect(ft).not_to have_end expect(ft).to have_trailing_newline expect(ft =~ /\burl\b/).to be_truthy @@ -55,20 +53,6 @@ module Homebrew ft = formula_text "newline" expect(ft).to have_trailing_newline end - - specify "#data?" do - ft = formula_text "data", <<~RUBY - patch :DATA - RUBY - - expect(ft).to have_data - end - - specify "#end?" do - ft = formula_text "end", "", patch: "__END__\na patch here" - expect(ft).to have_end - expect(ft.without_patch).to eq("class End < Formula\n \nend") - end end describe FormulaAuditor do @@ -96,31 +80,6 @@ module Homebrew end describe "#audit_file" do - specify "DATA but no __END__" do - fa = formula_auditor "foo", <<~RUBY - class Foo < Formula - url "https://brew.sh/foo-1.0.tgz" - patch :DATA - end - RUBY - - fa.audit_file - expect(fa.problems).to eq(["'DATA' was found, but no '__END__'"]) - end - - specify "__END__ but no DATA" do - fa = formula_auditor "foo", <<~RUBY - class Foo < Formula - url "https://brew.sh/foo-1.0.tgz" - end - __END__ - a patch goes here - RUBY - - fa.audit_file - expect(fa.problems).to eq(["'__END__' was found, but 'DATA' is not used"]) - end - specify "no issue" do fa = formula_auditor "foo", <<~RUBY class Foo < Formula From ffb1cc8e19abf6da810470d4ce266ab05fe69115 Mon Sep 17 00:00:00 2001 From: Frank Lam Date: Sat, 27 Jun 2020 03:13:50 +0800 Subject: [PATCH 08/32] Find patch nodes nested inside blocks --- Library/Homebrew/rubocops/patches.rb | 2 +- Library/Homebrew/test/rubocops/patches_spec.rb | 13 +++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/Library/Homebrew/rubocops/patches.rb b/Library/Homebrew/rubocops/patches.rb index 5a0e0effae..13cffc6dca 100644 --- a/Library/Homebrew/rubocops/patches.rb +++ b/Library/Homebrew/rubocops/patches.rb @@ -18,7 +18,7 @@ module RuboCop patch_problems(url_string) end - inline_patches = find_method_calls_by_name(body, :patch) + inline_patches = find_every_method_call_by_name(body, :patch) inline_patches.each { |patch| inline_patch_problems(patch) } if inline_patches.empty? && patch_end? diff --git a/Library/Homebrew/test/rubocops/patches_spec.rb b/Library/Homebrew/test/rubocops/patches_spec.rb index 6b0fc2a1a5..42883c8458 100644 --- a/Library/Homebrew/test/rubocops/patches_spec.rb +++ b/Library/Homebrew/test/rubocops/patches_spec.rb @@ -175,6 +175,19 @@ describe RuboCop::Cop::FormulaAudit::Patches do RUBY end + it "reports no offenses for valid nested inline patches" do + expect_no_offenses(<<~RUBY) + class Foo < Formula + url 'https://brew.sh/foo-1.0.tgz' + stable do + patch :DATA + end + end + __END__ + patch content here + RUBY + end + it "reports an offense when DATA is found with no __END__" do expect_offense(<<~RUBY) class Foo < Formula From e82084583a1c6c497e062d33f8602b3eb29e58de Mon Sep 17 00:00:00 2001 From: Rylan Polster Date: Thu, 4 Jun 2020 21:52:20 -0400 Subject: [PATCH 09/32] style: set shell variables in hash When running Utils.popen (or similar popen command), having a line like "SHELL=bash ..." doesn't work properly. Instead, use: `Utils.popen({ "SHELL" => "bash" }, "...")` --- Library/Homebrew/rubocops/lines.rb | 31 ++++- Library/Homebrew/test/rubocops/lines_spec.rb | 138 ++++++++++++++++++- 2 files changed, 166 insertions(+), 3 deletions(-) diff --git a/Library/Homebrew/rubocops/lines.rb b/Library/Homebrew/rubocops/lines.rb index 38e26af8b5..faa5f6ff80 100644 --- a/Library/Homebrew/rubocops/lines.rb +++ b/Library/Homebrew/rubocops/lines.rb @@ -195,7 +195,7 @@ module RuboCop end end - class ShellCmd < FormulaCop + class SafePopenCommands < FormulaCop def audit_formula(_node, _class_node, _parent_class_node, body_node) test = find_block(body_node, :test) @@ -223,6 +223,35 @@ module RuboCop end end + class ShellVariables < FormulaCop + def audit_formula(_node, _class_node, _parent_class_node, body_node) + popen_commands = [ + :popen, + :popen_read, + :safe_popen_read, + :popen_write, + :safe_popen_write, + ] + + popen_commands.each do |command| + find_instance_method_call(body_node, "Utils", command) do |method| + next unless match = regex_match_group(parameters(method).first, /^([^"' ]+)=([^"' ]+)(?: (.*))?$/) + + good_args = "Utils.#{command}({ \"#{match[1]}\" => \"#{match[2]}\" }, \"#{match[3]}\")" + + problem "Use `#{good_args}` instead of `#{method.source}`" + end + end + end + + def autocorrect(node) + lambda do |corrector| + match = regex_match_group(node, /^([^"' ]+)=([^"' ]+)(?: (.*))?$/) + corrector.replace(node.source_range, "{ \"#{match[1]}\" => \"#{match[2]}\" }, \"#{match[3]}\"") + end + end + end + class Miscellaneous < FormulaCop def audit_formula(_node, _class_node, _parent_class_node, body_node) # FileUtils is included in Formula diff --git a/Library/Homebrew/test/rubocops/lines_spec.rb b/Library/Homebrew/test/rubocops/lines_spec.rb index bb31d2e935..9c92e47850 100644 --- a/Library/Homebrew/test/rubocops/lines_spec.rb +++ b/Library/Homebrew/test/rubocops/lines_spec.rb @@ -345,10 +345,10 @@ describe RuboCop::Cop::FormulaAudit::MpiCheck do end end -describe RuboCop::Cop::FormulaAudit::ShellCmd do +describe RuboCop::Cop::FormulaAudit::SafePopenCommands do subject(:cop) { described_class.new } - context "When auditing shell commands" do + context "When auditing popen commands" do it "Utils.popen_read should become Utils.safe_popen_read" do expect_offense(<<~RUBY) class Foo < Formula @@ -440,6 +440,140 @@ describe RuboCop::Cop::FormulaAudit::ShellCmd do end end +describe RuboCop::Cop::FormulaAudit::ShellVariables do + subject(:cop) { described_class.new } + + context "When auditing shell variables" do + it "Shell variables should be expanded in Utils.popen" do + expect_offense(<<~RUBY) + class Foo < Formula + def install + Utils.popen "SHELL=bash foo" + ^^^^^^^^^^^^^^ Use `Utils.popen({ "SHELL" => "bash" }, "foo")` instead of `Utils.popen "SHELL=bash foo"` + end + end + RUBY + end + + it "Shell variables should be expanded in Utils.safe_popen_read" do + expect_offense(<<~RUBY) + class Foo < Formula + def install + Utils.safe_popen_read "SHELL=bash foo" + ^^^^^^^^^^^^^^ Use `Utils.safe_popen_read({ "SHELL" => "bash" }, "foo")` instead of `Utils.safe_popen_read "SHELL=bash foo"` + end + end + RUBY + end + + it "Shell variables should be expanded in Utils.safe_popen_write" do + expect_offense(<<~RUBY) + class Foo < Formula + def install + Utils.safe_popen_write "SHELL=bash foo" + ^^^^^^^^^^^^^^ Use `Utils.safe_popen_write({ "SHELL" => "bash" }, "foo")` instead of `Utils.safe_popen_write "SHELL=bash foo"` + end + end + RUBY + end + + it "Shell variables should be expanded and keep inline string variables in the arguments" do + expect_offense(<<~RUBY) + class Foo < Formula + def install + Utils.popen "SHELL=bash \#{bin}/foo" + ^^^^^^^^^^^^^^^^^^^^^ Use `Utils.popen({ "SHELL" => "bash" }, "\#{bin}/foo")` instead of `Utils.popen "SHELL=bash \#{bin}/foo"` + end + end + RUBY + end + + it "corrects shell variables in Utils.popen" do + source = <<~RUBY + class Foo < Formula + def install + Utils.popen("SHELL=bash foo") + end + end + RUBY + + corrected_source = <<~RUBY + class Foo < Formula + def install + Utils.popen({ "SHELL" => "bash" }, "foo") + end + end + RUBY + + new_source = autocorrect_source(source) + expect(new_source).to eq(corrected_source) + end + + it "corrects shell variables in Utils.safe_popen_read" do + source = <<~RUBY + class Foo < Formula + def install + Utils.safe_popen_read("SHELL=bash foo") + end + end + RUBY + + corrected_source = <<~RUBY + class Foo < Formula + def install + Utils.safe_popen_read({ "SHELL" => "bash" }, "foo") + end + end + RUBY + + new_source = autocorrect_source(source) + expect(new_source).to eq(corrected_source) + end + + it "corrects shell variables in Utils.safe_popen_write" do + source = <<~RUBY + class Foo < Formula + def install + Utils.safe_popen_write("SHELL=bash foo") + end + end + RUBY + + corrected_source = <<~RUBY + class Foo < Formula + def install + Utils.safe_popen_write({ "SHELL" => "bash" }, "foo") + end + end + RUBY + + new_source = autocorrect_source(source) + expect(new_source).to eq(corrected_source) + end + + it "corrects shell variables with inline string variable in arguments" do + source = <<~RUBY + class Foo < Formula + def install + Utils.popen("SHELL=bash \#{bin}/foo") + end + end + RUBY + + corrected_source = <<~RUBY + class Foo < Formula + def install + Utils.popen({ "SHELL" => "bash" }, "\#{bin}/foo") + end + end + RUBY + + new_source = autocorrect_source(source) + expect(new_source).to eq(corrected_source) + end + end +end + describe RuboCop::Cop::FormulaAudit::Miscellaneous do subject(:cop) { described_class.new } From 9c44e39a39c71d066a7141672f273a09a73df24e Mon Sep 17 00:00:00 2001 From: Rylan Polster Date: Fri, 26 Jun 2020 21:32:18 -0400 Subject: [PATCH 10/32] Remove hyphen in ISO 8601 in rubocops/deprecate_spec.rb --- Library/Homebrew/test/rubocops/deprecate_spec.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Library/Homebrew/test/rubocops/deprecate_spec.rb b/Library/Homebrew/test/rubocops/deprecate_spec.rb index 49176069f9..f73ab0be03 100644 --- a/Library/Homebrew/test/rubocops/deprecate_spec.rb +++ b/Library/Homebrew/test/rubocops/deprecate_spec.rb @@ -6,7 +6,7 @@ describe RuboCop::Cop::FormulaAudit::Deprecate do subject(:cop) { described_class.new } context "When auditing formula for deprecate!" do - it "deprecation date is not ISO-8601 compliant" do + it "deprecation date is not ISO 8601 compliant" do expect_offense(<<~RUBY) class Foo < Formula url 'https://brew.sh/foo-1.0.tgz' @@ -16,7 +16,7 @@ describe RuboCop::Cop::FormulaAudit::Deprecate do RUBY end - it "deprecation date is ISO-8601 compliant" do + it "deprecation date is ISO 8601 compliant" do expect_no_offenses(<<~RUBY) class Foo < Formula url 'https://brew.sh/foo-1.0.tgz' @@ -34,7 +34,7 @@ describe RuboCop::Cop::FormulaAudit::Deprecate do RUBY end - it "auto corrects to ISO-8601 format" do + it "auto corrects to ISO 8601 format" do source = <<~RUBY class Foo < Formula url 'https://brew.sh/foo-1.0.tgz' From 6eb80c67a43a908883ceb9303806c910c48b04d5 Mon Sep 17 00:00:00 2001 From: Michka Popoff Date: Thu, 25 Jun 2020 22:17:45 +0200 Subject: [PATCH 11/32] install: add backtrace on failure The error message might be useless alone --- Library/Homebrew/cmd/install.rb | 1 + Library/Homebrew/formulary.rb | 1 + 2 files changed, 2 insertions(+) diff --git a/Library/Homebrew/cmd/install.rb b/Library/Homebrew/cmd/install.rb index ef95b00c76..d6ad01feef 100644 --- a/Library/Homebrew/cmd/install.rb +++ b/Library/Homebrew/cmd/install.rb @@ -267,6 +267,7 @@ module Homebrew # Need to rescue before `FormulaUnavailableError` (superclass of this) # is handled, as searching for a formula doesn't make sense here (the # formula was found, but there's a problem with its implementation). + $stderr.puts e.backtrace if Homebrew::EnvConfig.developer? ofail e.message rescue FormulaUnavailableError => e if e.name == "updog" diff --git a/Library/Homebrew/formulary.rb b/Library/Homebrew/formulary.rb index 086d8ff61f..336f34f757 100644 --- a/Library/Homebrew/formulary.rb +++ b/Library/Homebrew/formulary.rb @@ -35,6 +35,7 @@ module Formulary begin mod.module_eval(contents, path) rescue NameError, ArgumentError, ScriptError => e + $stderr.puts e.backtrace if Homebrew::EnvConfig.developer? raise FormulaUnreadableError.new(name, e) end class_name = class_s(name) From f684a59fa5f8dfe6c1498bdd35d08d1e1ab60e92 Mon Sep 17 00:00:00 2001 From: Dustin Rodrigues Date: Sun, 14 Jun 2020 20:49:52 -0400 Subject: [PATCH 12/32] bump-formula-pr: search for closed dupe PRs --- Library/Homebrew/dev-cmd/bump-formula-pr.rb | 22 ++++++++++++--------- Library/Homebrew/exceptions.rb | 2 +- Library/Homebrew/utils/github.rb | 6 ++---- 3 files changed, 16 insertions(+), 14 deletions(-) diff --git a/Library/Homebrew/dev-cmd/bump-formula-pr.rb b/Library/Homebrew/dev-cmd/bump-formula-pr.rb index c3426f0ae3..a718d92077 100644 --- a/Library/Homebrew/dev-cmd/bump-formula-pr.rb +++ b/Library/Homebrew/dev-cmd/bump-formula-pr.rb @@ -147,7 +147,6 @@ module Homebrew raise FormulaUnspecifiedError unless formula tap_full_name, origin_branch, previous_branch = use_correct_linux_tap(formula) - check_for_duplicate_pull_requests(formula, tap_full_name) requested_spec, formula_spec = if args.devel? devel_message = " (devel)" @@ -315,6 +314,8 @@ module Homebrew new_formula_version = formula_version(formula, requested_spec, new_contents) + check_for_duplicate_pull_requests(formula, tap_full_name, new_formula_version.to_s) + if !new_mirrors && !formula_spec.mirrors.empty? if args.force? opoo "#{formula}: Removing all mirrors because a --mirror= argument was not specified." @@ -468,23 +469,26 @@ module Homebrew end end - def fetch_pull_requests(formula, tap_full_name) - GitHub.issues_for_formula(formula.name, tap_full_name: tap_full_name).select do |pr| + def fetch_pull_requests(query, tap_full_name, state: nil) + GitHub.issues_for_formula(query, tap_full_name: tap_full_name, state: state).select do |pr| pr["html_url"].include?("/pull/") && - /(^|\s)#{Regexp.quote(formula.name)}(:|\s|$)/i =~ pr["title"] + /(^|\s)#{Regexp.quote(query)}(:|\s|$)/i =~ pr["title"] end rescue GitHub::RateLimitExceededError => e opoo e.message [] end - def check_for_duplicate_pull_requests(formula, tap_full_name) - pull_requests = fetch_pull_requests(formula, tap_full_name) - return unless pull_requests - return if pull_requests.empty? + def check_for_duplicate_pull_requests(formula, tap_full_name, version) + # check for open requests + pull_requests = fetch_pull_requests(formula.name, tap_full_name, state: "open") + + # if we haven't already found open requests, try for an exact match across all requests + pull_requests = fetch_pull_requests("#{formula.name} #{version}", tap_full_name) if pull_requests.blank? + return if pull_requests.blank? duplicates_message = <<~EOS - These open pull requests may be duplicates: + These pull requests may be duplicates: #{pull_requests.map { |pr| "#{pr["title"]} #{pr["html_url"]}" }.join("\n")} EOS error_message = "Duplicate PRs should not be opened. Use --force to override this error." diff --git a/Library/Homebrew/exceptions.rb b/Library/Homebrew/exceptions.rb index 94bcc05245..ba47d036b7 100644 --- a/Library/Homebrew/exceptions.rb +++ b/Library/Homebrew/exceptions.rb @@ -360,7 +360,7 @@ class BuildError < RuntimeError end def fetch_issues - GitHub.issues_for_formula(formula.name, tap: formula.tap) + GitHub.issues_for_formula(formula.name, tap: formula.tap, state: "open") rescue GitHub::RateLimitExceededError => e opoo e.message [] diff --git a/Library/Homebrew/utils/github.rb b/Library/Homebrew/utils/github.rb index 556089c091..121e66b351 100644 --- a/Library/Homebrew/utils/github.rb +++ b/Library/Homebrew/utils/github.rb @@ -293,10 +293,8 @@ module GitHub search("code", **qualifiers) end - def issues_for_formula(name, options = {}) - tap = options[:tap] || CoreTap.instance - tap_full_name = options[:tap_full_name] || tap.full_name - search_issues(name, state: "open", repo: tap_full_name, in: "title") + def issues_for_formula(name, tap: CoreTap.instance, tap_full_name: tap.full_name, state: nil) + search_issues(name, repo: tap_full_name, state: state, in: "title") end def user From 36b62c50e03759b81e3dbece03faaaf5e3a0e5dd Mon Sep 17 00:00:00 2001 From: Michka Popoff Date: Thu, 25 Jun 2020 22:24:46 +0200 Subject: [PATCH 13/32] software_spec: do not add empty resources Empty resources are allowed to exist under the following form: resource "filelock" do on_linux do url "https://files.pythonhosted.org/packages/14/ec/6ee2168387ce0154632f856d5cc5592328e9cf93127c5c9aeca92c8c16cb/filelock-3.0.12.tar.gz" sha256 "18d82244ee114f543149c66a6e0c14e9c4f8a1044b5cdaadd0f82159d6a6ff59" end end In this case (or for the on_macos only resource case), just ignore the resource block. Fixes: /usr/local/Homebrew/Library/Homebrew/dependency_collector.rb:148:in `resource_dep' /usr/local/Homebrew/Library/Homebrew/dependency_collector.rb:99:in `parse_spec' /usr/local/Homebrew/Library/Homebrew/dependency_collector.rb:53:in `build' /usr/local/Homebrew/Library/Homebrew/dependency_collector.rb:40:in `block in fetch' /usr/local/Homebrew/Library/Homebrew/dependency_collector.rb:40:in `fetch' /usr/local/Homebrew/Library/Homebrew/dependency_collector.rb:40:in `fetch' /usr/local/Homebrew/Library/Homebrew/dependency_collector.rb:30:in `add' /usr/local/Homebrew/Library/Homebrew/software_spec.rb:120:in `resource' /usr/local/Homebrew/Library/Homebrew/formula.rb:2439:in `block in resource' /usr/local/Homebrew/Library/Homebrew/formula.rb:2438:in `each' /usr/local/Homebrew/Library/Homebrew/formula.rb:2438:in `resource' /usr/local/Homebrew/Library/Taps/homebrew/homebrew-core/Formula/aws-google-auth.rb:149:in `' /usr/local/Homebrew/Library/Taps/homebrew/homebrew-core/Formula/aws-google-auth.rb:1:in `load_formula' /usr/local/Homebrew/Library/Homebrew/formulary.rb:38:in `rescue in load_formula' /usr/local/Homebrew/Library/Homebrew/formulary.rb:35:in `load_formula' /usr/local/Homebrew/Library/Homebrew/formulary.rb:56:in `load_formula_from_path' /usr/local/Homebrew/Library/Homebrew/formulary.rb:138:in `load_file' /usr/local/Homebrew/Library/Homebrew/formulary.rb:128:in `klass' /usr/local/Homebrew/Library/Homebrew/formulary.rb:124:in `get_formula' /usr/local/Homebrew/Library/Homebrew/formulary.rb:333:in `factory' /usr/local/Homebrew/Library/Homebrew/cli/args.rb:87:in `block in formulae' /usr/local/Homebrew/Library/Homebrew/cli/args.rb:86:in `map' /usr/local/Homebrew/Library/Homebrew/cli/args.rb:86:in `formulae' /usr/local/Homebrew/Library/Homebrew/cmd/install.rb:133:in `install' /usr/local/Homebrew/Library/Homebrew/brew.rb:111:in `
' --- Library/Homebrew/software_spec.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Library/Homebrew/software_spec.rb b/Library/Homebrew/software_spec.rb index 3635edd3df..9d3b208a1b 100644 --- a/Library/Homebrew/software_spec.rb +++ b/Library/Homebrew/software_spec.rb @@ -115,6 +115,8 @@ class SoftwareSpec raise DuplicateResourceError, name if resource_defined?(name) res = klass.new(name, &block) + return unless res.url + resources[name] = res dependency_collector.add(res) else From a1e2b8d75c47fe399e51e4726689e4ee390976f1 Mon Sep 17 00:00:00 2001 From: Sean Molenaar Date: Thu, 25 Jun 2020 11:06:22 +0200 Subject: [PATCH 14/32] docs: reference formula assertions for tests --- docs/Formula-Cookbook.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/Formula-Cookbook.md b/docs/Formula-Cookbook.md index ac475face9..19f38b0a2e 100644 --- a/docs/Formula-Cookbook.md +++ b/docs/Formula-Cookbook.md @@ -261,7 +261,7 @@ We want tests that don't require any user input and test the basic functionality See [`cmake`](https://github.com/Homebrew/homebrew-core/blob/HEAD/Formula/cmake.rb) for an example of a formula with a good test. The formula writes a basic `CMakeLists.txt` file into the test directory then calls CMake to generate Makefiles. This test checks that CMake doesn't e.g. segfault during basic operation. -You can check that the output is as expected with `assert_equal` or `assert_match` on the output of shell_output such as in this example from the [envv formula](https://github.com/Homebrew/homebrew-core/blob/HEAD/Formula/envv.rb): +You can check that the output is as expected with `assert_equal` or `assert_match` on the output of the [Formula assertions](https://rubydoc.brew.sh/Homebrew/Assertions.html) such as in this example from the [envv formula](https://github.com/Homebrew/homebrew-core/blob/HEAD/Formula/envv.rb): ```ruby assert_equal "mylist=A:C; export mylist", shell_output("#{bin}/envv del mylist B").strip From 15bc09d9160ff90668b4dd0cbf902ccc86206ab5 Mon Sep 17 00:00:00 2001 From: Sam Ford <1584702+samford@users.noreply.github.com> Date: Sat, 27 Jun 2020 21:03:16 -0400 Subject: [PATCH 15/32] version: add empty? method --- Library/Homebrew/test/version_spec.rb | 10 ++++++++++ Library/Homebrew/version.rb | 4 ++++ 2 files changed, 14 insertions(+) diff --git a/Library/Homebrew/test/version_spec.rb b/Library/Homebrew/test/version_spec.rb index 60128a8a2c..03b9ab7e88 100644 --- a/Library/Homebrew/test/version_spec.rb +++ b/Library/Homebrew/test/version_spec.rb @@ -173,6 +173,16 @@ describe Version do expect(versions.sort_by { |v| described_class.create(v) }).to eq(versions) end + describe "#empty?" do + it "returns true if version is empty" do + expect(described_class.create("").empty?).to eq(true) + end + + it "returns false if version is not empty" do + expect(described_class.create("1.2.3").empty?).to eq(false) + end + end + specify "hash equality" do v1 = described_class.create("0.1.0") v2 = described_class.create("0.1.0") diff --git a/Library/Homebrew/version.rb b/Library/Homebrew/version.rb index 27116a243e..275c99f6e9 100644 --- a/Library/Homebrew/version.rb +++ b/Library/Homebrew/version.rb @@ -429,6 +429,10 @@ class Version end alias eql? == + def empty? + version.empty? + end + def hash version.hash end From 68947f2af9ed83aa1d6ac5e54d87f3935dfd498c Mon Sep 17 00:00:00 2001 From: EricFromCanada Date: Fri, 26 Jun 2020 18:10:16 -0400 Subject: [PATCH 16/32] cmd/options: add flag to list a command's options --- Library/Homebrew/cmd/options.rb | 35 ++++++++++++++++++++++++++++++++- docs/Manpage.md | 2 ++ manpages/brew.1 | 4 ++++ 3 files changed, 40 insertions(+), 1 deletion(-) diff --git a/Library/Homebrew/cmd/options.rb b/Library/Homebrew/cmd/options.rb index 397a39df0c..c41d27f149 100644 --- a/Library/Homebrew/cmd/options.rb +++ b/Library/Homebrew/cmd/options.rb @@ -3,6 +3,7 @@ require "formula" require "options" require "cli/parser" +require "commands" module Homebrew module_function @@ -20,8 +21,10 @@ module Homebrew description: "Show options for formulae that are currently installed." switch "--all", description: "Show options for all available formulae." + flag "--command=", + description: "Show options for the specified ." switch :debug - conflicts "--installed", "--all" + conflicts "--installed", "--all", "--command" end end @@ -32,6 +35,22 @@ module Homebrew puts_options Formula.to_a.sort elsif args.installed? puts_options Formula.installed.sort + elsif !args.command.nil? + path = Commands.path(args.command) + odie "Unknown command: #{args.command}" unless path + cmd_options = if cmd_parser = CLI::Parser.from_cmd_path(path) + cmd_parser.processed_options.map do |short, long, _, desc| + [long || short, desc] + end + else + cmd_comment_options(path) + end + if args.compact? + puts cmd_options.sort.map(&:first) * " " + else + cmd_options.sort.each { |option, desc| puts "#{option}\n\t#{desc}" } + puts + end elsif args.no_named? raise FormulaUnspecifiedError else @@ -39,6 +58,20 @@ module Homebrew end end + def cmd_comment_options(cmd_path) + options = [] + comment_lines = cmd_path.read.lines.grep(/^#:/) + return options if comment_lines.empty? + + # skip the comment's initial usage summary lines + comment_lines.slice(2..-1).each do |line| + if / (?