From 601bf83feea37de75faa47417ac803a5acd19cfe Mon Sep 17 00:00:00 2001 From: Carlo Cabrera <30379873+carlocab@users.noreply.github.com> Date: Fri, 19 Nov 2021 17:42:17 +0800 Subject: [PATCH] linkage: add `--strict` flag to detect opportunistic linkage There was a previous discussion about making `brew linkage --test` fail for unrequested dependencies (#9172). I'm not sure what the outcome of that was, but it still seems like a good idea to try to help us find cases of opportunistic linkage as they happen rather than when they cause CI failures in another PR sometime later. Let's do this by adding a `--strict` flag to `brew linkage --test`. My intention is for `brew linkage --test --strict` failures to be warnings rather than errors in CI, which should mitigate some of the concerns about doing this that were raised in #9172. --- Library/Homebrew/cli/args.rbi | 3 +++ Library/Homebrew/dev-cmd/linkage.rb | 7 +++++-- Library/Homebrew/linkage_checker.rb | 8 +++++--- 3 files changed, 13 insertions(+), 5 deletions(-) diff --git a/Library/Homebrew/cli/args.rbi b/Library/Homebrew/cli/args.rbi index 3dbdf28fb8a0c..552720dbafc3a 100644 --- a/Library/Homebrew/cli/args.rbi +++ b/Library/Homebrew/cli/args.rbi @@ -3,6 +3,9 @@ module Homebrew module CLI class Args < OpenStruct + sig { returns(T::Boolean) } + def strict?; end + sig { returns(T::Boolean) } def HEAD?; end diff --git a/Library/Homebrew/dev-cmd/linkage.rb b/Library/Homebrew/dev-cmd/linkage.rb index d765f9ac5fbe8..21b69593df5a6 100644 --- a/Library/Homebrew/dev-cmd/linkage.rb +++ b/Library/Homebrew/dev-cmd/linkage.rb @@ -20,6 +20,9 @@ def linkage_args switch "--test", description: "Show only missing libraries and exit with a non-zero status if any missing "\ "libraries are found." + switch "--strict", + depends_on: "--test", + description: "Exit with a non-zero status if any undeclared dependencies with linkage are found." switch "--reverse", description: "For every library that a keg references, print its dylib path followed by the "\ "binaries that link to it." @@ -46,8 +49,8 @@ def linkage result = LinkageChecker.new(keg, cache_db: db) if args.test? - result.display_test_output - Homebrew.failed = true if result.broken_library_linkage? + result.display_test_output(strict: args.strict?) + Homebrew.failed = true if result.broken_library_linkage?(strict: args.strict?) elsif args.reverse? result.display_reverse_output else diff --git a/Library/Homebrew/linkage_checker.rb b/Library/Homebrew/linkage_checker.rb index 9af36adb0e65a..116abd76b10c8 100644 --- a/Library/Homebrew/linkage_checker.rb +++ b/Library/Homebrew/linkage_checker.rb @@ -62,17 +62,19 @@ def display_reverse_output end end - def display_test_output(puts_output: true) + def display_test_output(puts_output: true, strict: false) display_items "Missing libraries", broken_dylibs_with_expectations, puts_output: puts_output display_items "Unused missing linkage information", unexpected_present_dylibs, puts_output: puts_output display_items "Broken dependencies", @broken_deps, puts_output: puts_output display_items "Unwanted system libraries", @unwanted_system_dylibs, puts_output: puts_output display_items "Conflicting libraries", @version_conflict_deps, puts_output: puts_output + display_items "Undeclared dependencies with linkage", @undeclared_deps, puts_output: puts_output if strict end - sig { returns(T::Boolean) } - def broken_library_linkage? + sig { params(strict: T::Boolean).returns(T::Boolean) } + def broken_library_linkage?(strict: false) issues = [@broken_deps, @unwanted_system_dylibs, @version_conflict_deps] + issues << @undeclared_deps if strict [issues, unexpected_broken_dylibs, unexpected_present_dylibs].flatten.any?(&:present?) end