Introduce a UseSystemLibraries option to use the system libunwind #391
Conversation
repos/coreclr.proj
Outdated
| @@ -4,6 +4,7 @@ | |||
| <PropertyGroup> | |||
| <BuildArguments>$(Platform) $(Configuration) skiptests -PortableBuild=$(PortableBuild)</BuildArguments> | |||
| <BuildArguments Condition="'$(OS)' != 'Windows_NT'">$(BuildArguments) msbuildonunsupportedplatform</BuildArguments> | |||
| <BuildArguments Condition="'$(UseSystemLibraries)' != 'false'">$(BuildArguments) cmakeargs -DCLR_CMAKE_USE_SYSTEM_LIBUNWIND=TRUE</BuildArguments> | |||
There was a problem hiding this comment.
Normally we do ... == 'true' in MSBuild, is there a reason to take more than true for this arg in particular?
There was a problem hiding this comment.
No. I just didn't realize the convention.
|
@dotnet-bot test this please /cc @dleeapho |
UseSystemLibraries lets users control if they want to use the system versions of libraries or bundled libraries. The only use case where this applies currently is coreclr's libunwind.
omajid
force-pushed
the
system-library-libunwind
branch
from
8edfc91 to
614fa98
Compare
|
@dotnet-bot test this please |
| @@ -8,6 +8,7 @@ | |||
| <_IsBootstrapping Condition="'$(BootstrapBuildToolsDir)' != ''">true</_IsBootstrapping> | |||
|
|
|||
| <PortableBuild Condition="'$(PortableBuild)' == ''">false</PortableBuild> | |||
| <UseSystemLibraries Condition="'$(UseSystemLibraries)' == ''">true</UseSystemLibraries> | |||
There was a problem hiding this comment.
Is there a reason to default to true?
There was a problem hiding this comment.
I figured since the biggest users of source-build are Linux distributions and they prefer to use system libraries, the default here should be true. Should I conditionalize things depending on the platform and platform versions?
There was a problem hiding this comment.
I'm fine with keeping this as the default and adjusting later if we find issues.
There was a problem hiding this comment.
I think defaulting to true is the way to go for the reasons @omajid said. Conditionalizing it isn't needed IMO.
|
Thanks for merging this! |
See #380. For some examples of why we would want to use the system libraries, see:
UseSystemLibrarieslets users control if they want to use the system versions of libraries or bundled libraries. The only use case where this applies currently is coreclr's libunwind.This PR by itself wont do anything until we pull in a version of coreclr that includes the implementation of
CLR_CMAKE_USE_SYSTEM_LIBUNWIND. Currently the option is not understood and just ignored by coreclr's build system.