[Do not merge] ASP.NET composite image testing

[lbussell]

@trylek, @ivdiazsa, @davidwrighton, @mangod9, Here is a vertical slice of Debian Bookworm Dockerfiles that you can clone and experiment with. I included a script, test.ps1, that builds all of the images in sequence as well as a the aspnet sample for validation.

[lbussell]

OK, so I updated this PR with the Aspnet Composite tarball built out of the installer repo form @ivdiazsa. Unfortunately we still hit the same issue as this comment: #4532 (comment)

I used the SDK version from the last installer build before Ivan's branch, which is SDK version 8.0.100-preview.4.23204.16.

I used the dive tool to confirm no files are being overwritten by layering the SDK on top of the aspnet composite image. Also, the image layout looks correct when comparing to our official images:

PS C:\sources> docker run --rm --entrypoint /bin/sh mcr.microsoft.com/dotnet/nightly/sdk:8.0.100-preview.3 -c 'find / -iname *extensions.logging.abstractions*'
/usr/share/dotnet/shared/Microsoft.AspNetCore.App/8.0.0-preview.3.23177.8/Microsoft.Extensions.Logging.Abstractions.dll
/usr/share/dotnet/packs/Microsoft.AspNetCore.App.Ref/8.0.0-preview.3.23177.8/ref/net8.0/Microsoft.Extensions.Logging.Abstractions.xml
/usr/share/dotnet/packs/Microsoft.AspNetCore.App.Ref/8.0.0-preview.3.23177.8/ref/net8.0/Microsoft.Extensions.Logging.Abstractions.dll
/usr/share/dotnet/sdk/8.0.100-preview.3.23178.7/Microsoft.Extensions.Logging.Abstractions.dll
/usr/share/dotnet/sdk/8.0.100-preview.3.23178.7/DotnetTools/dotnet-format/Microsoft.Extensions.Logging.Abstractions.dll

PS C:\sources> docker run --rm --entrypoint /bin/sh sdk-composite -c 'find / -iname *extensions.logging.abstractions*'
/usr/share/dotnet/shared/Microsoft.AspNetCore.App/8.0.0-preview.4.23203.12/Microsoft.Extensions.Logging.Abstractions.dll
/usr/share/dotnet/packs/Microsoft.AspNetCore.App.Ref/8.0.0-preview.4.23203.12/ref/net8.0/Microsoft.Extensions.Logging.Abstractions.xml
/usr/share/dotnet/packs/Microsoft.AspNetCore.App.Ref/8.0.0-preview.4.23203.12/ref/net8.0/Microsoft.Extensions.Logging.Abstractions.dll
/usr/share/dotnet/sdk/8.0.100-preview.4.23204.16/Microsoft.Extensions.Logging.Abstractions.dll
/usr/share/dotnet/sdk/8.0.100-preview.4.23204.16/DotnetTools/dotnet-format/Microsoft.Extensions.Logging.Abstractions.dll

You should be able to reproduce the error by running test.ps1 on the latest commit.

[trylek]

Thanks Logan for sharing this. I tried to build this on Linux, I had to fix the backslashes around the Dockerfile references to make that work. Do you expect this to be buildable on Windows - I mean, I don't see why it shouldn't but I can easily imagine it's easier said than done. At this point I'm still seeing the build complain about MVID mismatches, that's most likely caused by some of the composite assemblies being rewritten by their non-composite versions, I'm working on figuring out why that happens.

[lbussell]

Thanks Logan for sharing this. I tried to build this on Linux, I had to fix the backslashes around the Dockerfile references to make that work. Do you expect this to be buildable on Windows - I mean, I don't see why it shouldn't but I can easily imagine it's easier said than done. At this point I'm still seeing the build complain about MVID mismatches, that's most likely caused by some of the composite assemblies being rewritten by their non-composite versions, I'm working on figuring out why that happens.

I have been running these scripts on Windows only using Docker with the WSL2 backend.

[lbussell]

This workaround does work. It's obviously not a long-term solution.

