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

Jump to bottom

[browser] Fix encoding problem when publishing with AOT #83510

Merged
merged 8 commits into from
Mar 21, 2023
Merged

[browser] Fix encoding problem when publishing with AOT #83510

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
38eb608
Fix.
ilonatommy Mar 16, 2023
cffbc4d
Merge branch 'dotnet:main' into fix-83035
ilonatommy Mar 16, 2023
b6153ba
Regex approach is not necessary.
ilonatommy Mar 16, 2023
e20ee8f
Fix encoding on Windows.
ilonatommy Mar 17, 2023
a9d854c
Fixed build error.
ilonatommy Mar 17, 2023
5ce9ca7
Reverted unnecessary changes. Blocked relink with unicode.
ilonatommy Mar 17, 2023
d2bbded
Revet + nit.
ilonatommy Mar 17, 2023
7641a20
Applied @kg's review.
ilonatommy Mar 20, 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
46 changes: 24 additions & 22 deletions 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 @@ -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 = $"build_publish_{buildArgs.Config}_{s_unicodeChar}";
ilonatommy marked this conversation as resolved.
Show resolved Hide resolved

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}");

// relinking for paths with unicode does not work:
// [ActiveIssue("https://github.com/dotnet/runtime/issues/83497")]
ilonatommy marked this conversation as resolved.
Show resolved Hide resolved
// 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,8 @@ public void BuildThenPublishWithAOT(BuildArgs buildArgs, RunHost host, string id

// no native files changed
pathsDict.UpdateTo(unchanged: true);
CompareStat(publishStat, secondBuildStat, pathsDict.Values);
// [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 +155,10 @@ 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);
}

}
}
22 changes: 11 additions & 11 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 @@ -31,7 +31,7 @@ public abstract class BuildTestBase : IClassFixture<SharedBuildPerTestClassFixtu
public const string DefaultTargetFramework = "net8.0";
public const string DefaultTargetFrameworkForBlazor = "net8.0";
private const string DefaultEnvironmentLocale = "en-US";
protected static readonly char s_unicodeChar = '煉';
protected static readonly char s_unicodeChar = '\u7149';
protected static readonly bool s_skipProjectCleanup;
protected static readonly string s_xharnessRunnerCommand;
protected string? _projectDir;
Expand Down 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
Open in desktop
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
Open in desktop
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