From 2a6a2b2fa0c208f25524ea6ebcb0a9f30315d360 Mon Sep 17 00:00:00 2001 From: Maxim Belkin Date: Sat, 17 Mar 2018 00:15:51 -0500 Subject: [PATCH 01/13] Fixes for linkage_checker 1. In "display_test_output": when printing dependencies without linkage, use "Dependencies with no linkage" string instead of "Possible unnecessary dependencies" for consistency with "display_normal_output" 2. In "check_dylibs": keep track of and skip repetitive checking of those dylibs that have been previously checked. This is applicable when different executables/libraries are linked against the same libraries. 3. In "check_undeclared_deps": when there are missing libraries, corresponding dependencies should be excluded from the list of dependencies with no linkage. --- Library/Homebrew/linkage_checker.rb | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/Library/Homebrew/linkage_checker.rb b/Library/Homebrew/linkage_checker.rb index fb1aba067e..19ac85ed79 100644 --- a/Library/Homebrew/linkage_checker.rb +++ b/Library/Homebrew/linkage_checker.rb @@ -22,6 +22,7 @@ class LinkageChecker end def check_dylibs + checked_dylibs = Set.new @keg.find do |file| next if file.symlink? || file.directory? next unless file.dylib? || file.binary_executable? || file.mach_o_bundle? @@ -30,6 +31,7 @@ class LinkageChecker # when checking for broken linkage file.dynamically_linked_libraries(except: :LC_LOAD_WEAK_DYLIB).each do |dylib| @reverse_links[dylib] << file + next if checked_dylibs.include? dylib if dylib.start_with? "@" @variable_dylibs << dylib else @@ -50,6 +52,7 @@ class LinkageChecker @brewed_dylibs[f] << dylib end end + checked_dylibs << dylib end end @@ -83,6 +86,12 @@ class LinkageChecker next true if Formula[name].bin.directory? @brewed_dylibs.keys.map { |x| x.split("/").last }.include?(name) end + missing = [] + @broken_dylibs.each do |str| + next unless str.start_with? "#{HOMEBREW_PREFIX}/opt" + missing << str.sub("#{HOMEBREW_PREFIX}/opt/", "").split("/")[0] + end + unnecessary_deps -= missing [indirect_deps, undeclared_deps, unnecessary_deps] end @@ -123,7 +132,7 @@ class LinkageChecker def display_test_output display_items "Missing libraries", @broken_dylibs - display_items "Possible unnecessary dependencies", @unnecessary_deps + display_items "Dependencies with no linkage", @unnecessary_deps puts "No broken dylib links" if @broken_dylibs.empty? end From 8634f19107d98e56fa12165e92800a2120e9b13e Mon Sep 17 00:00:00 2001 From: Maxim Belkin Date: Sun, 18 Mar 2018 17:40:52 -0500 Subject: [PATCH 02/13] linkage: consider missing links to Cellar --- Library/Homebrew/linkage_checker.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Library/Homebrew/linkage_checker.rb b/Library/Homebrew/linkage_checker.rb index 19ac85ed79..deeaa4dfaa 100644 --- a/Library/Homebrew/linkage_checker.rb +++ b/Library/Homebrew/linkage_checker.rb @@ -88,8 +88,8 @@ class LinkageChecker end missing = [] @broken_dylibs.each do |str| - next unless str.start_with? "#{HOMEBREW_PREFIX}/opt" - missing << str.sub("#{HOMEBREW_PREFIX}/opt/", "").split("/")[0] + next unless str.start_with?("#{HOMEBREW_PREFIX}/opt", HOMEBREW_CELLAR) + missing << str.sub("#{HOMEBREW_PREFIX}/opt/", "").sub("#{HOMEBREW_CELLAR}/", "").split("/")[0] end unnecessary_deps -= missing [indirect_deps, undeclared_deps, unnecessary_deps] From 9642485e733c58daa297ba435c23390a1bd5df49 Mon Sep 17 00:00:00 2001 From: Maxim Belkin Date: Sun, 18 Mar 2018 18:52:12 -0500 Subject: [PATCH 03/13] Missing libraries now report corresponding missing formulae --- Library/Homebrew/linkage_checker.rb | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/Library/Homebrew/linkage_checker.rb b/Library/Homebrew/linkage_checker.rb index deeaa4dfaa..080373cddd 100644 --- a/Library/Homebrew/linkage_checker.rb +++ b/Library/Homebrew/linkage_checker.rb @@ -12,7 +12,7 @@ class LinkageChecker @formula = formula || resolve_formula(keg) @brewed_dylibs = Hash.new { |h, k| h[k] = Set.new } @system_dylibs = Set.new - @broken_dylibs = Set.new + @broken_dylibs = Hash.new { |h, k| h[k] = Set.new } @variable_dylibs = Set.new @indirect_deps = [] @undeclared_deps = [] @@ -21,6 +21,10 @@ class LinkageChecker check_dylibs end + def dylib_to_dep(dylib) + dylib.sub("#{HOMEBREW_PREFIX}/opt/", "").sub("#{HOMEBREW_CELLAR}/", "").split("/")[0] + end + def check_dylibs checked_dylibs = Set.new @keg.find do |file| @@ -41,7 +45,7 @@ class LinkageChecker @system_dylibs << dylib rescue Errno::ENOENT next if harmless_broken_link?(dylib) - @broken_dylibs << dylib + @broken_dylibs[dylib_to_dep(dylib)] << dylib else tap = Tab.for_keg(owner).tap f = if tap.nil? || tap.core_tap? @@ -86,12 +90,14 @@ class LinkageChecker next true if Formula[name].bin.directory? @brewed_dylibs.keys.map { |x| x.split("/").last }.include?(name) end - missing = [] - @broken_dylibs.each do |str| + missing = Set.new + @broken_dylibs.each do |key, value| + value.each do |str| next unless str.start_with?("#{HOMEBREW_PREFIX}/opt", HOMEBREW_CELLAR) - missing << str.sub("#{HOMEBREW_PREFIX}/opt/", "").sub("#{HOMEBREW_CELLAR}/", "").split("/")[0] + missing << dylib_to_dep(str) end - unnecessary_deps -= missing + end + unnecessary_deps -= missing.to_a [indirect_deps, undeclared_deps, unnecessary_deps] end From 1501354ee7a8debd45c47a898636d5a6aefcfb1d Mon Sep 17 00:00:00 2001 From: Maxim Belkin Date: Sun, 18 Mar 2018 19:03:31 -0500 Subject: [PATCH 04/13] Fix brew style error and warning --- Library/Homebrew/linkage_checker.rb | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/Library/Homebrew/linkage_checker.rb b/Library/Homebrew/linkage_checker.rb index 080373cddd..5d02cb8421 100644 --- a/Library/Homebrew/linkage_checker.rb +++ b/Library/Homebrew/linkage_checker.rb @@ -91,11 +91,10 @@ class LinkageChecker @brewed_dylibs.keys.map { |x| x.split("/").last }.include?(name) end missing = Set.new - @broken_dylibs.each do |key, value| - value.each do |str| - next unless str.start_with?("#{HOMEBREW_PREFIX}/opt", HOMEBREW_CELLAR) - missing << dylib_to_dep(str) - end + @broken_dylibs.each_value do |value| + value.each do |str| + missing << dylib_to_dep(str) if str.start_with?("#{HOMEBREW_PREFIX}/opt", HOMEBREW_CELLAR) + end end unnecessary_deps -= missing.to_a [indirect_deps, undeclared_deps, unnecessary_deps] From 81042c63fb138b50bfa27d499742181aef210348 Mon Sep 17 00:00:00 2001 From: Maxim Belkin Date: Tue, 20 Mar 2018 10:06:52 -0500 Subject: [PATCH 05/13] dylib_to_dep: use Regexp --- Library/Homebrew/linkage_checker.rb | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/Library/Homebrew/linkage_checker.rb b/Library/Homebrew/linkage_checker.rb index 5d02cb8421..85b1eeba10 100644 --- a/Library/Homebrew/linkage_checker.rb +++ b/Library/Homebrew/linkage_checker.rb @@ -22,7 +22,11 @@ class LinkageChecker end def dylib_to_dep(dylib) - dylib.sub("#{HOMEBREW_PREFIX}/opt/", "").sub("#{HOMEBREW_CELLAR}/", "").split("/")[0] + if dylib =~ %r{#{Regexp.escape(HOMEBREW_PREFIX)}/(opt|Cellar)/([\w+-.@]+)/} + Regexp.last_match(2) + else + "Not a Homebrew library" + end end def check_dylibs @@ -90,13 +94,7 @@ class LinkageChecker next true if Formula[name].bin.directory? @brewed_dylibs.keys.map { |x| x.split("/").last }.include?(name) end - missing = Set.new - @broken_dylibs.each_value do |value| - value.each do |str| - missing << dylib_to_dep(str) if str.start_with?("#{HOMEBREW_PREFIX}/opt", HOMEBREW_CELLAR) - end - end - unnecessary_deps -= missing.to_a + unnecessary_deps -= @broken_dylibs.map { |_, v| v.map { |d| dylib_to_dep(d) } }.flatten [indirect_deps, undeclared_deps, unnecessary_deps] end From 0513d9de5c3c1cd860474df3dfba3847e70331ff Mon Sep 17 00:00:00 2001 From: Maxim Belkin Date: Tue, 20 Mar 2018 12:30:14 -0500 Subject: [PATCH 06/13] display_items: handle nil keys --- Library/Homebrew/linkage_checker.rb | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/Library/Homebrew/linkage_checker.rb b/Library/Homebrew/linkage_checker.rb index 85b1eeba10..45739b29f4 100644 --- a/Library/Homebrew/linkage_checker.rb +++ b/Library/Homebrew/linkage_checker.rb @@ -22,11 +22,8 @@ class LinkageChecker end def dylib_to_dep(dylib) - if dylib =~ %r{#{Regexp.escape(HOMEBREW_PREFIX)}/(opt|Cellar)/([\w+-.@]+)/} - Regexp.last_match(2) - else - "Not a Homebrew library" - end + dylib =~ %r{#{Regexp.escape(HOMEBREW_PREFIX)}/(opt|Cellar)/([\w+-.@]+)/} + Regexp.last_match(2) end def check_dylibs @@ -170,7 +167,7 @@ class LinkageChecker return if things.empty? puts "#{label}:" if things.is_a? Hash - things.sort.each do |list_label, list| + things.sort_by { |k, _| k.to_s }.each do |list_label, list| list.sort.each do |item| puts " #{item} (#{list_label})" end From 09eab0cc7693b34d008eea3e6ff0db7bc8eb6011 Mon Sep 17 00:00:00 2001 From: Maxim Belkin Date: Wed, 21 Mar 2018 13:10:23 -0500 Subject: [PATCH 07/13] unrolling one-liner into two do/end blocks --- Library/Homebrew/linkage_checker.rb | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/Library/Homebrew/linkage_checker.rb b/Library/Homebrew/linkage_checker.rb index 45739b29f4..86bd6acbe2 100644 --- a/Library/Homebrew/linkage_checker.rb +++ b/Library/Homebrew/linkage_checker.rb @@ -91,7 +91,12 @@ class LinkageChecker next true if Formula[name].bin.directory? @brewed_dylibs.keys.map { |x| x.split("/").last }.include?(name) end - unnecessary_deps -= @broken_dylibs.map { |_, v| v.map { |d| dylib_to_dep(d) } }.flatten + missing_deps = @broken_dylibs.values.map do |v| + v.map do |d| + dylib_to_dep(d) + end + end.flatten.compact + unnecessary_deps -= missing_deps [indirect_deps, undeclared_deps, unnecessary_deps] end @@ -167,7 +172,7 @@ class LinkageChecker return if things.empty? puts "#{label}:" if things.is_a? Hash - things.sort_by { |k, _| k.to_s }.each do |list_label, list| + things.sort_by { |k, | k.to_s }.each do |list_label, list| list.sort.each do |item| puts " #{item} (#{list_label})" end From 55620a736a70673f55f525d03af2fec73ec2ca4e Mon Sep 17 00:00:00 2001 From: Maxim Belkin Date: Fri, 30 Mar 2018 15:55:14 -0500 Subject: [PATCH 08/13] distinguish between broken_dylibs and broken_deps --- Library/Homebrew/linkage_checker.rb | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/Library/Homebrew/linkage_checker.rb b/Library/Homebrew/linkage_checker.rb index 86bd6acbe2..2209b12c36 100644 --- a/Library/Homebrew/linkage_checker.rb +++ b/Library/Homebrew/linkage_checker.rb @@ -4,7 +4,7 @@ require "formula" class LinkageChecker attr_reader :keg, :formula - attr_reader :brewed_dylibs, :system_dylibs, :broken_dylibs, :variable_dylibs + 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) @@ -12,7 +12,8 @@ class LinkageChecker @formula = formula || resolve_formula(keg) @brewed_dylibs = Hash.new { |h, k| h[k] = Set.new } @system_dylibs = Set.new - @broken_dylibs = Hash.new { |h, k| h[k] = Set.new } + @broken_dylibs = [] + @broken_deps = Hash.new { |h, k| h[k] = Set.new } @variable_dylibs = Set.new @indirect_deps = [] @undeclared_deps = [] @@ -46,7 +47,12 @@ class LinkageChecker @system_dylibs << dylib rescue Errno::ENOENT next if harmless_broken_link?(dylib) - @broken_dylibs[dylib_to_dep(dylib)] << dylib + dep = dylib_to_dep(dylib) + if dep.nil? + @broken_dylibs << dylib + else + @broken_deps[dep] << dylib + end else tap = Tab.for_keg(owner).tap f = if tap.nil? || tap.core_tap? @@ -91,7 +97,7 @@ class LinkageChecker next true if Formula[name].bin.directory? @brewed_dylibs.keys.map { |x| x.split("/").last }.include?(name) end - missing_deps = @broken_dylibs.values.map do |v| + missing_deps = @broken_deps.values.map do |v| v.map do |d| dylib_to_dep(d) end @@ -118,6 +124,7 @@ class LinkageChecker display_items "Indirect dependencies with linkage", @indirect_deps display_items "Variable-referenced libraries", @variable_dylibs display_items "Missing libraries", @broken_dylibs + display_items "Missing dependencies", @broken_deps display_items "Undeclared dependencies with linkage", @undeclared_deps display_items "Dependencies with no linkage", @unnecessary_deps end @@ -137,6 +144,7 @@ class LinkageChecker def display_test_output display_items "Missing libraries", @broken_dylibs + display_items "Missing dependencies", @broken_deps display_items "Dependencies with no linkage", @unnecessary_deps puts "No broken dylib links" if @broken_dylibs.empty? end From d6e0b7d2bfa41e529c9055c13695afdba50b4a9a Mon Sep 17 00:00:00 2001 From: Maxim Belkin Date: Fri, 30 Mar 2018 20:56:36 -0500 Subject: [PATCH 09/13] use new syntax --- Library/Homebrew/linkage_checker.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Library/Homebrew/linkage_checker.rb b/Library/Homebrew/linkage_checker.rb index 2209b12c36..7a44ad7db8 100644 --- a/Library/Homebrew/linkage_checker.rb +++ b/Library/Homebrew/linkage_checker.rb @@ -180,7 +180,7 @@ class LinkageChecker return if things.empty? puts "#{label}:" if things.is_a? Hash - things.sort_by { |k, | k.to_s }.each do |list_label, list| + things.sort_by(&:to_s).each do |list_label, list| list.sort.each do |item| puts " #{item} (#{list_label})" end From 71fef1493dd932784774bb531b3a6322666eb99f Mon Sep 17 00:00:00 2001 From: Maxim Belkin Date: Sat, 7 Apr 2018 13:28:15 -0500 Subject: [PATCH 10/13] Use flat_map and other shortcuts --- Library/Homebrew/linkage_checker.rb | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/Library/Homebrew/linkage_checker.rb b/Library/Homebrew/linkage_checker.rb index 7a44ad7db8..1e8cf6a19c 100644 --- a/Library/Homebrew/linkage_checker.rb +++ b/Library/Homebrew/linkage_checker.rb @@ -47,8 +47,7 @@ class LinkageChecker @system_dylibs << dylib rescue Errno::ENOENT next if harmless_broken_link?(dylib) - dep = dylib_to_dep(dylib) - if dep.nil? + if (dep = dylib_to_dep(dylib)) @broken_dylibs << dylib else @broken_deps[dep] << dylib @@ -97,11 +96,7 @@ class LinkageChecker next true if Formula[name].bin.directory? @brewed_dylibs.keys.map { |x| x.split("/").last }.include?(name) end - missing_deps = @broken_deps.values.map do |v| - v.map do |d| - dylib_to_dep(d) - end - end.flatten.compact + missing_deps = @broken_deps.values.flat_map { |d| dylib_to_dep(d) }.compact unnecessary_deps -= missing_deps [indirect_deps, undeclared_deps, unnecessary_deps] end @@ -124,7 +119,7 @@ class LinkageChecker display_items "Indirect dependencies with linkage", @indirect_deps display_items "Variable-referenced libraries", @variable_dylibs display_items "Missing libraries", @broken_dylibs - display_items "Missing dependencies", @broken_deps + display_items "Broken dependencies", @broken_deps display_items "Undeclared dependencies with linkage", @undeclared_deps display_items "Dependencies with no linkage", @unnecessary_deps end @@ -144,7 +139,7 @@ class LinkageChecker def display_test_output display_items "Missing libraries", @broken_dylibs - display_items "Missing dependencies", @broken_deps + display_items "Broken dependencies", @broken_deps display_items "Dependencies with no linkage", @unnecessary_deps puts "No broken dylib links" if @broken_dylibs.empty? end From e4a1b16180ab4981d3894c13fcba6a956362e241 Mon Sep 17 00:00:00 2001 From: Maxim Belkin Date: Sat, 7 Apr 2018 13:33:10 -0500 Subject: [PATCH 11/13] Being explicit about sorting keys first --- Library/Homebrew/linkage_checker.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Library/Homebrew/linkage_checker.rb b/Library/Homebrew/linkage_checker.rb index 1e8cf6a19c..1f0d62f150 100644 --- a/Library/Homebrew/linkage_checker.rb +++ b/Library/Homebrew/linkage_checker.rb @@ -175,8 +175,8 @@ class LinkageChecker return if things.empty? puts "#{label}:" if things.is_a? Hash - things.sort_by(&:to_s).each do |list_label, list| - list.sort.each do |item| + things.keys.sort.each do |list_label| + things[list_label].sort.each do |item| puts " #{item} (#{list_label})" end end From e15430c2cb02172e5ef47579a7029f48d70503be Mon Sep 17 00:00:00 2001 From: Maxim Belkin Date: Sat, 7 Apr 2018 14:46:31 -0500 Subject: [PATCH 12/13] fix if statement --- Library/Homebrew/linkage_checker.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Library/Homebrew/linkage_checker.rb b/Library/Homebrew/linkage_checker.rb index 1f0d62f150..c840b1fde7 100644 --- a/Library/Homebrew/linkage_checker.rb +++ b/Library/Homebrew/linkage_checker.rb @@ -48,9 +48,9 @@ class LinkageChecker rescue Errno::ENOENT next if harmless_broken_link?(dylib) if (dep = dylib_to_dep(dylib)) - @broken_dylibs << dylib - else @broken_deps[dep] << dylib + else + @broken_dylibs << dylib end else tap = Tab.for_keg(owner).tap From 3484db71d008caf65d1d16e0e3db8410d80e73cd Mon Sep 17 00:00:00 2001 From: Maxim Belkin Date: Sat, 7 Apr 2018 15:35:27 -0500 Subject: [PATCH 13/13] fix missing_deps --- Library/Homebrew/linkage_checker.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Library/Homebrew/linkage_checker.rb b/Library/Homebrew/linkage_checker.rb index c840b1fde7..75b50966c6 100644 --- a/Library/Homebrew/linkage_checker.rb +++ b/Library/Homebrew/linkage_checker.rb @@ -13,7 +13,7 @@ class LinkageChecker @brewed_dylibs = Hash.new { |h, k| h[k] = Set.new } @system_dylibs = Set.new @broken_dylibs = [] - @broken_deps = Hash.new { |h, k| h[k] = Set.new } + @broken_deps = Hash.new { |h, k| h[k] = [] } @variable_dylibs = Set.new @indirect_deps = [] @undeclared_deps = [] @@ -48,7 +48,7 @@ class LinkageChecker rescue Errno::ENOENT next if harmless_broken_link?(dylib) if (dep = dylib_to_dep(dylib)) - @broken_deps[dep] << dylib + @broken_deps[dep] |= [dylib] else @broken_dylibs << dylib end @@ -96,7 +96,7 @@ class LinkageChecker next true if Formula[name].bin.directory? @brewed_dylibs.keys.map { |x| x.split("/").last }.include?(name) end - missing_deps = @broken_deps.values.flat_map { |d| dylib_to_dep(d) }.compact + missing_deps = @broken_deps.values.flatten.map { |d| dylib_to_dep(d) } unnecessary_deps -= missing_deps [indirect_deps, undeclared_deps, unnecessary_deps] end