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

[browser][mt] Fix package microsoft.netcore.app.runtime.mono.multithread.browser-wasm is not found #98083

Merged
merged 3 commits into from
Feb 7, 2024

Conversation

ilonatommy
Copy link
Member

@ilonatommy ilonatommy commented Feb 7, 2024

Follow up for #97560. Fixes both problems mentioned in the comments (perf tests and wbt).

Version 9.0.0-dev of package microsoft.netcore.app.runtime.mono.multithread.browser-wasm is not found in NuGet feeds

log

The original PR had a misconception that we should create a multithreading nuget for multithreading runtime in _GetRuntimePackNuGetsToBuild and same for single thread. However, it's about creating the missing nuget as the comment says:

<!-- For local builds, only one of the 3 required runtime packs might be available. In that case,

We flip the value of WasmEnableThreads in the property _IsMTNugetMissing to achieve that.

Question:

the comment says "3 required runtimes". What is the 3rd one?
Answer: it used to be EP runtime, now it does not exist. The comment should be edited.

@ilonatommy ilonatommy added arch-wasm WebAssembly architecture area-VM-threading-mono labels Feb 7, 2024
@ilonatommy ilonatommy self-assigned this Feb 7, 2024
@ghost
Copy link

ghost commented Feb 7, 2024

Tagging subscribers to 'arch-wasm': @lewing
See info in area-owners.md if you want to be subscribed.

Issue Details

Follow up for #97560. Fixes both problems mentioned in the comments (perf tests and wbt).

Version 9.0.0-dev of package microsoft.netcore.app.runtime.mono.multithread.browser-wasm is not found in NuGet feeds

log

The original PR had a misconception that we should create a multithreading nuget for multithreading runtime in _GetRuntimePackNuGetsToBuild and same for single thread. However, it's about creating the missing nuget as the comment says:

<!-- For local builds, only one of the 3 required runtime packs might be available. In that case,

We flip the value of WasmEnableThreads in the property _IsMTNugetMissing to achieve that.

Question:

the comment says "3 required runtimes". What is the 3rd one?

Author: ilonatommy
Assignees: ilonatommy
Labels:

arch-wasm, area-VM-threading-mono

Milestone: -

@ilonatommy
Copy link
Member Author

/azp run runtime-wasm

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@@ -70,7 +71,7 @@
<_NuGetsToBuild Include="$(_DefaultRuntimePackNuGetPath)"
Project="$(InstallerProjectRoot)pkg/sfx/Microsoft.NETCore.App/Microsoft.NETCore.App.Runtime.sfxproj"
Dependencies="$(_DefaultRuntimePackNuGetPath)"
Properties="@(_DefaultPropsForNuGetBuild, ';');WasmEnableThreads=$(WasmEnableThreads)"
Properties="@(_DefaultPropsForNuGetBuild, ';');WasmEnableThreads=$(_IsMTNugetMissing)"
Copy link
Member

@pavelsavara pavelsavara Feb 7, 2024

Choose a reason for hiding this comment

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

WasmEnableThreads=!WasmEnableThreads is a lie. Why do we need it ? Could we do better ?

Copy link
Member Author

Choose a reason for hiding this comment

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

We could go back to the original version where we have 2 _NuGetsToBuild and a condition. One would be run for WasmEnableThreads=true and pass WasmEnableThreads=false to the Properties, the other would do the reverse. However, it's code duplication.

Copy link
Member

Choose a reason for hiding this comment

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

@akoeplinger could you please help Ilona to figure this out ?
It's some trick with creating fake nuget, but we don't understand the details.

I worry that this lie will trip-up somebody reading binlog and waste many hours trying to figure it out.

Copy link
Member

@maraf maraf Feb 7, 2024

Choose a reason for hiding this comment

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

The logic is correct and previously there was the similar thing - "build the other flavors". We can put an explanation in a comment

Copy link
Member

Choose a reason for hiding this comment

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

But the comment is already there

For local builds, only one of the 3 required runtime packs might be available. In that case,
build the other nugets with the same runtime but different names.

Maybe we can fix the "3", because there are only "2" currently

Copy link
Member

Choose a reason for hiding this comment

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

Hehe, comment already fixed :)

Copy link
Member

Choose a reason for hiding this comment

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

Comment would help a lot. Thanks

Copy link
Member Author

Choose a reason for hiding this comment

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

The problem is that there is a comment already (I quote it in the PR description). But the flow is so unintuitive that it did not help me and I made the mistake anyway and then it took me longer than needed to figure out what's wrong. Let's figure out the comment together then

Copy link
Member Author

Choose a reason for hiding this comment

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

  1. Moving the comment closer to _NuGetsToBuild. We can keep the original one at the beginning of the Task.
  2. The comment above _NuGetsToBuild:
    "We should have both: multithreaded and singlethreaded nugets in local packages. The one that is in line with the runtime type is already there, here we create the missing one."

Copy link
Member

Choose a reason for hiding this comment

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

"We need nugets for all wasm runtime flavors. The one corresponding the current property values is already built, the others needs to be add to the _NuGetsToBuild"?

@ilonatommy
Copy link
Member Author

/azp run runtime-wasm

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@ilonatommy ilonatommy merged commit 18655ee into dotnet:main Feb 7, 2024
195 of 206 checks passed
@LoopedBard3
Copy link
Member

Thank you! This fixed the issue.

@kg
Copy link
Contributor

kg commented Feb 8, 2024

While this sort of fixed the issue it mentions for me, I still can't build the necessary packs for WBT like I could before, so I can't run WBTs.

I get this issue until I build clr+libs subset for linux, when before I didn't need to:

  ** Building runtime pack for multithread **
        
  Determining projects to restore...
  Restored /home/kate/Projects/dotnet-runtime-wasm/src/installer/pkg/sfx/Microsoft.NETCore.App/Microsoft.NETCore.App.Runtime.sfxproj (in 885 ms).
  Microsoft.NETCore.App.Runtime -> 
/home/kate/Projects/dotnet-runtime-wasm/eng/liveBuilds.targets(62,5): error : The CoreCLR artifacts path does not exist '/home/kate/Projects/dotnet-runtime-wasm/artifacts/bin/coreclr/linux.x64.Release/'. The 'clr' subset must be built before building this project. Configuration: 'Release'. To use a different configuration, specify the 'RuntimeConfiguration' property. [/home/kate/Projects/dotnet-runtime-wasm/src/installer/pkg/sfx/Microsoft.NETCore.App/Microsoft.NETCore.App.Runtime.sfxproj]

Build FAILED.

/home/kate/Projects/dotnet-runtime-wasm/eng/liveBuilds.targets(62,5): error : The CoreCLR artifacts path does not exist '/home/kate/Projects/dotnet-runtime-wasm/artifacts/bin/coreclr/linux.x64.Release/'. The 'clr' subset must be built before building this project. Configuration: 'Release'. To use a different configuration, specify the 'RuntimeConfiguration' property. [/home/kate/Projects/dotnet-runtime-wasm/src/installer/pkg/sfx/Microsoft.NETCore.App/Microsoft.NETCore.App.Runtime.sfxproj]

And I can't make this problem go away period, I tried building the mono subset and then built without specifying a subset and it still doesn't work:

  ** Building AOT Cross compiler **
        
  Determining projects to restore...
  Restored /home/kate/Projects/dotnet-runtime-wasm/src/installer/pkg/sfx/Microsoft.NETCore.App/Microsoft.NETCore.App.MonoCrossAOT.sfxproj (in 92 ms).
  Microsoft.NETCore.App.MonoCrossAOT -> 
/home/kate/Projects/dotnet-runtime-wasm/src/installer/pkg/sfx/Microsoft.NETCore.App/Microsoft.NETCore.App.MonoCrossAOT.sfxproj(59,5): error : Cross compiler not found in /home/kate/Projects/dotnet-runtime-wasm/artifacts/bin/mono/linux.x64.Release/cross/linux-x64/. MonoAotCrossDir=/home/kate/Projects/dotnet-runtime-wasm/artifacts/bin/mono/linux.x64.Release/cross/linux-x64/

Should I file a new issue?

@kg
Copy link
Contributor

kg commented Feb 8, 2024

I looked through the subsets and found an aotcross one, but that doesn't work either. Looks like MonoAotTargetOS is empty. I tried both --os linux and --os browser.

kate@reeir-debian2:~/Projects/dotnet-runtime-wasm$ ./build.sh -subset mono.workloads+mono.aotcross+mono.wasmworkload --os linux -c Release
  Determining projects to restore...
  Tool 'coverlet.console' (version '6.0.0') was restored. Available commands: coverlet
  Tool 'dotnet-reportgenerator-globaltool' (version '5.0.2') was restored. Available commands: reportgenerator
  Tool 'microsoft.dotnet.xharness.cli' (version '9.0.0-prerelease.24077.1') was restored. Available commands: xharness
  Tool 'microsoft.visualstudio.slngen.tool' (version '11.1.0') was restored. Available commands: slngen
  
  Restore was successful.
  All projects are up-to-date for restore.
  Determining projects to restore...
  Restored /home/kate/Projects/dotnet-runtime-wasm/src/installer/pkg/sfx/Microsoft.NETCore.App/Microsoft.NETCore.App.MonoCrossAOT.sfxproj (in 95 ms).
  23 of 24 projects are up-to-date for restore.
  Determining projects to restore...
  Restored /home/kate/Projects/dotnet-runtime-wasm/src/mono/mono.proj (in 100 ms).
/home/kate/Projects/dotnet-runtime-wasm/src/mono/monoaotcross.proj(37,7): error MSB4184: The expression """.Substring(0, -1)" cannot be evaluated. length ('-1') must be a non-negative value. (Parameter 'length')
/home/kate/Projects/dotnet-runtime-wasm/src/mono/monoaotcross.proj(37,7): error MSB4184: Actual value was -1.

Build FAILED.

/home/kate/Projects/dotnet-runtime-wasm/src/mono/monoaotcross.proj(37,7): error MSB4184: The expression """.Substring(0, -1)" cannot be evaluated. length ('-1') must be a non-negative value. (Parameter 'length')
/home/kate/Projects/dotnet-runtime-wasm/src/mono/monoaotcross.proj(37,7): error MSB4184: Actual value was -1.
    0 Warning(s)
    1 Error(s)

EDIT: Manually editing the aot-cross csproj to hard-code browser and wasm just makes the build fail later, it looks like it's trying to do win32 stuff. Maybe I need a windows dev environment?

 Copying /home/kate/Projects/dotnet-runtime-wasm/artifacts/bin/mono/browser.wasm.Release/cross/browser-wasm/libc++.so.1;/home/kate/Projects/dotnet-runtime-wasm/artifacts/bin/mono/browser.wasm.Release/cross/browser-wasm/libc++abi.so.1;/home/kate/Projects/dotnet-runtime-wasm/artifacts/bin/mono/browser.wasm.Release/cross/browser-wasm/mono-aot-cross;/home/kate/Projects/dotnet-runtime-wasm/artifacts/bin/mono/browser.wasm.Release/cross/browser-wasm/mono-aot-cross.dbg to /home/kate/Projects/dotnet-runtime-wasm/artifacts/bin/mono/linux.x64.Release/cross\linux-x64/
  Determining projects to restore...
  All projects are up-to-date for restore.
/home/kate/Projects/dotnet-runtime-wasm/src/workloads/workloads.csproj(149,5): error MSB6003: The specified task executable "heat.exe" could not be run. System.ComponentModel.Win32Exception (8): An error occurred trying to start process '/home/kate/.nuget/packages/microsoft.signed.wix/1.0.0-v3.14.0.5722/tools/heat.exe' with working directory '/home/kate/Projects/dotnet-runtime-wasm/src/workloads'. Exec format error
/home/kate/Projects/dotnet-runtime-wasm/src/workloads/workloads.csproj(149,5): error MSB6003:    at System.Diagnostics.Process.ForkAndExecProcess(ProcessStartInfo startInfo, String resolvedFilename, String[] argv, String[] envp, String cwd, Boolean setCredentials, UInt32 userId, UInt32 groupId, UInt32[] groups, Int32& stdinFd, Int32& stdoutFd, Int32& stderrFd, Boolean usesTerminal, Boolean throwOnNoExec)
/home/kate/Projects/dotnet-runtime-wasm/src/workloads/workloads.csproj(149,5): error MSB6003:    at System.Diagnostics.Process.StartCore(ProcessStartInfo startInfo)
/home/kate/Projects/dotnet-runtime-wasm/src/workloads/workloads.csproj(149,5): error MSB6003:    at Microsoft.Build.Utilities.ToolTask.ExecuteTool(String pathToTool, String responseFileCommands, String commandLineCommands)
/home/kate/Projects/dotnet-runtime-wasm/src/workloads/workloads.csproj(149,5): error MSB6003:    at Microsoft.Build.Utilities.ToolTask.Execute()
/home/kate/Projects/dotnet-runtime-wasm/src/workloads/workloads.csproj(149,5): error MSB6003: The specified task executable "heat.exe" could not be run. System.ComponentModel.Win32Exception (8): An error occurred trying to start process '/home/kate/.nuget/packages/microsoft.signed.wix/1.0.0-v3.14.0.5722/tools/heat.exe' with working directory '/home/kate/Projects/dotnet-runtime-wasm/src/workloads'. Exec format error
/home/kate/Projects/dotnet-runtime-wasm/src/workloads/workloads.csproj(149,5): error MSB6003:    at System.Diagnostics.Process.ForkAndExecProcess(ProcessStartInfo startInfo, String resolvedFilename, String[] argv, String[] envp, String cwd, Boolean setCredentials, UInt32 userId, UInt32 groupId, UInt32[] groups, Int32& stdinFd, Int32& stdoutFd, Int32& stderrFd, Boolean usesTerminal, Boolean throwOnNoExec)
/home/kate/Projects/dotnet-runtime-wasm/src/workloads/workloads.csproj(149,5): error MSB6003:    at System.Diagnostics.Process.StartCore(ProcessStartInfo startInfo)
/home/kate/Projects/dotnet-runtime-wasm/src/workloads/workloads.csproj(149,5): error MSB6003:    at Microsoft.Build.Utilities.ToolTask.ExecuteTool(String pathToTool, String responseFileCommands, String commandLineCommands)
/home/kate/Projects/dotnet-runtime-wasm/src/workloads/workloads.csproj(149,5): error MSB6003:    at Microsoft.Build.Utilities.ToolTask.Execute()

@pavelsavara
Copy link
Member

pavelsavara commented Feb 8, 2024

@kg try to look at the commands that CI uses to build it. For me, following works:

.\build.cmd -bl /p:TargetArchitecture=wasm -subset mono+libs -c Debug 
.\dotnet.cmd build /p:TargetOS=browser /p:TargetArchitecture=wasm /p:Configuration=Debug /t:Test src/mono/wasm/Wasm.Build.Tests 

Assuming that

  • you don't have some stray dotnet on your PATH
  • DOTNET_ROOT is good
  • your nuget config is good
  • your artifact folder is empty

@kg
Copy link
Contributor

kg commented Feb 8, 2024

That build incantation doesn't work on linux, so it sounds like we're in a windows-only situation right now.

@kg
Copy link
Contributor

kg commented Feb 8, 2024

I think the problem is the makefile targets for WBT, i.e. make -C src/mono/browser run-build-tests. Those seem to be totally broken now. If I do a variation on the second step of pavel's suggestion, i.e. ./dotnet.sh build ... /t:Test ... with a ton of extra switches and env vars, that works fine. We should remove the makefile targets.

@ilonatommy
Copy link
Member Author

That build incantation doesn't work on linux

This issue was being fixed and tested on Linux (Codespaces), I did not have such issues. I will try to reproduce rn, maybe something new was changed on main

@kg
Copy link
Contributor

kg commented Feb 9, 2024 via email

@github-actions github-actions bot locked and limited conversation to collaborators Mar 10, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
arch-wasm WebAssembly architecture area-VM-threading-mono
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants