Skip to content
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.

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

[browser] Fix debugger support #97213

Merged
merged 27 commits into from
Feb 12, 2024
Merged

[browser] Fix debugger support #97213

merged 27 commits into from
Feb 12, 2024

Conversation

maraf
Copy link
Member

@maraf maraf commented Jan 19, 2024

  • Build-time
    • Enable debugging for all builds until WasmDebugLevel==0 set by user
    • Disable debugging for publish until CopyOutputSymbolsToPublishDirectory=true or WasmDebugLevel!=0 set by user
  • On runtime
    • Pass build-time debugLevel to mono runtime only when browser is supported for debugging, otherwise pass 0
    • Use build-time debugLevel for all logging

Fixes #96239
Fixes #94174
Fixes #93622

@maraf maraf added arch-wasm WebAssembly architecture area-Build-mono os-browser Browser variant of arch-wasm labels Jan 19, 2024
@maraf maraf added this to the 9.0.0 milestone Jan 19, 2024
@maraf maraf self-assigned this Jan 19, 2024
@ghost
Copy link

ghost commented Jan 19, 2024

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

Issue Details

TBD

Fixes #96239

TODO

  • Change the logic
  • Add WBT for various build configurations
  • Add WBT for various publish configurations
Author: maraf
Assignees: maraf
Labels:

arch-wasm, area-Build-mono, os-browser

Milestone: 9.0.0

@maraf
Copy link
Member Author

maraf commented Feb 9, 2024

Failures are #98215 and #97513

src/mono/browser/runtime/startup.ts Outdated Show resolved Hide resolved
if (!loaderHelpers.isDebuggingSupported() || !config.resources!.pdb) {
debugLevel = 0;
}
cwraps.mono_wasm_load_runtime("unused", debugLevel);
Copy link
Member

Choose a reason for hiding this comment

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

I don't know if mono_wasm_load_runtime is an API for somebody, but I guess we could drop the un-used param all the way to driver.c

Copy link
Member Author

Choose a reason for hiding this comment

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

I thought it's in the unmanaged side for some reason, otherwise you would have removed it already. I'll try and we'll see 🤞

src/mono/browser/runtime/loader/config.ts Show resolved Hide resolved
@maraf maraf merged commit 882d01f into dotnet:main Feb 12, 2024
34 of 36 checks passed
maraf added a commit to maraf/runtime that referenced this pull request Feb 15, 2024
@maraf maraf deleted the BrowserDebuggerSupport branch March 4, 2024 13:37
@github-actions github-actions bot locked and limited conversation to collaborators Apr 4, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
arch-wasm WebAssembly architecture area-Build-mono os-browser Browser variant of arch-wasm
Projects
None yet
3 participants