From: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Date: Tue, 31 Aug 2021 16:17:55 +0000 (+0200) Subject: [release/6.0] [wasm] Require workloads if using `@(NativeFileReference)` (#58290) X-Git-Tag: accepted/tizen/unified/20220110.054933~238 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=e8aab6ad482a814652bb240493fdb60d69f108f5;p=platform%2Fupstream%2Fdotnet%2Fruntime.git [release/6.0] [wasm] Require workloads if using `@(NativeFileReference)` (#58290) * [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`. * Bump sdk for workload testing to 6.0.100-rc.2.21425.12 * Fix path to workload.stamp file Co-authored-by: Ankit Jain --- diff --git a/eng/Versions.props b/eng/Versions.props index 979f4d0..2a24f1d 100644 --- a/eng/Versions.props +++ b/eng/Versions.props @@ -163,7 +163,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 diff --git a/src/libraries/Directory.Build.props b/src/libraries/Directory.Build.props index e81806e..0be1011 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))) 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 d9a9dc2..9b13301 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 2059142..9b77090 100644 --- a/src/tests/BuildWasmApps/Wasm.Build.Tests/BlazorWasmTests.cs +++ b/src/tests/BuildWasmApps/Wasm.Build.Tests/BlazorWasmTests.cs @@ -26,27 +26,17 @@ namespace Wasm.Build.Tests [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 @@ namespace Wasm.Build.Tests // 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 @@ namespace Wasm.Build.Tests { "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 @@ namespace Wasm.Build.Tests "; - 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 3cd2115..50425f9 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.Linq; 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 @@ namespace Wasm.Build.Tests } } + 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 @@ namespace Wasm.Build.Tests } } + 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 e06c099..eb30628 100644 --- a/src/tests/BuildWasmApps/Wasm.Build.Tests/NativeLibraryTests.cs +++ b/src/tests/BuildWasmApps/Wasm.Build.Tests/NativeLibraryTests.cs @@ -17,7 +17,7 @@ namespace Wasm.Build.Tests { } - [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 @@ namespace Wasm.Build.Tests 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)