Skip to content

Commit

Permalink
[wasm] Don't use _WasmDevel=true when publishing (#87544)
Browse files Browse the repository at this point in the history
* [wasm] Add missing Wasm.Build.Tests.Blazor.AppsettingsTests to list of tests to run

* [wasm] CI: Trigger only wasm jobs on workload-testing.targets changes, since only wasm is using this currently

* [wasm] host: Add `--no-silent` command line parameter

- this is the equivalent to `--verbose`
- The default is "silent" behavior

Cannot use `--verbose` because that conflicts with `dotnet run`.

* [wasm] WBT: Update template tests to use `--no-silent` with `dotnet run`

.. which would be useful in debugging tests.

* [wasm] Don't use _WasmDevel=true whenever publishing

`_WasmDevel=true` causes the optimization flags to be `-O0`.

- This *was* automatically set whenever `Configuration=Debug`, and
`WasmBuildNative=true`, which is useful when relinking in inner loop.
- But when publishing, and relinking/AOT, `-O0` results in larger output.

* Update src/mono/wasm/host/JSEngineHost.cs

* Address feedback from @ilonatommy
  • Loading branch information
radical authored Jun 15, 2023
1 parent 9a33ae7 commit 226e3ec
Show file tree
Hide file tree
Showing 7 changed files with 34 additions and 22 deletions.
1 change: 1 addition & 0 deletions eng/pipelines/common/evaluate-default-paths.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ parameters:
eng/testing/tests.browser.targets
eng/testing/tests.was*.targets
eng/testing/was*provisioning.targets
eng/testing/workloads-testing.targets
src/libraries/sendtohelix-wasm.targets
src/libraries/sendtohelix-wasi.targets
src/mono/mono/**/*wasm*
Expand Down
1 change: 1 addition & 0 deletions eng/testing/scenarios/BuildWasmAppsJobsList.txt
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ Wasm.Build.Tests.Blazor.BuildPublishTests
Wasm.Build.Tests.Blazor.MiscTests
Wasm.Build.Tests.Blazor.MiscTests2
Wasm.Build.Tests.Blazor.NativeRefTests
Wasm.Build.Tests.Blazor.AppsettingsTests
Wasm.Build.Tests.BuildPublishTests
Wasm.Build.Tests.CleanTests
Wasm.Build.Tests.ConfigSrcTests
Expand Down
32 changes: 18 additions & 14 deletions src/mono/wasm/Wasm.Build.Tests/WasmTemplateTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -154,9 +154,11 @@ public void ConsoleBuildThenPublish(string config)

AssertDotNetJsSymbols(Path.Combine(GetBinDir(config), "AppBundle"), fromRuntimePack: true, targetFramework: DefaultTargetFramework);

(int exitCode, string output) = RunProcess(s_buildEnv.DotNet, _testOutput, args: $"run --no-build -c {config}", workingDir: _projectDir);
Assert.Equal(0, exitCode);
Assert.Contains("Hello, Console!", output);
CommandResult res = new RunCommand(s_buildEnv, _testOutput)
.WithWorkingDirectory(_projectDir!)
.ExecuteWithCapturedOutput($"run --no-silent --no-build -c {config}")
.EnsureSuccessful();
Assert.Contains("Hello, Console!", res.Output);

if (!_buildContext.TryGetBuildFor(buildArgs, out BuildProduct? product))
throw new XunitException($"Test bug: could not get the build product in the cache");
Expand Down Expand Up @@ -224,12 +226,14 @@ private void ConsoleBuildAndRun(string config, bool relinking, string extraNewAr

AssertDotNetJsSymbols(Path.Combine(GetBinDir(config, expectedTFM), "AppBundle"), fromRuntimePack: !relinking, targetFramework: expectedTFM);

(int exitCode, string output) = RunProcess(s_buildEnv.DotNet, _testOutput, args: $"run --no-build -c {config} x y z", workingDir: _projectDir);
Assert.Equal(42, exitCode);
CommandResult res = new RunCommand(s_buildEnv, _testOutput)
.WithWorkingDirectory(_projectDir!)
.ExecuteWithCapturedOutput($"run --no-silent --no-build -c {config} x y z")
.EnsureExitCode(42);

Assert.Contains("args[0] = x", output);
Assert.Contains("args[1] = y", output);
Assert.Contains("args[2] = z", output);
Assert.Contains("args[0] = x", res.Output);
Assert.Contains("args[1] = y", res.Output);
Assert.Contains("args[2] = z", res.Output);
}

public static TheoryData<bool, bool, string> TestDataForAppBundleDir()
Expand Down Expand Up @@ -279,7 +283,7 @@ private async Task BrowserRunTwiceWithAndThenWithoutBuildAsync(string config, st
.WithWorkingDirectory(workingDir);

await using var runner = new BrowserRunner(_testOutput);
var page = await runner.RunAsync(runCommand, $"run -c {config} --project {projectFile} --forward-console");
var page = await runner.RunAsync(runCommand, $"run --no-silent -c {config} --project {projectFile} --forward-console");
await runner.WaitForExitMessageAsync(TimeSpan.FromMinutes(2));
Assert.Contains("Hello, Browser!", string.Join(Environment.NewLine, runner.OutputLines));
}
Expand All @@ -289,7 +293,7 @@ private async Task BrowserRunTwiceWithAndThenWithoutBuildAsync(string config, st
.WithWorkingDirectory(workingDir);

await using var runner = new BrowserRunner(_testOutput);
var page = await runner.RunAsync(runCommand, $"run -c {config} --no-build --project {projectFile} --forward-console");
var page = await runner.RunAsync(runCommand, $"run --no-silent -c {config} --no-build --project {projectFile} --forward-console");
await runner.WaitForExitMessageAsync(TimeSpan.FromMinutes(2));
Assert.Contains("Hello, Browser!", string.Join(Environment.NewLine, runner.OutputLines));
}
Expand All @@ -309,7 +313,7 @@ private Task ConsoleRunWithAndThenWithoutBuildAsync(string config, string extraP
string workingDir = runOutsideProjectDirectory ? BuildEnvironment.TmpPath : _projectDir!;

{
string runArgs = $"run -c {config} --project {projectFile}";
string runArgs = $"run --no-silent -c {config} --project {projectFile}";
runArgs += " x y z";
using var cmd = new RunCommand(s_buildEnv, _testOutput, label: id)
.WithWorkingDirectory(workingDir)
Expand All @@ -325,7 +329,7 @@ private Task ConsoleRunWithAndThenWithoutBuildAsync(string config, string extraP

{
// Run with --no-build
string runArgs = $"run -c {config} --project {projectFile} --no-build";
string runArgs = $"run --no-silent -c {config} --project {projectFile} --no-build";
runArgs += " x y z";
using var cmd = new RunCommand(s_buildEnv, _testOutput, label: id)
.WithWorkingDirectory(workingDir);
Expand Down Expand Up @@ -404,7 +408,7 @@ public void ConsolePublishAndRun(string config, bool aot, bool relinking)
AssertFilesDontExist(Path.Combine(GetBinDir(config), "AppBundle"), new[] { "dotnet.native.js.symbols" });
}

string runArgs = $"run --no-build -c {config}";
string runArgs = $"run --no-silent --no-build -c {config}";
runArgs += " x y z";
var res = new RunCommand(s_buildEnv, _testOutput, label: id)
.WithWorkingDirectory(_projectDir!)
Expand Down Expand Up @@ -440,7 +444,7 @@ public async Task BrowserBuildAndRun(string extraNewArgs, string targetFramework
.WithWorkingDirectory(_projectDir!);

await using var runner = new BrowserRunner(_testOutput);
var page = await runner.RunAsync(runCommand, $"run -c {config} --no-build -r browser-wasm --forward-console");
var page = await runner.RunAsync(runCommand, $"run --no-silent -c {config} --no-build -r browser-wasm --forward-console");
await runner.WaitForExitMessageAsync(TimeSpan.FromMinutes(2));
Assert.Contains("Hello, Browser!", string.Join(Environment.NewLine, runner.OutputLines));
}
Expand Down
2 changes: 1 addition & 1 deletion src/mono/wasm/build/WasmApp.Native.targets
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@

<_DriverGenCNeeded Condition="'$(_DriverGenCNeeded)' == '' and '$(_WasmShouldAOT)' == 'true'">true</_DriverGenCNeeded>

<_WasmDevel Condition="'$(_WasmDevel)' == '' and '$(WasmBuildNative)' == 'true' and '$(Configuration)' == 'Debug'">true</_WasmDevel>
<_WasmDevel Condition="'$(_WasmDevel)' == '' and '$(WasmBuildNative)' == 'true' and '$(Configuration)' == 'Debug' and '$(WasmBuildingForNestedPublish)' != 'true'">true</_WasmDevel>

<_EmccOptimizationFlagDefault Condition="'$(_WasmDevel)' == 'true'">-O0</_EmccOptimizationFlagDefault>
<_EmccOptimizationFlagDefault Condition="'$(_EmccOptimizationFlagDefault)' == '' and '$(Configuration)' == 'Debug' and '$(WasmBuildingForNestedPublish)' != 'true'">-O1</_EmccOptimizationFlagDefault>
Expand Down
4 changes: 2 additions & 2 deletions src/mono/wasm/host/CommonConfiguration.cs
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

#nullable enable

using Mono.Options;
using System.Linq;
using System;
Expand All @@ -23,6 +21,7 @@ internal sealed class CommonConfiguration
public HostConfig HostConfig { get; init; }
public WasmHostProperties HostProperties { get; init; }
public IEnumerable<string> HostArguments { get; init; }
public bool Silent { get; private set; } = true;

private string? hostArg;
private string? _runtimeConfigPath;
Expand All @@ -38,6 +37,7 @@ private CommonConfiguration(string[] args)
{ "host|h=", "Host config name", v => hostArg = v },
{ "runtime-config|r=", "runtimeconfig.json path for the app", v => _runtimeConfigPath = v },
{ "extra-host-arg=", "Extra argument to be passed to the host", hostArgsList.Add },
{ "no-silent", "Verbose output from WasmAppHost", _ => Silent = false }
};

RemainingArgs = options.Parse(args);
Expand Down
9 changes: 6 additions & 3 deletions src/mono/wasm/host/JSEngineHost.cs
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,14 @@ public JSEngineHost(JSEngineArguments args, ILogger logger)
_logger = logger;
}

public static async Task<int> InvokeAsync(CommonConfiguration commonArgs,
public static Task<int> InvokeAsync(CommonConfiguration commonArgs,
ILoggerFactory _,
ILogger logger,
CancellationToken _1)
{
var args = new JSEngineArguments(commonArgs);
args.Validate();
return await new JSEngineHost(args, logger).RunAsync();
return new JSEngineHost(args, logger).RunAsync();
}

private async Task<int> RunAsync()
Expand Down Expand Up @@ -88,8 +88,11 @@ private async Task<int> RunAsync()
int exitCode = await Utils.TryRunProcess(psi,
_logger,
msg => { if (msg != null) _logger.LogInformation(msg); },
msg => { if (msg != null) _logger.LogInformation(msg); });
msg => { if (msg != null) _logger.LogInformation(msg); },
silent: _args.CommonConfig.Silent);

if (!_args.CommonConfig.Silent)
Console.WriteLine($"{_args.Host} exited with {exitCode}");
return exitCode;
}
}
7 changes: 5 additions & 2 deletions src/mono/wasm/host/Utils.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,12 @@ public static async Task<int> TryRunProcess(
ILogger logger,
Action<string?>? logStdOut = null,
Action<string?>? logStdErr = null,
bool silent = false,
string? label = null)
{
string msgPrefix = label == null ? string.Empty : $"[{label}] ";
logger.LogInformation($"{msgPrefix}Running: {psi.FileName} {string.Join(" ", psi.ArgumentList)}");
if (!silent)
logger.LogInformation($"{msgPrefix}Running: {psi.FileName} {string.Join(" ", psi.ArgumentList)}");

psi.UseShellExecute = false;
psi.CreateNoWindow = true;
Expand All @@ -29,7 +31,8 @@ public static async Task<int> TryRunProcess(
if (logStdErr != null)
psi.RedirectStandardError = true;

logger.LogDebug($"{msgPrefix}Using working directory: {psi.WorkingDirectory ?? Environment.CurrentDirectory}", msgPrefix);
if (!silent)
logger.LogDebug($"{msgPrefix}Using working directory: {psi.WorkingDirectory ?? Environment.CurrentDirectory}", msgPrefix);

// if (psi.EnvironmentVariables.Count > 0)
// logger.LogDebug($"{msgPrefix}Setting environment variables for execution:", msgPrefix);
Expand Down

0 comments on commit 226e3ec

Please sign in to comment.