Skip to content

Commit

Permalink
[browser] Fix encoding problem when publishing with AOT (#83510)
Browse files Browse the repository at this point in the history 
* Fix.

* Regex approach is not necessary.

* Fix encoding on Windows.

* Fixed build error.

* Reverted unnecessary changes. Blocked relink with unicode.

* Revet + nit.

* Applied @kg's review.
  • Loading branch information
@ilonatommy
ilonatommy committed Mar 21, 2023
1 parent bb3dc9c commit 93dda3b
Show file tree
Hide file tree
Showing 4 changed files with 72 additions and 42 deletions.
53 changes: 30 additions & 23 deletions src/mono/wasm/Wasm.Build.Tests/BuildPublishTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ public BuildPublishTests(ITestOutputHelper output, SharedBuildPerTestClassFixtur
[BuildAndRun(host: RunHost.Chrome, aot: false, config: "Debug")]
public void BuildThenPublishNoAOT(BuildArgs buildArgs, RunHost host, string id)
{
string projectName = $"build_publish_{buildArgs.Config}{s_unicodeChar}";
string projectName = GetTestProjectPath(prefix: "build_publish", config: buildArgs.Config);

buildArgs = buildArgs with { ProjectName = projectName };
buildArgs = ExpandBuildArgs(buildArgs);
Expand Down Expand Up @@ -73,7 +73,7 @@ void Run() => RunAndTestWasmApp(
[BuildAndRun(host: RunHost.Chrome, aot: true, config: "Debug")]
public void BuildThenPublishWithAOT(BuildArgs buildArgs, RunHost host, string id)
{
string projectName = $"build_publish_{buildArgs.Config}";
string projectName = GetTestProjectPath(prefix: "build_publish", config: buildArgs.Config);

buildArgs = buildArgs with { ProjectName = projectName };
buildArgs = ExpandBuildArgs(buildArgs, extraProperties: "<_WasmDevel>true</_WasmDevel>");
Expand Down Expand Up @@ -108,23 +108,25 @@ public void BuildThenPublishWithAOT(BuildArgs buildArgs, RunHost host, string id

_testOutput.WriteLine($"{Environment.NewLine}Publishing with no changes ..{Environment.NewLine}");

// FIXME: relinking for paths with unicode does not work:
// [ActiveIssue("https://github.com/dotnet/runtime/issues/83497")]
// relink by default for Release+publish
(_, output) = BuildProject(buildArgs,
id: id,
new BuildProjectOptions(
DotnetWasmFromRuntimePack: false,
CreateProject: false,
Publish: true,
UseCache: false,
Label: "first_publish"));

var publishStat = StatFiles(pathsDict.Select(kvp => kvp.Value.fullPath));
Assert.True(publishStat["pinvoke.o"].Exists);
Assert.True(publishStat[$"{mainDll}.bc"].Exists);
CheckOutputForNativeBuild(expectAOT: true, expectRelinking: false, buildArgs, output);
CompareStat(firstBuildStat, publishStat, pathsDict.Values);

Run(expectAOT: true);
// (_, output) = BuildProject(buildArgs,
// id: id,
// new BuildProjectOptions(
// DotnetWasmFromRuntimePack: false,
// CreateProject: false,
// Publish: true,
// UseCache: false,
// Label: "first_publish"));

// var publishStat = StatFiles(pathsDict.Select(kvp => kvp.Value.fullPath));
// Assert.True(publishStat["pinvoke.o"].Exists);
// Assert.True(publishStat[$"{mainDll}.bc"].Exists);
// CheckOutputForNativeBuild(expectAOT: true, expectRelinking: false, buildArgs, output);
// CompareStat(firstBuildStat, publishStat, pathsDict.Values);

// Run(expectAOT: true);

// second build
(_, output) = BuildProject(buildArgs,
Expand All @@ -142,7 +144,9 @@ public void BuildThenPublishWithAOT(BuildArgs buildArgs, RunHost host, string id

// no native files changed
pathsDict.UpdateTo(unchanged: true);
CompareStat(publishStat, secondBuildStat, pathsDict.Values);
// FIXME: elinking for paths with unicode does not work:
// [ActiveIssue("https://github.com/dotnet/runtime/issues/83497")]
// CompareStat(publishStat, secondBuildStat, pathsDict.Values);

void Run(bool expectAOT) => RunAndTestWasmApp(
buildArgs with { AOT = expectAOT },
Expand All @@ -152,11 +156,14 @@ void Run(bool expectAOT) => RunAndTestWasmApp(

void CheckOutputForNativeBuild(bool expectAOT, bool expectRelinking, BuildArgs buildArgs, string buildOutput)
{
AssertSubstring($"{buildArgs.ProjectName}.dll -> {buildArgs.ProjectName}.dll.bc", buildOutput, expectAOT);
AssertSubstring($"{buildArgs.ProjectName}.dll.bc -> {buildArgs.ProjectName}.dll.o", buildOutput, expectAOT);
AssertSubstring($"{buildArgs.ProjectName}.dll -> {buildArgs.ProjectName}.dll.bc", buildOutput, contains: expectAOT);
AssertSubstring($"{buildArgs.ProjectName}.dll.bc -> {buildArgs.ProjectName}.dll.o", buildOutput, contains: expectAOT);

AssertSubstring("pinvoke.c -> pinvoke.o", buildOutput, expectRelinking || expectAOT);
AssertSubstring("pinvoke.c -> pinvoke.o", buildOutput, contains: expectRelinking || expectAOT);
}



// appending UTF-8 char makes sure project build&publish under all types of paths is supported
string GetTestProjectPath(string prefix, string config) => $"{prefix}_{config}_{s_unicodeChar}";
}
}
20 changes: 10 additions & 10 deletions src/mono/wasm/Wasm.Build.Tests/BuildTestBase.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -195,16 +195,8 @@ protected string RunAndTestWasmApp(BuildArgs buildArgs,
useWasmConsoleOutput: useWasmConsoleOutput
);

if (buildArgs.AOT)
{
Assert.Contains("AOT: image 'System.Private.CoreLib' found.", output);
Assert.Contains($"AOT: image '{buildArgs.ProjectName}' found.", output);
}
else
{
Assert.DoesNotContain("AOT: image 'System.Private.CoreLib' found.", output);
Assert.DoesNotContain($"AOT: image '{buildArgs.ProjectName}' found.", output);
}
AssertSubstring("AOT: image 'System.Private.CoreLib' found.", output, contains: buildArgs.AOT);
AssertSubstring($"AOT: image '{buildArgs.ProjectName}' found.", output, contains: buildArgs.AOT);

if (test != null)
test(output);
Expand Down Expand Up @@ -1204,6 +1196,14 @@ public static int Main()
RunHost.NodeJS => new NodeJSHostRunner(),
_ => new BrowserHostRunner(),
};

protected void AssertSubstring(string substring, string full, bool contains)
{
if (contains)
Assert.Contains(substring, full);
else
Assert.DoesNotContain(substring, full);
}
}

public record BuildArgs(string ProjectName,
Expand Down
8 changes: 0 additions & 8 deletions src/mono/wasm/Wasm.Build.Tests/NativeRebuildTests/NativeRebuildTestsBase.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -186,13 +186,5 @@ internal void CompareStat(IDictionary<string, FileStat> oldStat, IDictionary<str

return dict;
}

protected void AssertSubstring(string substring, string full, bool contains)
{
if (contains)
Assert.Contains(substring, full);
else
Assert.DoesNotContain(substring, full);
}
}
}
33 changes: 32 additions & 1 deletion src/tasks/AotCompilerTask/MonoAOTCompiler.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,8 @@ public class MonoAOTCompiler : Microsoft.Build.Utilities.Task
private int _numCompiled;
private int _totalNumAssemblies;

private readonly Dictionary<string, string> _symbolNameFixups = new();

private bool ProcessAndValidateArguments()
{
if (!File.Exists(CompilerBinaryPath))
Expand Down Expand Up @@ -1025,7 +1027,7 @@ private bool GenerateAotModulesTable(IEnumerable<ITaskItem> assemblies, string[]
if (!TryGetAssemblyName(asmPath, out string? assemblyName))
return false;

string symbolName = assemblyName.Replace ('.', '_').Replace ('-', '_').Replace(' ', '_');
string symbolName = FixupSymbolName(assemblyName);
symbols.Add($"mono_aot_module_{symbolName}_info");
}

Expand Down Expand Up @@ -1160,6 +1162,35 @@ private static List<ITaskItem> ConvertAssembliesDictToOrderedList(ConcurrentDict
return outItems;
}

private string FixupSymbolName(string name)
{
if (_symbolNameFixups.TryGetValue(name, out string? fixedName))
return fixedName;

UTF8Encoding utf8 = new();
byte[] bytes = utf8.GetBytes(name);
StringBuilder sb = new();

foreach (byte b in bytes)
{
if ((b >= (byte)'0' && b <= (byte)'9') ||
(b >= (byte)'a' && b <= (byte)'z') ||
(b >= (byte)'A' && b <= (byte)'Z') ||
(b == (byte)'_'))
{
sb.Append((char)b);
}
else
{
sb.Append('_');
}
}

fixedName = sb.ToString();
_symbolNameFixups[name] = fixedName;
return fixedName;
}

internal sealed class PrecompileArguments
{
public PrecompileArguments(string ResponseFilePath, IDictionary<string, string> EnvironmentVariables, string WorkingDir, ITaskItem AOTAssembly, IList<ProxyFile> ProxyFiles)
Expand Down

0 comments on commit 93dda3b

Please sign in to comment.