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

Update SDK to 6.0 Preview 5 #55283

Merged
merged 15 commits into from
Jul 22, 2021
Merged

Update SDK to 6.0 Preview 5 #55283

merged 15 commits into from
Jul 22, 2021

Conversation

ViktorHofer
Copy link
Member

Part of #55281

@ViktorHofer ViktorHofer added NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) area-Infrastructure labels Jul 7, 2021
@ViktorHofer ViktorHofer added this to the 6.0.0 milestone Jul 7, 2021
@ViktorHofer ViktorHofer requested a review from Anipik July 7, 2021 17:13
@ViktorHofer ViktorHofer self-assigned this Jul 7, 2021
@ghost
Copy link

ghost commented Jul 7, 2021

Tagging subscribers to this area: @dotnet/runtime-infrastructure
See info in area-owners.md if you want to be subscribed.

Issue Details

Part of #55281

Author: ViktorHofer
Assignees: ViktorHofer
Labels:

* NO MERGE *, area-Infrastructure

Milestone: 6.0.0

@Anipik Anipik removed the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Jul 13, 2021
@eerhardt
Copy link
Member

@jkotas @LakshanF @agocke @vitek-karas - The trimming test failures on Windows appear to be a bad bug with trimmed coreclr apps.

Error

System.TypeLoadException
  HResult=0x80131522
  Message=Could not load type 'System.Runtime.InteropServices.DispatchWrapper' from assembly 'System.Private.CoreLib, Version=6.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e'.
  Source=System.Private.CoreLib
  StackTrace:
   at System.Reflection.Emit.TypeBuilder.SetMethodIL(QCallModule , Int32 , Boolean , Byte[] , Int32 , Byte[] , Int32 , Int32 , ExceptionHandler[] , Int32 , Int32[] , Int32 )
   at System.Reflection.Emit.TypeBuilder.CreateTypeNoLock() in C:\git\runtime2\src\coreclr\System.Private.CoreLib\src\System\Reflection\Emit\TypeBuilder.cs:line 2061
   at System.Reflection.Emit.TypeBuilder.CreateTypeInfo() in C:\git\runtime2\src\coreclr\System.Private.CoreLib\src\System\Reflection\Emit\TypeBuilder.cs:line 1895
   at System.Xml.Serialization.XmlSerializationWriterILGen.GenerateEnd() in C:\git\runtime2\src\libraries\System.Private.Xml\src\System\Xml\Serialization\XmlSerializationWriterILGen.cs:line 72
   at System.Xml.Serialization.TempAssembly.GenerateRefEmitAssembly(XmlMapping[] , Type[] , String ) in C:\git\runtime2\src\libraries\System.Private.Xml\src\System\Xml\Serialization\Compilation.cs:line 498
   at System.Xml.Serialization.TempAssembly..ctor(XmlMapping[] , Type[] , String , String ) in C:\git\runtime2\src\libraries\System.Private.Xml\src\System\Xml\Serialization\Compilation.cs:line 67
   at System.Xml.Serialization.XmlSerializer.GenerateTempAssembly(XmlMapping , Type , String , String ) in C:\git\runtime2\src\libraries\System.Private.Xml\src\System\Xml\Serialization\XmlSerializer.cs:line 308
   at System.Xml.Serialization.XmlSerializer.GenerateTempAssembly(XmlMapping , Type , String ) in C:\git\runtime2\src\libraries\System.Private.Xml\src\System\Xml\Serialization\XmlSerializer.cs:line 291
   at System.Xml.Serialization.XmlSerializer..ctor(Type , String ) in C:\git\runtime2\src\libraries\System.Private.Xml\src\System\Xml\Serialization\XmlSerializer.cs:line 237
   at System.Xml.Serialization.XmlSerializer..ctor(Type ) in C:\git\runtime2\src\libraries\System.Private.Xml\src\System\Xml\Serialization\XmlSerializer.cs:line 195
   at System.Xml.Schema.XmlSchema.Write(XmlWriter writer, XmlNamespaceManager namespaceManager) in C:\git\runtime2\src\libraries\System.Private.Xml\src\System\Xml\Schema\XmlSchema.cs:line 133
   at XMLSchemaExamples.Main() in C:\git\runtime2\artifacts\bin\trimmingTests\projects\System.Private.Xml.TrimmingTests\XmlSchema.Write\win-x64\XmlSchema.Write.cs:line 51

