Skip to content

[browser] new EmccEnableAssertions and EmccEnvironment MSBuild props #82954

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.

Sign up for GitHub

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

Jump to bottom
Merged
pavelsavara merged 16 commits into from
Mar 15, 2023
Merged

[browser] new EmccEnableAssertions and EmccEnvironment MSBuild props #82954

pavelsavara merged 16 commits into from
Mar 15, 2023

Conversation

@pavelsavara
Copy link
Member

@pavelsavara pavelsavara commented Mar 3, 2023 •
edited
Loading

  • added EmccEnableAssertions boolean
    • makes it easier to debug issues with wasm/native code and posix emulation
    • removed _EmccAssertionLevelDefault
  • added EmccEnvironment comma separated list. Default web,webview,worker,node,shell.
    • allows to build slightly smaller dotnet.js without code for nodeJS
  • removes Module.FS_readFile
  • makes dotnet.js.symbols into non-VFS asset. Now it's loaded before mono starts.
  • added EmccExportedLibraryFunction for DEFAULT_LIBRARY_FUNCS_TO_INCLUDE

Usage:

    <EmccAssertions Condition="'$(Configuration)' == 'Debug'">true</EmccAssertions>
    <EmccEnvironment>web</EmccEnvironment>

Fixes #62708
Fixes #74346
Related dotnet/aspnetcore#13915

Tested with latest Blazor locally

image

@pavelsavara pavelsavara added this to the 8.0.0 milestone Mar 3, 2023
@pavelsavara pavelsavara self-assigned this Mar 3, 2023
@pavelsavara pavelsavara requested a review from maraf March 3, 2023 19:19
@ghost
Copy link

ghost commented Mar 3, 2023

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

Issue Details
  • added EmccAssertions boolean
    • makes it easier to debug issues with wasm/native code and posix emulation
  • added EmccEnvironment comma separated list. Default web,webview,worker,node,shell.
    • allows to build slightly smaller dotnet.js without code for nodeJS

Usage:

    <EmccAssertions Condition="'$(Configuration)' == 'Debug'">true</EmccAssertions>
    <EmccEnvironment>web</EmccEnvironment>

Fixes #62708
Fixes #74346
Related dotnet/aspnetcore#13915

Tested with latest Blazor locally

image

Author: pavelsavara
Assignees: pavelsavara
Labels:

arch-wasm, area-System.Runtime.InteropServices.JavaScript
Milestone: 8.0.0

lewing
lewing reviewed Mar 3, 2023
View reviewed changes
@build-analysis build-analysis bot mentioned this pull request Mar 3, 2023
Closed
@pavelsavara
Copy link
Member Author

pavelsavara commented Mar 3, 2023

/azp run runtime-wasm

@azure-pipelines
Copy link

azure-pipelines bot commented Mar 3, 2023

Azure Pipelines successfully started running 1 pipeline(s).

@pavelsavara pavelsavara marked this pull request as ready for review March 3, 2023 23:27
@pavelsavara pavelsavara requested a review from kg as a code owner March 3, 2023 23:27
@pavelsavara pavelsavara requested a review from radical March 3, 2023 23:27
kg
kg reviewed Mar 4, 2023
View reviewed changes
kg
kg reviewed Mar 4, 2023
View reviewed changes
kg
kg reviewed Mar 4, 2023
View reviewed changes
kg
kg reviewed Mar 4, 2023
View reviewed changes
kg
kg approved these changes Mar 4, 2023
View reviewed changes
Copy link
Member

@kg kg left a comment

Choose a reason for hiding this comment

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

Looks good with the caveat that I'm not an expert on the debugging bits

radical
radical reviewed Mar 6, 2023
View reviewed changes
radical
radical reviewed Mar 6, 2023
View reviewed changes
maraf
maraf reviewed Mar 7, 2023
View reviewed changes
@pavelsavara
Merge branch 'main' into browser_asserts
51391ea 
# Conflicts:
#	src/mono/wasm/build/WasmApp.Native.targets
#	src/mono/wasm/runtime/dotnet.d.ts
#	src/mono/wasm/runtime/startup.ts
#	src/mono/wasm/runtime/types/emscripten.ts
@pavelsavara
feedback
9d442a8
@pavelsavara
fix
4c2d6b8
@pavelsavara
Merge branch 'main' into browser_asserts
f78df93
@pavelsavara pavelsavara requested review from maraf and radical March 14, 2023 10:30
@pavelsavara pavelsavara changed the title [browser] new EmccAssertions and EmccEnvironment MSBuild props [browser] new EmccEnableAssertions and EmccEnvironment MSBuild props Mar 14, 2023
This was referenced Mar 14, 2023
Closed
Closed
maraf
maraf reviewed Mar 14, 2023
View reviewed changes
maraf
maraf approved these changes Mar 15, 2023
View reviewed changes
Copy link
Member

@maraf maraf left a comment

Choose a reason for hiding this comment

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

Looks good to me 👍

@pavelsavara
Copy link
Member Author

pavelsavara commented Mar 15, 2023 •
edited
Loading

There was probably unrelated DebuggerTests.BreakpointTests.StepThroughOrNonUserCodeAttributeWithUserBp on last CI run.

Edit: yes, flaky test

@pavelsavara
Copy link
Member Author

pavelsavara commented Mar 15, 2023

Failures are known 'NuGet-Migrations'

@pavelsavara pavelsavara merged commit cacf5ac into dotnet:main Mar 15, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Apr 14, 2023
@pavelsavara pavelsavara deleted the browser_asserts branch September 2, 2024 15:30
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@lewing lewing lewing left review comments

@kg kg kg approved these changes

@maraf maraf maraf approved these changes

@radical radical Awaiting requested review from radical

@marek-safar marek-safar Awaiting requested review from marek-safar

Assignees

@pavelsavara pavelsavara

Labels

arch-wasm WebAssembly architecture area-System.Runtime.InteropServices.JavaScript

Projects

None yet

Milestone

8.0.0

Development

Successfully merging this pull request may close these issues.

[wasm] make it possible to enable emscripten assertions -s ASSERTIONS=1 [wasm] Allow user to set -s ENVIRONMENT setting

5 participants

@pavelsavara @radical @lewing @kg @maraf