-
-
Notifications
You must be signed in to change notification settings - Fork 950
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
fix(macos): fix boost on macos #2733
fix(macos): fix boost on macos #2733
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2733 +/- ##
=========================================
- Coverage 8.96% 8.89% -0.08%
=========================================
Files 94 94
Lines 17392 17385 -7
Branches 8270 8270
=========================================
- Hits 1560 1546 -14
- Misses 12967 12973 +6
- Partials 2865 2866 +1
Flags with carried forward coverage won't be shown. Click here to find out more. |
68333dc
to
0c67e74
Compare
if(APPLE) | ||
option(BOOST_USE_STATIC "Use static boost libraries." OFF) | ||
else() | ||
option(BOOST_USE_STATIC "Use static boost libraries." ON) | ||
endif() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you still getting issues with BOOST_USE_STATIC=ON
? If not we should revert this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Without manually setting icu path, it won't build with ON, so to make easier to build locally for development, I think OFF is a good default for Clion starting setup. We could let be ON by default, but then we need to update the build docs with a step to set icu path and flags manually.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, I'm fine with it as is if we must.
Before we have different defaults though, could you investigate finding the icu path with CMake functions, such as find_package
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've tried a bit, but had no success, as icu4c is a keg-only formula, it's not widely available in the system, I plan on finding a way to do that, but I'd rather fix the build first for most users, then improve it, since I don't know how long it will take me to do it and I won't be able to build the project for a few weeks, starting today.
Co-authored-by: ReenigneArcher <42013603+ReenigneArcher@users.noreply.github.com>
6319441
to
94afcef
Compare
Co-authored-by: ReenigneArcher <42013603+ReenigneArcher@users.noreply.github.com>
Description
macOS has some issues with Boost linking, this PR fixes the runtime error when statically linking boost, fixes the dynamic macOS build missing -licudata (icu4c) and improves brew installation options.
Checklist
Type of Change
.github/...
)Checklist
Branch Updates
LizardByte requires that branches be up-to-date before merging. This means that after any PR is merged, this branch
must be updated before it can be merged. You must also
Allow edits from maintainers.