Skip to content

Commit

Permalink
[wasm] Fix up conditions to trigger relink, and require wasm-tools
Browse files Browse the repository at this point in the history
…workload (#89754)

- Trigger relinking (`WasmBuildNative=true`) if:
   - `WasmNativeStrip=false`
   - `WasmEnableSIMD=false`
   - `WasmEnableExceptionHandling=false`
   - The above are in addition to the existing conditions

- Also, trigger "workload required" when:
   - `WasmNativeStrip=false`
   - `WasmEnableExceptionHandling=true`
   - `InvariantGlobalization=true`
   - `InvariantTimeZone=true`
   - The above are in addition to the existing conditions

- Rationalize `WasmNativeDebugSymbols`, and `WasmNativeStrip`
	- `WasmNativeDebugSymbols` will cause symbols to be included
	  (essentially `-g`)
	- `WasmNativeStrip` will cause these to be stripped with `wasm-opt
	  --strip-dwarf ...`

Fixes #85778 .
  • Loading branch information
radical authored Aug 11, 2023
1 parent 678fd6a commit 26ae097
Show file tree
Hide file tree
Showing 11 changed files with 315 additions and 130 deletions.
1 change: 1 addition & 0 deletions eng/testing/scenarios/BuildWasmAppsJobsList.txt
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ Wasm.Build.Tests.Blazor.MiscTests2
Wasm.Build.Tests.Blazor.MiscTests3
Wasm.Build.Tests.Blazor.NativeTests
Wasm.Build.Tests.Blazor.NoopNativeRebuildTest
Wasm.Build.Tests.Blazor.WorkloadRequiredTests
Wasm.Build.Tests.Blazor.IcuTests
Wasm.Build.Tests.BuildPublishTests
Wasm.Build.Tests.ConfigSrcTests
Expand Down
2 changes: 1 addition & 1 deletion eng/testing/tests.browser.targets
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
- For regular library tests, it will use the symbols file from the runtime pack.
- for AOT library tests, we use WasmNativeStrip=false, so we already have symbols
-->
<WasmNativeStrip Condition="'$(WasmNativeStrip)' == ''">false</WasmNativeStrip>
<WasmNativeStrip Condition="'$(WasmNativeStrip)' == '' and '$(RunAOTCompilation)' == 'true'">false</WasmNativeStrip>
<WasmEmitSymbolMap Condition="'$(WasmEmitSymbolMap)' == ''">true</WasmEmitSymbolMap>

<_WasmMainJSFileName Condition="'$(WasmMainJSPath)' != ''">$([System.IO.Path]::GetFileName('$(WasmMainJSPath)'))</_WasmMainJSFileName>
Expand Down
8 changes: 0 additions & 8 deletions eng/testing/workloads-testing.targets
Original file line number Diff line number Diff line change
Expand Up @@ -77,14 +77,6 @@

<Exec Condition="$([MSBuild]::IsOSPlatform('windows'))"
Command='powershell -ExecutionPolicy ByPass -NoProfile -command "&amp; $(_DotNetInstallCommand)"' />

<!-- HACK: Remove the now invalid manifest `microsoft.net.workload.mono.toolchain` as a workaround
till the sdk removes it completely. -->
<ItemGroup>
<_ManifestsToRemove Include="$(_SdkWithNoWorkloadPath)\sdk-manifests\8.0.100\microsoft.net.workload.mono.toolchain" />
</ItemGroup>
<Message Text="Removing @(_ManifestsToRemove)" Condition="Exists(%(_ManifestsToRemove.Identity))" Importance="High" />
<RemoveDir Directories="@(_ManifestsToRemove)" Condition="Exists(%(_ManifestsToRemove.Identity))" />
</Target>

<Target Name="_GetDotNetVersion">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,23 @@
</PropertyGroup>

<PropertyGroup Condition="'$(RuntimeIdentifier)' == 'browser-wasm' and '$(TargetFrameworkIdentifier)' == '.NETCoreApp'">
<!-- Keep in sync with WasmApp.Native.targets -->
<_WasmPropertiesDifferFromRuntimePackThusNativeBuildNeeded Condition="
'$(WasmEnableLegacyJsInterop)' == 'false' or
'$(WasmEnableSIMD)' == 'false' or
'$(WasmEnableExceptionHandling)' == 'false' or
'$(InvariantTimezone)' == 'true' or
'$(InvariantGlobalization)' == 'true' or
'$(WasmNativeStrip)' == 'false'">true</_WasmPropertiesDifferFromRuntimePackThusNativeBuildNeeded>

<!-- $(WasmBuildNative)==true is needed to enable workloads, when using native references, without AOT -->
<!-- FIXME: is the blazor condition here correct? -->
<_WasmNativeWorkloadNeeded Condition="'$(RunAOTCompilation)' == 'true' or '$(WasmEnableSIMD)' == 'true' or '$(WasmEnableLegacyJsInterop)' == 'false' or '$(WasmBuildNative)' == 'true' or
'$(WasmGenerateAppBundle)' == 'true' or '$(_UsingBlazorOrWasmSdk)' != 'true'" >true</_WasmNativeWorkloadNeeded>
<_WasmNativeWorkloadNeeded Condition="
'$(_WasmPropertiesDifferFromRuntimePackThusNativeBuildNeeded)' == 'true' or
'$(RunAOTCompilation)' == 'true' or
'$(WasmBuildNative)' == 'true' or
'$(WasmGenerateAppBundle)' == 'true' or
'$(_UsingBlazorOrWasmSdk)' != 'true'" >true</_WasmNativeWorkloadNeeded>

<UsingBrowserRuntimeWorkload Condition="'$(_BrowserWorkloadNotSupportedForTFM)' == 'true'">false</UsingBrowserRuntimeWorkload>
<UsingBrowserRuntimeWorkload Condition="'$(UsingBrowserRuntimeWorkload)' == '' and '$(_WasmNativeWorkloadNeeded)' == 'true'">true</UsingBrowserRuntimeWorkload>
Expand Down Expand Up @@ -185,8 +198,8 @@
Text="WebAssembly workloads, required for native relinking, are only supported for projects targeting net6.0+ . Set %24(WasmBuildNative)=false to disable it." />
</Target>

<Target Name="_ErrorDualWasmThreadProps"
Condition="('$(TargetsCurrent)' == 'true' or '$(TargetsNet7)' == 'true') and '$(RuntimeIdentifier)' == 'browser-wasm' and
<Target Name="_ErrorDualWasmThreadPropsOn7"
Condition="'$(TargetsNet7)' == 'true' and '$(RuntimeIdentifier)' == 'browser-wasm' and
'$(BrowserWorkloadDisabled)' != 'true' and '$(WasmEnableThreads)' == 'true' and '$(WasmEnablePerfTrace)' == 'true'"
BeforeTargets="Build">
<Error Text="WebAssembly workloads can only support one active threading mode at a time. Either set WasmEnableThreads or WasmEnablePerfTracing to true, but not both." />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ public record BlazorBuildOptions
string TargetFramework = BuildTestBase.DefaultTargetFrameworkForBlazor,
bool IsPublish = false,
bool WarnAsError = true,
bool ExpectSuccess = true,
bool ExpectRelinkDirWhenPublishing = false,
bool ExpectFingerprintOnDotnetJs = false,
RuntimeVariant RuntimeType = RuntimeVariant.SingleThreaded,
Expand Down
42 changes: 24 additions & 18 deletions src/mono/wasm/Wasm.Build.Tests/Blazor/BlazorWasmTestBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,10 @@ public string CreateBlazorWasmTemplateProject(string id)
if (options.WarnAsError)
extraArgs = extraArgs.Append("/warnaserror").ToArray();

(CommandResult res, string logPath) = BlazorBuildInternal(options.Id, options.Config, publish: false, setWasmDevel: false, extraArgs);
(CommandResult res, string logPath) = BlazorBuildInternal(options.Id, options.Config, publish: false, setWasmDevel: false, expectSuccess: options.ExpectSuccess, extraArgs);

AssertBundle(res.Output, options with { IsPublish = false });
if (options.ExpectSuccess)
AssertBundle(res.Output, options with { IsPublish = false });

return (res, logPath);
}
Expand All @@ -70,25 +71,29 @@ public string CreateBlazorWasmTemplateProject(string id)
if (options.WarnAsError)
extraArgs = extraArgs.Append("/warnaserror").ToArray();

