Merge branch 'master' into hbc_analytics

CONTRIBUTING.md

@@ -1,5 +1,5 @@
 # Contributing to Homebrew
-First time contributing to Homebrew? Read our [Code of Conduct](https://github.com/Homebrew/brew/blob/master/CODE_OF_CONDUCT.md#code-of-conduct).
+First time contributing to Homebrew? Read our [Code of Conduct](https://github.com/Homebrew/brew/blob/master/CODE_OF_CONDUCT.md#code-of-conduct) and review [How To Open a Homebrew Pull Request](https://docs.brew.sh/How-To-Open-a-Homebrew-Pull-Request).
 
 ### Report a bug
 

Library/Homebrew/brew.rb

@@ -114,7 +114,7 @@ rescue UsageError => e
   require "help"
   Homebrew::Help.help cmd, usage_error: e.message
 rescue SystemExit => e
-  onoe "Kernel.exit" if ARGV.verbose? && !e.success?
+  onoe "Kernel.exit" if ARGV.debug? && !e.success?
   $stderr.puts e.backtrace if ARGV.debug?
   raise
 rescue Interrupt

Library/Homebrew/build.rb

@@ -11,6 +11,8 @@ require "extend/ENV"
 require "debrew"
 require "fcntl"
 require "socket"
+require "json"
+require "json/add/core"
 
 class Build
   attr_reader :formula, :deps, :reqs
@@ -190,7 +192,17 @@ begin
   build   = Build.new(formula, options)
   build.install
 rescue Exception => e # rubocop:disable Lint/RescueException
-  Marshal.dump(e, error_pipe)
+  error_hash = JSON.parse e.to_json
+
+  # Special case: We need to toss our build state into the error hash
+  # for proper analytics reporting and sensible error messages.
+  if e.is_a?(BuildError)
+    error_hash["cmd"] = e.cmd
+    error_hash["args"] = e.args
+    error_hash["env"] = e.env
+  end
+
+  error_pipe.write error_hash.to_json
   error_pipe.close
   exit! 1
 end

Library/Homebrew/cleanup.rb

@@ -245,9 +245,9 @@ module Homebrew
     end
 
     def cleanup_lockfiles(*lockfiles)
-      return unless HOMEBREW_LOCK_DIR.directory?
+      return if dry_run?
 
-      if lockfiles.empty?
+      if lockfiles.empty? && HOMEBREW_LOCK_DIR.directory?
         lockfiles = HOMEBREW_LOCK_DIR.children.select(&:file?)
       end
 
@@ -256,7 +256,7 @@ module Homebrew
         next unless file.open(File::RDWR).flock(File::LOCK_EX | File::LOCK_NB)
 
         begin
-          cleanup_path(file) { file.unlink }
+          file.unlink
         ensure
           file.open(File::RDWR).flock(File::LOCK_UN) if file.exist?
         end

Library/Homebrew/cmd/upgrade.rb

@@ -4,7 +4,7 @@
 #:    Options for the `install` command are also valid here.
 #:
 #:    If `--cleanup` is specified or `HOMEBREW_UPGRADE_CLEANUP` is set then remove
-#:    previously installed <formula> version(s).
+#:    previously installed version(s) of upgraded <formulae>.
 #:
 #:    If `--fetch-HEAD` is passed, fetch the upstream repository to detect if
 #:    the HEAD installation of the formula is outdated. Otherwise, the

Library/Homebrew/dev-cmd/audit.rb

@@ -772,7 +772,7 @@ module Homebrew
       end
       bin_names.each do |name|
         ["system", "shell_output", "pipe_output"].each do |cmd|
-          if text =~ %r{(def test|test do).*(#{Regexp.escape(HOMEBREW_PREFIX)}/bin/)?#{cmd}[\(\s]+['"]#{Regexp.escape(name)}[\s'"]}m
+          if text =~ /test do.*#{cmd}[\(\s]+['"]#{Regexp.escape(name)}[\s'"]/m
             problem %Q(fully scope test #{cmd} calls e.g. #{cmd} "\#{bin}/#{name}")
           end
         end
@@ -803,10 +803,6 @@ module Homebrew
 
       problem "Use separate make calls" if line.include?("make && make")
 
-      if line =~ /shell_output\(['"].+['"], 0\)/
-        problem "Passing 0 to shell_output() is redundant"
-      end
-
       if line =~ /JAVA_HOME/i && !formula.requirements.map(&:class).include?(JavaRequirement)
         problem "Use `depends_on :java` to set JAVA_HOME"
       end

Library/Homebrew/dev-cmd/test.rb

@@ -93,12 +93,14 @@ module Homebrew
             exec(*args)
           end
         end
-      rescue ::Test::Unit::AssertionFailedError => e
+      rescue ChildProcessError => e
         ofail "#{f.full_name}: failed"
-        puts e.message
+        case e.inner["json_class"]
-      rescue Exception => e # rubocop:disable Lint/RescueException
+        when "Test::Unit::AssertionFailedError"
-        ofail "#{f.full_name}: failed"
+          puts e.inner["m"]
-        puts e, e.backtrace
+        else
+          puts e.inner["json_class"], e.backtrace
+        end
       ensure
         ENV.replace(env)
       end

Library/Homebrew/exceptions.rb

@@ -352,14 +352,16 @@ class FormulaAmbiguousPythonError < RuntimeError
 end
 
 class BuildError < RuntimeError
-  attr_reader :formula, :env
+  attr_reader :formula, :cmd, :args, :env
   attr_accessor :options
 
   def initialize(formula, cmd, args, env)
     @formula = formula
+    @cmd = cmd
+    @args = args
     @env = env
-    args = args.map { |arg| arg.to_s.gsub " ", "\\ " }.join(" ")
+    pretty_args = args.map { |arg| arg.to_s.gsub " ", "\\ " }.join(" ")
-    super "Failed executing: #{cmd} #{args}"
+    super "Failed executing: #{cmd} #{pretty_args}"
   end
 
   def issues
@@ -596,3 +598,20 @@ class BottleFormulaUnavailableError < RuntimeError
     EOS
   end
 end
+
+# Raised when a child process sends us an exception over its error pipe.
+class ChildProcessError < RuntimeError
+  attr_reader :inner
+
+  def initialize(inner)
+    @inner = inner
+
+    super <<~EOS
+      An exception occured within a build process:
+        #{inner["json_class"]}: #{inner["m"]}
+    EOS
+
+    # Clobber our real (but irrelevant) backtrace with that of the inner exception.
+    set_backtrace inner["b"]
+  end
+end

Library/Homebrew/formula_installer.rb

@@ -763,14 +763,25 @@ class FormulaInstaller
       raise "Empty installation"
     end
   rescue Exception => e # rubocop:disable Lint/RescueException
-    e.options = display_options(formula) if e.is_a?(BuildError)
+    # If we've rescued a ChildProcessError and that ChildProcessError
+    # contains a BuildError, then we reconstruct the inner build error
+    # to make analytics happy.
+    if e.is_a?(ChildProcessError) && e.inner["json_class"] == "BuildError"
+      build_error = BuildError.new(formula, e["cmd"], e["args"], e["env"])
+      build_error.set_backtrace e.backtrace
+      build_error.options = display_options(formula)
+
+      e = build_error
+    end
+
     ignore_interrupts do
       # any exceptions must leave us with nothing installed
       formula.update_head_version
       formula.prefix.rmtree if formula.prefix.directory?
       formula.rack.rmdir_if_possible
     end
-    raise
+
+    raise e
   end
 
   def link(keg)

Library/Homebrew/postinstall.rb

@@ -4,6 +4,7 @@ require "global"
 require "debrew"
 require "fcntl"
 require "socket"
+require "json/add/core"
 
 begin
   error_pipe = UNIXSocket.open(ENV["HOMEBREW_ERROR_PIPE"], &:recv_io)
@@ -15,7 +16,7 @@ begin
   formula.extend(Debrew::Formula) if ARGV.debug?
   formula.run_post_install
 rescue Exception => e # rubocop:disable Lint/RescueException
-  Marshal.dump(e, error_pipe)
+  error_pipe.write e.to_json
   error_pipe.close
   exit! 1
 end

Library/Homebrew/rubocops/class_cop.rb

@@ -22,6 +22,41 @@ module RuboCop
           end
         end
       end
+
+      class TestCalls < FormulaCop
+        def audit_formula(_node, _class_node, _parent_class_node, body_node)
+          test = find_block(body_node, :test)
+          return unless test
+
+          test_calls(test) do |node, params|
+            p1, p2 = params
+            if match = string_content(p1).match(%r{(/usr/local/(s?bin))})
+              offending_node(p1)
+              problem "use \#{#{match[2]}} instead of #{match[1]} in #{node}"
+            end
+
+            if node == :shell_output && node_equals?(p2, 0)
+              offending_node(p2)
+              problem "Passing 0 to shell_output() is redundant"
+            end
+          end
+        end
+
+        def autocorrect(node)
+          lambda do |corrector|
+            case node.type
+            when :str, :dstr
+              corrector.replace(node.source_range, node.source.to_s.sub(%r{(/usr/local/(s?bin))}, '#{\2}'))
+            when :int
+              corrector.remove(range_with_surrounding_comma(range_with_surrounding_space(range: node.source_range, side: :left)))
+            end
+          end
+        end
+
+        def_node_search :test_calls, <<~EOS
+          (send nil? ${:system :shell_output :pipe_output} $...)
+        EOS
+      end
     end
 
     module FormulaAuditStrict

Library/Homebrew/test.rb

@@ -7,6 +7,7 @@ require "debrew"
 require "formula_assertions"
 require "fcntl"
 require "socket"
+require "json/add/core"
 
 TEST_TIMEOUT_SECONDS = 5 * 60
 
@@ -28,7 +29,7 @@ begin
     raise "test returned false" if formula.run_test == false
   end
 rescue Exception => e # rubocop:disable Lint/RescueException
-  Marshal.dump(e, error_pipe)
+  error_pipe.write e.to_json
   error_pipe.close
   exit! 1
 end

Library/Homebrew/test/rubocops/class_cop_spec.rb

@@ -48,6 +48,61 @@ describe RuboCop::Cop::FormulaAudit::ClassName do
   end
 end
 
+describe RuboCop::Cop::FormulaAudit::TestCalls do
+  subject(:cop) { described_class.new }
+
+  it "reports an offense when /usr/local/bin is found in test calls" do
+    expect_offense(<<~RUBY)
+      class Foo < Formula
+        url 'https://example.com/foo-1.0.tgz'
+
+        test do
+          system "/usr/local/bin/test"
+                 ^^^^^^^^^^^^^^^^^^^^^ use \#{bin} instead of /usr/local/bin in system
+        end
+      end
+    RUBY
+  end
+
+  it "reports an offense when passing 0 as the second parameter to shell_output" do
+    expect_offense(<<~RUBY)
+      class Foo < Formula
+        url 'https://example.com/foo-1.0.tgz'
+
+        test do
+          shell_output("\#{bin}/test", 0)
+                                      ^ Passing 0 to shell_output() is redundant
+        end
+      end
+    RUBY
+  end
+
+  it "supports auto-correcting test calls" do
+    source = <<~RUBY
+      class Foo < Formula
+        url 'https://example.com/foo-1.0.tgz'
+
+        test do
+          shell_output("/usr/local/sbin/test", 0)
+        end
+      end
+    RUBY
+
+    corrected_source = <<~RUBY
+      class Foo < Formula
+        url 'https://example.com/foo-1.0.tgz'
+
+        test do
+          shell_output("\#{sbin}/test")
+        end
+      end
+    RUBY
+
+    new_source = autocorrect_source(source)
+    expect(new_source).to eq(corrected_source)
+  end
+end
+
 describe RuboCop::Cop::FormulaAuditStrict::Test do
   subject(:cop) { described_class.new }
 

Library/Homebrew/utils/fork.rb

@@ -1,5 +1,7 @@
 require "fcntl"
 require "socket"
+require "json"
+require "json/add/core"
 
 module Utils
   def self.safe_fork(&_block)
@@ -15,7 +17,7 @@ module Utils
             write.fcntl(Fcntl::F_SETFD, Fcntl::FD_CLOEXEC)
             yield
           rescue Exception => e # rubocop:disable Lint/RescueException
-            Marshal.dump(e, write)
+            write.write e.to_json
             write.close
             exit!
           else
@@ -36,7 +38,7 @@ module Utils
           data = read.read
           read.close
           Process.wait(pid) unless socket.nil?
-          raise Marshal.load(data) unless data.nil? || data.empty? # rubocop:disable Security/MarshalLoad
+          raise ChildProcessError, JSON.parse(data) unless data.nil? || data.empty?
           raise Interrupt if $CHILD_STATUS.exitstatus == 130
           raise "Forked child process failed: #{$CHILD_STATUS}" unless $CHILD_STATUS.success?
         end

README.md

@@ -40,7 +40,7 @@ Homebrew's lead maintainer is [Mike McQuaid](https://github.com/mikemcquaid).
 
 Homebrew's project leadership committee is [Mike McQuaid](https://github.com/mikemcquaid), [JCount](https://github.com/jcount), [Misty De Meo](https://github.com/mistydemeo) and [Markus Reiter](https://github.com/reitermarkus).
 
-Homebrew/brew's other current maintainers are [Dominyk Tiller](https://github.com/DomT4), [Claudia](https://github.com/claui), [Michka Popoff](https://github.com/imichka), [Shaun Jackman](https://github.com/sjackman), [Chongyu Zhu](https://github.com/lembacon), [commitay](https://github.com/commitay), [Vitor Galvao](https://github.com/vitorgalvao), [JCount](https://github.com/jcount), [Misty De Meo](https://github.com/mistydemeo), [Gautham Goli](https://github.com/GauthamGoli), [Markus Reiter](https://github.com/reitermarkus) and [William Woodruff](https://github.com/woodruffw).
+Homebrew/brew's other current maintainers are [Dominyk Tiller](https://github.com/DomT4), [Claudia](https://github.com/claui), [Michka Popoff](https://github.com/imichka), [Shaun Jackman](https://github.com/sjackman), [Chongyu Zhu](https://github.com/lembacon), [commitay](https://github.com/commitay), [Vitor Galvao](https://github.com/vitorgalvao), [JCount](https://github.com/jcount), [Misty De Meo](https://github.com/mistydemeo), [Gautham Goli](https://github.com/GauthamGoli), [Markus Reiter](https://github.com/reitermarkus), [Jonathan Chang](https://github.com/jonchang) and [William Woodruff](https://github.com/woodruffw).
 
 Homebrew/brew's Linux support (and Linuxbrew) maintainers are [Michka Popoff](https://github.com/imichka) and [Shaun Jackman](https://github.com/sjackman).
 

docs/Manpage.md

@@ -576,7 +576,7 @@ With `--verbose` or `-v`, many commands print extra debugging information. Note
     Options for the `install` command are also valid here.
 
     If `--cleanup` is specified or `HOMEBREW_UPGRADE_CLEANUP` is set then remove
-    previously installed `formula` version(s).
+    previously installed version(s) of upgraded `formulae`.
 
     If `--fetch-HEAD` is passed, fetch the upstream repository to detect if
     the HEAD installation of the formula is outdated. Otherwise, the
@@ -967,9 +967,9 @@ With `--verbose` or `-v`, many commands print extra debugging information. Note
 
   
 
-      `brew bundle check` [`--no-upgrade`] [`--file`=`path`|`--global`]
+      `brew bundle check` [`--no-upgrade`] [`--file`=`path`|`--global`] [`--verbose`]
 
-      Check if all dependencies are installed in a Brewfile.
+      Check if all dependencies are installed in a Brewfile. Missing dependencies are listed in verbose mode. `check` will exit on the first category missing a dependency unless in verbose mode.
 
   
 
@@ -1316,7 +1316,7 @@ Homebrew's lead maintainer is Mike McQuaid.
 
 Homebrew's project leadership committee is Mike McQuaid, JCount, Misty De Meo and Markus Reiter.
 
-Homebrew/brew's other current maintainers are Dominyk Tiller, Claudia, Michka Popoff, Shaun Jackman, Chongyu Zhu, commitay, Vitor Galvao, JCount, Misty De Meo, Gautham Goli, Markus Reiter and William Woodruff.
+Homebrew/brew's other current maintainers are Dominyk Tiller, Claudia, Michka Popoff, Shaun Jackman, Chongyu Zhu, commitay, Vitor Galvao, JCount, Misty De Meo, Gautham Goli, Markus Reiter, Jonathan Chang and William Woodruff.
 
 Homebrew/brew's Linux support (and Linuxbrew) maintainers are Michka Popoff and Shaun Jackman.
 

manpages/brew.1

@@ -524,7 +524,7 @@ If \fB\-\-force\fR (or \fB\-f\fR) is specified then always do a slower, full upd
 Options for the \fBinstall\fR command are also valid here\.
 .
 .IP
-If \fB\-\-cleanup\fR is specified or \fBHOMEBREW_UPGRADE_CLEANUP\fR is set then remove previously installed \fIformula\fR version(s)\.
+If \fB\-\-cleanup\fR is specified or \fBHOMEBREW_UPGRADE_CLEANUP\fR is set then remove previously installed version(s) of upgraded \fIformulae\fR\.
 .
 .IP
 If \fB\-\-fetch\-HEAD\fR is passed, fetch the upstream repository to detect if the HEAD installation of the formula is outdated\. Otherwise, the repository\'s HEAD will be checked for updates when a new stable or devel version has been released\.
@@ -914,10 +914,10 @@ Write all installed casks/formulae/taps into a Brewfile\.
 Uninstall all dependencies not listed in a Brewfile\.
 .
 .IP
-\fBbrew bundle check\fR [\fB\-\-no\-upgrade\fR] [\fB\-\-file\fR=\fIpath\fR|\fB\-\-global\fR]
+\fBbrew bundle check\fR [\fB\-\-no\-upgrade\fR] [\fB\-\-file\fR=\fIpath\fR|\fB\-\-global\fR] [\fB\-\-verbose\fR]
 .
 .IP
-Check if all dependencies are installed in a Brewfile\.
+Check if all dependencies are installed in a Brewfile\. Missing dependencies are listed in verbose mode\. \fBcheck\fR will exit on the first category missing a dependency unless in verbose mode\.
 .
 .IP
 \fBbrew bundle exec\fR \fIcommand\fR
@@ -1275,7 +1275,7 @@ Homebrew\'s lead maintainer is Mike McQuaid\.
 Homebrew\'s project leadership committee is Mike McQuaid, JCount, Misty De Meo and Markus Reiter\.
 .
 .P
-Homebrew/brew\'s other current maintainers are Dominyk Tiller, Claudia, Michka Popoff, Shaun Jackman, Chongyu Zhu, commitay, Vitor Galvao, JCount, Misty De Meo, Gautham Goli, Markus Reiter and William Woodruff\.
+Homebrew/brew\'s other current maintainers are Dominyk Tiller, Claudia, Michka Popoff, Shaun Jackman, Chongyu Zhu, commitay, Vitor Galvao, JCount, Misty De Meo, Gautham Goli, Markus Reiter, Jonathan Chang and William Woodruff\.
 .
 .P
 Homebrew/brew\'s Linux support (and Linuxbrew) maintainers are Michka Popoff and Shaun Jackman\.