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

Remove RemoteExecutor from library test linker inclusions #46814

Merged

Conversation

mdh1418
Copy link
Member

@mdh1418 mdh1418 commented Jan 11, 2021

When running wasm library tests with EnableAggressiveTrimming enabled to reduce library testing size, some test suites such as System.Buffers resulted in a dependency on Microsoft.DotNet.RemoteExecutor because of a linker bug, even though wasm does not support RemoteExecutor. With the linker fix being merged in #46805, this linker inclusion that cause failures in #46651 can be removed once the linker NuPkg takes effect.

Testing by verifying that System.Buffers test suite passes without the inclusion

Running ./dotnet.sh build /t:Test /p:TargetOS=Browser /p:TargetArchitecture=wasm /p:Configuration=Release /p:EnableAggressiveTrimming=true /p:RunAOTCompilation=true src/libraries/System.Buffers/tests/System.Buffers.Tests.csproj


Not rebased off of the linker update PR

Fatal error in IL Linker
  Unhandled exception. Mono.Linker.InternalErrorException: Step 'OutputStep' failed when processing assembly 'Microsoft.DotNet.RemoteExecutor, Version=6.0.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35'.
   ---> Mono.Linker.LinkerFatalErrorException: ILLink: error IL1011: Failed to write '/Users/mdhwang/runtime_wasm_clean/artifacts/obj/System.Buffers.Tests/net6.0-Release/browser-wasm/linked/Microsoft.DotNet.RemoteExecutor.dll
   ---> System.ArgumentException: Member 'System.Int32 Microsoft.DotNet.RemoteExecutor.Program::Main(System.String[])' is declared in another module and needs to be imported
     at Mono.Cecil.MetadataBuilder.LookupToken(IMetadataTokenProvider provider)
     at Mono.Cecil.MetadataBuilder.BuildModule()
     at Mono.Cecil.MetadataBuilder.BuildMetadata()
     at Mono.Cecil.ModuleWriter.<>c.<BuildMetadata>b__2_0(MetadataBuilder builder, MetadataReader _)
     at Mono.Cecil.ModuleDefinition.Read[TItem,TRet](TItem item, Func`3 read)
     at Mono.Cecil.ModuleWriter.BuildMetadata(ModuleDefinition module, MetadataBuilder metadata)
     at Mono.Cecil.ModuleWriter.Write(ModuleDefinition module, Disposable`1 stream, WriterParameters parameters)
     at Mono.Cecil.ModuleWriter.WriteModule(ModuleDefinition module, Disposable`1 stream, WriterParameters parameters)
     at Mono.Cecil.ModuleDefinition.Write(String fileName, WriterParameters parameters)
     at Mono.Cecil.AssemblyDefinition.Write(String fileName, WriterParameters parameters)
     at Mono.Linker.Steps.OutputStep.WriteAssembly(AssemblyDefinition assembly, String directory, WriterParameters writerParameters)
     --- End of inner exception stack trace ---
     at Mono.Linker.Steps.OutputStep.WriteAssembly(AssemblyDefinition assembly, String directory, WriterParameters writerParameters)
     at Mono.Linker.Steps.OutputStep.WriteAssembly(AssemblyDefinition assembly, String directory)
     at Mono.Linker.Steps.OutputStep.OutputAssembly(AssemblyDefinition assembly)
     at Mono.Linker.Steps.OutputStep.ProcessAssembly(AssemblyDefinition assembly)
     at Mono.Linker.Steps.BaseStep.Process(LinkContext context)
     --- End of inner exception stack trace ---
     at Mono.Linker.Steps.BaseStep.Process(LinkContext context)
     at Mono.Linker.Pipeline.ProcessStep(LinkContext context, IStep step)
     at Mono.Linker.Pipeline.Process(LinkContext context)
     at Mono.Linker.Driver.Run(ILogger customLogger)
     at Mono.Linker.Driver.Main(String[] args)
  Optimizing assemblies for size, which may change the behavior of the app. Be sure to test after publishing. See: https://aka.ms/dotnet-illink
/Users/mdhwang/runtime_wasm_clean/.dotnet/sdk/5.0.100/Sdks/Microsoft.NET.Sdk/targets/Microsoft.NET.ILLink.targets(41,5): error NETSDK1144: Optimizing assemblies for size failed. Optimization can be disabled by setting the PublishTrimmed property to false. [/Users/mdhwang/runtime_wasm_clean/src/libraries/System.Buffers/tests/System.Buffers.Tests.csproj]

Rebased off the linker update PR

XHarness command issued: wasm test --app=. --engine=V8 --engine-arg=--stack-trace-limit=1000 --js-file=runtime.js --output-directory=/Users/mdhwang/runtime_wasm_clean/artifacts/bin/System.Buffers.Tests/net6.0-Release/browser-wasm/AppBundle/xharness-output -- --run WasmTestRunner.dll System.Buffers.Tests.dll -notrait category=OuterLoop -notrait category=failing
  info: 13:26:53.0282570 v8 --expose_wasm --stack-trace-limit=1000 runtime.js -- --run WasmTestRunner.dll System.Buffers.Tests.dll -notrait category=OuterLoop -notrait category=failing

  info: Arguments: --run,WasmTestRunner.dll,System.Buffers.Tests.dll,-notrait,category=OuterLoop,-notrait,category=failing
  info: console.debug: MONO_WASM: Initializing mono runtime
  info: console.debug: MONO_WASM: ICU data archive(s) loaded, disabling invariant mode
  info: console.debug: mono_wasm_runtime_ready fe00e07a-5519-4dfe-b35a-f867dbaf2e28
  info: Initializing.....
  info: Discovering: System.Buffers.Tests.dll (method display = ClassAndMethod, method display options = None)
  info: Discovered:  System.Buffers.Tests.dll (found 22 of 22 test cases)
  info: Starting:    System.Buffers.Tests.dll
  info: Finished:    System.Buffers.Tests.dll
  info:
  info: === TEST EXECUTION SUMMARY ===
  info: Total: 22, Errors: 0, Failed: 0, Skipped: 0, Time: 3.756443s
  info:
  info: 13:26:58.1507840 Process v8 exited with 0

  XHarness exit code: 0
  XHarness artifacts: /Users/mdhwang/runtime_wasm_clean/artifacts/bin/System.Buffers.Tests/net6.0-Release/browser-wasm/AppBundle/xharness-output

@ghost
Copy link

ghost commented Jan 11, 2021

Tagging subscribers to this area: @safern, @ViktorHofer
See info in area-owners.md if you want to be subscribed.

Issue Details

When running wasm library tests with EnableAggressiveTrimming enabled to reduce library testing size, some test suites such as System.Buffers resulted in a dependency on Microsoft.DotNet.RemoteExecutor because of a linker bug, even though wasm does not support RemoteExecutor. With the linker fix being merged in #46805, this linker inclusion that cause failures in #46651 can be removed once the linker NuPkg takes effect.

Testing

Verifying that System.Buffers test suite passes without the inclusion

Author: mdh1418
Assignees: -
Labels:

arch-wasm, area-Infrastructure-libraries

Milestone: -

@ViktorHofer
Copy link
Member

Do you want to update the linker package as well with this PR?

@mdh1418
Copy link
Member Author

mdh1418 commented Jan 11, 2021

Good point! I rebased off of the update PR to test the changes locally, and it worked for System.Buffers.Tests.csproj. Will rebase this PR off of master when that update PR gets merged.

@mdh1418
Copy link
Member Author

mdh1418 commented Jan 12, 2021

The failures all seem unrelated: #46861 #46863 #46865

@mdh1418 mdh1418 merged commit 052846d into dotnet:master Jan 12, 2021
@mdh1418 mdh1418 deleted the mdhwang/remove_remote_executor_linker_inclusion branch January 12, 2021 18:06
@ghost ghost locked as resolved and limited conversation to collaborators Feb 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants