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

Adding changes to support use monovsdbg to debug wasm apps. #7220

Open
wants to merge 19 commits into
base: main
Choose a base branch
from

Conversation

thaystg
Copy link
Member

@thaystg thaystg commented Jun 10, 2024

Adding changes to support debug wasm apps using monovsdbg.

image

Tested scenarios with the new setting enabled and this PR:

  • With Assets and C# DevKit Not Installed targetting .NET 8 (Blazor Wasm) - DISABLED

  • With Assets and C# DevKit Not Installed targetting .NET 8 (Blazor Server Wasm) - DISABLED

  • With Assets and C# DevKit Not Installed targetting .NET 9 (Blazor Wasm) - ENABLED

  • With Assets and C# DevKit Not Installed targetting .NET 9 (Blazor Server Wasm) - ENABLED

  • With Assets and C# DevKit Installed targetting .NET 8 (Blazor Wasm) - DISABLED

  • With Assets and C# DevKit Installed targetting .NET 8 (Blazor Server Wasm) - DISABLED

  • With Assets and C# DevKit Installed targetting .NET 9 (Blazor Wasm) - ENABLED

  • With Assets and C# DevKit Installed targetting .NET 9 (Blazor Server Wasm) - ENABLED

  • Without Assets and C# DevKit Installed targetting .NET 8 (Blazor Wasm) - DISABLED

  • Without Assets and C# DevKit Installed targetting .NET 8 (Blazor Server Wasm) - DISABLED

  • Without Assets and C# DevKit Installed targetting .NET 9 (Blazor Wasm) - ENABLED

  • Without Assets and C# DevKit Installed targetting .NET 9 (Blazor Server Wasm) - ENABLED

Tested scenarios only updating C# DevKit with the monovsdbg support:

  • Without Assets and C# DevKit Installed with C# extension without this PR targetting .NET 8 (Blazor Wasm) - DISABLED
  • Without Assets and C# DevKit Installed with C# extension without this PR targetting .NET 8 (Blazor Server Wasm) - DISABLED
  • Without Assets and C# DevKit Installed with C# extension without this PR targetting .NET 9 (Blazor Wasm) - DISABLED
  • Without Assets and C# DevKit Installed with C# extension without this PR targetting .NET 9 (Blazor Server Wasm) - DISABLED

Tested scenarios with this PR and C# DevKit without the monovsdbg support:

  • With Assets and C# DevKit Installed targetting .NET 8 (Blazor Wasm) - DISABLED

  • With Assets and C# DevKit Installed targetting .NET 8 (Blazor Server Wasm) - DISABLED

  • With Assets and C# DevKit Installed targetting .NET 9 (Blazor Wasm) - ENABLED

  • With Assets and C# DevKit Installed targetting .NET 9 (Blazor Server Wasm) - ENABLED

  • Without Assets and C# DevKit Installed targetting .NET 8 (Blazor Wasm) - DISABLED

  • Without Assets and C# DevKit Installed targetting .NET 8 (Blazor Server Wasm) - DISABLED

  • Without Assets and C# DevKit Installed targetting .NET 9 (Blazor Wasm) - DISABLED

  • Without Assets and C# DevKit Installed targetting .NET 9 (Blazor Server Wasm) - DISABLED

package.json Outdated Show resolved Hide resolved
package.nls.json Outdated Show resolved Hide resolved
@@ -43,6 +43,14 @@ export function isWebProject(projectPath: string): boolean {
return projectFileText.toLowerCase().indexOf('sdk="microsoft.net.sdk.web"') >= 0;
}

export function isWebAssemblyProject(projectPath: string): boolean {
const projectFileText = fs.readFileSync(projectPath, 'utf8');
Copy link
Contributor

Choose a reason for hiding this comment

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

const projectFileText = fs.readFileSync(projectPath, 'utf8');

Instead of reading the file a second time, I would suggest changing isWebProject to getProjectFlags and returning both.

Copy link
Member Author

Choose a reason for hiding this comment

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

done

@thaystg thaystg marked this pull request as ready for review June 22, 2024 00:38
@thaystg thaystg requested review from a team as code owners June 22, 2024 00:38
@thaystg thaystg requested a review from gregg-miskelly June 22, 2024 00:52
@thaystg
Copy link
Member Author

thaystg commented Jun 28, 2024

@gregg-miskelly can you please review again?
I think I addressed all your comments.

src/shared/assets.ts Outdated Show resolved Hide resolved
src/shared/assets.ts Outdated Show resolved Hide resolved
src/shared/assets.ts Outdated Show resolved Hide resolved
src/shared/utils.ts Outdated Show resolved Hide resolved
@thaystg thaystg requested a review from gregg-miskelly July 2, 2024 15:28
@thaystg
Copy link
Member Author

thaystg commented Jul 8, 2024

@gregg-miskelly I think I'm done again. If you can review it will be great.

Copy link
Contributor

@gregg-miskelly gregg-miskelly left a comment

Choose a reason for hiding this comment

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

Otherwise LGTM

@WardenGnaw
Copy link
Contributor

Did you want to update a case for the updatePackageDependencies script?

@thaystg
Copy link
Member Author

thaystg commented Jul 16, 2024

Did you want to update a case for the updatePackageDependencies script?

I'm using npm run updatePackageDependencies to update the webassembly package but I didn't need to change this file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants