Skip to content
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.

Sign up for GitHub

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

source-build: bundle NativeAOT libraries with the SDK. #41198

Merged
merged 7 commits into from
Jun 19, 2024
Merged

source-build: bundle NativeAOT libraries with the SDK. #41198

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
ec9a3f1
source-build: bundle NativeAOT libraries with the SDK.
tmds May 28, 2024
3d1c224
Add DotNetBuildOrchestrator condition for setting BundleNativeAotComp…
tmds Jun 18, 2024
713a6ff
Find out what happens when doubling the test timeout.
tmds Jun 19, 2024
cd188f3
Try to get some more output from the CI test run.
tmds Jun 19, 2024
71f5d98
Skip 'ilc' in SourceBuiltSdkContainsNativeDebugSymbols.
tmds Jun 19, 2024
7be0c03
Revert runTestsTimeout change.
tmds Jun 19, 2024
22a593b
Set MSBUILDENSURESTDOUTFORTASKPROCESSES in scripts when running tests.
tmds Jun 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions src/Installer/redist-installer/targets/GenerateLayout.targets
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,17 @@
<RelativeLayoutPath>packs/%(PackageName)/%(PackageVersion)</RelativeLayoutPath>
</BundledLayoutPackage>

<BundledLayoutPackage Include="MicrosoftDotNetILCompilerPackNupkg" Condition="'$(BundleNativeAotCompiler)' == 'true'">
<PackageName>runtime.$(SharedFrameworkRid).Microsoft.DotNet.ILCompiler</PackageName>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Just want to make sure we are aware of what to expect)
This will only provide ILCompiler package for current RID. Note that PublishAot=true downloads separate ILCompiler packages when we issue dotnet publish -p:PublishAot=true -r <some other rid> (e.g. fedora-arm64 cross-publish on fedora-x64 system), in which case it will download the Microsoft linux-arm64 package from nuget feed. If we don't like this behavior, then I think we have two options:

  1. distro maintainers publish ilc packages with non-portable RID to nuget.org (or some dedicated nuget feed for dotnet distros).
  2. In nativeaot BuildIntegration issue a warning message for source-build runtimes and continue, or error that cross-compile is not supported and stop. (former is probably less drastic)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this is the behavior we want. We are enabling publishing for the SDK rid with a packed ILCompiler for that rid only. For other rids, the SDK will download a package from nuget.org.

<PackageVersion>$(MicrosoftNETCoreAppRuntimePackageVersion)</PackageVersion>
<TargetFramework>$(TargetFramework)</TargetFramework>
<RelativeLayoutPath>packs/%(PackageName)/%(PackageVersion)</RelativeLayoutPath>
</BundledLayoutPackage>

<BundledLayoutLibraryPackage Include="$(SourceBuiltShippingPackagesDir)/../runtime/Microsoft.DotNet.ILCompiler.$(MicrosoftNETCoreAppRuntimePackageVersion).nupkg" Condition="'$(BundleNativeAotCompiler)' == 'true'" />
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$(SourceBuiltShippingPackagesDir)/../runtime/ this moves from the sdk repo to the runtime repo shipping packages. Perhaps there's a better way to get to that directory.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it different than other runtime packs (apphost, corossgen2 etc.)?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These remain as .nupkg files which are stored in the library-packs directory.

The others are "restored" against a the target framework into the packs directory. You see the extracted package content.

Copy link
Member

@ViktorHofer ViktorHofer Jun 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps there's a better way to get to that directory.

I think you want to add two PackageDownload items for both packages so that they get downloaded in the context of dotnet/sdk and then construct the path to them from the cache (NuGetPackageRoot). The nuget package itself is part of the nuget cache.

FSharp and Roslyn do something similar but not when bundling the SDK but when publishing packages:

sdk/eng/Publishing.props

Line 85 in dad1976

<FSharpPackagesToPush Include="$(NuGetPackageRoot)\Microsoft.FSharp.Compiler\$(MicrosoftFSharpCompilerPackageVersion)\contentFiles\$(FSharpCorePath)\FSharp.Core.*.nupkg" />

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cc @MiYanni @dsplaisted @marcpopMSFT (owner of the bundling logic in sdk)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For now, I've added the DotNetBuildOrchestrator property and I'll create a tracking issue for eliminating it.
I think what you are proposing sounds good. I don't know how to put it all together without digging deeper.

tmds marked this conversation as resolved.
Show resolved Hide resolved

<BundledLayoutLibraryPackage Include="$(SourceBuiltShippingPackagesDir)/../runtime/Microsoft.NET.ILLink.Tasks.$(MicrosoftNETILLinkTasksPackageVersion).nupkg" Condition="'$(BundleNativeAotCompiler)' == 'true'" />

<BundledInstallerComponent Include="DownloadedRuntimeDepsInstallerFile"
Condition="('$(IsDebianBaseDistro)' == 'true' OR '$(IsRPMBasedDistro)' == 'true') And '$(SkipBuildingInstallers)' != 'true' And '$(InstallerExtension)' != '' And !$(Architecture.StartsWith('arm'))">
<BaseUrl>$(NetRuntimeRootUrl)</BaseUrl>
Expand Down Expand Up @@ -460,6 +471,11 @@
SkipUnchangedFiles="true"
/>

<Copy SourceFiles="@(BundledLayoutLibraryPackage)"
ViktorHofer marked this conversation as resolved.
Show resolved Hide resolved
DestinationFolder="$(RedistLayoutPath)/library-packs"
SkipUnchangedFiles="true"
/>

<!-- From Version.targets in SDK redist -->
<PropertyGroup>
<ArtifactNameSdk>dotnet-toolset-internal</ArtifactNameSdk>
Expand Down
15 changes: 10 additions & 5 deletions src/Tasks/Microsoft.NET.Build.Tasks/ProcessFrameworkReferences.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -788,17 +788,22 @@ private ToolPackSupport AddToolPack(

var runtimePackName = packNamePattern.Replace("**RID**", hostRuntimeIdentifier);

if (EnableRuntimePackDownload)
var runtimePackItem = new TaskItem(runtimePackName);
runtimePackItem.SetMetadata(MetadataKeys.NuGetPackageId, runtimePackName);
runtimePackItem.SetMetadata(MetadataKeys.NuGetPackageVersion, packVersion);

string runtimePackPath = GetPackPath(runtimePackName, packVersion);
if (runtimePackPath != null)
{
runtimePackItem.SetMetadata(MetadataKeys.PackageDirectory, runtimePackPath);
}
else if (EnableRuntimePackDownload)
{
// We need to download the runtime pack
runtimePackToDownload = new TaskItem(runtimePackName);
runtimePackToDownload.SetMetadata(MetadataKeys.Version, packVersion);
}

var runtimePackItem = new TaskItem(runtimePackName);
runtimePackItem.SetMetadata(MetadataKeys.NuGetPackageId, runtimePackName);
runtimePackItem.SetMetadata(MetadataKeys.NuGetPackageVersion, packVersion);

switch (toolPackType)
{
case ToolPackType.Crossgen2:
Expand Down
Loading