[release/6.0] [wasm] Require workloads if using `@(NativeFileReference)` (#58290)
authorgithub-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Tue, 31 Aug 2021 16:17:55 +0000 (18:17 +0200)
committerGitHub <noreply@github.com>
Tue, 31 Aug 2021 16:17:55 +0000 (18:17 +0200)
* [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 <radical@gmail.com>
eng/Versions.props
src/libraries/Directory.Build.props
src/mono/nuget/Microsoft.NET.Workload.Mono.Toolchain.Manifest/WorkloadManifest.targets.in
src/tests/BuildWasmApps/Wasm.Build.Tests/BlazorWasmTests.cs
src/tests/BuildWasmApps/Wasm.Build.Tests/BuildTestBase.cs
src/tests/BuildWasmApps/Wasm.Build.Tests/NativeLibraryTests.cs

index 979f4d0137001a23de9ca42ac83d637d7e77dc1c..2a24f1d3dc6abb33beaafa58db84243e053f6fc3 100644 (file)
     <SQLitePCLRawbundle_greenVersion>2.0.4</SQLitePCLRawbundle_greenVersion>
     <MoqVersion>4.12.0</MoqVersion>
     <FsCheckVersion>2.14.3</FsCheckVersion>
-    <SdkVersionForWorkloadTesting>6.0.100-rc.1.21412.8</SdkVersionForWorkloadTesting>
+    <SdkVersionForWorkloadTesting>6.0.100-rc.2.21425.12</SdkVersionForWorkloadTesting>
     <!-- Docs -->
     <MicrosoftPrivateIntellisenseVersion>5.0.0-preview-20201009.2</MicrosoftPrivateIntellisenseVersion>
     <!-- ILLink -->
index e81806e83421f2f58e4b8a575e1bd9dd9651214e..0be101164991b03d4fed8b8a549d20e59dbb2cd7 100644 (file)
     <SdkWithNoWorkloadForTestingPath>$([MSBuild]::NormalizeDirectory($(SdkWithNoWorkloadForTestingPath)))</SdkWithNoWorkloadForTestingPath>
 
     <SdkWithNoWorkloadStampPath>$(SdkWithNoWorkloadForTestingPath)version-$(SdkVersionForWorkloadTesting).stamp</SdkWithNoWorkloadStampPath>
-    <SdkWithNoWorkload_WorkloadStampPath>$(SdkWithWorkloadForTestingPath)workload.stamp</SdkWithNoWorkload_WorkloadStampPath>
+    <SdkWithNoWorkload_WorkloadStampPath>$(SdkWithNoWorkloadForTestingPath)workload.stamp</SdkWithNoWorkload_WorkloadStampPath>
 
     <SdkWithWorkloadForTestingPath>$(ArtifactsBinDir)dotnet-workload\</SdkWithWorkloadForTestingPath>
     <SdkWithWorkloadForTestingPath>$([MSBuild]::NormalizeDirectory($(SdkWithWorkloadForTestingPath)))</SdkWithWorkloadForTestingPath>
index d9a9dc2a72d463f526d7479bd1f3adb5ef80f12a..9b13301584dc5d82b1121974e1c24b52e2a13e40 100644 (file)
@@ -7,13 +7,9 @@
                                             !$([MSBuild]::VersionGreaterThanOrEquals('$(TargetFrameworkVersion)', '6.0'))">true</BrowserWorkloadDisabled>
     </PropertyGroup>
 
-    <PropertyGroup Condition="'$(RuntimeIdentifier)' == 'browser-wasm' and '$(BrowserWorkloadDisabled)' == 'true'">
-        <_NativeBuildNeeded Condition="'$(RunAOTCompilation)' == 'true'">true</_NativeBuildNeeded>
-        <WorkloadDisabledWithReason Condition="'$(_NativeBuildNeeded)' == 'true'">WebAssembly workloads (required for AOT) are only supported for projects targeting net6.0+</WorkloadDisabledWithReason>
-    </PropertyGroup>
-
     <PropertyGroup Condition="'$(RuntimeIdentifier)' == 'browser-wasm' AND '$(UsingBrowserRuntimeWorkload)' == ''">
-        <UsingBrowserRuntimeWorkload Condition="'$(RunAOTCompilation)' == 'true' or '$(UsingMicrosoftNETSdkBlazorWebAssembly)' != 'true'" >true</UsingBrowserRuntimeWorkload>
+        <!-- $(WasmBuildNative)==true is needed to enable workloads, when using native references, without AOT -->
+        <UsingBrowserRuntimeWorkload Condition="'$(RunAOTCompilation)' == 'true' or '$(WasmBuildNative)' == 'true' or '$(UsingMicrosoftNETSdkBlazorWebAssembly)' != 'true'" >true</UsingBrowserRuntimeWorkload>
         <UsingBrowserRuntimeWorkload Condition="'$(UsingBrowserRuntimeWorkload)' == ''" >$(WasmNativeWorkload)</UsingBrowserRuntimeWorkload>
     </PropertyGroup>
 
                         />
     </ItemGroup>
 
-    <Target Name="ErrorDisabledWorkload" Condition="'$(WorkloadDisabledWithReason)' != ''" BeforeTargets="Publish">
-      <Error Text="$(WorkloadDisabledWithReason)" />
+    <!-- we can't condition sdk imports on the item @(NativeFileReference). Instead, explicitly check before the build
+         and emit a warning -->
+    <Target Name="_CheckBrowserWorkloadNeededButNotAvailable"
+            Condition="'$(RuntimeIdentifier)' == 'browser-wasm' and '$(BrowserWorkloadDisabled)' != 'true' and '$(WasmNativeWorkload)' != 'true'"
+            BeforeTargets="Build">
+
+      <Warning Condition="@(NativeFileReference->Count()) > 0"
+               Text="%40(NativeFileReference) is not empty, but the native references won't be linked in, because neither %24(WasmBuildNative), nor %24(RunAOTCompilation) are 'true'. NativeFileReference=@(NativeFileReference)" />
+    </Target>
+
+    <Target Name="_ErrorDisabledWorkload" Condition="'$(BrowserWorkloadDisabled)' == 'true'" BeforeTargets="Build">
+      <Error Condition="'$(RunAOTCompilation)' == 'true'"
+             Text="WebAssembly workloads, required for AOT, are only supported for projects targeting net6.0+ . Set %24(RunAOTCompilation)=false to disable it." />
+
+      <Error Condition="@(NativeFileReference->Count()) > 0"
+             Text="WebAssembly workloads, required for linking native files (from %40(NativeFileReference)), are only supported for projects targeting net6.0+ ." />
     </Target>
 </Project>
index 2059142de09715e9e032efb3ecb4d6352aec1dbc..9b7709098ed92905ff43fbcfa9ee962d87fe3d02 100644 (file)
@@ -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: "<NativeFileReference Include=\"native-lib.o\" />");
+            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: "<RunAOTCompilation>true</RunAOTCompilation>");
+            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: "<RunAOTCompilation>true</RunAOTCompilation>",
+                                    extraItems: "<NativeFileReference Include=\"native-lib.o\" />");
+
+            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"), "<Project />");
+                File.WriteAllText(Path.Combine(_projectDir!, "Directory.Build.targets"), "<Project />");
+            }
+
+            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, @"<NativeFileReference Include=""native-lib.o"" />");
+
         public static TheoryData<string, bool, bool> 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 = @"<Project>
                 <Target Name=""PrintAllProjects"" BeforeTargets=""Build"">
@@ -90,26 +141,27 @@ namespace Wasm.Build.Tests
                 </Target>
             </Project>";
 
-            File.Copy(Path.Combine(BuildEnvironment.TestDataPath, "nuget6.config"), Path.Combine(_projectDir, "nuget.config"));
-            File.WriteAllText(Path.Combine(_projectDir, "Directory.Build.props"), "<Project />");
-            File.WriteAllText(Path.Combine(_projectDir, "Directory.Build.targets"), directoryBuildTargets);
+            File.WriteAllText(Path.Combine(_projectDir!, "Directory.Build.props"), "<Project />");
+            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
             {
index 3cd21151a07859e18ff4746025652449de2d57e6..50425f9844cb8c39ba9a4386a9965fd7ef2f4563 100644 (file)
@@ -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)
index e06c099b19c4bf9f8dc10cb07de1758fef5edd4c..eb306286ba5e0dd896c0a3a6508492e4305974a7 100644 (file)
@@ -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)