[browser][publish] Unicode in the path are not parsed correctly in Windows for AOT/relink by emcc

ilonatommy commented 1 year ago

Original issue:

The problem exists only on Windows. When we want to relink or we switch on AOT in Blazor app, we are triggering a task that is executing emscripten like this: https://github.com/dotnet/runtime/blob/99aa25fee09a3a66fb698720a234eb3d7770ca1a/src/mono/wasm/build/WasmApp.Native.targets#L475

_EmccLinkRsp parameter contains the project name and in case this name has unicode chars in it, emscripten will parse it as "?" and will not be able to find the path:

Failed opening 'artifacts\bin\Wasm.Build.Tests\Debug\net8.0\browser-wasm\wbt\blz_dllimp_Release_?build_then_publish\obj\Release\net8.0\wasm\for-build\dotnet.wasm'
        emcc : error : 'artifacts\bin\dotnet-latest\packs\Microsoft.NET.Runtime.Emscripten.3.1.12.Sdk.win-x64\8.0.0-preview.3.23127.2\tools\bin\wasm-emscripten-finalize -g --dyncalls-i64 --dwarf \runtime-fork2\artifacts\bin\Wasm.Build.Tests\Debug\net8.0\browser-wasm\wbt\blz_dllimp_Release_?build_then_publish\obj\Release\net8.0\wasm\for-build\dotnet.wasm -o \runtime-fork2\artifacts\bin\Wasm.Build.Tests\Debug\net8.0\browser-wasm\wbt\blz_dllimp_Release_?build_then_publish\obj\Release\net8.0\wasm\for-build\dotnet.wasm --detect-features' failed (returned 1)
        artifacts\bin\dotnet-latest\packs\Microsoft.NET.Runtime.WebAssembly.Sdk\8.0.0-dev\Sdk\WasmApp.Native.targets(468,5): error MSB3073: The command "emcc "@\artifacts\bin\dotnet-latest\packs\Microsoft.NETCore.App.Runtime.Mono.browser-wasm\8.0.0-dev\runtimes\browser-wasm\native\src\emcc-default.rsp" "@artifacts\bin\dotnet-latest\packs\Microsoft.NETCore.App.Runtime.Mono.browser-wasm\8.0.0-dev\runtimes\browser-wasm\native\src\emcc-link.rsp" "@artifacts\bin\Wasm.Build.Tests\Debug\net8.0\browser-wasm\wbt\blz_dllimp_Release_?build_then_publish\obj\Release\net8.0\wasm\for-build\emcc-link.rsp"" exited with code 1

The issue was discovered in https://github.com/dotnet/runtime/pull/82833. The test for relink on Blazor for unicode characters https://github.com/dotnet/runtime/blob/99aa25fee09a3a66fb698720a234eb3d7770ca1a/src/mono/wasm/Wasm.Build.Tests/Blazor/BuildPublishTests.cs#L46 got disabled.

Current state:

Fatal: Failed opening 'C:\Users\username\source\repos\runtime-fork\artifacts\bin\Wasm.Build.Tests\Release\net9.0\win-x64\wbt artifacts\xpk1al41_fpx_?\obj\Debug\net9.0\wasm\for-publish\dotnet.native.wasm'
        [] emcc : error : 'C:\Users\username\source\repos\runtime-fork\artifacts\bin\dotnet-latest\packs\Microsoft.NET.Runtime.Emscripten.3.1.34.Sdk.win-x64\9.0.0-preview.6.24277.2\tools\bin\wasm-opt --strip-dwarf --post-emscripten -Oz --low-memory-unused --zero-filled-memory --pass-arg=directize-initial-contents-immutable --strip-debug --strip-producers "C:\Users\username\source\repos\runtime-fork\artifacts\bin\Wasm.Build.Tests\Release\net9.0\win-x64\wbt artifacts\xpk1al41_fpx_\u9fc0\obj\Debug\net9.0\wasm\for-publish\dotnet.native.wasm" -o "C:\Users\username\source\repos\runtime-fork\artifacts\bin\Wasm.Build.Tests\Release\net9.0\win-x64\wbt artifacts\xpk1al41_fpx_\u9fc0\obj\Debug\net9.0\wasm\for-publish\dotnet.native.wasm" --mvp-features --enable-exception-handling --enable-mutable-globals --enable-sign-ext --enable-simd' failed (returned 1) 
        [] C:\Users\username\source\repos\runtime-fork\artifacts\bin\dotnet-latest\packs\Microsoft.NET.Runtime.WebAssembly.Sdk\9.0.0-dev\Sdk\BrowserWasmApp.targets(494,5): error MSB3073: The command "emcc "@C:\Users\username\source\repos\runtime-fork\artifacts\bin\dotnet-latest\packs\Microsoft.NETCore.App.Runtime.Mono.browser-wasm\9.0.0-dev\runtimes\browser-wasm\native\src\emcc-default.rsp" -msimd128 "@C:\Users\username\source\repos\runtime-fork\artifacts\bin\dotnet-latest\packs\Microsoft.NETCore.App.Runtime.Mono.browser-wasm\9.0.0-dev\runtimes\browser-wasm\native\src\emcc-link.rsp" "@C:\Users\itomkowicz\source\repos\runtime-fork\artifacts\bin\Wasm.Build.Tests\Release\net9.0\win-x64\wbt artifacts\xpk1al41_fpx_├ÜÔöÉ├ç\obj\Debug\net9.0\wasm\for-publish\emcc-link.rsp"" exited with code 1.

see the comments below.

Reproduction:

Enable TestDataForDefaultTemplate_WithWorkload blocked by this issue. Run:

./dotnet.cmd build -bl ./src/mono/wasm/Wasm.Build.Tests/Wasm.Build.Tests.csproj -c Release -t:Test -p:XUnitClassName=Wasm.Build.Tests.Blazor.BuildPublishTests -p:TargetOS=browser -p:TargetArchitecture=wasm

ghost commented 1 year ago

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

Issue Details

When we want to relink we are triggering a task that is executing emscripten like this: https://github.com/dotnet/runtime/blob/99aa25fee09a3a66fb698720a234eb3d7770ca1a/src/mono/wasm/build/WasmApp.Native.targets#L475 `_EmccLinkRsp` parameter contains the project name and in case this name has unicode chars in it, emscripten will parse it as "?" and will not be able to find the path: ``` Failed opening 'artifacts\bin\Wasm.Build.Tests\Debug\net8.0\browser-wasm\wbt\blz_dllimp_Release_?build_then_publish\obj\Release\net8.0\wasm\for-build\dotnet.wasm' emcc : error : 'artifacts\bin\dotnet-latest\packs\Microsoft.NET.Runtime.Emscripten.3.1.12.Sdk.win-x64\8.0.0-preview.3.23127.2\tools\bin\wasm-emscripten-finalize -g --dyncalls-i64 --dwarf \runtime-fork2\artifacts\bin\Wasm.Build.Tests\Debug\net8.0\browser-wasm\wbt\blz_dllimp_Release_?build_then_publish\obj\Release\net8.0\wasm\for-build\dotnet.wasm -o \runtime-fork2\artifacts\bin\Wasm.Build.Tests\Debug\net8.0\browser-wasm\wbt\blz_dllimp_Release_?build_then_publish\obj\Release\net8.0\wasm\for-build\dotnet.wasm --detect-features' failed (returned 1) artifacts\bin\dotnet-latest\packs\Microsoft.NET.Runtime.WebAssembly.Sdk\8.0.0-dev\Sdk\WasmApp.Native.targets(468,5): error MSB3073: The command "emcc "@\artifacts\bin\dotnet-latest\packs\Microsoft.NETCore.App.Runtime.Mono.browser-wasm\8.0.0-dev\runtimes\browser-wasm\native\src\emcc-default.rsp" "@artifacts\bin\dotnet-latest\packs\Microsoft.NETCore.App.Runtime.Mono.browser-wasm\8.0.0-dev\runtimes\browser-wasm\native\src\emcc-link.rsp" "@artifacts\bin\Wasm.Build.Tests\Debug\net8.0\browser-wasm\wbt\blz_dllimp_Release_?build_then_publish\obj\Release\net8.0\wasm\for-build\emcc-link.rsp"" exited with code 1 ``` The issue was discovered in https://github.com/dotnet/runtime/pull/82833. The test for relink on Blazor for unicode characters https://github.com/dotnet/runtime/blob/99aa25fee09a3a66fb698720a234eb3d7770ca1a/src/mono/wasm/Wasm.Build.Tests/Blazor/BuildPublishTests.cs#L46 got disabled.

Author:	ilonatommy
Assignees:	-
Labels:	`arch-wasm`, `disabled-test`, `area-Build-mono`
Milestone:	-

ilonatommy commented 1 year ago

The cmd script that makes emscripten fail is encoded correctly and looks as follows:

%SystemRoot%\System32\chcp.com 65001>nul
setlocal
set errorlevel=dummy
set errorlevel=
emcc "@C:\Users\username\runtime\artifacts\bin\dotnet-latest\packs\Microsoft.NETCore.App.Runtime.Mono.browser-wasm\8.0.0-dev\runtimes\browser-wasm\native\src\emcc-default.rsp" "@C:\Users\username\runtime\artifacts\bin\Wasm.Build.Tests\Release\net8.0\browser-wasm\wbt\blz_dllimp_Debug_煉publish\obj\Debug\net8.0\wasm\for-publish\emcc-compile.rsp" -c -o "C:\Users\username\AppData\Local\Temp\tmps4na2x.tmp" "C:\Users\username\runtime\artifacts\bin\dotnet-latest\packs\Microsoft.NETCore.App.Runtime.Mono.browser-wasm\8.0.0-dev\runtimes\browser-wasm\native\src\pinvoke.c"

The best way to reproduce would seem to: Run the above script in cmd (with access to emcc, either in PATH or from the emcc location src\mono\wasm\emsdk\upstream\emscripten) and make sure the paths point to existing WBT folder (WBT removes the AppBundle after each test). However, it finishes without any error.

When we are checking if the property _EmccLinkRsp in WasmApp.Native.targets contains the unicode sign,

<Message Text="Is unicode still fine -> $(_EmccLinkRsp.Contains('煉'))" Importance="High" />

just before calling

<Exec Command='emcc "@$(_EmccDefaultFlagsRsp)" "@$(_EmccDefaultLinkFlagsRsp)" "@$(_EmccLinkRsp)"' EnvironmentVariables="@(EmscriptenEnvVars)" ConsoleToMSBuild="true">
      <Output TaskParameter="ConsoleOutput" PropertyName="_EmccLinkStepConsoleOutput" />
      <Output TaskParameter="ExitCode" PropertyName="_EmccLinkStepExitCode" />
    </Exec>

the result is True, so Exec gets the string in correct form.

We are using emscripten 3.1.34 and according to https://github.com/emscripten-core/emscripten/issues/17817, it used to have problems with non-ASCII but only up to version 3.1.8. Issue https://github.com/emscripten-core/emscripten/issues/19229 seems fresh, though. Let's mark this issue as blocked till they resolve it and then we'll try again.

lewing commented 1 year ago

cc @radical

lewing commented 1 year ago

I know we do some stuff to force the utf8 for some of the emscripten stuff, I wonder if this is breaking when it doesn't match other parts of the build

ilonatommy commented 1 year ago

The error is caused by binaryen treating utf16 chars on Windows as utf8, issue: https://github.com/WebAssembly/binaryen/issues/4995. The fix is in the PR: https://github.com/WebAssembly/binaryen/pull/5671.

ilonatommy commented 5 months ago

The error changed to:

dotnet-latest\sdk\9.0.100-preview.5.24272.19\Microsoft.Common.CurrentVersion.targets(2401,5): error MSB3106: Assembly strong name "C:\Users\Desktop\runtime\artifacts\bin\Wasm.Build.Tests\Release\net9.0\win-x64\wbt artifacts\nuget\blz_aot_Debug_owxt2na2_p0r_╬ÿΓöÉ├ç╬ª┬ú├å┬╡Γéº┬ó╬ÿ├╝┬╜╬ú├¡┬╜╧ä├á├½\microsoft.aspnetcore.components.webassembly\9.0.0-preview.6.24272.7\lib\net9.0\Microsoft.AspNetCore.Components.WebAssembly.dll" is either a path which could not be found or it is a full assembly name which is badly formed. If it is a full assembly name it may contain characters that need to be escaped with backslash(\). Those characters are Equals(=), Comma(,), Quote("), Apostrophe('), Backslash(\). [C:\Users\Desktop\runtime\artifacts\bin\Wasm.Build.Tests\Release\net9.0\win-x64\wbt artifacts\blz_aot_Debug_owxt2na2_p0r_╬ÿΓöÉ├ç╬ª┬ú├å┬╡Γéº┬ó╬ÿ├╝┬╜╬ú├¡┬╜╧ä├á├½\blz_aot_Debug_owxt2na2_p0r_╬ÿΓöÉ├ç╬ª┬ú├å┬╡Γéº┬ó╬ÿ├╝┬╜╬ú├¡┬╜╧ä├á├½.csproj]

thrown by https://github.com/dotnet/msbuild/blob/db79545e5940a69560c4885b792ef3a359f0de68/src/Tasks/AssemblyDependency/ResolveAssemblyReference.cs#L1265 caused by (stack trace):

https://github.com/dotnet/msbuild/blob/db79545e5940a69560c4885b792ef3a359f0de68/src/Tasks/AssemblyDependency/ResolveAssemblyReference.cs#L2431 -> https://github.com/dotnet/msbuild/blob/db79545e5940a69560c4885b792ef3a359f0de68/src/Tasks/AssemblyDependency/ReferenceTable.cs#L1614 -> https://github.com/dotnet/msbuild/blob/db79545e5940a69560c4885b792ef3a359f0de68/src/Tasks/AssemblyDependency/ReferenceTable.cs#L531 -> https://github.com/dotnet/msbuild/blob/db79545e5940a69560c4885b792ef3a359f0de68/src/Tasks/AssemblyDependency/ReferenceTable.cs#L686

Investigate in what form the string lands in ResolveReference https://github.com/dotnet/msbuild/blob/db79545e5940a69560c4885b792ef3a359f0de68/src/Tasks/AssemblyDependency/ReferenceTable.cs#L633 to define what is the root cause. MsBuild Task ResolveAssemblyReference gets Assemblies in a correct form, see the binlog of WBT: blz_aot_Release_bnam5mbd_zb4_build.zip

ilonatommy commented 5 months ago

cc @surayya-MS, @JanKrivanek looks like MsBuild issue now. It's publishing a Blazor app in path with non-Ascii chars.

JanKrivanek commented 5 months ago

FYI @dotnet/kitten

AR-May commented 5 months ago

This issue does not repro on my machine and @ilonatommy also seem to have issue with consistent repro of the latest error as well. Returning this issue back to @ilonatommy for the time being, as we agreed.

ilonatommy commented 5 months ago

I have a stable repro and there are 2 problems:

Window's limit for long paths, even after enabling long paths we fail to publish:

C:\Users\username\work\runtime\runtime\artifacts\bin\Wasm.Build.Tests\Release\net9.0\win-x64\wbt artifacts\blz_aot_Release_3souqva1_5gw_鿀蜒枛遫䡫煉\obj\Release\net9.0\wasm\for-publish\tokens\blz_aot_Release_3souqva1_5gw__E9__BF__80__E8__9C__92__E6__9E__9B__E9__81__AB__E4__A1__AB__E7__85__89__dll_compiled_methods.txt doesn't exist.

the original error changed and throws with path for-publish:

C:\Users\username\source\repos\runtime-fork\artifacts\bin\dotnet-latest\packs\Microsoft.NET.Runtime.WebAssembly.Sdk\9.0.0-dev\Sdk\BrowserWasmApp.targets(494,5): error MSB3073: The command "emcc "@C:\Users\itomkowicz\source\repos\runtime-fork\artifacts\bin\dotnet-latest\packs\Microsoft.NETCore.App.Runtime.Mono.browser-wasm\9.0.0-dev\runtimes\browser-wasm\native\src\emcc-default.rsp" -msimd128 "@C:\Users\itomkowicz\source\repos\runtime-fork\artifacts\bin\dotnet-latest\packs\Microsoft.NETCore.App.Runtime.Mono.browser-wasm\9.0.0-dev\runtimes\browser-wasm\native\src\emcc-link.rsp" "@C:\Users\itomkowicz\source\repos\runtime-fork\artifacts\bin\Wasm.Build.Tests\Release\net9.0\win-x64\wbt artifacts\xpk1al41_fpx_├ÜÔöÉ├ç\obj\Debug\net9.0\wasm\for-publish\emcc-link.rsp"" exited with code 1.

I cannot reproduce the Assembly strong name.... error on other machines and I cannot find the reason for it on the VM I hit it.

JanKrivanek commented 5 months ago

@ilonatommy

Can you please confirm or clarify?:

You are hitting some publish error (the 'Assembly strong name..') on a VM, but not able to repro it elsewhere - is that correct? If yes - would you be able to provide remote debugging session (if it's managed VM) or a VM drive + snapshot (if it's your local)?
Yor are hitting another publish error (caused by long paths) that is stable reproducible - is that correct? If yes - can you provide script/test/etc. that could be executed by someone on our team and getting the repro?

If I misunderstood please bear with me and try to specify more :-)

ilonatommy commented 5 months ago

You are hitting some publish error (the 'Assembly strong name..') on a VM, but not able to repro it elsewhere - is that correct? If yes - would you be able to provide remote debugging session (if it's managed VM) or a VM drive + snapshot (if it's your local)?

Correct, neither my local machine nor Alina's was able to reproduce it. The error was hit on my own azure VM that I use for testing, I can pass accesses.

Yor are hitting another publish error (caused by long paths) that is stable reproducible - is that correct? If yes - can you provide script/test/etc. that could be executed by someone on our team and getting the repro?

The path length issue was found by @AR-May and can be reproduced using the command from the description. I will attach the binlog that is produced when we change the {s_unicodeChars} to TEST_OF_LONG_PATH (to make sure unicodes don't matter here). All files that have paths shorter than 260 chars are ILStripped with success on publish, these over the limit fail. Build works fine.

blz_aot_Debug_tumnkn5b_5fc_TEST_OF_LONG_PATH-build_and_publish.zip

ilonatommy commented 5 months ago

About the error connected with emcc: the PR was merged just after 116 was released, so the change should be in binaryen 117 that was bumped to emcc here https://github.com/emscripten-core/emscripten/pull/21451. At earliest it can get into emcc 3.1.55 and now we use 3.1.34. I was too fast with removing the blocked tag, we have to wait a bit more

AR-May commented 5 months ago

You are hitting some publish error (the 'Assembly strong name..') on a VM, but not able to repro it elsewhere - is that correct? If yes - would you be able to provide remote debugging session (if it's managed VM) or a VM drive + snapshot (if it's your local)?

Correct, neither my local machine nor Alina's was able to reproduce it. The error was hit on my own azure VM that I use for testing, I can pass accesses.

Yor are hitting another publish error (caused by long paths) that is stable reproducible - is that correct? If yes - can you provide script/test/etc. that could be executed by someone on our team and getting the repro?

The path length issue was found by @AR-May and can be reproduced using the command from the description. I will attach the binlog that is produced when we change the {s_unicodeChars} to TEST_OF_LONG_PATH (to make sure unicodes don't matter here). All files that have paths shorter than 260 chars are ILStripped with success on publish, these over the limit fail. Build works fine.

blz_aot_Debug_tumnkn5b_5fc_TEST_OF_LONG_PATH-build_and_publish.zip

I talked with @rainersigwald about this publishing error

So, the ILStrip task is not to blame for this error. It seems that the MonoAOTCompiler is unable to produce the file due to the long path. From the log:

Unable to open trimming-eligible-methods-outfile specified file C:\Users\username\source\repos\runtime-fork\artifacts\bin\Wasm.Build.Tests\Release\net9.0\win-x64\wbt artifacts\blz_aot_Debug_tumnkn5b_5fc_TEST_OF_LONG_PATH\obj\Debug\net9.0\wasm\for-publish\tokens\Microsoft_Extensions_DependencyInjection_dll_compiled_methods.txt
Mono Ahead of Time compiler - compiling assembly C:\Users\username\source\repos\runtime-fork\artifacts\bin\Wasm.Build.Tests\Release\net9.0\win-x64\wbt artifacts\blz_aot_Debug_tumnkn5b_5fc_TEST_OF_LONG_PATH\obj\Debug\net9.0\wasm\for-publish\aot-in\Microsoft.Extensions.DependencyInjection.dll
Compiled: 451/461
JIT time: 329 ms, Generation time: 148 ms, Assembly+Link time: 0 ms.

For the long path to work, you both need to turn it on the windows level with registry key AND have a specific entry in the application manifest, see https://learn.microsoft.com/en-us/windows/win32/fileio/maximum-file-path-limitation?tabs=registry#enable-long-paths-in-windows-10-version-1607-and-later. This second thing could be missing.

ilonatommy commented 4 months ago

Thank you for help, I decided to open a separate issue for long paths: https://github.com/dotnet/runtime/issues/103625. The problem is clearly connected with our mono C code and can be fixed when we track down all the places where we do not take special steps to allow long paths. This issue will be tracking progress on unicode problem and should be reviewed on emscripten bump.

dotnet / runtime