[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 <radical@gmail.com>

diff --git a/eng/Versions.props b/eng/Versions.props
index 979f4d0..2a24f1d 100644 (file)
--- a/eng/Versions.props
+++ b/eng/Versions.props
@@ -163,7 +163,7 @@
     <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 -->

diff --git a/src/libraries/Directory.Build.props b/src/libraries/Directory.Build.props
index e81806e..0be1011 100644 (file)
--- a/src/libraries/Directory.Build.props
+++ b/src/libraries/Directory.Build.props
@@ -138,7 +138,7 @@
     <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>

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 (file)
--- 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</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>
 
@@ -128,7 +124,21 @@
                         />
     </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>

diff --git a/src/tests/BuildWasmApps/Wasm.Build.Tests/BlazorWasmTests.cs b/src/tests/BuildWasmApps/Wasm.Build.Tests/BlazorWasmTests.cs
index 2059142..9b77090 100644 (file)
--- 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: "<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
             {

diff --git a/src/tests/BuildWasmApps/Wasm.Build.Tests/BuildTestBase.cs b/src/tests/BuildWasmApps/Wasm.Build.Tests/BuildTestBase.cs
index 3cd2115..50425f9 100644 (file)
--- 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 (file)
--- 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)