Skip to content

[browser] new EmccEnableAssertions and EmccEnvironment MSBuild props - #82954

Merged
pavelsavara merged 16 commits into
dotnet:mainfrom
pavelsavara:browser_asserts
Mar 15, 2023
Merged

[browser] new EmccEnableAssertions and EmccEnvironment MSBuild props#82954
pavelsavara merged 16 commits into
dotnet:mainfrom
pavelsavara:browser_asserts

Conversation

@pavelsavara

@pavelsavarapavelsavara commented Mar 3, 2023•
edited
Loading

Copy link
Copy Markdown
Member
  • 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

@pavelsavarapavelsavara added this to the 8.0.0 milestone Mar 3, 2023
@pavelsavarapavelsavara self-assigned this Mar 3, 2023
@pavelsavara
pavelsavara requested a review from marafMarch 3, 2023 19:19
@ghost

ghost commented Mar 3, 2023

Copy link
Copy Markdown

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
Comment threadsrc/mono/wasm/wasm.proj
@build-analysisbuild-analysisBot mentioned this pull request Mar 3, 2023
Closed
@pavelsavara

pavelsavara commented Mar 3, 2023

Copy link
Copy Markdown
MemberAuthor

/azp run runtime-wasm

@azure-pipelines

azure-pipelinesBot commented Mar 3, 2023

Copy link
Copy Markdown
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 ownerMarch 3, 2023 23:27
@pavelsavara
pavelsavara requested a review from radicalMarch 3, 2023 23:27
kg
kg reviewed Mar 4, 2023
View reviewed changes
Comment threadsrc/mono/wasm/runtime/types.ts Outdated
kg
kg reviewed Mar 4, 2023
View reviewed changes
Comment threadsrc/mono/wasm/wasm.proj Outdated
kg
kg reviewed Mar 4, 2023
View reviewed changes
Comment threadsrc/mono/wasm/wasm.proj
kg
kg reviewed Mar 4, 2023
View reviewed changes
Comment threadsrc/tasks/WasmAppBuilder/WasmAppBuilder.cs Outdated
kg
kg approved these changes Mar 4, 2023
View reviewed changes

@kgkg left a comment

Copy link
Copy Markdown
Contributor

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
Comment threadsrc/mono/wasm/build/WasmApp.targets Outdated
radical
radical reviewed Mar 6, 2023
View reviewed changes
Comment threadsrc/tasks/WasmAppBuilder/WasmAppBuilder.cs Outdated
maraf
maraf reviewed Mar 7, 2023
View reviewed changes
Comment threadsrc/mono/wasm/build/WasmApp.Native.targets
@pavelsavara@radical
Update src/tasks/WasmAppBuilder/WasmAppBuilder.cs
dd7561f
Co-authored-by: Ankit Jain <radical@gmail.com>
@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
@pavelsavarapavelsavara changed the title [browser] new EmccAssertions and EmccEnvironment MSBuild props[browser] new EmccEnableAssertions and EmccEnvironment MSBuild propsMar 14, 2023
This was referenced Mar 14, 2023
Closed
Closed
maraf
maraf reviewed Mar 14, 2023
View reviewed changes
Comment threadsrc/mono/wasm/runtime/assets.ts Outdated
Comment threadsrc/mono/wasm/runtime/assets.ts Outdated
Comment threadsrc/mono/wasm/build/WasmApp.Native.targets Outdated
Comment threadsrc/mono/wasm/wasm.proj
maraf
maraf approved these changes Mar 15, 2023
View reviewed changes

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

Looks good to me 👍

Comment threadsrc/mono/wasm/runtime/assets.ts Outdated
@pavelsavara@maraf
Update src/mono/wasm/runtime/assets.ts
ef636bb
Co-authored-by: Marek Fišera <mara@neptuo.com>
@pavelsavara

pavelsavara commented Mar 15, 2023•
edited
Loading

Copy link
Copy Markdown
MemberAuthor

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

Edit: yes, flaky test

@pavelsavara

pavelsavara commented Mar 15, 2023

Copy link
Copy Markdown
MemberAuthor

Failures are known 'NuGet-Migrations'

@pavelsavara
pavelsavara merged commit cacf5ac into dotnet:mainMar 15, 2023
@ghostghost 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 freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@lewinglewinglewing left review comments
@marafmarafmaraf approved these changes
@radicalradicalAwaiting requested review from radical
@marek-safarmarek-safarAwaiting requested review from marek-safar
+1 more reviewer
@kgkgkg approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@pavelsavarapavelsavara

Labels

arch-wasmWebAssembly architecturearea-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