Skip to content

linkage_checker: Distinguish indirect deps from undeclared deps #3785

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Feb 11, 2018
Merged

linkage_checker: Distinguish indirect deps from undeclared deps #3785

merged 1 commit into from
Feb 11, 2018

Conversation

apjanke
Copy link
Contributor

@apjanke apjanke commented Feb 10, 2018 •
edited
Loading

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same change?
  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes? Here's an example. No - there are no linkage_checker tests yet, and I don't know where to start for that.
  • Have you successfully run brew style with your changes locally? No, but I don't think it's a regression.
  • Have you successfully run brew tests with your changes locally?

Follows up on #2297 and Homebrew/homebrew-core#23933 (comment).

This modifies brew linkage to separate out the "undeclared dependencies" in to two lists: an "indirect dependencies" list for linkage dependencies that are picked up transitively but not directly declared in the formula, and "undeclared dependencies" for those that are not in the recursive dependencies at all.

Before:

[/usr/local/Homebrew/Library/Taps/homebrew/homebrew-core/Formula on ⇄ ad8a26413e]
$ brew linkage mldonkey
System libraries:
  /usr/lib/libSystem.B.dylib
  /usr/lib/libbz2.1.0.dylib
  /usr/lib/libc++.1.dylib
  /usr/lib/libcharset.1.dylib
  /usr/lib/libiconv.2.dylib
  /usr/lib/libz.1.dylib
Homebrew libraries:
  /usr/local/opt/fontconfig/lib/libfontconfig.1.dylib (fontconfig)
  /usr/local/opt/freetype/lib/libfreetype.6.dylib (freetype)
  /usr/local/opt/gd/lib/libgd.3.dylib (gd)
  /usr/local/opt/jpeg/lib/libjpeg.9.dylib (jpeg)
  /usr/local/opt/libmagic/lib/libmagic.1.dylib (libmagic)
  /usr/local/opt/libpng/lib/libpng16.16.dylib (libpng)
  /usr/local/opt/libtiff/lib/libtiff.5.dylib (libtiff)
  /usr/local/opt/webp/lib/libwebp.7.dylib (webp)
Undeclared dependencies with linkage:
  fontconfig
  freetype
  jpeg
  libmagic
  libtiff
  webp
[/usr/local/Homebrew/Library/Taps/homebrew/homebrew-core/Formula on ⇄ ad8a26413e]
$ brew linkage atk
System libraries:
  /System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation
  /usr/lib/libSystem.B.dylib
Homebrew libraries:
  /usr/local/opt/gettext/lib/libintl.8.dylib (gettext)
  /usr/local/opt/glib/lib/libglib-2.0.0.dylib (glib)
  /usr/local/opt/glib/lib/libgobject-2.0.0.dylib (glib)
Undeclared dependencies with linkage:
  gettext
[/usr/local/Homebrew/Library/Taps/homebrew/homebrew-core/Formula on ⇄ ad8a26413e]
$ brew linkage gtk+3
System libraries:
  /System/Library/Frameworks/AppKit.framework/Versions/C/AppKit
  /System/Library/Frameworks/ApplicationServices.framework/Versions/A/ApplicationServices
  /System/Library/Frameworks/Cocoa.framework/Versions/A/Cocoa
  /System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation
  /System/Library/Frameworks/CoreGraphics.framework/Versions/A/CoreGraphics
  /System/Library/Frameworks/CoreServices.framework/Versions/A/CoreServices
  /System/Library/Frameworks/Foundation.framework/Versions/C/Foundation
  /System/Library/Frameworks/Security.framework/Versions/A/Security
  /System/Library/Frameworks/SystemConfiguration.framework/Versions/A/SystemConfiguration
  /usr/lib/libSystem.B.dylib
  /usr/lib/libcups.2.dylib
  /usr/lib/libiconv.2.dylib
  /usr/lib/libobjc.A.dylib
  /usr/lib/libresolv.9.dylib
  /usr/lib/libz.1.dylib
Homebrew libraries:
  /usr/local/opt/atk/lib/libatk-1.0.0.dylib (atk)
  /usr/local/opt/cairo/lib/libcairo-gobject.2.dylib (cairo)
  /usr/local/opt/cairo/lib/libcairo.2.dylib (cairo)
  /usr/local/opt/fontconfig/lib/libfontconfig.1.dylib (fontconfig)
  /usr/local/opt/freetype/lib/libfreetype.6.dylib (freetype)
  /usr/local/opt/gdk-pixbuf/lib/libgdk_pixbuf-2.0.0.dylib (gdk-pixbuf)
  /usr/local/opt/gettext/lib/libintl.8.dylib (gettext)
  /usr/local/opt/glib/lib/libgio-2.0.0.dylib (glib)
  /usr/local/opt/glib/lib/libglib-2.0.0.dylib (glib)
  /usr/local/opt/glib/lib/libgmodule-2.0.0.dylib (glib)
  /usr/local/opt/glib/lib/libgobject-2.0.0.dylib (glib)
  /usr/local/Cellar/gtk+3/3.22.26/lib/libgdk-3.0.dylib (gtk+3)
  /usr/local/Cellar/gtk+3/3.22.26/lib/libgtk-3.0.dylib (gtk+3)
  /usr/local/opt/harfbuzz/lib/libharfbuzz.0.dylib (harfbuzz)
  /usr/local/opt/libepoxy/lib/libepoxy.dylib (libepoxy)
  /usr/local/opt/pango/lib/libpango-1.0.0.dylib (pango)
  /usr/local/opt/pango/lib/libpangocairo-1.0.0.dylib (pango)
  /usr/local/opt/pango/lib/libpangoft2-1.0.0.dylib (pango)
Undeclared dependencies with linkage:
  cairo
  fontconfig
  freetype
  gettext
  harfbuzz
Dependencies with no linkage:
  gsettings-desktop-schemas
  hicolor-icon-theme

After:

[/usr/local/Homebrew/Library/Taps/homebrew/homebrew-core/Formula on ⇄ ad8a26413e]
$ brew linkage mldonkey
System libraries:
  /usr/lib/libSystem.B.dylib
  /usr/lib/libbz2.1.0.dylib
  /usr/lib/libc++.1.dylib
  /usr/lib/libcharset.1.dylib
  /usr/lib/libiconv.2.dylib
  /usr/lib/libz.1.dylib
Homebrew libraries:
  /usr/local/opt/fontconfig/lib/libfontconfig.1.dylib (fontconfig)
  /usr/local/opt/freetype/lib/libfreetype.6.dylib (freetype)
  /usr/local/opt/gd/lib/libgd.3.dylib (gd)
  /usr/local/opt/jpeg/lib/libjpeg.9.dylib (jpeg)
  /usr/local/opt/libmagic/lib/libmagic.1.dylib (libmagic)
  /usr/local/opt/libpng/lib/libpng16.16.dylib (libpng)
  /usr/local/opt/libtiff/lib/libtiff.5.dylib (libtiff)
  /usr/local/opt/webp/lib/libwebp.7.dylib (webp)
Indirect dependencies:
  fontconfig
  freetype
  jpeg
  libtiff
  webp
Undeclared dependencies with linkage:
  libmagic
[/usr/local/Homebrew/Library/Taps/homebrew/homebrew-core/Formula on ⇄ ad8a26413e]
$ brew linkage atk
System libraries:
  /System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation
  /usr/lib/libSystem.B.dylib
Homebrew libraries:
  /usr/local/opt/gettext/lib/libintl.8.dylib (gettext)
  /usr/local/opt/glib/lib/libglib-2.0.0.dylib (glib)
  /usr/local/opt/glib/lib/libgobject-2.0.0.dylib (glib)
Indirect dependencies:
  gettext
[/usr/local/Homebrew/Library/Taps/homebrew/homebrew-core/Formula on ⇄ ad8a26413e]
$ brew linkage gtk+3
System libraries:
  /System/Library/Frameworks/AppKit.framework/Versions/C/AppKit
  /System/Library/Frameworks/ApplicationServices.framework/Versions/A/ApplicationServices
  /System/Library/Frameworks/Cocoa.framework/Versions/A/Cocoa
  /System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation
  /System/Library/Frameworks/CoreGraphics.framework/Versions/A/CoreGraphics
  /System/Library/Frameworks/CoreServices.framework/Versions/A/CoreServices
  /System/Library/Frameworks/Foundation.framework/Versions/C/Foundation
  /System/Library/Frameworks/Security.framework/Versions/A/Security
  /System/Library/Frameworks/SystemConfiguration.framework/Versions/A/SystemConfiguration
  /usr/lib/libSystem.B.dylib
  /usr/lib/libcups.2.dylib
  /usr/lib/libiconv.2.dylib
  /usr/lib/libobjc.A.dylib
  /usr/lib/libresolv.9.dylib
  /usr/lib/libz.1.dylib