[ivdiazsa]

Adding my comment from earlier here for visibility.

We've got a problem on our hands. So, @lbussell is facing the MVID mismatch error again here: #4538 (comment)

The problem here is... our culprit lies elsewhere. It is the Microsoft.Extensions.Logging.Abstractions located under the sdk path. For some reason, it also wants the composite. At some point I dug into that potential rabbit hole, and discovered that if I copied the Microsoft.Extensions.Logging.Abstractions from shared/Microsoft.AspNetCore.App to sdk, the error went away and it worked well. This is the workaround I gave Logan that he mentioned in the comment above.

We could hack our way into getting it copied when generating the composite, but I fear we will get a lot of push back from the SDK people, since our project's scope is just ASP.NET.

[mangod9]

What is special about that assembly? Can we check with the owners why it might be causing the MVID mismatch? Is it because its not part of the composite so we are getting an older version or something?

[ivdiazsa]

What is special about that assembly? Can we check with the owners why it might be causing the MVID mismatch? Is it because its not part of the composite so we are getting an older version or something?

The reason it is causing the MVID mismatch is because it's part of the composite. And the one in sdk was not included in the composite, hence it fails with that mismatch.

Now, this is an interesting observation. How important is Microsoft.Extensions.Logging.Abstractions? I just tested that if I remove it from the composite, then everything works fine. No more mismatches or anything. Is it something we can afford to do? It might be our only choice at this point.

[mangod9]

Hmm interesting. Perhaps @trylek can do a sanity check that the performance is still reasonable if its removed from the composite. If no perf impact removing it for now would make sense.

[trylek]

I tried to measure the plaintext benchmark on Linux using the ASP.NET perf lab with and without the assembly Microsoft.Extensions.Logging.Abstractions in the partial composite image and I see no perf difference (the startup time was 88 msecs in both cases). Having said that, I think it would be good to understand the status of this assembly:

• If it's supposed to be part of ASP.NET (as seems to be the case if we end up putting it in the composite image), I don't see why SDK should carry around its own version, it should use the one provided by the "runtime+ASP.NET" layer.
  In fact, if SDK carries around its own version, it makes it susceptible to potential break - if we needed to make a breaking change involving the assembly Microsoft.Extensions.Logging.Abstractions and some other parts of the runtime, the slightly different SDK version could be out of sync with the underlying runtime.

• If it's not supposed to be part of ASP.NET, then I completely agree we should remove it from the composite; indeed we should make sure we don't publish it as part of ASP.NET if it just happens to be unintentionally picked up by the publishing process based on its name. We should also remove it from the partial composite assembly list where it's currently included.

Adding @wtgodbe for ASP.NET and @marcpopMSFT for SDK to chime in.

[mangod9]

Also do we know whether this was the same assembly which was failing when using the composite from asp.net repo? Or was that causing other conflicts as well?

[wtgodbe]

Microsoft.Extensions.Logging.Abstractions is built by dotnet/runtime, and carried by the shared framework/targeting pack from aspnetcore. We do this for several dotnet/runtime libraries that are not part of their SharedFx, but are used by libraries in our SharedFx. The list of such libraries is here: https://github.com/dotnet/aspnetcore/blob/31d335b8562aa6d9e137676ef41368e82355e6da/eng/SharedFramework.External.props#L10-L45. I think the SDK should just be using the .dll from aspnetcore's SharedFx.

[trylek]

@mangod9 - I believe the primary error I hit in my local testing of Logan's script on Ubuntu was the one discussed above, MS.Extensions.Logging.Abstractions; I believe Ivan mentions higher up this thread that after removing the one assembly from the composite it started to work for him; based on Will's feedback I believe the next step is to understand why SDK "rolls its own" and how to properly retarget it to use the version provided by the aspnetcore layer.

[ivdiazsa]

