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

VMR repositories should receive configuration properties like OS, Arch and RID and respect them #4784

Open
ViktorHofer opened this issue Dec 3, 2024 · 8 comments

Comments

@ViktorHofer
Copy link
Member

ViktorHofer commented Dec 3, 2024

The .NET SDK targets a specific RID, i.e. win-x64. When that SDK is being used in a vertical that should produce win-x86 assets, the wrong assets are restored.

Any component that restores RID specific assets needs to know the RID that the VMR is building for. That means any project that sets <OutputType>Exe</OutputType> or similarly WinExe, SelfContained, publishes, etc. RID specific assets are apphost packs, runtime packs and many more.

We need to the following things:

  1. Pass a set of properties into every VMR repo's build. Initially TargetOS, TargetArchitecture and TargetRuntimeIdentifier might be enough.
  2. Repositories should respect those properties. Probably the best way to achieve this is by leveraging the Arcade SDK to read from those properties and set the correct defaults. See the example below.

Arcade SDK:

<PropertyGroup>
  <!-- source-build sets the following already in a few repos that depend on live runtime, i.e. aspnetcore. -->
  <DefaultAppHostRuntimeIdentifier>$(TargetRuntimeIdentifier)</DefaultAppHostRuntimeIdentifier>

  <!-- the following might be better as it affects all packs, not just the apphost ones: -->
  <NETCoreSdkPortableRuntimeIdentifier>$(TargetRuntimeIdentifier)</NETCoreSdkPortableRuntimeIdentifier>
  <NETCoreSdkRuntimeIdentifier>$(TargetRuntimeIdentifier)</NETCoreSdkRuntimeIdentifier >
</PropertyGroup>

There are probably more properties that should be set that control what RID to use for restoring packs. @dsplaisted do we have a list of those?

Copy link

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

Copy link

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

@ViktorHofer
Copy link
Member Author

ViktorHofer commented Dec 3, 2024

I wonder if overriding NETCoreSdkPortableRuntimeIdentifier and NETCoreSdkRuntimeIdentifier is supported so that the SDK only downloads assets for that RID.

@mmitche
Copy link
Member

mmitche commented Dec 3, 2024

I don't think this is the case for all repos. I think this is only for repos that depend on the live runtime AND produce rid specific assets. Roslyn is not in this category, for instance, and should not be. The assets it produces (including an exes) are not rid specific.

@ViktorHofer
Copy link
Member Author

ViktorHofer commented Dec 4, 2024

The assets it produces (including an exes) are not rid specific.

The apphost which is included for OutputType=Exe by default (unless you set UseAppHost=false) is a RID specific asset. See the SDK's definition of when a project is RID agnostic: https://github.com/dotnet/sdk/blob/363ff49b2cbed4e6978c40750c5e59b3a1fc56c6/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Sdk.targets#L117C35-L117C48

This is really easy to reproduce locally. Just create a dotnet new console project and look at the app output. The binlog will also show you that a PackageDownload item exists for the apphost packages (clear you C:\Program Files\dotnet\packs folder before you do this though, otherwise NuGet doesn't get called).


OK, now bear with me with a pretty verbose description of what's going on:

Note that this isn't a VMR specific problem. There's a mismatch whenever the host RID is different from the target RID. In GNU configure terms this is called a cross configuration.
Example: I'm on an win-x64 OS (called build) and use win-x64 SDK (called host). I use that to restore my project (called target) which sets OutputType=Exe or SelfContained=true (or any other property that results in RID specific assets getting downloaded). I want my project to only restore win-x86 RID assets and I don't specify RuntimeIdentifier(s), I let the SDK handle this for me.

In that configuration you download win-x64 RID assets because that's the host's RID. If you want your target to only ever downlodad win-x86 RID specific assets (if necessary), you apparently need to override NETCoreSdkPortableRuntimeIdentifier and NETCoreSdkRuntimeIdentifier properties.

For the VMR we definitely care about this problem as a win-x86 vertical should depend on win-x86 assets. Arcade or any other repository that doesn't depend on live runtime isn't failing because those repos automatically use the RID packs that are available in the SDK's packs folder (pre-cached). For repositories that depend on live runtime, @pavel-purma's build which uses a unique OfficialBuildId demonstrates that we are now trying to restore RID packs for the wrong RID.

As an example, take the Microsoft.DotNet.Tar project which sets OutputType=Exe but doesn't set a RuntimeIdentifier. When building that in the win-x86 vertical, still the win-x64 apphost gets picked:

Image

This is often not a problem at consumption time as we usually don't use an app's apphost directly and invoke the executable with the dotnet host instead. Also, PackAsTool or NuGet Pack by default don't include the apphost.

cc @baronfel @richlander to fact check me here

@ViktorHofer
Copy link
Member Author

ViktorHofer commented Dec 4, 2024

The XUnitConsoleRunner package is an example of an apphost being included that targets the host RID instead of the target RID: https://dnceng.visualstudio.com/public/_artifacts/feed/dotnet-eng/NuGet/Microsoft.DotNet.XUnitConsoleRunner/overview/2.9.2-beta.24602.4

You could argue that the package is incorrectly authored and shouldn't include the apphost or instead just use the PackAsTool functionality - this is just an example and I bet that's not the only one in our stack.

Image

@mmitche
Copy link
Member

mmitche commented Dec 4, 2024

Got it, that makes sense.

If you set RuntimeIdentifier to win-x86 when building using the x64 SDK, do you end up downloading x64 apphost as well as the x86 apphost? Or does it properly constrain the SDK?

@ViktorHofer
Copy link
Member Author

The RuntimeIdentifier / RuntimeIdentifiers property wins over any SDK default. That's why we had to make these changes in couple of repos: https://github.com/dotnet/aspnetcore/blob/849235790fe151ab311192d7e6c5860e6c177f5e/src/Servers/testassets/ServerComparison.TestSites/ServerComparison.TestSites.csproj#L8

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: No status
Development

No branches or pull requests

3 participants