Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[wasm] Avoid unnecessary relinking when publishing a blazor project for AOT #82748

Merged
merged 11 commits into from
Mar 2, 2023
Merged

[wasm] Avoid unnecessary relinking when publishing a blazor project for AOT #82748

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
b7e9946
[wasm] Wasm.Build.Tests: Add blazor+aot run test
radical Feb 28, 2023
fcc38bb
cleanup
radical Feb 28, 2023
4be35a7
[wasm] Wasm.Build.Tests: Use $(_IsPublishing) to avoid unncessary
radical Feb 28, 2023
306eb03
fix blazor tests
radical Feb 28, 2023
8950797
Wasm.Build.Tests: Add support for running published blazor projects
radical Feb 28, 2023
e5c4c0d
Detect runtime assertions in blazor runs
radical Mar 1, 2023
5e7c968
Update test to track changes
radical Mar 1, 2023
09052b6
cleanup
radical Mar 1, 2023
17a8255
Disable BlazorPublishRunTest because of https://github.com/dotnet/run…
radical Mar 1, 2023
5ff3c1c
Update test to track changes
radical Mar 1, 2023
2ba2ac7
Update xharness dependency
radical Mar 2, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
89 changes: 74 additions & 15 deletions src/mono/wasm/Wasm.Build.Tests/Blazor/BuildPublishTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,11 @@ public void DefaultTemplate_WithoutWorkload(string config)
CreateBlazorWasmTemplateProject(id);

// Build
BuildInternal(id, config, publish: false);
BlazorBuildInternal(id, config, publish: false);
AssertBlazorBootJson(config, isPublish: false);

// Publish
BuildInternal(id, config, publish: true);
BlazorBuildInternal(id, config, publish: true);
AssertBlazorBootJson(config, isPublish: true);
}

Expand All @@ -52,11 +52,11 @@ public void DefaultTemplate_NoAOT_WithWorkload(string config)
if (config == "Release")
{
// relinking in publish for Release config
BlazorPublish(new BlazorBuildOptions(id, config, NativeFilesType.Relinked));
BlazorPublish(new BlazorBuildOptions(id, config, NativeFilesType.Relinked, ExpectRelinkDirWhenPublishing: true));
}
else
{
BlazorPublish(new BlazorBuildOptions(id, config, NativeFilesType.FromRuntimePack));
BlazorPublish(new BlazorBuildOptions(id, config, NativeFilesType.FromRuntimePack, ExpectRelinkDirWhenPublishing: true));
}
}

Expand All @@ -82,12 +82,23 @@ public void DefaultTemplate_NoAOT_WithWorkload(string config)
//}