(CommandResult res, string logPath) = BlazorBuildInternal(options.Id, options.Config, publish: true, setWasmDevel: false, extraArgs);
AssertBundle(res.Output, options with { IsPublish = true });
(CommandResult res, string logPath) = BlazorBuildInternal(options.Id, options.Config, publish: true, setWasmDevel: false, expectSuccess: options.ExpectSuccess, extraArgs);

if (options.ExpectedFileType == NativeFilesType.AOT)
if (options.ExpectSuccess)
{
// check for this too, so we know the format is correct for the negative
// test for jsinterop.webassembly.dll
Assert.Contains("Microsoft.JSInterop.dll -> Microsoft.JSInterop.dll.bc", res.Output);
AssertBundle(res.Output, options with { IsPublish = true });

// make sure this assembly gets skipped
Assert.DoesNotContain("Microsoft.JSInterop.WebAssembly.dll -> Microsoft.JSInterop.WebAssembly.dll.bc", res.Output);
}
if (options.ExpectedFileType == NativeFilesType.AOT)
{
// check for this too, so we know the format is correct for the negative
// test for jsinterop.webassembly.dll
Assert.Contains("Microsoft.JSInterop.dll -> Microsoft.JSInterop.dll.bc", res.Output);

string objBuildDir = Path.Combine(_projectDir!, "obj", options.Config, options.TargetFramework!, "wasm", "for-build");
// Check that we linked only for publish
if (options.ExpectRelinkDirWhenPublishing)
Assert.True(Directory.Exists(objBuildDir), $"Could not find expected {objBuildDir}, which gets created when relinking during Build. This is likely a test authoring error");
else
Assert.False(File.Exists(Path.Combine(objBuildDir, "emcc-link.rsp")), $"Found unexpected files in {objBuildDir}, which gets created when relinking during Build");
// make sure this assembly gets skipped
Assert.DoesNotContain("Microsoft.JSInterop.WebAssembly.dll -> Microsoft.JSInterop.WebAssembly.dll.bc", res.Output);
}

string objBuildDir = Path.Combine(_projectDir!, "obj", options.Config, options.TargetFramework!, "wasm", "for-build");
// Check that we linked only for publish
if (options.ExpectRelinkDirWhenPublishing)
Assert.True(Directory.Exists(objBuildDir), $"Could not find expected {objBuildDir}, which gets created when relinking during Build. This is likely a test authoring error");
else
Assert.False(File.Exists(Path.Combine(objBuildDir, "emcc-link.rsp")), $"Found unexpected files in {objBuildDir}, which gets created when relinking during Build");
}

return (res, logPath);
}
Expand All @@ -98,14 +103,15 @@ public string CreateBlazorWasmTemplateProject(string id)
string config,
bool publish = false,
bool setWasmDevel = true,
bool expectSuccess = true,
params string[] extraArgs)
{
try
{
return BuildProjectWithoutAssert(
id,
config,
new BuildProjectOptions(CreateProject: false, UseCache: false, Publish: publish),
new BuildProjectOptions(CreateProject: false, UseCache: false, Publish: publish, ExpectSuccess: expectSuccess),
extraArgs.Concat(new[]
{
"-p:BlazorEnableCompression=false",
Expand Down
89 changes: 89 additions & 0 deletions src/mono/wasm/Wasm.Build.Tests/Blazor/WorkloadRequiredTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Collections.Generic;
using System.IO;
using Xunit;
using Xunit.Abstractions;

#nullable enable

namespace Wasm.Build.Tests.Blazor;

public class WorkloadRequiredTests : BlazorWasmTestBase
{
/* Keep in sync with settings in wasm.proj, and WasmApp.Native.targets .
* The `triggerValue` here is opposite of the default used when building the runtime pack
* (see wasm.proj), and thus requiring a native build
*/
public static (string propertyName, bool triggerValue)[] PropertiesWithTriggerValues = new[]
{
("RunAOTCompilation", true),
("WasmEnableLegacyJsInterop", false),
("WasmEnableSIMD", false),
("WasmEnableExceptionHandling", false),
("InvariantTimezone", true),
("InvariantGlobalization", true),
("WasmNativeStrip", false)
};

public WorkloadRequiredTests(ITestOutputHelper output, SharedBuildPerTestClassFixture buildContext)
: base(output, buildContext)
{
}

public static TheoryData<string, string, bool> SettingDifferentFromValuesInRuntimePack(bool forPublish)
{
TheoryData<string, string, bool> data = new();

string[] configs = new[] { "Debug", "Release" };
foreach (var defaultPair in PropertiesWithTriggerValues)
{
foreach (string config in configs)
{
data.Add(config, $"<{defaultPair.propertyName}>{defaultPair.triggerValue}</{defaultPair.propertyName}>", true);
data.Add(config, $"<{defaultPair.propertyName}>{!defaultPair.triggerValue}</{defaultPair.propertyName}>", false);
}
}

return data;
}

[Theory, TestCategory("no-workload")]
[MemberData(nameof(SettingDifferentFromValuesInRuntimePack), parameters: false)]
public void WorkloadRequiredForBuild(string config, string extraProperties, bool workloadNeeded)
=> CheckWorkloadRequired(config, extraProperties, workloadNeeded, publish: false);

[Theory, TestCategory("no-workload")]
[MemberData(nameof(SettingDifferentFromValuesInRuntimePack), parameters: false)]
public void WorkloadRequiredForPublish(string config, string extraProperties, bool workloadNeeded)
=> CheckWorkloadRequired(config, extraProperties, workloadNeeded, publish: true);

private void CheckWorkloadRequired(string config, string extraProperties, bool workloadNeeded, bool publish)
{
string id = $"props_req_workload_{(publish ? "publish" : "build")}_{GetRandomId()}";
string projectFile = CreateWasmTemplateProject(id, "blazorwasm");
AddItemsPropertiesToProject(projectFile, extraProperties,
atTheEnd: @"<Target Name=""StopBuildBeforeCompile"" BeforeTargets=""Compile"">
<Error Text=""Stopping the build"" />
</Target>");

CommandResult result;
if (publish)
(result, _) = BlazorPublish(new BlazorBuildOptions(id, config, ExpectSuccess: false));
else
(result, _) = BlazorBuild(new BlazorBuildOptions(id, config, ExpectSuccess: false));

if (workloadNeeded)
{
Assert.Contains("following workloads must be installed: wasm-tools", result.Output);
Assert.DoesNotContain("error : Stopping the build", result.Output);
}
else
{
Assert.DoesNotContain("following workloads must be installed: wasm-tools", result.Output);
Assert.Contains("error : Stopping the build", result.Output);
}
}
}
3 changes: 0 additions & 3 deletions src/mono/wasm/Wasm.Build.Tests/Common/BuildEnvironment.cs
Original file line number Diff line number Diff line change
Expand Up @@ -128,9 +128,6 @@ public BuildEnvironment()
EnvVars["WasmEnableWebCil"] = "false";
}

// helps with debugging
EnvVars["WasmNativeStrip"] = "false";

DotNet = Path.Combine(sdkForWorkloadPath!, "dotnet");
if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
DotNet += ".exe";
Expand Down
Loading

0 comments on commit 26ae097

Please sign in to comment.