Skip to content

[wasm][debugger] Using "rollForwardOnNoCandidateFx": 2 for BrowserDebugHost - #88919

Merged
thaystg merged 1 commit into
dotnet:mainfrom
thaystg:thays_fix_88391
Jul 17, 2023
Merged

[wasm][debugger] Using "rollForwardOnNoCandidateFx": 2 for BrowserDebugHost#88919
thaystg merged 1 commit into
dotnet:mainfrom
thaystg:thays_fix_88391

Conversation

@thaystg

@thaystg thaystg commented Jul 14, 2023

Copy link
Copy Markdown
Member

I was able to reproduce the issue and check that this PR fixes the issue.

This is the same configuration used on Blazor.DevServer.
https://github.com/dotnet/aspnetcore/blob/579d547d708eb19f8b05b00f5386649d6dac7b6a/src/Components/WebAssembly/DevServer/src/blazor-devserver.runtimeconfig.json.in#L8C6-L8C32

Fixes: #88391

@thaystg
thaystg requested a review from ilonatommy July 14, 2023 16:42
@thaystg
thaystg requested a review from radical as a code owner July 14, 2023 16:42
@ghost ghost added the area-Debugger-mono label Jul 14, 2023
@thaystg
thaystg requested a review from lewing July 14, 2023 16:43
@ghost ghost assigned thaystg Jul 14, 2023
@ghost

ghost commented Jul 14, 2023

Copy link
Copy Markdown

Tagging subscribers to this area: @thaystg
See info in area-owners.md if you want to be subscribed.

Issue Details

I was able to reproduce the issue and check that this PR fixes the issue.

This is the same configuration used on Blazor.DevServer.
https://github.com/dotnet/aspnetcore/blob/579d547d708eb19f8b05b00f5386649d6dac7b6a/src/Components/WebAssembly/DevServer/src/blazor-devserver.runtimeconfig.json.in#L8C6-L8C32

Fixes: #88391

Author: thaystg
Assignees: -
Labels:

area-Debugger-mono

Milestone: -

@thaystg

thaystg commented Jul 14, 2023

Copy link
Copy Markdown
Member Author

@lewing can I backport it to .net6 and .net7?

@radical

radical commented Jul 14, 2023

Copy link
Copy Markdown
Member

https://learn.microsoft.com/en-us/dotnet/core/tools/dotnet-environment-variables suggests that maybe DOTNET_ROLL_FORWARD is the one to use now? Maybe RollForward=LatestMajor?

@thaystg

thaystg commented Jul 14, 2023

Copy link
Copy Markdown
Member Author

https://learn.microsoft.com/en-us/dotnet/core/tools/dotnet-environment-variables suggests that maybe DOTNET_ROLL_FORWARD is the one to use now? Maybe RollForward=LatestMajor?

I don't have any preference at all.
But I think that if DevServer runs in an environment BrowserDebugProxy should also run that is why I decided to use the same configuration that they use to avoid any weird error on running BrowserDebugProxy.
If you prefer environment variable I can open a PR on aspnetcore repo, if you prefer RollForward=LatestMajor here, I can also update the PR.

@radical

radical commented Jul 14, 2023

Copy link
Copy Markdown
Member

Whatever is acceptable for blazor should be good for us too.

@radical

radical commented Jul 14, 2023

Copy link
Copy Markdown
Member

The test failure shouldn't be related to this, right?

@radical radical added the arch-wasm WebAssembly architecture label Jul 14, 2023
@ghost

ghost commented Jul 14, 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

I was able to reproduce the issue and check that this PR fixes the issue.

This is the same configuration used on Blazor.DevServer.
https://github.com/dotnet/aspnetcore/blob/579d547d708eb19f8b05b00f5386649d6dac7b6a/src/Components/WebAssembly/DevServer/src/blazor-devserver.runtimeconfig.json.in#L8C6-L8C32

Fixes: #88391

Author: thaystg
Assignees: thaystg
Labels:

arch-wasm, area-Debugger-mono

Milestone: -

@radical radical added this to the 8.0.0 milestone Jul 14, 2023
@thaystg
thaystg merged commit b7618ab into dotnet:main Jul 17, 2023
@thaystg

thaystg commented Jul 17, 2023

Copy link
Copy Markdown
Member Author

/backport to release/7.0-staging

@thaystg

thaystg commented Jul 17, 2023

Copy link
Copy Markdown
Member Author

/backport to release/6.0-staging

@github-actions

Copy link
Copy Markdown
Contributor

Started backporting to release/7.0-staging: https://github.com/dotnet/runtime/actions/runs/5576097797

@github-actions

Copy link
Copy Markdown
Contributor

Started backporting to release/6.0-staging: https://github.com/dotnet/runtime/actions/runs/5576099118

@vitek-karas

Copy link
Copy Markdown
Member

Please note that rollForwardOnNoCandidate is deprecated. The host has to support it for backward compat, but we may decide to remove it one day in the future. The recommended option is rollForward. In this case, since we're just building a normal app the best solution is to set RollForward=Major in your project file - which will correctly populate the runtimeconfig.json and so on.

@thaystg

thaystg commented Aug 3, 2023

Copy link
Copy Markdown
Member Author

@mkArtakMSFT, I think this is an important information for you to change the DevServer behavior, then we also change the BrowserDebugProxy behavior. I really don't want to get a different behavior between them to avoid the customer to see an error only when trying to debug the app. I think we should start the change on DevServer side and then propagate the same behavior here.

@vitek-karas

Copy link
Copy Markdown
Member

I want to make it clear that rollForwardOnNoCandidateFx=2 and rollForward=Major are functionally equivalent. You will not see differences in behavior (the implementation actually translates the former to the latter). But if we could, making the change to use RollFoward would be better - it's fully supported by SDK, unlike the old setting.

@thaystg

thaystg commented Aug 3, 2023

Copy link
Copy Markdown
Member Author

#89960

@ghost ghost locked as resolved and limited conversation to collaborators Sep 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

arch-wasm WebAssembly architecture area-Debugger-mono

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Blazor WASM: BrowserDebugHost.dll running from DevServer is missing roll-forward runtime policy

4 participants