Author: | eerhardt |
---|---|
Assignees: | - |
Labels: | `untriaged`, `area-Extensions-DependencyInjection` |
Milestone: | - |
Open eerhardt opened 1 year ago
Tagging subscribers to this area: @dotnet/area-extensions-dependencyinjection See info in area-owners.md if you want to be subscribed.
Author: | eerhardt |
---|---|
Assignees: | - |
Labels: | `untriaged`, `area-Extensions-DependencyInjection` |
Milestone: | - |
After doing some tweaks to the repro steps, this issue still exists if I do:
dotnet publish /p:PublishTrimmed=true /p:PublishSingleFile=true /p:TrimMode=full /p:EnableRequestDelegateGenerator=true
but it no longer happens when I do:
dotnet publish /p:PublishTrimmed=true /p:TrimMode=full /p:EnableRequestDelegateGenerator=true --sc
So it doesn't appear to be a ReadyToRun issue. But appears to be a combination of PublishTrimmed
and PublishSingleFile
. Looking at the stacktrace, PublishTrimmed
is necessary because Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteFactory.ValidateTrimmingAnnotations
only happens in publish trimmed apps. So the minimal repro might be calling CustomAttributeData.ConstructorArguments
in a PublishSingleFile app. I've updated the top comment for the minimal app I can make that repros this issue.
<FrameworkReference Update="Microsoft.NETCore.App"
RuntimeFrameworkVersion="8.0.0-preview.4.23181.16" />
This creates version mismatch. It upgrades the reference to the runtime pack, but not to the apphost pack. Single file apphost in apphost pack contains the build of the whole runtime. It means that we end up using new CoreLib with old runtime that results into this crash.
It does not repro with PublishSingleFile=false
since the runtime and Corelib both come from the runtime pack and there is no mismatch.
Tagging subscribers to this area: @vitek-karas, @agocke, @vsadov See info in area-owners.md if you want to be subscribed.
Author: | eerhardt |
---|---|
Assignees: | - |
Labels: | `area-VM-coreclr`, `area-Host`, `untriaged` |
Milestone: | - |
FYI @sebastienros - this sounds be an issue with how we run jobs with dotnet/crank.
We also have https://github.com/dotnet/runtime/issues/81382 for similar request with NativeAOT.
@agocke @vitek-karas @sbomer - What is the recommended/supported approach for specifying the Runtime Version explicitly? The crank benchmark infrastructure runs benchmarks on the latest runtime version without waiting for dotnet/runtime to flow all the way to dotnet/installer.
We do have documentation for how to use daily runtime packages at https://github.com/dotnet/runtime/blob/main/docs/project/dogfooding.md#option-2-self-contained . Unfortunately, these instructions do not work. Setting RuntimeFrameworkVersion
updates all packs versions including ASP.NET, but there is no guarantee that the ASP.NET packs of given version exist (they typically do not). These instructions should be updated.
I'm not sure what the right answer is here. I think RuntimeFrameworkVersion
is ideally the right name -- and I would not expect it to alter the ASP.NET version. But it seems like that would be a breaking change now.
Is introducing a new NetRuntimeFrameworkVersion
property the right answer?
But it seems like that would be a breaking change now.
Yeah, and it sounds like the current behavior was intentional, based on https://github.com/dotnet/sdk/issues/2792.
@dsplaisted might have opinions on the naming or the approach.
Bike-shedding on the name. Maybe
$(MicrosoftNETCoreAppVersion)
$(MicrosoftNETCoreAppRuntimeVersion)
$(MicrosoftNETCoreAppFrameworkVersion)
$(MicrosoftNETCoreAppRuntimeFrameworkVersion)
The first 2 seem like good names, but they are already used inside dotnet repos. So maybe we don't want to pick ones that are already used?
It's explicitly not just netcore app though -- it's also the host, and the Microsoft.DotNet.ILCompiler package.
the problem is much broader than this if you consider workloads which have similar version grouping problems (a compiler, other tasks, sdk packs)
Instead of or in addition to creating new properties, we may want to document how to use the existing mechanisms to override default packs versions:
For example, I use this snippet when I want to compile a test app against my local netcoreapp ref pack and native aot publish it with my local native AOT pack:
<ItemGroup>
<KnownFrameworkReference Update="Microsoft.NETCore.App">
<TargetingPackVersion>8.0.0-dev</TargetingPackVersion>
</KnownFrameworkReference>
</ItemGroup>
<ItemGroup>
<KnownILCompilerPack Update="Microsoft.DotNet.ILCompiler">
<ILCompilerPackVersion>8.0.0-dev</ILCompilerPackVersion>
</KnownILCompilerPack>
</ItemGroup>
It's explicitly not just netcore app though -- it's also the host, and the Microsoft.DotNet.ILCompiler package.
To a "normal" dev (someone outside of dotnet/runtime), all of those things are part of Microsoft.NETCore.App
, i.e. the runtime's shared framework. They are all built in the dotnet/runtime's official build.
Maybe this doesn't need to be an "officially" supported property, it really is for daily builds, which I don't think normal customers need to worry about.
So maybe calling it something that is tied to the repo's name would work.
Instead of or in addition to creating new properties, we may want to document how to use the existing mechanisms to override default packs versions:
I think documenting is a good idea. But I don't think it solves the problem because it is error prone. I need to know what kind of project is being built to know which snippet I need. For example, if I PublishSingleFile=true
, I need a different snippet than above. And if I PublishTrimmed=true
, I need yet another snippet. It makes it hard for a tool (like dotnet/crank) to figure out what to inject into the project. Where if we just had a single property, it makes it really hard to get it wrong.
it really is for daily builds
The daily build problem is going to be solved by switching to unified build in .NET 9. There is going to be official daily build from dotnet/dotnet that has everything (runtime, ASP.NET, SDK) with all same versions.
How are tests going to work in that world? We will still have the same problem - there will be an SDK (with a runtime version) used for building ASP.NET. And there will be a separate runtime that ASP.NET is building against. When we run our tests, we will need our tests to run with the runtime that ASP.NET is built against, and not the runtime that came with the SDK.
I would expect that you will set RuntimeFrameworkVersion
to the latest daily build version (the daily build is going to have all packs with given version available so they will restore fine) and then you add the local ASP.NET projects or packages that you want to test.
So we couldn't run these ASP.NET tests until after the full daily build (including Windows.Desktop, SDK, etc) is done?
In the unified build plan, there is just one official build that lives in dotnet/dotnet.
I agree with Jan, that seems like a simpler model. In particular,
To a "normal" dev (someone outside of dotnet/runtime), all of those things are part of Microsoft.NETCore.App,
I think that's incorrect -- to a "normal" dev, I think all these things are part of the .NET SDK that they download. They have no idea about our internal package divisions. Using a "nightly" build of the SDK, or a property that we tell them is equivalent to using a "nightly" build of the SDK, is a simple concept that most developers will probably understand and be able to use easily.
Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas See info in area-owners.md if you want to be subscribed.
Author: | eerhardt |
---|---|
Assignees: | - |
Labels: | `area-NativeAOT-coreclr` |
Milestone: | 9.0.0 |
Tagging subscribers to this area: @agocke, @vitek-karas, @vsadov See info in area-owners.md if you want to be subscribed.
I'm seeing the following exception from the ASP.NET benchmarks for PublishTrimmed/ReadyToRun/SingleFile:
Minimal Repro instructions
dotnet publish
the following app:.\bin\Release\net8.0\win-x64\publish\Net8Console.exe
Original Repro instructions
git clone https://github.com/aspnet/Benchmarks
cd benchmarks\src\BenchmarksApps\BasicMinimalApi
BasicMinimalApi.csproj
to ensure you are using a recent runtime version (I know that recent SDKs don't have the latest runtime currently):dotnet publish /p:PublishTrimmed=true /p:PublishReadyToRun=true /p:PublishSingleFile=true /p:TrimMode=full /p:EnableRequestDelegateGenerator=true
bin\Release\net8.0\win-x64\publish\BasicMinimalApi.exe
Expected result
The app should run successfully
Actual result
The above exception crashes the app
Notes
From looking at the last passing run to the first failing run, this appears to start occurring between https://github.com/dotnet/runtime/compare/60b4804...390c2d5df324139c716c12bedf5a8dee737fe994.
Looking through the commit list above I believe this is caused by https://github.com/dotnet/runtime/pull/84159, since that change deleted the unmanged TypeNameParser and the exception stacktrace contains
System.Reflection.TypeNameParser.GetTypeReferencedByCustomAttribute
. But I haven't verified that is the reason for this failure.cc @jkotas @AaronRobinsonMSFT