Comparing working to non-working

In the "working" version, CoreLib contains the DispatchWrapper class. In the "non-working" version, DispatchWrapper is gone.

Change that caused it

In Preview5, we are setting more feature switches by default, specifically we are defaulting --feature System.Runtime.InteropServices.BuiltInComInterop.IsSupported false. This apparently means that IL Ref Emit no longer works on Windows when BuiltInComInterop is disabled.

I'm opening an issue for this now.

@eerhardt
Copy link
Member

I've opened #55600 for the windows trimming failures. The tests caught a bug in our product.

@Anipik
Copy link
Contributor

Anipik commented Jul 13, 2021

@eerhardt can we turn off this feature in order to unblock this pr ?

@eerhardt
Copy link
Member

can we turn off this feature in order to unblock this pr ?

One quick way to do that is to set <BuiltInComInteropSupport>true</BuiltInComInteropSupport> in the trimming .csproj template here:

<TargetingPackDir>{TargetingPackDir}</TargetingPackDir>
<NETCoreAppMaximumVersion>{NetCoreAppMaximumVersion}</NETCoreAppMaximumVersion>
<MicrosoftNETCoreAppVersion>{MicrosoftNETCoreAppVersion}</MicrosoftNETCoreAppVersion>
<_ExtraTrimmerArgs>{ExtraTrimmerArgs} $(_ExtraTrimmerArgs)</_ExtraTrimmerArgs>

@agocke
Copy link
Member

agocke commented Jul 13, 2021

Yes, this seems like a good solution to unblock the infra change.

@hoyosjs
Copy link
Member

hoyosjs commented Jul 16, 2021

hmm, now newtonsoft is missing?

@eerhardt
Copy link
Member

eerhardt commented Jul 16, 2021

The dotnet-linker-tests wasm leg is failing due to:


/__w/1/s/.dotnet/sdk/6.0.100-preview.5.21302.13/Sdks/Microsoft.NET.Sdk/targets/Microsoft.NET.Sdk.ImportWorkloads.targets(33,5): error NETSDK1147: To build this project, the following workloads must be installed: microsoft-net-sdk-blazorwebassembly-aot [/__w/1/s/artifacts/bin/trimmingTests/projects/Microsoft.Extensions.DependencyInjection.TrimmingTests/ActivatorUtilitiesTests/browser-wasm/project.csproj]
/__w/1/s/.dotnet/sdk/6.0.100-preview.5.21302.13/Sdks/Microsoft.NET.Sdk/targets/Microsoft.NET.Sdk.ImportWorkloads.targets(33,5): error NETSDK1147: To install these workloads, run the following command: dotnet workload install microsoft-net-sdk-blazorwebassembly-aot [/__w/1/s/artifacts/bin/trimmingTests/projects/Microsoft.Extensions.DependencyInjection.TrimmingTests/ActivatorUtilitiesTests/browser-wasm/project.csproj]
##[error].dotnet/sdk/6.0.100-preview.5.21302.13/Sdks/Microsoft.NET.Sdk/targets/Microsoft.NET.Sdk.ImportWorkloads.targets(33,5): error NETSDK1147: (NETCORE_ENGINEERING_TELEMETRY=Build) To build this project, the following workloads must be installed: microsoft-net-sdk-blazorwebassembly-aot
To install these workloads, run the following command: dotnet workload install microsoft-net-sdk-blazorwebassembly-aot

@lewing @radical @SamMonoRT - any thoughts? Why would this error start happening by upgrading the SDK from preview4 to preview5?

Looks like it isn't just the trimming tests, I see it on the other wasm legs:

