Skip to content

[wasm] Correctly escape library names when generating symbols for .c - #79007

Merged
radical merged 8 commits into
dotnet:mainfrom
radical:pinvoke-escape-name
Nov 30, 2022
Merged

[wasm] Correctly escape library names when generating symbols for .c#79007
radical merged 8 commits into
dotnet:mainfrom
radical:pinvoke-escape-name

Conversation

@radical

Copy link
Copy Markdown
Member

.. files.

Use the existing FixupSymbolName method for fixing library names too,
when converting to symbols.

Fixes#78992 .

.. files.
Use th existing `FixupSymbolName` method for fixing library names too,
when converting to symbols.
Fixesdotnet#78992 .
.. as it is called frequently, and for repeated strings. For a
consolewasm template build, we get 490 calls but only 140 of them are
for unique strings.
@radicalradical added arch-wasm WebAssembly architecture area-Build-mono labels Nov 29, 2022
@ghostghost assigned radicalNov 29, 2022
@ghost

Copy link
Copy Markdown

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

Issue Details

.. files.

Use the existing FixupSymbolName method for fixing library names too,
when converting to symbols.

Fixes #78992 .

Author:radical
Assignees:-
Labels:

arch-wasm, area-Build-mono

Milestone:-

@marafmaraf left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@radical

Copy link
Copy Markdown
MemberAuthor

The test failure is #78194, and unrelated.

@radical

Copy link
Copy Markdown
MemberAuthor

/backport to release/7.0

@github-actions

Copy link
Copy Markdown
Contributor

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

@github-actions

Copy link
Copy Markdown
Contributor

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

$ git am --3way --ignore-whitespace --keep-non-patch changes.patch
Applying: [wasm] Correctly escape library names when generating symbols for .c
Applying: [wasm] PInvokeTableGenerator: Use FixupSymbolName in more places
Applying: [wasm] *TableGenerator task: Cache the symbol name fixups
Applying: [wasm] IcallTableGenerator: fixup assembly name also, when used as a symbol
Applying: cleanup
Applying: Add tests
Using index info to reconstruct a base tree...
A	src/mono/wasm/Wasm.Build.Tests/PInvokeTableGeneratorTests.cs
Falling back to patching base and 3-way merge...
Auto-merging src/tests/BuildWasmApps/Wasm.Build.Tests/PInvokeTableGeneratorTests.cs
CONFLICT (content): Merge conflict in src/tests/BuildWasmApps/Wasm.Build.Tests/PInvokeTableGeneratorTests.cs
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0006 Add tests
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

Copy link
Copy Markdown
Contributor

@radical 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.

radical added a commit to radical/runtime that referenced this pull request Dec 2, 2022
…otnet#79007)
* [wasm] Correctly escape library names when generating symbols for .c files.
Use the existing `FixupSymbolName` method for fixing library names too,
when converting to symbols.
* [wasm] *TableGenerator task: Cache the symbol name fixups
.. as it is called frequently, and for repeated strings. For a
consolewasm template build, we get 490 calls but only 140 of them are
for unique strings.
* Add tests
Fixesdotnet#78992 .
(cherry picked from commit 85a9dfc)
@ghostghost locked as resolved and limited conversation to collaborators Jan 2, 2023
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

arch-wasmWebAssembly architecturearea-Build-mono

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[wasm] Compilation with WasmBuildNative fails with dependency containing hyphen in the name

2 participants

@radical@maraf