Remove Blazor WASM SDK from repo #29427

captainsafia · 2021-01-19T17:10:16Z

Part of #27455.

Bump SDK and runtime versions in global.json to required versions
Remove Blazor WASM SDK projects and references
Disable ErrorOnDuplicatePublishOutputFiles in helix builds to workaround issue with eager conflict resolution checks in new SDK
Update RunTests.proj target to net6.0 to support new runtime
Add platform compat checks to DataProtection following changes in new runtime
Update output check in TemplatePackageInstaller to check for stderr
Skip broke Publish_NoBuild_CopiesStaticWebAssetsToDestinationFolder test

src/DataProtection/DataProtection/src/LoggingExtensions.cs

eng/Workarounds.props

wtgodbe

Infra parts LGTM other than the nit

ghost · 2021-01-20T20:45:06Z

Hello @captainsafia!

Because this pull request has the auto-merge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (`@msftbot`) and give me an instruction to get started! Learn more here.

dougbu · 2021-01-20T22:16:04Z

eng/Workarounds.props

+       and ends up marking the same files in different projects as duplicates of each other. We disable this check here to work
+       around this issue. -->
+  <PropertyGroup>
+    <ErrorOnDuplicatePublishOutputFiles>false</ErrorOnDuplicatePublishOutputFiles>


Why is this now an issue❔ If anything, the build should run smoother without the Blazor WASM SDK.

We bumped the version of the SDK we are one and picked up a change to the conflict resolution targets mention here so this workaround is necessary since this is new code.

Is this necessary everywhere (as it is now) or just in a couple of projects❔ Having it here concerns me…

This only affects a couple of projects, particularly the test sites that we use which end up having a lot of common files (wwwroot/style.css for example) which trigger this buggy code path. I'm working on opening a bug in the SDK repo to see if there is an issue with this.

dougbu · 2021-01-20T22:17:15Z

eng/helix/content/RunTests/RunTests.csproj

@@ -2,7 +2,7 @@

  <PropertyGroup>
    <OutputType>Exe</OutputType>
-    <TargetFramework>net5.0</TargetFramework>
+    <TargetFramework>net6.0</TargetFramework>


Why is this changing here❔ We've been intentionally using the older TFM since the net6.0 transition without any related problems.

I had to update the runtime when updating the SDK and was trying to fix the following issue:

Running tests: /datadisks/disk1/work/9804089C/w/B36509CF/e/.dotnet9476/dotnet run --no-restore --project RunTests/RunTests.csproj -- --target Diagnostics.FunctionalTests.dll --runtime 6.0.0-ci --queue Ubuntu.1604.Amd64.Open --arch x64 --quarantined false --ef 6.0.0-alpha.1.21069.1 --helixTimeout 00:30:00 It was not possible to find any compatible framework version The framework 'Microsoft.NETCore.App', version '5.0.0' was not found. - The following frameworks were found: 6.0.0-alpha.1.21061.20 at [/datadisks/disk1/work/9804089C/w/B36509CF/e/.dotnet9476/shared/Microsoft.NETCore.App] 6.0.0-alpha.1.21070.2 at [/datadisks/disk1/work/9804089C/w/B36509CF/e/.dotnet9476/shared/Microsoft.NETCore.App]

That sounds like a side effect of the SDK no longer bundling the runtimes though it might be a bug in the specific version. @dsplaisted any suggestions❔

eng/targets/CSharp.Common.props

global.json

dougbu · 2021-01-20T22:20:22Z

src/DataProtection/DataProtection/src/DataProtectionServiceCollectionExtensions.cs

@@ -67,6 +70,8 @@ private static void AddDataProtectionServices(IServiceCollection services)
        {
            if (OSVersionUtil.IsWindows())
            {
+                // Assertion for platform compat analyzer
+                Debug.Assert(RuntimeInformation.IsOSPlatform(OSPlatform.Windows));


What changed to make this necessary❔ Same for src/DataProtection/DataProtection/src/EphemeralDataProtectionProvider.cs

New runtime dependency introduced some platform compat changes.

What "new runtime dependency"❔ Tests should be using the runtimes we explicitly install and not the default version specified in the SDK.

My statement wasn't very clear. I updated the dotnet version we specify in global.json which brought in some API changes related to platform compat that I addressed here and below.

dougbu · 2021-01-20T22:24:14Z

src/DataProtection/DataProtection/src/LoggingExtensions.cs

-                logLevel: LogLevel.Debug,
-                formatString: "Reading data from registry key '{RegistryKeyName}', value '{Value}'.");
+
+            if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))


What led to the new platform-compatibility checks here❔

* Remove Blazor WASM Sdk in repo * Bump dotnet and fix platform compat issues * Remove Blazor WASM SDK from solution files * Disable removal of Web.Spa 6.0 templates * Update check for uninstalled templates * Commit broken tests and fix installer check * Remove BlazorWebAssemblySdkDirectoryRoot references * Try to resolve Helix publish issues * Disable ErrorOnDuplicatePublishOutputFiles property * Bump tfm in Helix test project * Add explainer note in Workaround.props Commit migrated from dotnet/aspnetcore@bdec803c73df

captainsafia force-pushed the safia/remove-blazor-sdk branch 2 times, most recently from a16b387 to 7562cb2 Compare January 19, 2021 17:19

captainsafia added 3 commits January 19, 2021 15:15

Remove Blazor WASM Sdk in repo

6c4a773

Bump dotnet and fix platform compat issues

555c401

Remove Blazor WASM SDK from solution files

fbe1288

captainsafia force-pushed the safia/remove-blazor-sdk branch from bad39d3 to fbe1288 Compare January 19, 2021 23:15

captainsafia added 2 commits January 19, 2021 15:57

Disable removal of Web.Spa 6.0 templates

6807ab2

Update check for uninstalled templates

0f7c674

captainsafia force-pushed the safia/remove-blazor-sdk branch from d396189 to 0f7c674 Compare January 20, 2021 01:17

captainsafia added 5 commits January 19, 2021 19:51

Commit broken tests and fix installer check

5cd2bda

Remove BlazorWebAssemblySdkDirectoryRoot references

193c562

Try to resolve Helix publish issues

959b987

Disable ErrorOnDuplicatePublishOutputFiles property

5eedf68

Bump tfm in Helix test project

0c5305c

Pilchie added the area-blazor Includes: Blazor, Razor Components label Jan 20, 2021

captainsafia changed the title ~~Remove Blazor WASM Sdk in repo~~ Remove Blazor WASM SDK from repo Jan 20, 2021

captainsafia requested review from dougbu and pranavkm January 20, 2021 18:34

captainsafia marked this pull request as ready for review January 20, 2021 18:34

captainsafia requested a review from a team as a code owner January 20, 2021 18:34

captainsafia commented Jan 20, 2021

View reviewed changes

src/DataProtection/DataProtection/src/LoggingExtensions.cs Show resolved Hide resolved

pranavkm approved these changes Jan 20, 2021

View reviewed changes

src/DataProtection/DataProtection/src/LoggingExtensions.cs Show resolved Hide resolved

captainsafia requested a review from wtgodbe January 20, 2021 20:08

wtgodbe reviewed Jan 20, 2021

View reviewed changes

eng/Workarounds.props Show resolved Hide resolved

wtgodbe approved these changes Jan 20, 2021

View reviewed changes

Add explainer note in Workaround.props

686fb0b

captainsafia added the auto-merge label Jan 20, 2021

ghost merged commit bdec803 into master Jan 20, 2021

ghost deleted the safia/remove-blazor-sdk branch January 20, 2021 22:02

dougbu reviewed Jan 20, 2021

View reviewed changes

MichaelSimons mentioned this pull request Mar 25, 2021

ArPow stage 1: local source-build infrastructure #31235

Merged

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove Blazor WASM SDK from repo #29427

Remove Blazor WASM SDK from repo #29427

captainsafia commented Jan 19, 2021 •

edited

Loading

wtgodbe left a comment

ghost commented Jan 20, 2021

dougbu Jan 20, 2021

captainsafia Jan 21, 2021

dougbu Jan 21, 2021

captainsafia Jan 22, 2021

dougbu Jan 20, 2021

captainsafia Jan 21, 2021

dougbu Jan 21, 2021

dougbu Jan 20, 2021

captainsafia Jan 21, 2021

dougbu Jan 21, 2021

captainsafia Jan 22, 2021

dougbu Jan 20, 2021

Remove Blazor WASM SDK from repo #29427

Remove Blazor WASM SDK from repo #29427

Conversation

captainsafia commented Jan 19, 2021 • edited Loading

wtgodbe left a comment

Choose a reason for hiding this comment

ghost commented Jan 20, 2021

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

captainsafia commented Jan 19, 2021 •

edited

Loading

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (`@msftbot`) and give me an instruction to get started! Learn more here.