https://helix.dot.net/api/2019-06-17/jobs/bb7717ef-0390-4821-b3e4-61ba0340e1d5/workitems/JIT.Generics/console

@lewing
Copy link
Member

lewing commented Jul 16, 2021

Yes. This is likely going to be tricky.

@lewing
Copy link
Member

lewing commented Jul 16, 2021

cc @radekdoulik

safern and others added 3 commits July 16, 2021 12:14
For in-tree builds, and tests we don't want to use workloads from dotnet
being used to build these.

For the projects being built on the build machine, we can disable them
via `Directory.Build.props`, and wasm's InTree/LocalBuild props. But for
projects that get built on helix, eg. the runtime tests, we are setting
the property values as environment variables.
@radical
Copy link
Member

radical commented Jul 17, 2021

Hopefully, this will fix the breakage.

@eerhardt
Copy link
Member

@agocke @vitek-karas - the installer host model tests are now failing:

application dependencies manifest (LightupLib.deps.json) was not found:\n package: 'Newtonsoft.Json', version: '9.0.1'\n path:

@@ -15,6 +15,8 @@
<TargetingPackDir>{TargetingPackDir}</TargetingPackDir>
<NETCoreAppMaximumVersion>{NetCoreAppMaximumVersion}</NETCoreAppMaximumVersion>
<MicrosoftNETCoreAppVersion>{MicrosoftNETCoreAppVersion}</MicrosoftNETCoreAppVersion>
<!-- Workaround issue: https://github.com/dotnet/runtime/issues/55600 -->
<BuiltInComInteropSupport>true</BuiltInComInteropSupport>
Copy link
Member

Choose a reason for hiding this comment

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

This can be reverted now that #55756 is merged.

radical and others added 2 commits July 19, 2021 01:19
In preview5, the workload manifest overrides
`$(UsingBrowserRuntimeWorkload)` setting, so pass it on the command
line.

``xml
    <PropertyGroup Condition="'$(RuntimeIdentifier)' == 'browser-wasm'">
        <UsingBrowserRuntimeWorkload Condition="'$(RunAOTCompilation)' == 'true' or '$(UsingMicrosoftNETSdkBlazorWebAssembly)' != 'true'" >true</UsingBrowserRuntimeWorkload>
        <UsingBrowserRuntimeWorkload Condition="'$(UsingBrowserRuntimeWorkload)' == ''" >$(WasmNativeWorkload)</UsingBrowserRuntimeWorkload>
    </PropertyGroup>
