Consume cDac package and other cleanup #5482
Conversation
mikem8361
force-pushed
the
use_cdac_package
branch
from
d383025 to
a14c784
Compare
|
@hoyosjs I'll still except and apply any feedback you have for this PR. |
|
I can't see the failure details anymore. How do other outside dev's figure out the failure details? |
Create a InstallNativePackages.proj that downloads the native diasymreader and cdac packages Add InstallNativePackages to native-prereqs.proj Move the native binary copies out of the build.sh/build-native.cmd scripts to the appropriate managed project Move the rest of the binary copying from the cmake files to the managed projects Switch the Windows native builds to use ninja. Add the -msbuild/-ninja build-native.cmd options.
mikem8361
force-pushed
the
use_cdac_package
branch
from
a14c784 to
95e84fc
Compare
| @@ -5,6 +5,9 @@ | |||
| cmake_minimum_required(VERSION 3.15) | |||
|
|
|||
| cmake_policy(SET CMP0042 NEW) # MACOSX_RPATH is enabled by default. | |||
| if(CLR_CMAKE_HOST_WIN32) | |||
| cmake_policy(SET CMP0177 NEW) # install() paths are normalized | |||
There was a problem hiding this comment.
nit: Any reason no to normalize everywhere
There was a problem hiding this comment.
This is just to stop the warning on Windows. It is fine on other build platforms
There was a problem hiding this comment.
I've seen it in newer linux versions of the containers.
steveisok
force-pushed
the
use_cdac_package
branch
from
fdee73e to
42d80d8
Compare
| <PropertyGroup Condition="'$(TargetRid)' == ''"> | ||
| <TargetRidOS>$(TargetOS)</TargetRidOS> | ||
| <TargetRidOS Condition="'$(TargetOS)' == 'Windows_NT'">win</TargetRidOS> | ||
| <TargetRid>$(TargetRidOS)-$(TargetArch)</TargetRid> |
There was a problem hiding this comment.
Won't this give us linux-x64 on musl? I remember mike having to use this for a case, but not sure if it's doing what's expected.
There was a problem hiding this comment.
This won't apparently break obviously since it gets funneled down from buildnative. This does mean something like dotnet build <project> won't work in linux-musl
There was a problem hiding this comment.
TargetRid is set by the build native script so yes dotnet build <project> doesn't work. This does assume everything is built with our build.sh/build.ps1 scripts.
| <!-- | ||
| Copies all the native binaries to a rid-specific directory under the output and publish paths. | ||
| --> | ||
| <Target Name="InstallNativeFiles" AfterTargets="Build"> |
There was a problem hiding this comment.
This likely should be pulled out to sos-packaging.targets. Another question is - what's using them from the publish sub directory? It's not a regular publish since this would break given it's just depending on build. This also means incrementality might be broken - After/Before always run.
There was a problem hiding this comment.
When -bundletools is set on CI, I think we'll publish single file. I suspect that's why.
| @@ -133,6 +133,9 @@ jobs: | |||
| - _TestArgs: '-test' | |||
| - _Cross: '' | |||
|
|
|||
| - ${{ if ne(parameters.osGroup, 'Windows_NT') }}: | |||
| - _buildLog: '-binaryLog' | |||
There was a problem hiding this comment.
Let's just inline this to the call after CI unconditionally. It's arguably a bug in arcade (win always produces unless we exclude, and linux only if you include)
|
Looking at the official build - we are not carrying the cDAC for musl. Also, this increases install size from ~21mb to ~84mb. Each linux cDAC is 12mb. Is this expected? |
|
There's also none for win-x86, linux-arm So we are still missing about 50mb of payload, effectively 6x of what was there. |
|
Also - we probably also want these in the SOS transport packages. Like sos.installhelpers |
It wasn't ready back when this PR was created. They are now and dotnet/runtime#117321 will fix that. |
Create a InstallNativePackages.proj that downloads the native diasymreader and cdac packages
Add InstallNativePackages to native-prereqs.proj
Move the native binary copies out of the build.sh/build-native.cmd scripts to the appropriate managed project
Move the rest of the binary copying from the cmake files to the managed projects
Switch the Windows native builds to use ninja. Add the -msbuild/-ninja build-native.cmd options.