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

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

Merged
merged 1 commit into from
Jul 17, 2023

Conversation

thaystg
Copy link
Member

@thaystg thaystg commented Jul 14, 2023

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

@ghost
Copy link

ghost commented Jul 14, 2023

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
Copy link
Member Author

thaystg commented Jul 14, 2023

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

@radical
Copy link
Member

radical commented Jul 14, 2023

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
Copy link
Member Author

thaystg commented Jul 14, 2023

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

radical commented Jul 14, 2023

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

@radical
Copy link
Member

radical commented Jul 14, 2023

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

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

ghost commented Jul 14, 2023

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
12 of 15 checks passed
@thaystg
Copy link
Member Author

thaystg commented Jul 17, 2023

/backport to release/7.0-staging

@thaystg
Copy link
Member Author

thaystg commented Jul 17, 2023

/backport to release/6.0-staging

@github-actions
Copy link
Contributor

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

@github-actions
Copy link
Contributor

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

@vitek-karas
Copy link
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
Copy link
Member Author

thaystg commented Aug 3, 2023

@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
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
Copy link
Member Author

thaystg commented Aug 3, 2023

#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