dotnet
Pass TargetRid and SourceBuildNonPortable to the native scripts
Fixes https://github.com/dotnet/runtime/issues/74577.
cc @MichaelSimons @omajid
Tagging subscribers to this area: @hoyosjs See info in area-owners.md if you want to be subscribed.
Issue Details
The native build script uses the portablebuild argument when calculating the distro rid.
cc @MichaelSimons @omajid
| Author: | tmds |
|---|---|
| Assignees: | - |
| Labels: |
|
| Milestone: | - |
This PR passes the TargetRid and PortableBuild arguments to the native scripts. The native scripts take these into account now when calculating __DistroRid. When source-build passes a TargetRid that gets used as a __DistroRid. Previously PortableBuild was ignored and assumed to be portable causing __DistroRid to always be the portable rid.
I've simplified init-distro-rid.sh a bit by only calling initNonPortableDistroRid for non-portable builds.
corehost managed and native now both use __DistroRid to determine the output directory.
This is now working.
When I run the ./build.sh command and pass in an unknown rid --packagerid fedora.40-x64 everything in the default set builds, with the exception of Microsoft.NETCore.App.Crossgen2.sfxproj which is using the PackageRID:
https://github.com/dotnet/runtime/blob/7aef48a531e814bb8e8e0f6e060b100b51b27c80/src/installer/pkg/sfx/Microsoft.NETCore.App/Microsoft.NETCore.App.Crossgen2.sfxproj#L32-L48
Using /p:AdditionalRuntimeIdentifiers=fedora.40-x64 it should be possible to make Microsoft.NETCore.Platforms.csproj generate a package that can be used to let the this project know about the fedora.40-x64 rid. I'm don't know how to do this in practice. @ericstj do you have some suggestions?
This PR is up for review.
generate a package that can be used to let the this project know about the fedora.40-x64 rid.
Looks like this is trying to publish crossgen2 as a self-contained application. I would imagine it needs https://github.com/dotnet/runtime/pull/69455 and then needs to set BundledRuntimeIdentifierGraphFile. Still unclear to me if that's enough since I'm not sure how that project fetches the runtime that it's trying to link in.
Thanks @ericstj. I've created https://github.com/dotnet/runtime/issues/74721 to look further into Microsoft.NETCore.App.Crossgen2.sfxproj for source-build.
This is ready for review.
For source-build, before we were passing TargetRid as PackageRID, causing OutputRid and PackageRID to be the same.
With this change, we're passing it as OutputRid. This allows it to be a yet unknown version of a distribution.
I've updated the SourceBuild leg to test the case where they are not the same, and used an OutputRid that is not in the graph to ensure it only gets used for naming things.
From the log:
...
Successfully created package '/__w/1/s/artifacts/source-build/self/src/artifacts/packages/Release/Shipping/Microsoft.NETCore.App.Host.banana.24-x64.8.0.0-dev.nupkg'.
Successfully created package '/__w/1/s/artifacts/source-build/self/src/artifacts/packages/Release/Shipping/Microsoft.NETCore.App.Host.banana.24-x64.8.0.0-dev.symbols.nupkg'.
...
Successfully created package '/__w/1/s/artifacts/source-build/self/src/artifacts/packages/Release/Shipping/Microsoft.NETCore.App.Runtime.banana.24-x64.8.0.0-dev.nupkg'.
Successfully created package '/__w/1/s/artifacts/source-build/self/src/artifacts/packages/Release/Shipping/Microsoft.NETCore.App.Runtime.banana.24-x64.8.0.0-dev.symbols.nupkg'.
...
@dotnet/runtime-infrastructure can you please take a look? This touches host and source build properties that I'm not super aware of.
Can you merge this?
This enables building runtime repo for source-build on platforms where the target rid is not yet known in the rid graph.
Thanks for the work so far Tom. Unfortunately as already I mentioned, I'm not the right person to sign-off this change and I just asked the CLR team again to take a look.
If the Build Linux x64 release SourceBuild passes, that means this is in a good state.
@MichaelSimons can you review/approve from source-build perspective?
I've just extended SourceBuild.props so it figures out the appropriate RuntimeOS instead of taking it as an argument.
This needs to do a CI run.
@am11 @MichalStrehovsky can you please take a look? Am11 hope that's OK to ask you for a review but if I remember correctly, you made the original changes around RuntimeOS/TargetArch/TargetRid/... and have the most expertise in that area.
@am11 @MichalStrehovsky can you please take a look? Am11 hope that's OK to ask you for a review but if I remember correctly, you made the original changes around RuntimeOS/TargetArch/TargetRid/... and have the most expertise in that area.
I can only be authoritative for the diff in src/coreclr/tools/aot/ILCompiler/ILCompiler.csproj. But this is an area that went through like 5 different values in the past year (used to be defined as __DistroRid, then ToolsRid, then Crossgen2PackageRid, then OutputRid, and now with this change PackageRid). I don't know what any of those mean. As long as the CI passes and we're still generating ARM64 bits when crossbuilding ARM64 from x64 (which would not be covered by the CI), that particular diff LGTM.
Am11 hope that's OK to ask you for a review
@ViktorHofer, I approved this PR three weeks ago. :)
Unrelated, but there was a segfault when invoking msbuild:
/__w/1/s/.dotnet/sdk/7.0.100-preview.7.22377.5/MSBuild.dll /nologo -maxcpucount /m -verbosity:m /v:minimal /bl:/__w/1/s/artifacts/log/Checked/ToolsetRestore.binlog /clp:Summary /clp:ErrorsOnly;NoSummary /nr:false /p:TreatWarningsAsErrors=true /p:ContinuousIntegrationBuild=true /p:__ToolsetLocationOutputFile=/__w/1/s/artifacts/toolset/8.0.0-beta.22469.1.txt /t:__WriteToolsetLocation /warnaserror /__w/1/s/artifacts/toolset/restore.proj
/__w/1/s/eng/common/tools.sh: line 474: 512 Segmentation fault (core dumped) "$_InitializeBuildTool" "$@"
Build failed with exit code 139. Check errors above.
Finishing: Build and generate native prerequisites
Unfortunately I couldn't find a dump in the artifacts payload. Configuration: CoreCLR Product Build Linux x86 checked