```
In P5 we don't generate .runtimeconfig.dev.json anymore. Some tests started to fail because they relied on the .runtime.dev.json to include local nuget cache in the probing paths.

I changed one of those tests to force-generate .runtimeconfig.dev.json as for the tested scenario it seems to make sense. For the other test I modified it to copy the necessary dependency into the right location instead.
@vitek-karas
Copy link
Member

vitek-karas commented Jul 19, 2021

I have a fix for the host tests here: https://github.com/vitek-karas/runtime/tree/FixHostTests.
Pushed the change to this PR as well.

@radical
Copy link
Member

radical commented Jul 20, 2021

Wasm/windows:

  1. System.Net.Http.Functional.Tests timing out when running with chrome:
ChromeDriver was started successfully.
[22:40:23] info: Loaded RemoteLoopServer.GenericHandler middleware
[22:40:23] info: Loaded NetCoreServer.GenericHandler middleware
[22:40:23] info: browser: Console websocket connected.
[22:40:23] info: Arguments: --setenv=DOTNET_TEST_WEBSOCKETHOST=127.0.0.1:49471,--setenv=DOTNET_TEST_HTTPHOST=127.0.0.1:49471,--setenv=DOTNET_TEST_REMOTE_LOOP_HOST=127.0.0.1:49471,--setenv=DOTNET_TEST_SECUREWEBSOCKETHOST=127.0.0.1:49472,--setenv=DOTNET_TEST_SECUREHTTPHOST=127.0.0.1:49472,--setenv=DOTNET_TEST_HTTP2HOST=127.0.0.1:49472,--run,WasmTestRunner.dll,System.Net.Http.Functional.Tests.dll,-notrait,category=IgnoreForCI,-notrait,category=OuterLoop,-notrait,category=failing
[22:40:29] info: Initializing.....
[22:40:32] info: Discovering: System.Net.Http.Functional.Tests.dll (method display = ClassAndMethod, method display options = None)
[22:40:32] info: Discovered:  System.Net.Http.Functional.Tests.dll (found 432 of 1542 test cases)
[22:40:32] info: Starting:    System.Net.Http.Functional.Tests.dll
[22:55:23] fail: Application has finished with exit code TIMED_OUT but 0 was expected
XHarness exit code: 71 (GENERAL_FAILURE)

@radekdoulik any idea about why this might be timing out?

  1. System.Net.WebSockets.Client.Tests failures are [wasm] SendReceive_VaryingLengthBuffers_Success failing in CI #53957

@Anipik
Copy link
Contributor

Anipik commented Jul 22, 2021

Known Failure #55999

@Anipik Anipik merged commit 159a01a into main Jul 22, 2021
@Anipik Anipik deleted the ViktorHofer-patch-2 branch July 22, 2021 19:27
mmitche pushed a commit that referenced this pull request Jul 23, 2021
* Update SDK to 6.0 Preview 6

Part of #55281

* Enable COM support to work around missing symbols in tests

* Update eng/testing/linker/project.csproj.template

* Disable the workload targets

* Disable workloads for wasm builds

For in-tree builds, and tests we don't want to use workloads from dotnet
being used to build these.

For the projects being built on the build machine, we can disable them
via `Directory.Build.props`, and wasm's InTree/LocalBuild props. But for
projects that get built on helix, eg. the runtime tests, we are setting
the property values as environment variables.

* Fix setting envvars for disabling workloads

* Another attempt to fix wasm tests

In preview5, the workload manifest overrides
`$(UsingBrowserRuntimeWorkload)` setting, so pass it on the command
line.

``xml
    <PropertyGroup Condition="'$(RuntimeIdentifier)' == 'browser-wasm'">
        <UsingBrowserRuntimeWorkload Condition="'$(RunAOTCompilation)' == 'true' or '$(UsingMicrosoftNETSdkBlazorWebAssembly)' != 'true'" >true</UsingBrowserRuntimeWorkload>
        <UsingBrowserRuntimeWorkload Condition="'$(UsingBrowserRuntimeWorkload)' == ''" >$(WasmNativeWorkload)</UsingBrowserRuntimeWorkload>
    </PropertyGroup>
```

* Fix host tests after upgrade to P5

In P5 we don't generate .runtimeconfig.dev.json anymore. Some tests started to fail because they relied on the .runtime.dev.json to include local nuget cache in the probing paths.

I changed one of those tests to force-generate .runtimeconfig.dev.json as for the tested scenario it seems to make sense. For the other test I modified it to copy the necessary dependency into the right location instead.

* Fix up reflection to private FileStatus field

* Disable workload resolver for wasm.build.tests, EMSDK run

* Disable workloads for wasm with MSBuildEnableWorkloadResolver=false everywhere

* remove linker workaround

* [wasm] Remove args unnecessary for disabling workloads

* Pass MSBuildEnableWorkloadResolver property to individual trimming projects

Co-authored-by: Juan Sebastian Hoyos Ayala <juhoyosa@microsoft.com>
Co-authored-by: Santiago Fernandez Madero <safern@microsoft.com>
Co-authored-by: Larry Ewing <lewing@microsoft.com>
Co-authored-by: Ankit Jain <radical@gmail.com>
Co-authored-by: vitek-karas <vitek.karas@microsoft.com>
Co-authored-by: Eric Erhardt <eric.erhardt@microsoft.com>
Co-authored-by: Anirudh Agnihotry <anirudhagnihotry098@gmail.com>
@ghost ghost locked as resolved and limited conversation to collaborators Aug 21, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants