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] Fix analyzer support in templates #77704

Merged
merged 3 commits into from
Nov 1, 2022

Conversation

radical
Copy link
Member

@radical radical commented Nov 1, 2022

@radical radical added arch-wasm WebAssembly architecture area-Build-mono labels Nov 1, 2022
@ghost
Copy link

ghost commented Nov 1, 2022

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

Issue Details
Author: radical
Assignees: -
Labels:

arch-wasm, area-Build-mono

Milestone: -

@ghost ghost assigned radical Nov 1, 2022
@radical
Copy link
Member Author

radical commented Nov 1, 2022

/azp run runtime-wasm

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@radical
Copy link
Member Author

radical commented Nov 1, 2022

/azp run runtime-wasm

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

- Add
  `[assembly:System.Runtime.Versioning.SupportedOSPlatform("browser")]`
  to the browser, and console templates. This would allow the analyzers,
  if enabled, to treat the assembly as one that will run only on
  browser.

- Also, populate `@(SupportedPlatform)` with only `browser`, for *wasm*
  projects, similar to https://github.com/dotnet/sdk/blob/fef8cedfb6b4ac85a7e135f3e4f155e29cdcbdf1/src/BlazorWasmSdk/Targets/Microsoft.NET.Sdk.BlazorWebAssembly.5_0.targets#L75-L79
@radical
Copy link
Member Author

radical commented Nov 1, 2022

/azp run runtime-wasm

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@radical
Copy link
Member Author

radical commented Nov 1, 2022

/azp run runtime-wasm

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

[assembly:System.Runtime.Versioning.SupportedOSPlatform("browser")]
Copy link
Member

Choose a reason for hiding this comment

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

Is this the best approach we can offer?

Copy link
Member Author

Choose a reason for hiding this comment

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

IIUC, this is needed. I'm not sure how else we can do this, but open to suggestions!

@pavelsavara
Copy link
Member

If this is the right way how to do it, then the samples\wasm should get the same treatment and then we could <RunAnalyzers>true</RunAnalyzers> in it.

@radical
Copy link
Member Author

radical commented Nov 1, 2022

@akoeplinger does my implementation look correct?

I added it for the samples, but didn't enable the analyzers. We could try that.

@pavelsavara
Copy link
Member

Related #77712 and #77630

@radical
Copy link
Member Author

radical commented Nov 1, 2022

This is good to go, once we have an approval.

@radical
Copy link
Member Author

radical commented Nov 1, 2022

/azp run runtime-wasm

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@steveisok steveisok self-requested a review November 1, 2022 17:03
@radical radical merged commit 6631b47 into dotnet:main Nov 1, 2022
@radical radical deleted the fix-sample-analyzer branch November 1, 2022 17:48
@ghost ghost locked as resolved and limited conversation to collaborators Dec 1, 2022
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants