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.
| if(CLR_CMAKE_HOST_WIN32) | ||
| cmake_policy(SET CMP0177 NEW) # install() paths are normalized | ||
| endif(CLR_CMAKE_HOST_WIN32) |
There was a problem hiding this comment.
| if(CLR_CMAKE_HOST_WIN32) | |
| cmake_policy(SET CMP0177 NEW) # install() paths are normalized | |
| endif(CLR_CMAKE_HOST_WIN32) | |
| if(POLICY CMP0177) | |
| cmake_policy(SET CMP0177 NEW) # install() paths are normalized | |
| endif(POLICY CMP0177) |
| @@ -257,7 +247,9 @@ if [[ "$__InstallRuntimes" == 1 || "$__PrivateBuild" == 1 ]]; then | |||
| /t:InstallTestRuntimes \ | |||
There was a problem hiding this comment.
We should log an issue - deleting .dotnet-test every build feels unnecessary for dev inner loop.
There was a problem hiding this comment.
I don't think the InstallTestRuntimes target deletes .dotnet-test every time.
| <!-- | ||
| 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.
| @@ -43,6 +43,30 @@ | |||
| <Uri>https://github.com/dotnet/dotnet</Uri> | |||
| <Sha>f5705c8f4c5079bba77bae8698ba1583bde0388c</Sha> | |||
| </Dependency> | |||
| <Dependency Name="runtime.win-x64.Microsoft.DotNet.Cdac.Transport" Version="10.0.0-preview.5.25261.1"> | |||
| <Uri>https://github.com/dotnet/runtime</Uri> | |||
There was a problem hiding this comment.
We should move these references to point to the VMR and take the latest
| @@ -14,6 +14,10 @@ | |||
| <AssemblyName>.NET Diagnostics</AssemblyName> | |||
| </PropertyGroup> | |||
|
|
|||
| <Target Name="BuildPrereqs" | |||
| BeforeTargets="Build" | |||
| DependsOnTargets="GenerateRuntimeVersionFile;InstallNativePackages" /> | |||
There was a problem hiding this comment.
May be cleaner to redefine build after the import of the notargets at the end of the file.
| <!-- | ||
| 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.
Can we please make these be
| <Target Name="InstallNativeFiles" AfterTargets="Build"> | |
| <Target Name="InstallNativeFiles" AfterTargets="AfterBuild"> |
At least this solves the build incrementality issues.
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.