@mangod9 - I believe the primary error I hit in my local testing of Logan's script on Ubuntu was the one discussed above, MS.Extensions.Logging.Abstractions; I believe Ivan mentions higher up this thread that after removing the one assembly from the composite it started to work for him; based on Will's feedback I believe the next step is to understand why SDK "rolls its own" and how to properly retarget it to use the version provided by the aspnetcore layer.

Right. I took it off the text list of the partial composite and that change was enough to make everything work again.

[trylek]

Right. I took it off the text list of the partial composite and that change was enough to make everything work again.

That is actually interesting, I'm surprised why that just by itself made a difference - the list I merged in a couple of weeks back just indicates for which assemblies Crossgen2 should precompile native code, technically all input assemblies get rewritten and provided with the forwarding composite header, there should be no difference as to version considerations. But maybe the fact that we're unable to resolve a single method from such assembly somehow guides the native runtime the right way. Still, if at some point we decided to fully remove a particular assembly from the composite, it should be done at msbuild level in the script

https://github.com/dotnet/aspnetcore/blob/fdd5c923fa8bb0a2ff97400287fa8a5a26ab9220/src/Framework/App.Runtime/src/Microsoft.AspNetCore.App.Runtime.csproj#L507

by selectively removing a given set of assemblies from BaseAssemblies; I would consider this as the last resort, after all Will's above response quotes a long list of assemblies with similar behavior, removing all of them from the composite would likely have significant perf effects.

[mangod9]

Yeah we should certainly investigate the reason for the mismatch. But in the near term if removing that assembly from the composite makes things work (esp. if same applies to the composite built in asp.net repo) we should make progress that way. We can subsequently get the right fix in.

[ivdiazsa]

I agree with Manish. I think that we should definitely understand what's going on, but we no longer have the luxury of time to do so. Since taking that assembly off removes all these roadblocks, I think we should go for that and we can investigate without the rush while the composite images are being tested.

[trylek]

I'm certainly not opposed to merging in a short-term workaround, I just think that the clean way to do that is to remove the assembly from the BaseAssembly set in the msbuild script. Ivan, please let me know if you want me to put up the PR for this change.

[ivdiazsa]

@trylek Sounds good, thanks a lot Tomas!

[trylek]

Unfortunately it turns out that exclusing the assembly Microsoft.Extensions.Logging.Abstractions from the composite build closure causes build errors - apparently the group of Microsoft.Extensions.Logging assemblies include partial classes that don't build when the above assembly is missing. At this point it seems to me that we would need to exclude the entire subgroup of logging assemblies with potentially heavier perf impact. As a first step I'm taking a look how hard it would be to fix the problem at the SDK side by reusing the assembly from the runtime+ASP.NET packages instead of using its own version as I believe it's the correct long-term fix.

[trylek]

Thankfully I misinterpreted the build failure - it was just a bug in my script change, not a fundamental problem. I should be able to publish the PR with the workaround shortly.

[trylek]

This is the workaround PR for reference: dotnet/aspnetcore#47747

[eerhardt]

As a first step I'm taking a look how hard it would be to fix the problem at the SDK side by reusing the assembly from the runtime+ASP.NET packages instead of using its own version as I believe it's the correct long-term fix.

I agree this is a better fix than excluding one-off assemblies in the composite image. I'm not sure why the Microsoft.Extensions versions in the dotnet/sdk are pinned to 6.0. They should be taking the "current" version coming from dotnet/runtime - just like the rest of the assemblies coming from dotnet/runtime.

[lbussell]

I am trying to test the workaround from dotnet/aspnetcore#47747 (comment). @mangod9 says the MVID mismatch shouldn't be happening anymore. That might be the case but I can't be sure yet. The issue is that there isn't a corresponding installer/SDK build (that I can find) that has a coherent runtime with the aspnet build 8.0.0-preview.4.23218.4. Taking the latest SDK build from installer produces some weird results, likely due to the differing runtime versions - dotnet help now runs but dotnet --info fails with an MVID mismatch on Microsoft.Extensions.Logging (NOT Logging.Extensions). Actual dotnet build/restore commands fail with the original issue which is the runtime version mismatch.

