Skip to content

linkage_checker: always report incorrect :no_linkage#22887

Merged
cho-m merged 1 commit into
mainfrom
linkage_checker-always-report-no_linkage
Jun 26, 2026
Merged

linkage_checker: always report incorrect :no_linkage#22887
cho-m merged 1 commit into
mainfrom
linkage_checker-always-report-no_linkage

Conversation

@cho-m

@cho-m cho-m commented Jun 26, 2026

Copy link
Copy Markdown
Member

  • Have you followed our Contributing guidelines?
  • Have you checked for other open Pull Requests for the same change?
  • Have you explained what your changes do? Performance claims (e.g. "this is faster") must include Hyperfine benchmarks.
  • Have you explained why you'd like these changes included, not just what they do?
  • For bug fixes, have you given step-by-step brew commands to reproduce the bug?
  • Have you written new tests (excluding integration tests)? Here's an example.
  • Have you successfully run brew lgtm (style, typechecking and tests) locally?

  • AI was used to generate or assist with generating this PR.

:no_linkage only makes sense if there is actually no linkage. There shouldn't be any false positives here (though the linkage may differ on macOS/Linux which is possible to handle via on_system blocks).

Just helps improve the metadata. Won't have a functional impact even if incorrect :no_linkage as we don't use this data outside of brew linkage output. But helps keep a canonical representation of a formula.


Among remaining that are strict-only:

  • Missing RPATH has a lot of false positives as shared modules/bundle (macOS .so files) usually don't use dyld but we still report those.
  • @executable_path are usually app bundles. This could be a potential issue when an app bundle includes copies of libraries, but we sometimes fix these up via symlinks so can be a false positive. Not sure if we can guarantee no usage of this.

For declared deps, I think that will fail the test if a dependency is linked into library but not installed during test. Though I can't tell if there are any false positives here so will leave this strict-only.

Copilot AI review requested due to automatic review settings June 26, 2026 14:29

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adjusts LinkageChecker so that formulas declaring dependencies as :no_linkage are always reported (and treated as failures in --test mode) when linkage is actually present, rather than only surfacing this under --strict.

Changes:

  • Always display “Unexpected linkage for no_linkage dependencies” in display_test_output, regardless of strict.
  • Treat @unexpected_linkage_deps as a --test failure condition even when strict: false.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread Library/Homebrew/linkage_checker.rb

@MikeMcQuaid MikeMcQuaid left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea, thanks!

@cho-m cho-m added this pull request to the merge queue Jun 26, 2026
Merged via the queue into main with commit f9b65a4 Jun 26, 2026
43 checks passed
@cho-m cho-m deleted the linkage_checker-always-report-no_linkage branch June 26, 2026 19:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants