Skip to content
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

linkage_checker: skip broken linkage in Julia #18235

Merged
merged 1 commit into from
Sep 10, 2024
Merged

linkage_checker: skip broken linkage in Julia #18235

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions Library/Homebrew/linkage_checker.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,13 @@
Regexp.last_match(2)
end

sig { params(file: String).returns(T::Boolean) }
def broken_dylibs_allowed?(file)
return false if formula.name != "julia"
Copy link
Member Author

@cho-m cho-m Sep 10, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I noticed there is some logic afterward that checks if formula. I guess there is a chance formula will not be resolved?

brew/Library/Homebrew/linkage_checker.rb

Lines 317 to 320 in 5b0a84b

def resolve_formula(keg)
Formulary.from_keg(keg)
rescue FormulaUnavailableError
opoo "Formula unavailable: #{keg.name}"

If so, can modify to check if formula.blank? && ...

EDIT: Most likely typechecking will fail once we add strict typing. Can look into adding types along with handling this.

Copy link
Member

Choose a reason for hiding this comment

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

Could just do

if formula&.name != "julia"


file.start_with?("#{formula.prefix.realpath}/share/julia/compiled/")

Check warning on line 99 in Library/Homebrew/linkage_checker.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/linkage_checker.rb#L99 

Added line #L99 was not covered by tests
end

def check_dylibs(rebuild_cache:)
keg_files_dylibs = nil

Expand Down Expand Up @@ -128,7 +135,7 @@
if !file_has_any_rpath_dylibs && (dylib.start_with? "@rpath/")
file_has_any_rpath_dylibs = true
pathname = Pathname(file)
@files_missing_rpaths << file if pathname.rpaths.empty?
@files_missing_rpaths << file if pathname.rpaths.empty? && !broken_dylibs_allowed?(file.to_s)
end

next if checked_dylibs.include? dylib
Expand Down Expand Up @@ -156,7 +163,7 @@
# If we cannot associate the dylib with a dependency, then it may be a system library.
# If dlopen finds the dylib, then the linkage is not broken.
@system_dylibs << dylib
elsif !system_framework?(dylib)
elsif !system_framework?(dylib) && !broken_dylibs_allowed?(file.to_s)
@broken_dylibs << dylib
end
else
Expand Down