Homebrew libraries:
  /usr/local/opt/atk/lib/libatk-1.0.0.dylib (atk)
  /usr/local/opt/cairo/lib/libcairo-gobject.2.dylib (cairo)
  /usr/local/opt/cairo/lib/libcairo.2.dylib (cairo)
  /usr/local/opt/fontconfig/lib/libfontconfig.1.dylib (fontconfig)
  /usr/local/opt/freetype/lib/libfreetype.6.dylib (freetype)
  /usr/local/opt/gdk-pixbuf/lib/libgdk_pixbuf-2.0.0.dylib (gdk-pixbuf)
  /usr/local/opt/gettext/lib/libintl.8.dylib (gettext)
  /usr/local/opt/glib/lib/libgio-2.0.0.dylib (glib)
  /usr/local/opt/glib/lib/libglib-2.0.0.dylib (glib)
  /usr/local/opt/glib/lib/libgmodule-2.0.0.dylib (glib)
  /usr/local/opt/glib/lib/libgobject-2.0.0.dylib (glib)
  /usr/local/Cellar/gtk+3/3.22.26/lib/libgdk-3.0.dylib (gtk+3)
  /usr/local/Cellar/gtk+3/3.22.26/lib/libgtk-3.0.dylib (gtk+3)
  /usr/local/opt/harfbuzz/lib/libharfbuzz.0.dylib (harfbuzz)
  /usr/local/opt/libepoxy/lib/libepoxy.dylib (libepoxy)
  /usr/local/opt/pango/lib/libpango-1.0.0.dylib (pango)
  /usr/local/opt/pango/lib/libpangocairo-1.0.0.dylib (pango)
  /usr/local/opt/pango/lib/libpangoft2-1.0.0.dylib (pango)
Indirect dependencies:
  cairo
  fontconfig
  freetype
  gettext
  harfbuzz
Dependencies with no linkage:
  gsettings-desktop-schemas
  hicolor-icon-theme

The brew style error:

$ brew style ./linkage_checker.rb
warning: parser/current is loading parser/ruby23, which recognizes
warning: 2.3.5-compliant syntax, but you are running 2.3.3.
warning: please see https://github.com/whitequark/parser#compatibility-with-ruby-mri.
== linkage_checker.rb ==
C: 31: 41: Use hash rockets syntax.

1 file inspected, 1 offense detected

That's from a line not touched by this PR.

If tests are necessary - should I just dump a pre-built keg with known linkage qualities in to the test fixtures? Or install a formula with known linkage properties? Or...?

@ilovezfs
Copy link
Contributor

Should we have it say Indirect dependencies: or Indirect dependencies with linkage:?

@apjanke
Copy link
Contributor Author

apjanke commented Feb 10, 2018

Indirect dependencies with linkage: is better. I've updated it.

ilovezfs
ilovezfs reviewed Feb 10, 2018
View reviewed changes
Library/Homebrew/os/mac/linkage_checker.rb Outdated
end
end
sort_by_formula_full_name!(undeclared_deps)
sort_by_formula_full_name!(indirect_deps)
Copy link
Contributor

Choose a reason for hiding this comment

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

would probably do this sort first just for aesthetics since they're mentioned in indirect, then undeclared, order above not the other way around

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense. Updated.

@ilovezfs
Copy link
Contributor

CC @zmwangx FYI

@alyssais alyssais mentioned this pull request Feb 11, 2018
Merged
6 tasks
@MikeMcQuaid
Copy link
Member

Great work, thanks @apjanke, good to see your face around these parts 😁!

@MikeMcQuaid MikeMcQuaid merged commit b6a1302 into Homebrew:master Feb 11, 2018
This was referenced Mar 5, 2018
Merged
Closed
@Homebrew Homebrew locked and limited conversation to collaborators May 4, 2018
@apjanke apjanke deleted the linkage-indirect-deps branch June 11, 2018 19:41
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
1 more reviewer

@ilovezfs ilovezfs ilovezfs approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@apjanke @ilovezfs @MikeMcQuaid