From 4a03145c1c02a57bdef3043e8e3d987e5db06d8a Mon Sep 17 00:00:00 2001 From: Mike McQuaid Date: Tue, 24 Apr 2018 09:52:51 +0100 Subject: [PATCH] linkage: fix --test exit code. Ensure that a non-zero exit code is set both for missing random dylibs and random missing dependencies. Additionally, while we are here, drastically trim down the public interface for this class to the bare minimum and allow getting the output from `display_test_output` as a variable. Fixes issue mentioned by @ilovezfs in: https://github.com/Homebrew/brew/pull/3940#issuecomment-383794520 --- Library/Homebrew/dev-cmd/linkage.rb | 2 +- .../extend/os/mac/formula_cellar_checks.rb | 4 +- Library/Homebrew/linkage_checker.rb | 97 +++++++++---------- 3 files changed, 47 insertions(+), 56 deletions(-) diff --git a/Library/Homebrew/dev-cmd/linkage.rb b/Library/Homebrew/dev-cmd/linkage.rb index c33c181a1e..834ed066ed 100644 --- a/Library/Homebrew/dev-cmd/linkage.rb +++ b/Library/Homebrew/dev-cmd/linkage.rb @@ -21,7 +21,7 @@ module Homebrew result = LinkageChecker.new(keg) if ARGV.include?("--test") result.display_test_output - Homebrew.failed = true if result.broken_dylibs? + Homebrew.failed = true if result.broken_library_linkage? elsif ARGV.include?("--reverse") result.display_reverse_output else diff --git a/Library/Homebrew/extend/os/mac/formula_cellar_checks.rb b/Library/Homebrew/extend/os/mac/formula_cellar_checks.rb index 0b1a1643e1..1b43b0448b 100644 --- a/Library/Homebrew/extend/os/mac/formula_cellar_checks.rb +++ b/Library/Homebrew/extend/os/mac/formula_cellar_checks.rb @@ -66,10 +66,10 @@ module FormulaCellarChecks keg = Keg.new(formula.prefix) checker = LinkageChecker.new(keg, formula) - return unless checker.broken_dylibs? + return unless checker.broken_library_linkage? output = <<~EOS #{formula} has broken dynamic library links: - #{checker.broken_dylibs.to_a * "\n "} + #{checker.display_test_output} EOS tab = Tab.for_keg(keg) if tab.poured_from_bottle diff --git a/Library/Homebrew/linkage_checker.rb b/Library/Homebrew/linkage_checker.rb index d601a31d7f..bedb7fa00c 100644 --- a/Library/Homebrew/linkage_checker.rb +++ b/Library/Homebrew/linkage_checker.rb @@ -2,10 +2,6 @@ require "keg" require "formula" class LinkageChecker - attr_reader :keg, :formula - attr_reader :brewed_dylibs, :system_dylibs, :broken_dylibs, :broken_deps, :variable_dylibs - attr_reader :undeclared_deps, :unnecessary_deps, :reverse_links - def initialize(keg, formula = nil) @keg = keg @formula = formula || resolve_formula(keg) @@ -21,6 +17,44 @@ class LinkageChecker check_dylibs end + def display_normal_output + display_items "System libraries", @system_dylibs + display_items "Homebrew libraries", @brewed_dylibs + display_items "Indirect dependencies with linkage", @indirect_deps + display_items "Variable-referenced libraries", @variable_dylibs + display_items "Missing libraries", @broken_dylibs + display_items "Broken dependencies", @broken_deps + display_items "Undeclared dependencies with linkage", @undeclared_deps + display_items "Dependencies with no linkage", @unnecessary_deps + end + + def display_reverse_output + return if @reverse_links.empty? + sorted = @reverse_links.sort + sorted.each do |dylib, files| + puts dylib + files.each do |f| + unprefixed = f.to_s.strip_prefix "#{@keg}/" + puts " #{unprefixed}" + end + puts unless dylib == sorted.last[0] + end + end + + def display_test_output(puts_output: true) + display_items "Missing libraries", @broken_dylibs, puts_output: puts_output + display_items "Broken dependencies", @broken_deps, puts_output: puts_output + puts "No broken library linkage" unless broken_library_linkage? + end + + def broken_library_linkage? + !@broken_dylibs.empty? || !@broken_deps.empty? + end + + private + + attr_reader :keg, :formula + def dylib_to_dep(dylib) dylib =~ %r{#{Regexp.escape(HOMEBREW_PREFIX)}/(opt|Cellar)/([\w+-.@]+)/} Regexp.last_match(2) @@ -113,51 +147,6 @@ class LinkageChecker end end - def display_normal_output - display_items "System libraries", @system_dylibs - display_items "Homebrew libraries", @brewed_dylibs - display_items "Indirect dependencies with linkage", @indirect_deps - display_items "Variable-referenced libraries", @variable_dylibs - display_items "Missing libraries", @broken_dylibs - display_items "Broken dependencies", @broken_deps - display_items "Undeclared dependencies with linkage", @undeclared_deps - display_items "Dependencies with no linkage", @unnecessary_deps - end - - def display_reverse_output - return if @reverse_links.empty? - sorted = @reverse_links.sort - sorted.each do |dylib, files| - puts dylib - files.each do |f| - unprefixed = f.to_s.strip_prefix "#{@keg}/" - puts " #{unprefixed}" - end - puts unless dylib == sorted.last[0] - end - end - - def display_test_output - display_items "Missing libraries", @broken_dylibs - display_items "Broken dependencies", @broken_deps - display_items "Dependencies with no linkage", @unnecessary_deps - puts "No broken dylib links" if @broken_dylibs.empty? - end - - def broken_dylibs? - !@broken_dylibs.empty? - end - - def undeclared_deps? - !@undeclared_deps.empty? - end - - def unnecessary_deps? - !@unnecessary_deps.empty? - end - - private - # Whether or not dylib is a harmless broken link, meaning that it's # okay to skip (and not report) as broken. def harmless_broken_link?(dylib) @@ -171,20 +160,22 @@ class LinkageChecker # Display a list of things. # Things may either be an array, or a hash of (label -> array) - def display_items(label, things) + def display_items(label, things, puts_output: true) return if things.empty? - puts "#{label}:" + output = "#{label}:" if things.is_a? Hash things.keys.sort.each do |list_label| things[list_label].sort.each do |item| - puts " #{item} (#{list_label})" + output += "\n #{item} (#{list_label})" end end else things.sort.each do |item| - puts " #{item}" + output += "\n #{item}" end end + puts output if puts_output + output end def resolve_formula(keg)