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

[main] Update dotnet/runtime #35048

Merged
merged 32 commits into from
Sep 20, 2023
Merged

[main] Update dotnet/runtime #35048

merged 32 commits into from
Sep 20, 2023

Conversation

lewing
Copy link
Member

@lewing lewing commented Aug 30, 2023

Fix the 9.0.100 band in the emsdk transport package name

@lewing
Copy link
Member Author

lewing commented Aug 30, 2023

this may need additional tweaks to deal with the band versioning

@lewing lewing requested a review from agocke August 30, 2023 17:54
@lewing
Copy link
Member Author

lewing commented Aug 30, 2023

Installer needs a bunch of updates as well once this lands

@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged Request triage from a team member label Aug 30, 2023
@jeffschwMSFT
Copy link
Member

who is on point to either identifying the individual failures and farming out the work and/or handling them?

@mmitche
Copy link
Member

mmitche commented Aug 30, 2023

@jeffschwMSFT There shouldn't be too much. This was the commit to switch to 8: 8119940

@lewing
Copy link
Member Author

lewing commented Aug 30, 2023

opened dotnet/arcade#14002 for future me

@lewing
Copy link
Member Author

lewing commented Aug 30, 2023

Looks like it is failing on rid resolution now @elinor-fung can you take a look?

@elinor-fung
Copy link
Member

The built tests are configured to run on a runtime from before dotnet/docs#36466 for some reason.

