From f6b0feaa2fe243922de7950ac35de27a1f28c523 Mon Sep 17 00:00:00 2001 From: Ankit Jain Date: Thu, 26 Aug 2021 05:57:51 -0400 Subject: [PATCH 1/3] [wasm] Require workloads, if a project is using native references Currently, if the `wasm-tools` workload is not installed, and a project uses AOT, then the build fails with an error saying that the workload is needed. But if the project is using native references, but not AOT, then the build does not fail. Instead, the `@(NativeFileReference)` just gets ignored. Even though the wasm workload is needed to relink dotnet.wasm with the native libraries. Implementation: - `$(RunAOTCompilation)` is a property, so it can be checked, and wasm workload imports can be enabled. - But `@(NativeFileReference)` is an item, and that gets evaluated in the second phase, so we can't use that to affect the imports. - Instead, we emit a warning from a target run before Build, if the project has any native references, but the workload isn't enabled. - Users can explicitly enable the workload by setting `$(WasmBuildNative)==true`. --- .../WorkloadManifest.targets.in | 26 ++-- .../Wasm.Build.Tests/BlazorWasmTests.cs | 134 ++++++++++++------ .../Wasm.Build.Tests/BuildTestBase.cs | 41 ++++++ .../Wasm.Build.Tests/NativeLibraryTests.cs | 4 +- 4 files changed, 154 insertions(+), 51 deletions(-) diff --git a/src/mono/nuget/Microsoft.NET.Workload.Mono.Toolchain.Manifest/WorkloadManifest.targets.in b/src/mono/nuget/Microsoft.NET.Workload.Mono.Toolchain.Manifest/WorkloadManifest.targets.in index d9a9dc2a72d463..9b13301584dc5d 100644 --- a/src/mono/nuget/Microsoft.NET.Workload.Mono.Toolchain.Manifest/WorkloadManifest.targets.in +++ b/src/mono/nuget/Microsoft.NET.Workload.Mono.Toolchain.Manifest/WorkloadManifest.targets.in @@ -7,13 +7,9 @@ !$([MSBuild]::VersionGreaterThanOrEquals('$(TargetFrameworkVersion)', '6.0'))">true - - <_NativeBuildNeeded Condition="'$(RunAOTCompilation)' == 'true'">true - WebAssembly workloads (required for AOT) are only supported for projects targeting net6.0+ - - - true + + true $(WasmNativeWorkload) @@ -128,7 +124,21 @@ /> - - + + + + + + + + + + diff --git a/src/tests/BuildWasmApps/Wasm.Build.Tests/BlazorWasmTests.cs b/src/tests/BuildWasmApps/Wasm.Build.Tests/BlazorWasmTests.cs index 2059142de09715..9b7709098ed929 100644 --- a/src/tests/BuildWasmApps/Wasm.Build.Tests/BlazorWasmTests.cs +++ b/src/tests/BuildWasmApps/Wasm.Build.Tests/BlazorWasmTests.cs @@ -26,27 +26,17 @@ public BlazorWasmTests(ITestOutputHelper output, SharedBuildPerTestClassFixture [InlineData("Release", true)] public void PublishTemplateProject(string config, bool aot) { - string id = $"blazorwasm_{config}_aot_{aot}"; - InitPaths(id); - if (Directory.Exists(_projectDir)) - Directory.Delete(_projectDir, recursive: true); - Directory.CreateDirectory(_projectDir); - Directory.CreateDirectory(Path.Combine(_projectDir, ".nuget")); - - File.Copy(Path.Combine(BuildEnvironment.TestDataPath, "nuget6.config"), Path.Combine(_projectDir, "nuget.config")); - File.Copy(Path.Combine(BuildEnvironment.TestDataPath, "Blazor.Directory.Build.props"), Path.Combine(_projectDir, "Directory.Build.props")); - File.Copy(Path.Combine(BuildEnvironment.TestDataPath, "Blazor.Directory.Build.targets"), Path.Combine(_projectDir, "Directory.Build.targets")); - - string logPath = Path.Combine(s_buildEnv.LogRootPath, id); + string id = $"blazorwasm_{config}_aot_{aot}_{Path.GetRandomFileName()}"; + InitBlazorWasmProjectDir(id); new DotNetCommand(s_buildEnv, useDefaultArgs: false) - .WithWorkingDirectory(_projectDir) + .WithWorkingDirectory(_projectDir!) .ExecuteWithCapturedOutput("new blazorwasm") .EnsureSuccessful(); - string publishLogPath = Path.Combine(logPath, $"{id}.binlog"); + string publishLogPath = Path.Combine(s_buildEnv.LogRootPath, id, $"{id}.binlog"); new DotNetCommand(s_buildEnv) - .WithWorkingDirectory(_projectDir) + .WithWorkingDirectory(_projectDir!) .ExecuteWithCapturedOutput("publish", $"-bl:{publishLogPath}", aot ? "-p:RunAOTCompilation=true" : "", $"-p:Configuration={config}") .EnsureSuccessful(); @@ -57,6 +47,76 @@ public void PublishTemplateProject(string config, bool aot) // playwright? } + [ConditionalTheory(typeof(BuildTestBase), nameof(IsNotUsingWorkloads))] + [InlineData("Debug")] + [InlineData("Release")] + public void NativeRef_EmitsWarningBecauseItRequiresWorkload(string config) + { + CommandResult res = PublishForRequiresWorkloadTest(config, extraItems: ""); + res.EnsureSuccessful(); + + Assert.Contains("but the native references won't be linked in", res.Output); + } + + [ConditionalTheory(typeof(BuildTestBase), nameof(IsNotUsingWorkloads))] + [InlineData("Debug")] + [InlineData("Release")] + public void AOT_FailsBecauseItRequiresWorkload(string config) + { + CommandResult res = PublishForRequiresWorkloadTest(config, extraProperties: "true"); + Assert.NotEqual(0, res.ExitCode); + Assert.Contains("following workloads must be installed: wasm-tools", res.Output); + } + + [ConditionalTheory(typeof(BuildTestBase), nameof(IsNotUsingWorkloads))] + [InlineData("Debug")] + [InlineData("Release")] + public void AOT_And_NativeRef_FailsBecauseItRequireWorkload(string config) + { + CommandResult res = PublishForRequiresWorkloadTest(config, + extraProperties: "true", + extraItems: ""); + + Assert.NotEqual(0, res.ExitCode); + Assert.Contains("following workloads must be installed: wasm-tools", res.Output); + } + + private CommandResult PublishForRequiresWorkloadTest(string config, string extraItems="", string extraProperties="") + { + string id = $"needs_workload_{config}_{Path.GetRandomFileName()}"; + InitBlazorWasmProjectDir(id); + + new DotNetCommand(s_buildEnv, useDefaultArgs: false) + .WithWorkingDirectory(_projectDir!) + .ExecuteWithCapturedOutput("new blazorwasm") + .EnsureSuccessful(); + + if (IsNotUsingWorkloads) + { + // no packs installed, so no need to update the paths for runtime pack etc + File.WriteAllText(Path.Combine(_projectDir!, "Directory.Build.props"), ""); + File.WriteAllText(Path.Combine(_projectDir!, "Directory.Build.targets"), ""); + } + + AddItemsPropertiesToProject(Path.Combine(_projectDir!, $"{id}.csproj"), + extraProperties: extraProperties, + extraItems: extraItems); + + string publishLogPath = Path.Combine(s_buildEnv.LogRootPath, id, $"{id}.binlog"); + return new DotNetCommand(s_buildEnv) + .WithWorkingDirectory(_projectDir!) + .ExecuteWithCapturedOutput("publish", + $"-bl:{publishLogPath}", + $"-p:Configuration={config}", + "-p:MSBuildEnableWorkloadResolver=true"); // WasmApp.LocalBuild.* disables this, but it is needed for this test + } + + [Theory] + [InlineData("Debug")] + [InlineData("Release")] + public void Net50Projects_NativeReference(string config) + => BuildNet50Project(config, aot: false, expectError: true, @""); + public static TheoryData Net50TestData = new() { { "Debug", /*aot*/ true, /*expectError*/ true }, @@ -65,24 +125,15 @@ public void PublishTemplateProject(string config, bool aot) { "Release", /*aot*/ false, /*expectError*/ false } }; - [ConditionalTheory(typeof(BuildTestBase), nameof(IsNotUsingWorkloads))] + [Theory] [MemberData(nameof(Net50TestData))] - public void Net50ProjectsWithNoPacksInstalled(string config, bool aot, bool expectError) - => BuildNet50Project(config, aot, expectError); + public void Net50Projects_AOT(string config, bool aot, bool expectError) + => BuildNet50Project(config, aot: aot, expectError: expectError); - [ConditionalTheory(typeof(BuildTestBase), nameof(IsUsingWorkloads))] - [MemberData(nameof(Net50TestData))] - public void Net50ProjectsWithPacksInstalled(string config, bool aot, bool expectError) - => BuildNet50Project(config, aot, expectError); - - private void BuildNet50Project(string config, bool aot, bool errorExpected) + private void BuildNet50Project(string config, bool aot, bool expectError, string? extraItems=null) { - string id = $"Blazor_net50_{config}_{aot}"; - InitPaths(id); - if (Directory.Exists(_projectDir)) - Directory.Delete(_projectDir, recursive: true); - Directory.CreateDirectory(_projectDir); - Directory.CreateDirectory(Path.Combine(_projectDir, ".nuget")); + string id = $"Blazor_net50_{config}_{aot}_{Path.GetRandomFileName()}"; + InitBlazorWasmProjectDir(id); string directoryBuildTargets = @" @@ -90,26 +141,27 @@ private void BuildNet50Project(string config, bool aot, bool errorExpected) "; - File.Copy(Path.Combine(BuildEnvironment.TestDataPath, "nuget6.config"), Path.Combine(_projectDir, "nuget.config")); - File.WriteAllText(Path.Combine(_projectDir, "Directory.Build.props"), ""); - File.WriteAllText(Path.Combine(_projectDir, "Directory.Build.targets"), directoryBuildTargets); + File.WriteAllText(Path.Combine(_projectDir!, "Directory.Build.props"), ""); + File.WriteAllText(Path.Combine(_projectDir!, "Directory.Build.targets"), directoryBuildTargets); string logPath = Path.Combine(s_buildEnv.LogRootPath, id); Utils.DirectoryCopy(Path.Combine(BuildEnvironment.TestAssetsPath, "Blazor_net50"), Path.Combine(_projectDir!)); + string projectFile = Path.Combine(_projectDir!, "Blazor_net50.csproj"); + AddItemsPropertiesToProject(projectFile, extraItems: extraItems); + string publishLogPath = Path.Combine(logPath, $"{id}.binlog"); CommandResult result = new DotNetCommand(s_buildEnv) - .WithWorkingDirectory(_projectDir) - .ExecuteWithCapturedOutput("publish", - $"-bl:{publishLogPath}", - (aot ? "-p:RunAOTCompilation=true" : ""), - $"-p:Configuration={config}"); + .WithWorkingDirectory(_projectDir!) + .ExecuteWithCapturedOutput("publish", + $"-bl:{publishLogPath}", + (aot ? "-p:RunAOTCompilation=true" : ""), + $"-p:Configuration={config}"); - if (errorExpected) + if (expectError) { result.EnsureExitCode(1); - Assert.Contains("** UsingBrowserRuntimeWorkload: 'false'", result.Output); - Assert.Contains("error : WebAssembly workloads (required for AOT) are only supported for projects targeting net6.0+", result.Output); + Assert.Contains("are only supported for projects targeting net6.0+", result.Output); } else { diff --git a/src/tests/BuildWasmApps/Wasm.Build.Tests/BuildTestBase.cs b/src/tests/BuildWasmApps/Wasm.Build.Tests/BuildTestBase.cs index 3cd21151a07859..50425f9844cb8c 100644 --- a/src/tests/BuildWasmApps/Wasm.Build.Tests/BuildTestBase.cs +++ b/src/tests/BuildWasmApps/Wasm.Build.Tests/BuildTestBase.cs @@ -11,6 +11,7 @@ using System.Runtime.InteropServices; using System.Text; using System.Text.RegularExpressions; +using System.Xml; using Xunit; using Xunit.Abstractions; using Xunit.Sdk; @@ -357,6 +358,19 @@ protected static BuildArgs ExpandBuildArgs(BuildArgs buildArgs, string extraProp } } + public void InitBlazorWasmProjectDir(string id) + { + InitPaths(id); + if (Directory.Exists(_projectDir)) + Directory.Delete(_projectDir, recursive: true); + Directory.CreateDirectory(_projectDir); + Directory.CreateDirectory(Path.Combine(_projectDir, ".nuget")); + + File.Copy(Path.Combine(BuildEnvironment.TestDataPath, "nuget6.config"), Path.Combine(_projectDir, "nuget.config")); + File.Copy(Path.Combine(BuildEnvironment.TestDataPath, "Blazor.Directory.Build.props"), Path.Combine(_projectDir, "Directory.Build.props")); + File.Copy(Path.Combine(BuildEnvironment.TestDataPath, "Blazor.Directory.Build.targets"), Path.Combine(_projectDir, "Directory.Build.targets")); + } + static void AssertRuntimePackPath(string buildOutput) { var match = s_runtimePackPathRegex.Match(buildOutput); @@ -601,6 +615,33 @@ void LogData(string label, string? message) } } + public static string AddItemsPropertiesToProject(string projectFile, string? extraProperties=null, string? extraItems=null) + { + if (extraProperties == null && extraItems == null) + return projectFile; + + XmlDocument doc = new(); + doc.Load(projectFile); + + if (extraItems != null) + { + XmlNode node = doc.CreateNode(XmlNodeType.Element, "ItemGroup", null); + node.InnerXml = extraItems; + doc.DocumentElement!.AppendChild(node); + } + + if (extraProperties != null) + { + XmlNode node = doc.CreateNode(XmlNodeType.Element, "PropertyGroup", null); + node.InnerXml = extraProperties; + doc.DocumentElement!.AppendChild(node); + } + + doc.Save(projectFile); + + return projectFile; + } + public void Dispose() { if (_projectDir != null && _enablePerTestCleanup) diff --git a/src/tests/BuildWasmApps/Wasm.Build.Tests/NativeLibraryTests.cs b/src/tests/BuildWasmApps/Wasm.Build.Tests/NativeLibraryTests.cs index e06c099b19c4bf..eb306286ba5e0d 100644 --- a/src/tests/BuildWasmApps/Wasm.Build.Tests/NativeLibraryTests.cs +++ b/src/tests/BuildWasmApps/Wasm.Build.Tests/NativeLibraryTests.cs @@ -17,7 +17,7 @@ public NativeLibraryTests(ITestOutputHelper output, SharedBuildPerTestClassFixtu { } - [Theory] + [ConditionalTheory(typeof(BuildTestBase), nameof(IsUsingWorkloads))] [BuildAndRun(aot: false)] [BuildAndRun(aot: true)] public void ProjectWithNativeReference(BuildArgs buildArgs, RunHost host, string id) @@ -48,7 +48,7 @@ public void ProjectWithNativeReference(BuildArgs buildArgs, RunHost host, string Assert.Contains("from pinvoke: 142", output); } - [Theory] + [ConditionalTheory(typeof(BuildTestBase), nameof(IsUsingWorkloads))] [BuildAndRun(aot: false)] [BuildAndRun(aot: true)] public void ProjectUsingSkiaSharp(BuildArgs buildArgs, RunHost host, string id) From 9abb01f1a387ea47648256a3811953de0d78a960 Mon Sep 17 00:00:00 2001 From: Ankit Jain Date: Thu, 26 Aug 2021 06:03:55 -0400 Subject: [PATCH 2/3] Bump sdk for workload testing to 6.0.100-rc.2.21425.12 --- eng/Versions.props | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/eng/Versions.props b/eng/Versions.props index b7e3c660f4cdd3..f05d832e6bccdf 100644 --- a/eng/Versions.props +++ b/eng/Versions.props @@ -164,7 +164,7 @@ 2.0.4 4.12.0 2.14.3 - 6.0.100-rc.1.21412.8 + 6.0.100-rc.2.21425.12 5.0.0-preview-20201009.2 From 0f5cc66a22443daaf93f00ff1661e4db98bd13fe Mon Sep 17 00:00:00 2001 From: Ankit Jain Date: Thu, 26 Aug 2021 06:13:03 -0400 Subject: [PATCH 3/3] Fix path to workload.stamp file --- src/libraries/Directory.Build.props | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/Directory.Build.props b/src/libraries/Directory.Build.props index e81806e83421f2..0be101164991b0 100644 --- a/src/libraries/Directory.Build.props +++ b/src/libraries/Directory.Build.props @@ -138,7 +138,7 @@ $([MSBuild]::NormalizeDirectory($(SdkWithNoWorkloadForTestingPath))) $(SdkWithNoWorkloadForTestingPath)version-$(SdkVersionForWorkloadTesting).stamp - $(SdkWithWorkloadForTestingPath)workload.stamp + $(SdkWithNoWorkloadForTestingPath)workload.stamp $(ArtifactsBinDir)dotnet-workload\ $([MSBuild]::NormalizeDirectory($(SdkWithWorkloadForTestingPath)))