[Theory]
[InlineData("Debug")]
[InlineData("Release")]
public async Task WithDllImportInMainAssembly(string config)
[InlineData("Debug", /*build*/true, /*publish*/false)]
[InlineData("Debug", /*build*/false, /*publish*/true)]
[InlineData("Debug", /*build*/true, /*publish*/true)]
[InlineData("Release", /*build*/true, /*publish*/false)]
[InlineData("Release", /*build*/false, /*publish*/true)]
[InlineData("Release", /*build*/true, /*publish*/true)]
public async Task WithDllImportInMainAssembly(string config, bool build, bool publish)
{
// Based on https://github.com/dotnet/runtime/issues/59255
string id = $"blz_dllimp_{config}";
string id = $"blz_dllimp_{config}_";
if (build && publish)
id += "build_then_publish";
else if (build)
id += "build";
else
id += "publish";

string projectFile = CreateProjectWithNativeReference(id);
string nativeSource = @"
#include <stdio.h>
Expand Down Expand Up @@ -118,18 +129,29 @@ public static class MyDllImports
outputText = $"{result}";
""");

BlazorBuild(new BlazorBuildOptions(id, config, NativeFilesType.Relinked));
CheckNativeFileLinked(forPublish: false);
if (build)
{
BlazorBuild(new BlazorBuildOptions(id, config, NativeFilesType.Relinked));
CheckNativeFileLinked(forPublish: false);
}

BlazorPublish(new BlazorBuildOptions(id, config, NativeFilesType.Relinked));
CheckNativeFileLinked(forPublish: true);
if (publish)
{
BlazorPublish(new BlazorBuildOptions(id, config, NativeFilesType.Relinked, ExpectRelinkDirWhenPublishing: build));
CheckNativeFileLinked(forPublish: true);
}

if (publish)
await BlazorRunForPublishWithWebServer(config, TestDllImport);
else
await BlazorRunForBuildWithDotnetRun(config, TestDllImport);

await BlazorRun(config, async (page) =>
async Task TestDllImport(IPage page)
{
await page.Locator("text=\"cpp_add\"").ClickAsync();
var txt = await page.Locator("p[role='test']").InnerHTMLAsync();
Assert.Equal("Output: 22", txt);
});
}

void CheckNativeFileLinked(bool forPublish)
{
Expand Down Expand Up @@ -181,7 +203,7 @@ public void BugRegression_60479_WithRazorClassLib()
BlazorBuild(new BlazorBuildOptions(id, config, NativeFilesType.FromRuntimePack));

// will relink
BlazorPublish(new BlazorBuildOptions(id, config, NativeFilesType.Relinked));
BlazorPublish(new BlazorBuildOptions(id, config, NativeFilesType.Relinked, ExpectRelinkDirWhenPublishing: true));

// publish/wwwroot/_framework/blazor.boot.json
string frameworkDir = FindBlazorBinFrameworkDir(config, forPublish: true);
Expand All @@ -197,4 +219,41 @@ public void BugRegression_60479_WithRazorClassLib()

Assert.Contains("RazorClassLibrary.dll", lazyVal.EnumerateObject().Select(jp => jp.Name));
}

[ConditionalTheory(typeof(BuildTestBase), nameof(IsUsingWorkloads))]
[InlineData("Debug")]
[InlineData("Release")]
public async Task BlazorBuildRunTest(string config)
{
string id = $"blazor_{config}_{Path.GetRandomFileName()}";
string projectFile = CreateWasmTemplateProject(id, "blazorwasm");

new DotNetCommand(s_buildEnv, _testOutput)
.WithWorkingDirectory(_projectDir!)
.Execute($"build -c {config} -bl:{Path.Combine(s_buildEnv.LogRootPath, $"{id}.binlog")}")
.EnsureSuccessful();

await BlazorRunForBuildWithDotnetRun(config);
}

[ConditionalTheory(typeof(BuildTestBase), nameof(IsUsingWorkloads))]
[InlineData("Debug", false)]
[InlineData("Debug", true)]
[InlineData("Release", false)]
[InlineData("Release", true)]
public async Task BlazorPublishRunTest(string config, bool aot)
{
string id = $"blazor_{config}_{Path.GetRandomFileName()}";
string projectFile = CreateWasmTemplateProject(id, "blazorwasm");
if (aot)
AddItemsPropertiesToProject(projectFile, "<RunAOTCompilation>true</RunAOTCompilation>");

new DotNetCommand(s_buildEnv, _testOutput)
.WithWorkingDirectory(_projectDir!)
.Execute($"publish -c {config} -bl:{Path.Combine(s_buildEnv.LogRootPath, $"{id}.binlog")}")
.EnsureSuccessful();

await BlazorRunForPublishWithWebServer(config);
}

}
4 changes: 2 additions & 2 deletions src/mono/wasm/Wasm.Build.Tests/Blazor/MiscTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ public void NativeBuild_WithDeployOnBuild_UsedByVS(string config, bool nativeRel
AddItemsPropertiesToProject(projectFile, extraProperties: extraProperties);

// build with -p:DeployOnBuild=true, and that will trigger a publish
(CommandResult res, _) = BuildInternal(id, config, publish: false, setWasmDevel: false, "-p:DeployOnBuild=true");
(CommandResult res, _) = BlazorBuildInternal(id, config, publish: false, setWasmDevel: false, "-p:DeployOnBuild=true");

var expectedFileType = nativeRelink ? NativeFilesType.Relinked : NativeFilesType.AOT;

Expand Down Expand Up @@ -83,7 +83,7 @@ public void DefaultTemplate_AOT_InProjectFile(string config)
BlazorBuild(new BlazorBuildOptions(id, config, NativeFilesType.FromRuntimePack));

// will aot
BlazorPublish(new BlazorBuildOptions(id, config, NativeFilesType.AOT));
BlazorPublish(new BlazorBuildOptions(id, config, NativeFilesType.AOT, ExpectRelinkDirWhenPublishing: true));

// build again
BlazorBuild(new BlazorBuildOptions(id, config, NativeFilesType.FromRuntimePack));
Expand Down
6 changes: 3 additions & 3 deletions src/mono/wasm/Wasm.Build.Tests/Blazor/NativeRefTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ public void WithNativeReference_AOTInProjectFile(string config)

BlazorBuild(new BlazorBuildOptions(id, config, NativeFilesType.Relinked));

BlazorPublish(new BlazorBuildOptions(id, config, NativeFilesType.AOT));
BlazorPublish(new BlazorBuildOptions(id, config, NativeFilesType.AOT, ExpectRelinkDirWhenPublishing: true));

// will relink
BlazorBuild(new BlazorBuildOptions(id, config, NativeFilesType.Relinked));
Expand All @@ -53,10 +53,10 @@ public void WithNativeReference_AOTOnCommandLine(string config)

BlazorBuild(new BlazorBuildOptions(id, config, NativeFilesType.Relinked));

BlazorPublish(new BlazorBuildOptions(id, config, NativeFilesType.AOT), "-p:RunAOTCompilation=true");
BlazorPublish(new BlazorBuildOptions(id, config, NativeFilesType.AOT, ExpectRelinkDirWhenPublishing: true), "-p:RunAOTCompilation=true");

// no aot!
BlazorPublish(new BlazorBuildOptions(id, config, NativeFilesType.Relinked));
BlazorPublish(new BlazorBuildOptions(id, config, NativeFilesType.Relinked, ExpectRelinkDirWhenPublishing: true));
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/mono/wasm/Wasm.Build.Tests/BrowserRunner.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ public async Task<IPage> RunAsync(ToolCommand cmd, string args, bool headless =
throw new Exception($"Process ended before the url was found");
}
if (!urlAvailable.Task.IsCompleted)
throw new Exception("Timed out waiting for the app host url");
throw new Exception("Timed out waiting for the web server url");

var url = new Uri(urlAvailable.Task.Result);
Playwright = await Microsoft.Playwright.Playwright.CreateAsync();
Expand Down
1 change: 0 additions & 1 deletion src/mono/wasm/Wasm.Build.Tests/BuildPublishTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ public void BuildThenPublishNoAOT(BuildArgs buildArgs, RunHost host, string id)
Publish: false
));


Run();

if (!_buildContext.TryGetBuildFor(buildArgs, out BuildProduct? product))
Expand Down
43 changes: 36 additions & 7 deletions src/mono/wasm/Wasm.Build.Tests/BuildTestBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -544,7 +544,7 @@ public string CreateBlazorWasmTemplateProject(string id)
if (options.WarnAsError)
extraArgs = extraArgs.Append("/warnaserror").ToArray();

var res = BuildInternal(options.Id, options.Config, publish: false, setWasmDevel: false, extraArgs);
var res = BlazorBuildInternal(options.Id, options.Config, publish: false, setWasmDevel: false, extraArgs);
_testOutput.WriteLine($"BlazorBuild, options.tfm: {options.TargetFramework}");
AssertDotNetNativeFiles(options.ExpectedFileType, options.Config, forPublish: false, targetFramework: options.TargetFramework);
AssertBlazorBundle(options.Config,
Expand All @@ -557,7 +557,7 @@ public string CreateBlazorWasmTemplateProject(string id)

protected (CommandResult, string) BlazorPublish(BlazorBuildOptions options, params string[] extraArgs)
{
var res = BuildInternal(options.Id, options.Config, publish: true, setWasmDevel: false, extraArgs);
var res = BlazorBuildInternal(options.Id, options.Config, publish: true, setWasmDevel: false, extraArgs);
AssertDotNetNativeFiles(options.ExpectedFileType, options.Config, forPublish: true, targetFramework: options.TargetFramework);
AssertBlazorBundle(options.Config,
isPublish: true,
Expand All @@ -572,11 +572,20 @@ public string CreateBlazorWasmTemplateProject(string id)

// make sure this assembly gets skipped
Assert.DoesNotContain("Microsoft.JSInterop.WebAssembly.dll -> Microsoft.JSInterop.WebAssembly.dll.bc", res.Item1.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 liokely a test authoring error");
else
Assert.False(Directory.Exists(objBuildDir), $"Found unexpected {objBuildDir}, which gets created when relinking during Build");

return res;
}

protected (CommandResult, string) BuildInternal(string id, string config, bool publish=false, bool setWasmDevel=true, params string[] extraArgs)
protected (CommandResult, string) BlazorBuildInternal(string id, string config, bool publish=false, bool setWasmDevel=true, params string[] extraArgs)
{
string label = publish ? "publish" : "build";
_testOutput.WriteLine($"{Environment.NewLine}** {label} **{Environment.NewLine}");
Expand Down Expand Up @@ -923,13 +932,26 @@ public void BlazorAddRazorButton(string buttonText, string customCode, string me
File.WriteAllText(counterRazorPath, oldContent + additionalCode);
}

public async Task BlazorRun(string config, Func<IPage, Task>? test=null, string extraArgs="--no-build")
// Keeping these methods with explicit Build/Publish in the name
// so in the test code it is evident which is being run!
public Task BlazorRunForBuildWithDotnetRun(string config, Func<IPage, Task>? test=null, string extraArgs="--no-build")
=> BlazorRunTest($"run -c {config} {extraArgs}", _projectDir!, test);

public Task BlazorRunForPublishWithWebServer(string config, Func<IPage, Task>? test=null, string extraArgs="")
=> BlazorRunTest($"{s_xharnessRunnerCommand} wasm webserver --app=. --web-server-use-default-files {extraArgs}",
Path.GetFullPath(Path.Combine(FindBlazorBinFrameworkDir(config, forPublish: true), "..")),
test);

public async Task BlazorRunTest(string runArgs,
string workingDirectory,
Func<IPage, Task>? test = null,
bool detectRuntimeFailures = true)
{
using var runCommand = new RunCommand(s_buildEnv, _testOutput)
.WithWorkingDirectory(_projectDir!);
.WithWorkingDirectory(workingDirectory);

await using var runner = new BrowserRunner(_testOutput);
var page = await runner.RunAsync(runCommand, $"run -c {config} {extraArgs}", onConsoleMessage: OnConsoleMessage);
var page = await runner.RunAsync(runCommand, runArgs, onConsoleMessage: OnConsoleMessage);

await page.Locator("text=Counter").ClickAsync();
var txt = await page.Locator("p[role='status']").InnerHTMLAsync();
Expand All @@ -947,6 +969,12 @@ void OnConsoleMessage(IConsoleMessage msg)
if (EnvironmentVariables.ShowBuildOutput)
Console.WriteLine($"[{msg.Type}] {msg.Text}");
_testOutput.WriteLine($"[{msg.Type}] {msg.Text}");

if (detectRuntimeFailures)
{
if (msg.Text.Contains("[MONO] * Assertion") || msg.Text.Contains("Error: [MONO] "))
throw new XunitException($"Detected a runtime failure at line: {msg.Text}");
}
}
}

Expand Down Expand Up @@ -1211,7 +1239,8 @@ public record BlazorBuildOptions
string Config,
NativeFilesType ExpectedFileType,
string TargetFramework = BuildTestBase.DefaultTargetFrameworkForBlazor,
bool WarnAsError = true
bool WarnAsError = true,
bool ExpectRelinkDirWhenPublishing=false
);

public enum GlobalizationMode
Expand Down
4 changes: 2 additions & 2 deletions src/mono/wasm/Wasm.Build.Tests/CleanTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,15 +73,15 @@ private void Blazor_BuildNativeNonNative_ThenCleanTest(string config, bool first
AddItemsPropertiesToProject(projectFile, extraProperties: extraProperties);

bool relink = firstBuildNative;
BuildInternal(id, config, publish: false,
BlazorBuildInternal(id, config, publish: false,
extraArgs: relink ? "-p:WasmBuildNative=true" : string.Empty);

string relinkDir = Path.Combine(_projectDir!, "obj", config, DefaultTargetFrameworkForBlazor, "wasm", "for-build");
if (relink)
Assert.True(Directory.Exists(relinkDir), $"Could not find expected relink dir: {relinkDir}");

relink = !firstBuildNative;
BuildInternal(id, config, publish: false,
BlazorBuildInternal(id, config, publish: false,
extraArgs: relink ? "-p:WasmBuildNative=true" : string.Empty);

if (relink)
Expand Down
4 changes: 3 additions & 1 deletion src/mono/wasm/Wasm.Build.Tests/WasmNativeDefaultsTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,9 @@ public void Defaults(string config, string extraProperties, bool aot, bool build
{
string output = CheckWasmNativeDefaultValue("native_defaults_publish", config, extraProperties, aot, dotnetWasmFromRuntimePack: !publishValue);

Assert.Contains($"** WasmBuildNative: '{buildValue.ToString().ToLower()}', WasmBuildingForNestedPublish: ''", output);
// for build
Assert.DoesNotContain($"** WasmBuildNative: '{buildValue.ToString().ToLower()}', WasmBuildingForNestedPublish: ''", output);
// for publish
Assert.Contains($"** WasmBuildNative: '{publishValue.ToString().ToLower()}', WasmBuildingForNestedPublish: 'true'", output);
Assert.Contains("Stopping the build", output);
}
Expand Down
15 changes: 0 additions & 15 deletions src/mono/wasm/Wasm.Build.Tests/WasmTemplateTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -418,21 +418,6 @@ public void ConsolePublishAndRun(string config, bool aot, bool relinking)
Assert.Contains("args[2] = z", res.Output);
}

[ConditionalFact(typeof(BuildTestBase), nameof(IsUsingWorkloads))]
public async Task BlazorRunTest()
{
string config = "Debug";
string id = $"blazor_{config}_{Path.GetRandomFileName()}";
string projectFile = CreateWasmTemplateProject(id, "blazorwasm");

new DotNetCommand(s_buildEnv, _testOutput)
.WithWorkingDirectory(_projectDir!)
.Execute($"build -c {config} -bl:{Path.Combine(s_buildEnv.LogRootPath, $"{id}.binlog")}")
.EnsureSuccessful();

await BlazorRun(config);
}

[ConditionalTheory(typeof(BuildTestBase), nameof(IsUsingWorkloads))]
[InlineData("", BuildTestBase.DefaultTargetFramework)]
// [ActiveIssue("https://github.com/dotnet/runtime/issues/79313")]
Expand Down
5 changes: 3 additions & 2 deletions src/mono/wasm/build/WasmApp.targets
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,8 @@
<_WasmNestedPublishAppPreTarget Condition="'$(DisableAutoWasmPublishApp)' != 'true'">Publish</_WasmNestedPublishAppPreTarget>

<EnableDefaultWasmAssembliesToBundle Condition="'$(EnableDefaultWasmAssembliesToBundle)' == ''">true</EnableDefaultWasmAssembliesToBundle>
<WasmBuildOnlyAfterPublish Condition="'$(WasmBuildOnlyAfterPublish)' == '' and '$(DeployOnBuild)' == 'true'">true</WasmBuildOnlyAfterPublish>
<!-- VS uses DeployOnBuild, and sdk sets _IsPublishing -->
<WasmBuildOnlyAfterPublish Condition="'$(WasmBuildOnlyAfterPublish)' == '' and ('$(DeployOnBuild)' == 'true' or '$(_IsPublishing)' == 'true')">true</WasmBuildOnlyAfterPublish>
<WasmGenerateAppBundle Condition="'$(WasmGenerateAppBundle)' == '' and '$(OutputType)' != 'Library'">true</WasmGenerateAppBundle>
<WasmGenerateAppBundle Condition="'$(WasmGenerateAppBundle)' == ''">false</WasmGenerateAppBundle>
<UseAppHost>false</UseAppHost>
Expand Down Expand Up @@ -172,7 +173,7 @@
<!-- Use a unique property, so the already run wasm targets can also run -->
<MSBuild Projects="$(MSBuildProjectFile)"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm curious how this works.
Do we run separate MSBuild process here (which doesn't share variables and items from parent ?)
Do we still need to have it as separate process ?
Do we still need _WasmInNestedPublish_UniqueProperty_XYZ ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we run separate MSBuild process here (which doesn't share variables and items from parent ?)

No, but it runs it in a fresh context. I will be removing this, but I'm waiting for the blazor<->runtime build changes to happen.

Do we still need to have it as separate process ?

It's not necessarily a separate process. And this PR doesn't change anything about this.

Do we still need _WasmInNestedPublish_UniqueProperty_XYZ ?

Yep

Essentially, this PR doesn't change any behavior w.r.t the nested publish. The nested publish bit was a hacky solution, and will be removed. But I'm holding that off till we get further on the build consolidation with blazor.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ilonatommy opened issue for ICU related dependency on not nesting it
#82819

Targets="WasmNestedPublishApp"
Properties="_WasmInNestedPublish_UniqueProperty_XYZ=true;;WasmBuildingForNestedPublish=true;DeployOnBuild=">
Properties="_WasmInNestedPublish_UniqueProperty_XYZ=true;;WasmBuildingForNestedPublish=true;DeployOnBuild=;_IsPublishing=">
<Output TaskParameter="TargetOutputs" ItemName="WasmNestedPublishAppResultItems" />
</MSBuild>

Expand Down