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.

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

build: use -Wl,--gc-sections when appropriate #6497

Merged
merged 1 commit into from
Dec 10, 2024

Conversation

garlick
Copy link
Member

@garlick garlick commented Dec 10, 2024

Problem: libflux-optparse.so has unresolved jansson symbols on focal.

A recent macos portability change dropped the linker option to garbage collect unused sections, resulting in unresolved json_* symbols in libflux-optparse.so from libmissing.la on focal.

The change, introduced in #6476, was supposed to set a make variable $ld_gc_sections when that option is available, but the change was incomplete and the variable was never set.

Add the missing configure logic.

Confirmed:

  • json_ symbols are no longer unresolved in libflux-optparse.so on focal
  • macos still builds, and json_ symbols do not appear there

Fixes #6496

Problem: libflux-optparse.so has unresolved jansson symbols on focal.

A recent macos portability change dropped the linker option to
garbage collect unused sections, resulting in unresolved json_*
symbols in libflux-optparse.so from libmissing.la on focal.

The change, introduced in flux-framework#6476, was supposed
to set a make variable $ld_gc_sections when that option is available,
but the change was incomplete and the variable was never set.

Add the missing configure logic.

Confirmed:
- json_ symbols are no longer unresolved in libflux-optparse.so on focal
- macos still builds, and json_ symbols do not appear there

Fixes flux-framework#6496
Copy link
Contributor

@grondo grondo left a comment

Choose a reason for hiding this comment

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

LGTM!

@garlick
Copy link
Member Author

garlick commented Dec 10, 2024

Thanks! Sorry for the sloppiness and thanks for requesting follow-up in the pmix issue.

@mergify mergify bot merged commit 3cae472 into flux-framework:master Dec 10, 2024
34 checks passed
Copy link

codecov bot commented Dec 10, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.61%. Comparing base (7cd6705) to head (ff1a93f).
Report is 2 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #6497   +/-   ##
=======================================
  Coverage   83.61%   83.61%           
=======================================
  Files         524      524           
  Lines       87623    87623           
=======================================
+ Hits        73268    73270    +2     
+ Misses      14355    14353    -2     

see 8 files with indirect coverage changes

@garlick garlick deleted the issue#6496 branch December 10, 2024 18:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

liboptparse.so has mysterious unresolved jansson symbols on focal
2 participants