If there is a set of SDK and aspnetcore versions that share the same runtime and contain the MVID workaround, we can test that. Otherwise, we need to wait until the aspnet workaround flows into installer and produce a composite tarball out of there.

I can update this PR with what I had tried testing shortly.

[mangod9]

@ivdiazsa can you please work with Logan on this, since you were able to verify that removing the Logging assembly from the composite got things "working". Perhaps you just tried dotnet help, but not other sdk commands?

[ivdiazsa]

First let's wait for Tomas' changes to flow into installer. This just doesn't make any sense. If we're still having problems with Tomas' workaround, then we'll have to look into it but now with the full base.

[ivdiazsa]

I'm afraid we've got a bigger problem on our hands. I repro'd the problem @lbussell mentioned and every time I fix an assembly, I get errors from another one. I haven't reached the end of this Pandora's Box yet. That said, there seems to be a pattern. It's been only the Microsoft.Extensions.* assemblies that have been causing the problems so far.

[trylek]

@ivdiazsa / @lbussell - I have merged in the aspnetcore PR removing the remaining problematic assemblies identified by Ivan from the ASP.NET composite image. My hope is that this change should finally unblock the build of the .NET composite container for Preview 4. An aspnetcore official build is currently running, I guess the next one will start once this one finishes i.e. in a few hours. Based on this fact I believe that tomorrow (on Friday) we should have all the remaining pieces at our disposal. If one or both of you could give that a shot, that would be fantastic. I have booked vacation for tomorrow as I plan to spend the evening with my team peer Mukund who's traveling across Europe but I should be available in early morning PST hours and then in the late afternoon. I might be able to get a bit of additional work done over the weekend if needed.

[lbussell]

Based on this fact I believe that tomorrow (on Friday) we should have all the remaining pieces at our disposal. If one or both of you could give that a shot, that would be fantastic.

@trylek @ivdiazsa I just want to be completely transparent here and make sure we're on the same page. There are two known issues preventing us from shipping aspnet composite images in .NET Docker -

1. MVID mismatch - should be fixed now but awaiting validation
2. runtime incoherency between aspnetcore and the SDK in nightly builds - would be fixed by building out of dotnet/installer.

The runtime coherency issue makes these MVID fixes difficult to validate before they flow into dotnet/installer. Ivan's changes to build the aspnet composite tarball out of dotnet/installer are required for us to deliver the composite docker images. We can't have non-functional builds in our nightly repo. I would expect these changes to be in PR/review soon. I have limited bandwidth for manual testing but I can certainly test any builds Ivan provides. You may be able to create a build by flowing dependencies into installer manually with darc.

For .NET Docker specifically we have additional time after code complete to make changes, but without the coherency issue addressed and builds published from installer, we're not ready to move forward.

[mangod9]

Perhaps we should have a quick sync on this. From what I have understood both the issues were related to the same root cause and ignoring the assemblies from the composite should make things work. At least @ivdiazsa was able to get the sdk to function after building it locally.

[lbussell]

Perhaps we should have a quick sync on this. From what I have understood both the issues were related to the same root cause and ignoring the assemblies from the composite should make things work. At least @ivdiazsa was able to get the sdk to function after building it locally.

Maybe I have misunderstood the fix, I would love to be proven wrong. I will try a quick test with a new aspnet build.

[lbussell]

I tested the latest SDK build and latest aspnet build together and things appear to work. I would like to know what other validation we can do here - does the SDK work when both

• SDK runtime > ASP.NET runtime?
• ASP.NET runtime > SDK runtime?

[lbussell]

I updated this with the latest dependency flows from installer based on #4532 and updated the test script to construct two versions of the SDK, one based on the runtime and one based on the asp.net composite image. The SDK-on-aspnet-composite image fails to build as described in #4532 (comment), while the SDK-on-runtime image appears to work. We are also able to build the aspnetapp sample on the SDK-on-runtime image and run it on the aspnet composite image.

[lbussell]

It seems this is no longer necessary since #4594 is in good shape.