If I build main, the runtimeconfig for the Microsoft.NET.Build.Tests has the runtime version corresponding to Version.props/Version.Details.xml:

  "runtimeOptions": {
    "tfm": "net8.0",
    "rollForward": "Major",
    "framework": {
      "name": "Microsoft.NETCore.App",
      "version": "8.0.0-rc.1.23421.3"
    },

But from this branch, it has:

  "runtimeOptions": {
    "tfm": "net8.0",
    "rollForward": "Major",
    "framework": {
      "name": "Microsoft.NETCore.App",
      "version": "8.0.0-preview.7.23375.6"
    },

I'm not sure why yet - cc @dsplaisted in case it is something obvious to you

@elinor-fung
Copy link
Member

This is what does the updating to the runtime version that gets used by tests:

<!-- Update KnownFrameworkReferences to target the right version of the runtime -->
<ItemGroup Condition="'$(TargetFrameworkIdentifier)' == '.NETCoreApp'
and $(MicrosoftNETCoreAppRefPackageVersion.StartsWith('$(_TargetFrameworkVersionWithoutV)'))
and '$(MSBuildProjectName)' != 'toolset-tasks'">
<FrameworkReference
Update="Microsoft.NETCore.App"
TargetingPackVersion="$(MicrosoftNETCoreAppRefPackageVersion)"
RuntimeFrameworkVersion="$(MicrosoftNETCoreAppRuntimePackageVersion)" />
</ItemGroup>

But with this PR, $(MicrosoftNETCoreAppRefPackageVersion.StartsWith('$(_TargetFrameworkVersionWithoutV)')) is false:

  • MicrosoftNETCoreAppRefPackageVersion is 9.0.0-alpha.1.23430.1
  • _TargetFrameworkVersionWithoutV is 8.0

@elinor-fung
Copy link
Member

@dsplaisted / @marcpopMSFT how does SDK usually handle updating major versions and targeting the consumed runtime version?

The retargeting for ASP.NET references has a similar condition:

<ItemGroup Condition="$(MicrosoftAspNetCoreAppRefPackageVersion.StartsWith('$(_TargetFrameworkVersionWithoutV)'))">
<KnownFrameworkReference Update="Microsoft.AspNetCore.App">
<LatestRuntimeFrameworkVersion>$(MicrosoftAspNetCoreAppRefPackageVersion)</LatestRuntimeFrameworkVersion>
<RuntimePackRuntimeIdentifiers>${SupportedRuntimeIdentifiers}</RuntimePackRuntimeIdentifiers>
<TargetingPackVersion>$(MicrosoftAspNetCoreAppRefPackageVersion)</TargetingPackVersion>
<DefaultRuntimeFrameworkVersion>$(MicrosoftAspNetCoreAppRefPackageVersion)</DefaultRuntimeFrameworkVersion>
</KnownFrameworkReference>
</ItemGroup>

@v-wuzhai v-wuzhai requested a review from a team as a code owner August 31, 2023 10:24
@dsplaisted
Copy link
Member

Now that the dependency flow is bringing in .NET 9 instead of .NET 8, the build doesn't have a version of .NET 8 to update the targeting to. I think the thing to do here is to update the stage 0 SDK to one that has a recent enough version of the runtime.

FYI for those not aware, here is where we are trying to keep track of the changes needed each time we update to a new TargetFramework: https://github.com/dotnet/sdk/blob/main/documentation/general/UpdateToNewTargetFramework.md Feel free to update it as appropriate.

@lewing
Copy link
Member Author

lewing commented Sep 2, 2023

@ViktorHofer @dotnet/domestic-cat please take a look

Comment on lines +61 to 78
var originalBundledNETCoreAppPackageVersion = propertyGroup.Element(ns + "BundledNETCoreAppPackageVersion").Value;
var parsedOriginalBundledPackageVersion = SemanticVersion.Parse(originalBundledNETCoreAppPackageVersion);
var parsedMicrosoftNETCoreAppRefPackageVersion =
SemanticVersion.Parse(microsoftNETCoreAppRefPackageVersion);

// In the case where we have a new major version, it'll come in first through the dotnet/runtime flow of the
// SDK's own package references. The Stage0 SDK's bundled version props file will still be on the older major version
// (and the older TFM that goes along with that). If we just replaced the bundled version with the new major version,
// apps that target the 'older' TFM would fail to build. So we need to keep the bundled version from the existing
// bundledversions.props in this one specific case.

var newBundledPackageVersion =
parsedOriginalBundledPackageVersion.Major == parsedMicrosoftNETCoreAppRefPackageVersion.Major
? microsoftNETCoreAppRefPackageVersion
: originalBundledNETCoreAppPackageVersion;

propertyGroup.Element(ns + "BundledNETCoreAppPackageVersion").Value = newBundledPackageVersion;

Copy link
Member

Choose a reason for hiding this comment

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

@dsplaisted would you mind double-checking this and seeing if it matched what you were talking about?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, this matches what I expected. It might be better to use NuGetVersion instead of SemanticVersion. I'm not too sure of the differences between them, but the code I've seen has used NuGetVersion.

@marcpopMSFT
Copy link
Member

latest change has us down to 34 test failures. @dotnet/source-build-internal @agocke @sbomer

  1. Microsoft.NET.Sdk.BlazorWebAssembly.AoT.Tests.WasmAoTPublishIntegrationTest.AoT_Publish_WithExistingWebConfig_Works
    emcc : error : C:\h\w\B1C3097B\t\dotnetSdkTests\ij4x4eyn.d0b\.nuget\packages\microsoft.netcore.app.runtime.mono.browser-wasm\8.0.0-rc.1.23414.4\runtimes\browser-wasm\native\libmono-wasm-simd.a: No such file or directory ("C:\h\w\B1C3097B\t\dotnetSdkTests\ij4x4eyn.d0b\.nuget\packages\microsoft.netcore.app.runtime.mono.browser-wasm\8.0.0-rc.1.23414.4\runtimes\browser-wasm\native\libmono-wasm-simd.a" was expected to be an input file, based on the commandline arguments provided) [C:\h\w\B1C3097B\t\dotnetSdkTests\ij4x4eyn.d0b\AoT_Publish_W---DA56BF44\blazorwasm\blazorwasm.csproj]
  2. Microsoft.NET.Publish.Tests.GivenThatWeWantToPublishAnAotApp.NativeAotLib_errors_out_when_eventpipe_is_enabled
    Expected command to fail but it did not.
  3. Source build is flagging the 8.0 downgrades I checked in to fix the System.Text.Json failure. Not sure what to do about this as it's meant to be temporary
Package IDs are:
Microsoft.Bcl.AsyncInterfaces.8.0.0-rc.1.23414.4
Microsoft.Extensions.DependencyModel.8.0.0-rc.1.23414.4
Microsoft.NET.HostModel.8.0.0-rc.1.23414.4```

There are a few other individual tests failures are unbuntu it looks like that also need to be looked into.

@elinor-fung
Copy link
Member

Microsoft.NET.Publish.Tests.GivenThatWeWantToPublishAnAotApp.NativeAotLib_errors_out_when_eventpipe_is_enabled

Microsoft.DotNet.ILCompiler being used is now 8.0.0-rc.1.23414.4 - which is from before dotnet/runtime#90811 (main is using 8.0.0-rc.1.23421.3). Maybe related to the BundledVersions changes in this PR?

Side note: not the issue being hit in this PR, but that error is also being downgraded to a warning (dotnet/runtime#91715), so the test will also need to be updated when that change flows over.

@lewing
Copy link
Member Author

lewing commented Sep 15, 2023

I've updated the runtime flow to the most recent build. The wasm build is odd @maraf and @radical can you please take a look.

@elinor-fung
Copy link
Member

C:\h\w\B1C3097B\t\dotnetSdkTests\ij4x4eyn.d0b.nuget\packages\microsoft.netcore.app.runtime.mono.browser-wasm\8.0.0-rc.1.23414.4\runtimes\browser-wasm\native\libmono-wasm-simd.a: No such file or directory

Maybe the wasm error is the same as root cause as the native aot one - seems like the test environment gets constructed such that it is using older packages. As with Microsoft.DotNet.ILCompiler, the version of Microsoft.NETCore.App.Runtime.Mono.browser-wasm being used is an older version - 8.0.0-rc.1.23414.4 - which is before libmono-wasm-simd.a was added.

@radical
Copy link
Member

radical commented Sep 15, 2023

C:\h\w\B1C3097B\t\dotnetSdkTests\ij4x4eyn.d0b.nuget\packages\microsoft.netcore.app.runtime.mono.browser-wasm\8.0.0-rc.1.23414.4\runtimes\browser-wasm\native\libmono-wasm-simd.a: No such file or directory

This is because 9.0 targets are being used with a 8.0 runtime pack. I'm trying to reproduce this locally now.

The tests fail with:
```
emcc : error : /private/tmp/helix/working/A19408C3/w/B54A09BA/e/testExecutionDirectory/.nuget/packages/microsoft.netcore.app.runtime.mono.browser-wasm/8.0.0-rc.1.23414.4/runtimes/browser-wasm/native/libmono-wasm-simd.a: No such file or directory
```

.. because the build is using 9.0 targets, but 8.0 runtime pack.

This commit works around that issue by overriding the runtime pack path
to point to the correct one. `WasmOverridePacks.targets` is taken from
`Wasm.Build.Tests` in `dotnet/runtime`.

This workaround can be removed once we have a proper 9.0 workload.
@lewing
Copy link
Member Author

lewing commented Sep 18, 2023

Looks like we are down to NativeAOT publish failures now?

@marek-safar
Copy link
Contributor

@sbomer @LakshanF could you help with resolving tests failures in AotStaticLibraryPublishWithEventPipe ?

@marek-safar
Copy link
Contributor

@dotnet/source-build-internal could you please advise how to track where these prebuilts are coming from and how to version baseline-comparison.xml in 8.0 arcade for 9.0 main?

</Dependency>
<Dependency Name="Microsoft.NETCore.DotNetHostResolver" Version="8.0.0-rc.1.23421.3">
<Dependency Name="Microsoft.NETCore.DotNetHostResolver" Version="9.0.0-alpha.1.23455.1">
<Uri>https://github.com/dotnet/runtime</Uri>
Copy link
Member

@MichaelSimons MichaelSimons Sep 19, 2023

Choose a reason for hiding this comment

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

Is this intentionally incoherent with the other runtime dependencies? They are on version 9.0.0-alpha.1.23464.19 and sha a6c64c3c26417f579022e20cbcee992b4814ed2c. This is the source of the following source-build prebuilts that are being reported:

    <Usage Id="Microsoft.NETCore.DotNetAppHost" Version="9.0.0-alpha.1.23455.1" File="src/artifacts/obj/Microsoft.DotNet.MSBuildSdkResolver/project.assets.json" />
    <Usage Id="Microsoft.NETCore.DotNetAppHost" Version="9.0.0-alpha.1.23455.1" File="src/artifacts/obj/Microsoft.DotNet.SdkResolver/project.assets.json" />
    <Usage Id="Microsoft.NETCore.DotNetHostResolver" Version="9.0.0-alpha.1.23455.1" File="src/artifacts/obj/Microsoft.DotNet.MSBuildSdkResolver/project.assets.json" IsDirectDependency="true" />
    <Usage Id="Microsoft.NETCore.DotNetHostResolver" Version="9.0.0-alpha.1.23455.1" File="src/artifacts/obj/Microsoft.DotNet.SdkResolver/project.assets.json" IsDirectDependency="true" />

Copy link
Member

Choose a reason for hiding this comment

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

We actually removed Microsoft.NETCore.DotNetHostResolver in runtime. It looks like this repo is using it to grab hostfxr for Framework builds. I think it can use Microsoft.NETCore.App.Runtime.* packages - I'll take a look.

@MichaelSimons
Copy link
Member

@dotnet/source-build-internal could you please advise how to track where these prebuilts are coming from and how to version baseline-comparison.xml in 8.0 arcade for 9.0 main?

I took a quick look. Here is what I did:

  1. Reviewed the prebuilt baseline-comparison.xml.
  2. Noted two classes of prebuilts as they come from two different project groupings.
  3. First grouping is from src/artifacts/obj/Release/Sdks/Microsoft.NET.Sdk/tools/project.assets.json which when looking at the changes led me to this conclusion.
  4. Second grouping is from src/artifacts/obj/Microsoft.DotNet.MSBuildSdkResolver/project.assets.json and src/artifacts/obj/Microsoft.DotNet.SdkResolver/project.assets.json which led me to review the version of the referenced prebuilts in the versions.props and version.details.xml and noticed this incoherency.

@elinor-fung
Copy link
Member

Second grouping from Microsoft.DotNet.MSBuildSdkResolver / Microsoft.DotNet.SdkResolver is resolved.

#35048 (comment) is all that is left.

@lewing
Copy link
Member Author

lewing commented Sep 19, 2023

@marcpopMSFT ping

@marcpopMSFT marcpopMSFT merged commit 1bdc5a5 into main Sep 20, 2023
@marcpopMSFT marcpopMSFT deleted the update-emsdk-manifest branch September 20, 2023 06:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-CodeFlow untriaged Request triage from a team member
Projects
None yet
Development

Successfully merging this pull request may close these issues.