From 8537a68533d45024e8ec03d91288322e239baed5 Mon Sep 17 00:00:00 2001 From: Mike McQuaid Date: Thu, 16 Aug 2018 09:39:44 +0100 Subject: [PATCH 01/13] README: add jonchang. Another maintainer! --- README.md | 2 +- docs/Manpage.md | 2 +- manpages/brew.1 | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 00724142d0..b1732901d5 100644 --- a/README.md +++ b/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). diff --git a/docs/Manpage.md b/docs/Manpage.md index 6d0d1c837a..743230e68a 100644 --- a/docs/Manpage.md +++ b/docs/Manpage.md @@ -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. diff --git a/manpages/brew.1 b/manpages/brew.1 index 28e12b6011..36ce04d666 100644 --- a/manpages/brew.1 +++ b/manpages/brew.1 @@ -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\. From 40fc520a45ccca3142a1bad81e8ddcc3ecda6a0d Mon Sep 17 00:00:00 2001 From: Mike McQuaid Date: Thu, 16 Aug 2018 09:58:39 +0100 Subject: [PATCH 02/13] brew.rb: only output "Kernel.exit" in debugging mode. This doesn't seem to make sense for `--verbose`. --- Library/Homebrew/brew.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Library/Homebrew/brew.rb b/Library/Homebrew/brew.rb index e956eba77d..527dec79e7 100644 --- a/Library/Homebrew/brew.rb +++ b/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 From 0c7dbd07bc35e01a2759f3d129609e917af65580 Mon Sep 17 00:00:00 2001 From: Mike McQuaid Date: Thu, 16 Aug 2018 10:48:11 +0100 Subject: [PATCH 03/13] manpage: add `brew bundle check --verbose` --- docs/Manpage.md | 4 ++-- manpages/brew.1 | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/Manpage.md b/docs/Manpage.md index 743230e68a..ef9a188664 100644 --- a/docs/Manpage.md +++ b/docs/Manpage.md @@ -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. diff --git a/manpages/brew.1 b/manpages/brew.1 index 36ce04d666..9ebdc66b90 100644 --- a/manpages/brew.1 +++ b/manpages/brew.1 @@ -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 From efd1884c2e5ec23ccdc13078d08444f1bca2ea88 Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Thu, 16 Aug 2018 05:55:17 +0200 Subject: [PATCH 04/13] Hide lockfiles being cleaned up again. --- Library/Homebrew/cleanup.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Library/Homebrew/cleanup.rb b/Library/Homebrew/cleanup.rb index c58c3b6de8..a4fb1e0b9b 100644 --- a/Library/Homebrew/cleanup.rb +++ b/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 From 526262fca51e9effda6dd025667a4b38abbdd6c7 Mon Sep 17 00:00:00 2001 From: JBallin Date: Wed, 15 Aug 2018 09:49:44 -0700 Subject: [PATCH 05/13] Change to to match the suggested inputs --- Library/Homebrew/cmd/upgrade.rb | 2 +- docs/Manpage.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Library/Homebrew/cmd/upgrade.rb b/Library/Homebrew/cmd/upgrade.rb index ada3ca5e54..64674d8280 100644 --- a/Library/Homebrew/cmd/upgrade.rb +++ b/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 version(s). +#: previously installed version(s). #: #: If `--fetch-HEAD` is passed, fetch the upstream repository to detect if #: the HEAD installation of the formula is outdated. Otherwise, the diff --git a/docs/Manpage.md b/docs/Manpage.md index ef9a188664..f343c2ca8a 100644 --- a/docs/Manpage.md +++ b/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 `formulae` version(s). If `--fetch-HEAD` is passed, fetch the upstream repository to detect if the HEAD installation of the formula is outdated. Otherwise, the From 520b5a103ad251307b92403d28f74dc245769850 Mon Sep 17 00:00:00 2001 From: JBallin Date: Wed, 15 Aug 2018 09:53:20 -0700 Subject: [PATCH 06/13] Clarify that --cleanup only runs on formulae which were just upgraded --- Library/Homebrew/cmd/upgrade.rb | 2 +- docs/Manpage.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Library/Homebrew/cmd/upgrade.rb b/Library/Homebrew/cmd/upgrade.rb index 64674d8280..f170d1f220 100644 --- a/Library/Homebrew/cmd/upgrade.rb +++ b/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 version(s). +#: previously installed version(s) of upgraded . #: #: If `--fetch-HEAD` is passed, fetch the upstream repository to detect if #: the HEAD installation of the formula is outdated. Otherwise, the diff --git a/docs/Manpage.md b/docs/Manpage.md index f343c2ca8a..d716efd57a 100644 --- a/docs/Manpage.md +++ b/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 `formulae` 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 From f065c0a737a0c1ede1753b9b5668fa1f4bfe5623 Mon Sep 17 00:00:00 2001 From: JBallin Date: Thu, 16 Aug 2018 09:29:30 -0700 Subject: [PATCH 07/13] Generate man page --- manpages/brew.1 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/manpages/brew.1 b/manpages/brew.1 index 9ebdc66b90..fee4bd3757 100644 --- a/manpages/brew.1 +++ b/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\. From 24873454609a66c45ffdde7eb5931a7c6c55c921 Mon Sep 17 00:00:00 2001 From: Jonathan Chang Date: Wed, 15 Aug 2018 19:29:22 -0400 Subject: [PATCH 08/13] audit: fixes for test checks --- Library/Homebrew/dev-cmd/audit.rb | 6 +----- Library/Homebrew/rubocops/class_cop.rb | 21 +++++++++++++++++++++ 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/Library/Homebrew/dev-cmd/audit.rb b/Library/Homebrew/dev-cmd/audit.rb index 8818bf2474..de9a156c04 100644 --- a/Library/Homebrew/dev-cmd/audit.rb +++ b/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 diff --git a/Library/Homebrew/rubocops/class_cop.rb b/Library/Homebrew/rubocops/class_cop.rb index 8e0df3cbc8..0976104752 100644 --- a/Library/Homebrew/rubocops/class_cop.rb +++ b/Library/Homebrew/rubocops/class_cop.rb @@ -22,6 +22,27 @@ module RuboCop end end end + + class TestCalls < FormulaCop + def audit_formula(_node, _class_node, _parent_class_node, body_node) + test_calls(find_block(body_node, :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_node_search :test_calls, <<~EOS + (send nil? ${:system :shell_output :pipe_output} $...) + EOS + end end module FormulaAuditStrict From 5f981a8722487dd9ee127a59f4f0471fa35c3c4c Mon Sep 17 00:00:00 2001 From: Jonathan Chang Date: Wed, 15 Aug 2018 19:43:57 -0400 Subject: [PATCH 09/13] tests: add test for test calls audit cop --- .../Homebrew/test/rubocops/class_cop_spec.rb | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/Library/Homebrew/test/rubocops/class_cop_spec.rb b/Library/Homebrew/test/rubocops/class_cop_spec.rb index 6314978b45..469fbe8e5b 100644 --- a/Library/Homebrew/test/rubocops/class_cop_spec.rb +++ b/Library/Homebrew/test/rubocops/class_cop_spec.rb @@ -48,6 +48,36 @@ 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 +end + describe RuboCop::Cop::FormulaAuditStrict::Test do subject(:cop) { described_class.new } From ca59377a90970a88b58aeda8cf74b8803931dcdc Mon Sep 17 00:00:00 2001 From: Jonathan Chang Date: Thu, 16 Aug 2018 12:42:45 -0400 Subject: [PATCH 10/13] audit: add autocorrect and tests for test checks --- Library/Homebrew/rubocops/class_cop.rb | 11 ++++++++ .../Homebrew/test/rubocops/class_cop_spec.rb | 25 +++++++++++++++++++ 2 files changed, 36 insertions(+) diff --git a/Library/Homebrew/rubocops/class_cop.rb b/Library/Homebrew/rubocops/class_cop.rb index 0976104752..3b667e75c8 100644 --- a/Library/Homebrew/rubocops/class_cop.rb +++ b/Library/Homebrew/rubocops/class_cop.rb @@ -39,6 +39,17 @@ module RuboCop 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 diff --git a/Library/Homebrew/test/rubocops/class_cop_spec.rb b/Library/Homebrew/test/rubocops/class_cop_spec.rb index 469fbe8e5b..7ada0bebd3 100644 --- a/Library/Homebrew/test/rubocops/class_cop_spec.rb +++ b/Library/Homebrew/test/rubocops/class_cop_spec.rb @@ -76,6 +76,31 @@ describe RuboCop::Cop::FormulaAudit::TestCalls do 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 From 25d0c020c9a39f99917be6d968678599ceb346b0 Mon Sep 17 00:00:00 2001 From: Jonathan Chang Date: Thu, 16 Aug 2018 16:38:41 -0400 Subject: [PATCH 11/13] audit: don't error when test block is missing --- Library/Homebrew/rubocops/class_cop.rb | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Library/Homebrew/rubocops/class_cop.rb b/Library/Homebrew/rubocops/class_cop.rb index 3b667e75c8..cc24f5f595 100644 --- a/Library/Homebrew/rubocops/class_cop.rb +++ b/Library/Homebrew/rubocops/class_cop.rb @@ -25,7 +25,10 @@ module RuboCop class TestCalls < FormulaCop def audit_formula(_node, _class_node, _parent_class_node, body_node) - test_calls(find_block(body_node, :test)) do |node, params| + 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) From 5c90833f0a16e9326750959aec91d779b889ceca Mon Sep 17 00:00:00 2001 From: William Woodruff Date: Mon, 13 Aug 2018 23:46:51 -0400 Subject: [PATCH 12/13] Use JSON to marshal errors from children --- Library/Homebrew/build.rb | 14 +++++++++++++- Library/Homebrew/dev-cmd/test.rb | 12 +++++++----- Library/Homebrew/exceptions.rb | 25 ++++++++++++++++++++++--- Library/Homebrew/formula_installer.rb | 15 +++++++++++++-- Library/Homebrew/postinstall.rb | 3 ++- Library/Homebrew/test.rb | 3 ++- Library/Homebrew/utils/fork.rb | 6 ++++-- 7 files changed, 63 insertions(+), 15 deletions(-) diff --git a/Library/Homebrew/build.rb b/Library/Homebrew/build.rb index 4dfaf9e7e0..a0d453e50f 100644 --- a/Library/Homebrew/build.rb +++ b/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 diff --git a/Library/Homebrew/dev-cmd/test.rb b/Library/Homebrew/dev-cmd/test.rb index d21a62cde5..310ab94a6c 100644 --- a/Library/Homebrew/dev-cmd/test.rb +++ b/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 - rescue Exception => e # rubocop:disable Lint/RescueException - ofail "#{f.full_name}: failed" - puts e, e.backtrace + case e.inner["json_class"] + when "Test::Unit::AssertionFailedError" + puts e.inner["m"] + else + puts e.inner["json_class"], e.backtrace + end ensure ENV.replace(env) end diff --git a/Library/Homebrew/exceptions.rb b/Library/Homebrew/exceptions.rb index dc3f5f9e36..ab6f5c6724 100644 --- a/Library/Homebrew/exceptions.rb +++ b/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(" ") - super "Failed executing: #{cmd} #{args}" + pretty_args = args.map { |arg| arg.to_s.gsub " ", "\\ " }.join(" ") + 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 diff --git a/Library/Homebrew/formula_installer.rb b/Library/Homebrew/formula_installer.rb index 66a44a9cde..f0c834500f 100644 --- a/Library/Homebrew/formula_installer.rb +++ b/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) diff --git a/Library/Homebrew/postinstall.rb b/Library/Homebrew/postinstall.rb index 53a5b7e751..916dbada37 100644 --- a/Library/Homebrew/postinstall.rb +++ b/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 diff --git a/Library/Homebrew/test.rb b/Library/Homebrew/test.rb index 3d5e62a884..0c4b1bb36e 100644 --- a/Library/Homebrew/test.rb +++ b/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 diff --git a/Library/Homebrew/utils/fork.rb b/Library/Homebrew/utils/fork.rb index 5087ca716c..bf4e84143d 100644 --- a/Library/Homebrew/utils/fork.rb +++ b/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 From 4141c494d919a40d6ca68d92430c8d8ead47805e Mon Sep 17 00:00:00 2001 From: JBallin Date: Fri, 17 Aug 2018 12:53:12 -0700 Subject: [PATCH 13/13] Link first-timers to "How To Open A Homebrew Pull Request" --- CONTRIBUTING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 7ba6830c03..9d85186358 100644 --- a/CONTRIBUTING.md +++ b/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