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.

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 runtime packs with the sdk. #16499

Merged
merged 6 commits into from
May 30, 2023

Conversation

tmds
Copy link
Member

@tmds tmds commented May 25, 2023

@tmds
Copy link
Member Author

tmds commented May 25, 2023

Tested on my machine:

$ dotnet publish --sc -r fedora.37-x64
MSBuild version 17.7.0-preview-23272-02+d077d294f for .NET
  Determining projects to restore...
  Restored /tmp/web2/web2.csproj (in 98 ms).
/home/tmds/Downloads/extract2/with_aspnetcore/sdk/8.0.100-preview.5.23273.1/Sdks/Microsoft.NET.Sdk/targets/Microsoft.NET.RuntimeIdentifierInference.targets(314,5): message NETSDK1057: You are using a preview version of .NET. See: https://aka.ms/dotnet-support-policy [/tmp/web2/web2.csproj]
  web2 -> /tmp/web2/bin/Release/net8.0/fedora.37-x64/web2.dll
  web2 -> /tmp/web2/bin/Release/net8.0/fedora.37-x64/publish/

🎉

It would be nice to have an argument - similar to --use-current-runtime - that means: the non-portable rid.

Alternatively, one can already add:

<RuntimeIdentifier>$(NETCoreSdkRuntimeIdentifier)</RuntimeIdentifier>

Using rdfind I created hard links between the packs and their corresponding shared folder (assuming these would be distributed in the same repository package).

That leaves these two unique files in the .NET Core runtime pack:

-rwxr--r--. 1 tmds tmds  39K May 25 09:19 ./8.0.0-preview.5.23272.4/data/RuntimeList.xml
-rwxr--r--. 1 tmds tmds 4.1M May 25 09:19 ./8.0.0-preview.5.23272.4/runtimes/fedora.37-x64/native/libhostfxr.so

And these two unique files in the ASP.NET Core runtime pack:

-rwxr--r--. 1 tmds tmds  13K May 25 09:19 ./8.0.0-preview.5.23272.6/data/PlatformManifest.txt
-rwxr--r--. 1 tmds tmds  34K May 25 09:19 ./8.0.0-preview.5.23272.6/data/RuntimeList.xml

So we get source-build self-contained at the small cost of 4.2MB in size (provided the repo packages support hard links).

@tmds tmds force-pushed the bundle_runtime_packs branch from 7e3b49c to d29c9b8 Compare May 26, 2023 11:34
@tmds tmds force-pushed the bundle_runtime_packs branch from d29c9b8 to 243d157 Compare May 26, 2023 11:46
@tmds
Copy link
Member Author

tmds commented May 26, 2023

I've added a property (BundleRuntimePacks) that allows to toggle the feature on and off.

When turned on, we'll try to call rdfind to remove duplicates.
If the tool isn't available, a message is printed.

Because the tool may not be available on every distro, I'll look into replacing the rdfind invocation with a custom Task in another PR.

@tmds
Copy link
Member Author

tmds commented May 26, 2023

@ashnaga @MichaelSimons can this still be part or preview5?

Copy link
Member

@dsplaisted dsplaisted left a comment

Choose a reason for hiding this comment

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

Awesome!

src/redist/redist.csproj Outdated Show resolved Hide resolved
@@ -183,6 +183,20 @@
<RelativeLayoutPath>packs/%(PackageName)/%(PackageVersion)</RelativeLayoutPath>
</BundledLayoutPackage>

<BundledLayoutPackage Include="MicrosoftNetCoreAppRuntimePackNupkg" Condition="'$(BundleRuntimePacks)' == 'true'">
<PackageName>Microsoft.NETCore.App.Runtime.$(SharedFrameworkRid)</PackageName>
Copy link
Member

Choose a reason for hiding this comment

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

Is SharedFrameworkRid the same as ProductMonikerRid? ProductMonikerRid is used earlier for the known framework reference RIDs.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm using the same parameters as we were are already using for the app host, and they are both equal to the non-portable rid.

Co-authored-by: Daniel Plaisted <dsplaisted@gmail.com>
@MichaelSimons
Copy link
Member

@tmds - Can you log a follow-up test to ensure we adjust the source-build tests run a self-contained scenario and verify the runtime pack doesn't get pulled from NuGet? The change @mthalman is making in #16359 should make this a lot easier.

@ashnaga @MichaelSimons can this still be part or preview5?

I am going to defer this to @marcpopMSFT and @dsplaisted since this is an installer product change.

@tmds
Copy link
Member Author

tmds commented May 30, 2023

Can you log a follow-up test to ensure we adjust the source-build tests run a self-contained scenario and verify the runtime pack doesn't get pulled from NuGet?

Created dotnet/source-build#3485.

@tmds
Copy link
Member Author

tmds commented May 30, 2023

Because the tool may not be available on every distro, I'll look into replacing the rdfind invocation with a custom Task in another PR.

@MichaelSimons what is the best place to put this Task? Is it https://github.com/dotnet/installer/tree/main/src/core-sdk-tasks? Or does it belong somewhere else?

@MichaelSimons
Copy link
Member

@MichaelSimons what is the best place to put this Task? Is it https://github.com/dotnet/installer/tree/main/src/core-sdk-tasks? Or does it belong somewhere else?

I will defer to @dsplaisted.

@MichaelSimons
Copy link
Member

Per today's tactics meeting, we are at the point in the release cycle that only critical fixes are being accepted for preview 5. It sounds like this will have to be deferred to preview 6.

cc @mmitche for visibility.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants