[browser] Fix encoding problem when publishing #82833

ilonatommy · 2023-03-01T14:33:08Z

For usernames with Unicode characters we had a problem because Window's .cmd scripts are not supporting Unicode (https://ss64.com/nt/chcp.html). Temporary files paths, e.g. "C:\Users\煉\AppData\Local\Temp\tmpC538.tmp" were passed to emcc with incorrect encoding and then could not be found in the file system which caused exceptions.

Out-of-scope:

AOT tests for Unicode in the path

publish with relink tests for Blazor are disabled because emscripten canot process unicode correctly when we run it here:

runtime/src/mono/wasm/build/WasmApp.Native.targets

Line 475 in 99aa25f

    
           <Exec Command='emcc "@$(_EmccDefaultFlagsRsp)" "@$(_EmccDefaultLinkFlagsRsp)" "@$(_EmccLinkRsp)"' EnvironmentVariables="@(EmscriptenEnvVars)" ConsoleToMSBuild="true">

Regressions with non-ASCII characters in path names emscripten-core/emscripten#17817

ghost · 2023-03-01T14:33:25Z

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

Issue Details

Fixes #78953.

For usernames with Unicode characters we had a problem because Window's .cmd scripts are not supporting Unicode (https://ss64.com/nt/chcp.html). Temporary files paths, e.g. "C:\Users\煉\AppData\Local\Temp\tmpC538.tmp" were passed to emcc with incorrect encoding and then could not be found in the file system which caused exceptions.

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

radical · 2023-03-01T18:58:29Z

This needs a test in WBT. Instead of the username, the unicode characters can be in the project name.

src/mono/wasm/Wasm.Build.Tests/BuildPublishTests.cs

src/tasks/Common/Utils.cs

src/mono/wasm/Wasm.Build.Tests/BuildPublishTests.cs

radical · 2023-03-03T17:40:35Z

Failing on windows, and linux:
wasm-ld : error : /root/helix/work/workitem/e/wbt/pztditip.fs4/obj/Debug/net8.0/browser-wasm/wasm/for-publish/driver.o: undefined symbol: mono_aot_module_build_publish_Debug煉_info

runtime/src/tasks/AotCompilerTask/MonoAOTCompiler.cs

Lines 1027 to 1028 in 9fd3ece

    
           string symbolName = assemblyName.Replace ('.', '_').Replace ('-', '_').Replace(' ', '_'); 
        
           symbols.Add($"mono_aot_module_{symbolName}_info");

this probably needs to use FixupSymbolName too
And do we need to get aot compiler to use the fixed up name too?

@maraf could you please take a look?

pavelsavara · 2023-03-03T17:46:37Z

This does similar operation on assembly names for WASI

runtime/src/tasks/WasmAppBuilder/EmitWasmBundleBase.cs

Line 215 in 9fd3ece

private static string ToSafeSymbolName(string destinationFileName)

radical · 2023-03-03T18:55:01Z

We can replace the wasi one with FixupSymbolName which is more tested too.

ilonatommy · 2023-03-06T15:24:42Z

FixupSymbolName

Thank you for a hint. Using the FixumSymbolName does not seem to fix it yet. It changes the error into, e.g.:
wasm-ld : error : /workspaces/runtime/artifacts/bin/Wasm.Build.Tests/Debug/net8.0/browser-wasm/wbt/dhvzeehz.ggm/obj/Debug/net8.0/browser-wasm/wasm/for-publish/driver.o: undefined symbol: mono_aot_module_build_publish_Debug_E7__85__89__info
but the issue is still there. I will change this PR to non-AOT fix, then we can go on with AOT part in #83035.

src/mono/wasm/Wasm.Build.Tests/Blazor/BuildPublishTests.cs

ilonatommy · 2023-03-07T16:47:12Z

Stashing for later investigation:
running runtime (Build browser-wasm windows Release WasmBuildTests) 7/11 Wasm.Build.Tests.Blazor.BuildPublishTests fail with

Failed opening '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'
        emcc : error : '\runtime-fork2\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) [\runtime-fork2\artifacts\bin\Wasm.Build.Tests\Debug\net8.0\browser-wasm\wbt\blz_dllimp_Release_?build_then_publish\blz_dllimp_Release_?build_then_publish.csproj]
        \runtime-fork2\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 "@\runtime-fork2\artifacts\bin\dotnet-latest\packs\Microsoft.NETCore.App.Runtime.Mono.browser-wasm\8.0.0-dev\runtimes\browser-wasm\native\src\emcc-default.rsp" "@\runtime-fork2\artifacts\bin\dotnet-latest\packs\Microsoft.NETCore.App.Runtime.Mono.browser-wasm\8.0.0-dev\runtimes\browser-wasm\native\src\emcc-link.rsp" "@\repos\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\emcc-link.rsp"" exited with code 1. [runtime-fork2\artifacts\bin\Wasm.Build.Tests\Debug\net8.0\browser-wasm\wbt\blz_dllimp_Release_?build_then_publish\blz_dllimp_Release_?build_then_publish.csproj]

radical · 2023-03-07T17:16:54Z

7/11 Wasm.Build.Tests.Blazor.BuildPublishTests fail with

These seem to be failing on CI too.

ilonatommy · 2023-03-15T15:22:15Z

Publish with relink tests on Blazor do not work.
We are passing the unicode sign correctly but emcc cannot process it. The command it's failng on:

 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_no_aot_Release_煉\obj\Release\net8.0\wasm\for-publish\emcc-link.rsp"

emscripten-core/emscripten#17817. I will block these tests till emcc solves it.

src/mono/wasm/Wasm.Build.Tests/Blazor/BuildPublishTests.cs

radical

Other than the comment, LGTM 👍

ilonatommy · 2023-03-16T09:39:55Z

/backport to release/7.0

github-actions · 2023-03-16T09:40:08Z

Started backporting to release/7.0: https://github.com/dotnet/runtime/actions/runs/4435596854

github-actions · 2023-03-16T09:42:09Z

@ilonatommy backporting to release/7.0 failed, the patch most likely resulted in conflicts:

$ git am --3way --ignore-whitespace --keep-non-patch changes.patch

Applying: Fixed https://github.com/dotnet/runtime/issues/78953.
Applying: New wbt publish test case: Unicode sign in project name.
Using index info to reconstruct a base tree...
A	src/mono/wasm/Wasm.Build.Tests/BuildPublishTests.cs
Falling back to patching base and 3-way merge...
Applying: Removed duplicated, unused function + addressed @akoeplinger review.
Applying: Fix.
Applying: Revert AOT tests to fix them in a follow-up.
error: sha1 information is lacking or useless (src/mono/wasm/Wasm.Build.Tests/BuildPublishTests.cs).
error: could not build fake ancestor
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0005 Revert AOT tests to fix them in a follow-up.
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Error: The process '/usr/bin/git' failed with exit code 128

Please backport manually!

github-actions · 2023-03-16T09:42:11Z

@ilonatommy an error occurred while backporting to release/7.0, please check the run log for details!

Error: git am failed, most likely due to a merge conflict.

Fixed dotnet#78953.

0f6b9b1

ilonatommy added arch-wasm WebAssembly architecture area-Build-mono labels Mar 1, 2023

ilonatommy requested review from radical, pavelsavara and maraf March 1, 2023 14:33

ilonatommy self-assigned this Mar 1, 2023

lewing requested a review from akoeplinger March 1, 2023 14:51

build-analysis bot mentioned this pull request Mar 1, 2023

Tracking issue for CI build timeouts #76454

Closed

New wbt publish test case: Unicode sign in project name.

8456079

akoeplinger approved these changes Mar 2, 2023

View reviewed changes

radical reviewed Mar 2, 2023

View reviewed changes

src/mono/wasm/Wasm.Build.Tests/BuildPublishTests.cs Outdated Show resolved Hide resolved

akoeplinger reviewed Mar 2, 2023

View reviewed changes

src/tasks/Common/Utils.cs Outdated Show resolved Hide resolved

radical reviewed Mar 2, 2023

View reviewed changes

src/mono/wasm/Wasm.Build.Tests/BuildPublishTests.cs Outdated Show resolved Hide resolved

ilonatommy and others added 3 commits March 3, 2023 10:30

Removed duplicated, unused function + addressed @akoeplinger review.

54c0aff

Fix.

1645c72

Merge branch 'dotnet:main' into fix-78953

349bccb

build-analysis bot mentioned this pull request Mar 3, 2023

Alpine System.Net.Security.Tests failing because of "Cannot load library libgssapi_krb5.so.2" #82945

Closed

ilonatommy mentioned this pull request Mar 6, 2023

[browser] Fix encoding problem when publishing with AOT #83035

Closed

ilonatommy added 2 commits March 6, 2023 15:30

Revert AOT tests to fix them in a follow-up.

5af2012

Test Blazor as well.

805465f

radical reviewed Mar 6, 2023

View reviewed changes

src/mono/wasm/Wasm.Build.Tests/Blazor/BuildPublishTests.cs Outdated Show resolved Hide resolved

build-analysis bot mentioned this pull request Mar 6, 2023

IOException running NuGet-Migrations during tests in dotnet CLI first run #80619

Closed

@radical's review: test reduction + relinking

a363561

build-analysis bot mentioned this pull request Mar 7, 2023

AV during System.Net.Http.Functional.Tests on windows x86 #83098

Closed

dellis1972 mentioned this pull request Mar 9, 2023

MAUI project with non-ASCII project name cannot release to my Android phone dotnet/android#7710

Merged

lewing mentioned this pull request Mar 10, 2023

[mono] Use unsigned char when computing UTF8 string hashes #83273

Merged

Merge branch 'main' into fix-78953

ad557ed

Block relinking tests with Unicode signs.

36c8805

ilonatommy requested a review from radical March 16, 2023 07:04

radical reviewed Mar 16, 2023

View reviewed changes

src/mono/wasm/Wasm.Build.Tests/Blazor/BuildPublishTests.cs Show resolved Hide resolved

radical approved these changes Mar 16, 2023

View reviewed changes

ilonatommy mentioned this pull request Mar 16, 2023

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

Closed

Added active issue link.

6a4c874

ilonatommy merged commit 7f0c19a into dotnet:main Mar 16, 2023

build-analysis bot mentioned this pull request Mar 16, 2023

Crashes when initializing System.Net.Security.Native #83540

Closed

This was referenced Mar 17, 2023

[release/7.0] Fix encoding problem when publishing #83577

Merged

[browser] Escape sequence instead of unicode #83595

Merged

ghost locked as resolved and limited conversation to collaborators Apr 15, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[browser] Fix encoding problem when publishing #82833

[browser] Fix encoding problem when publishing #82833

ilonatommy commented Mar 1, 2023 •

edited

Loading

ghost commented Mar 1, 2023

radical commented Mar 1, 2023

radical commented Mar 3, 2023

pavelsavara commented Mar 3, 2023

radical commented Mar 3, 2023

ilonatommy commented Mar 6, 2023

ilonatommy commented Mar 7, 2023 •

edited

Loading

radical commented Mar 7, 2023

ilonatommy commented Mar 15, 2023 •

edited

Loading

radical left a comment

ilonatommy commented Mar 16, 2023

github-actions bot commented Mar 16, 2023

github-actions bot commented Mar 16, 2023

github-actions bot commented Mar 16, 2023

[browser] Fix encoding problem when publishing #82833

[browser] Fix encoding problem when publishing #82833

Conversation

ilonatommy commented Mar 1, 2023 • edited Loading

Out-of-scope:

ghost commented Mar 1, 2023

radical commented Mar 1, 2023

radical commented Mar 3, 2023

pavelsavara commented Mar 3, 2023

radical commented Mar 3, 2023

ilonatommy commented Mar 6, 2023

ilonatommy commented Mar 7, 2023 • edited Loading

radical commented Mar 7, 2023

ilonatommy commented Mar 15, 2023 • edited Loading

radical left a comment

Choose a reason for hiding this comment

ilonatommy commented Mar 16, 2023

github-actions bot commented Mar 16, 2023

github-actions bot commented Mar 16, 2023

github-actions bot commented Mar 16, 2023

ilonatommy commented Mar 1, 2023 •

edited

Loading

ilonatommy commented Mar 7, 2023 •

edited

Loading

ilonatommy commented Mar 15, 2023 •

edited

Loading