[wasm] Build improvements (dotnet#61276)

Fixes dotnet#61067 Fixes dotnet#59538 (cherry picked from commit 43c3e9a79a241d044f747373bf483da23807afff)
radical · Feb 7, 2022 · b0762e8 · b0762e8
1 parent 3105078
commit b0762e8
Show file tree

Hide file tree

Showing 4 changed files with 74 additions and 29 deletions.
diff --git a/src/mono/wasm/build/WasmApp.Native.targets b/src/mono/wasm/build/WasmApp.Native.targets
@@ -7,7 +7,6 @@
 
   <PropertyGroup>
     <_WasmBuildNativeCoreDependsOn>
-      _CheckEmccIsExpectedVersion;
       _PrepareForWasmBuildNative;
       _GenerateDriverGenC;
       _GeneratePInvokeTable;
@@ -261,14 +260,17 @@
     <IcallTableGenerator
       RuntimeIcallTableFile="$(_WasmRuntimeICallTablePath)"
       Assemblies="@(_WasmAssembliesInternal)"
-      OutputPath="$(_WasmICallTablePath)">
-      <Output TaskParameter="FileWrites" ItemName="FileWrites" />
-    </IcallTableGenerator>
+      OutputPath="$(_WasmICallTablePath)" />
+
+    <!-- Writing this explicitly, so it gets picked up when the target is skipped -->
+    <ItemGroup>
+      <FileWrites Include="$(_WasmICallTablePath)" />
+    </ItemGroup>
   </Target>
 
   <Target Name="_WasmSelectRuntimeComponentsForLinking" Condition="'$(WasmNativeWorkload)' == true" DependsOnTargets="_MonoSelectRuntimeComponents" />
 
-  <Target Name="_WasmCompileNativeFiles">
+  <Target Name="_WasmCompileNativeFiles" DependsOnTargets="_CheckEmccIsExpectedVersion">
     <PropertyGroup>
       <_EmBuilder Condition="$([MSBuild]::IsOSPlatform('WINDOWS'))">embuilder.bat</_EmBuilder>
       <_EmBuilder Condition="!$([MSBuild]::IsOSPlatform('WINDOWS'))">embuilder.py</_EmBuilder>
@@ -304,7 +306,7 @@
           Inputs="@(_BitcodeFile);$(_EmccDefaultFlagsRsp);$(_EmccCompileBitcodeRsp)"
           Outputs="@(_BitcodeFile->'%(ObjectFile)')"
           Condition="'$(_WasmShouldAOT)' == 'true' and @(_BitcodeFile->Count()) > 0"
-          DependsOnTargets="_WasmWriteRspForCompilingBitcode"
+          DependsOnTargets="_CheckEmccIsExpectedVersion;_WasmWriteRspForCompilingBitcode"
           Returns="@(FileWrites)">
 
     <ItemGroup>
@@ -371,7 +373,7 @@
   <Target Name="_WasmLinkDotNet"
           Inputs="@(_WasmLinkDependencies);$(_EmccDefaultFlagsRsp);$(_EmccLinkRsp)"
           Outputs="$(_WasmIntermediateOutputPath)dotnet.js;$(_WasmIntermediateOutputPath)dotnet.wasm"
-          DependsOnTargets="_WasmSelectRuntimeComponentsForLinking;_WasmCompileAssemblyBitCodeFilesForAOT;_WasmWriteRspFilesForLinking"
+          DependsOnTargets="_CheckEmccIsExpectedVersion;_WasmSelectRuntimeComponentsForLinking;_WasmCompileAssemblyBitCodeFilesForAOT;_WasmWriteRspFilesForLinking"
           Returns="@(FileWrites)" >
 
     <Message Text="Linking with emcc. This may take a while ..." Importance="High" />
@@ -426,10 +428,24 @@ EMSCRIPTEN_KEEPALIVE void mono_wasm_load_profiler_aot (const char *desc) { mono_
     <Error Condition="'$(RuntimeEmccVersionRaw)' == ''"
            Text="%24(RuntimeEmccVersionRaw) is not set. '$(_EmccPropsPath)' should have set that."/>
 
-    <Exec Command="emcc --version" WorkingDirectory="$(_WasmIntermediateOutputPath)" EnvironmentVariables="@(EmscriptenEnvVars)" ConsoleToMsBuild="true" StandardOutputImportance="Low">
+    <PropertyGroup>
+      <_EmccVersionCommand>emcc --version</_EmccVersionCommand>
+    </PropertyGroup>
+
+    <Exec Command="$(_EmccVersionCommand)" WorkingDirectory="$(_WasmIntermediateOutputPath)" EnvironmentVariables="@(EmscriptenEnvVars)" ConsoleToMsBuild="true" StandardOutputImportance="Low" IgnoreExitCode="true">
       <Output TaskParameter="ConsoleOutput" ItemName="_VersionLines" />
+      <Output TaskParameter="ExitCode" PropertyName="_EmccVersionExitCode" />
     </Exec>
 
+    <!-- If `emcc -version` failed, then run it again, so we can surface the output as *Errors*. This allows the errors to show up correctly,
+         versus trying to use the output lines with the Error task -->
+    <Exec Condition="$(_EmccVersionExitCode) != '0'"
+          Command="$(_EmccVersionCommand)"
+          WorkingDirectory="$(_WasmIntermediateOutputPath)"
+          EnvironmentVariables="@(EmscriptenEnvVars)"
+          CustomErrorRegularExpression=".*"
+          />
+
     <!-- we want to get the first line from the output, which has the version.
          Rest of the lines are the license -->
     <ItemGroup>

diff --git a/src/tasks/WasmAppBuilder/IcallTableGenerator.cs b/src/tasks/WasmAppBuilder/IcallTableGenerator.cs
@@ -31,8 +31,16 @@ public class IcallTableGenerator : Task
 
     public override bool Execute()
     {
-        GenIcallTable(RuntimeIcallTableFile!, Assemblies!.Select(item => item.ItemSpec).ToArray());
-        return true;
+        try
+        {
+            GenIcallTable(RuntimeIcallTableFile!, Assemblies!.Select(item => item.ItemSpec).ToArray());
+            return !Log.HasLoggedErrors;
+        }
+        catch (LogAsErrorException laee)
+        {
+            Log.LogError(laee.Message);
+            return false;
+        }
     }
 
     //
@@ -152,20 +160,9 @@ private void ProcessType (Type type)
             icallClass.Icalls.TryGetValue (method.Name, out icall);
             if (icall == null)
             {
-                // Then with signature
-                var sig = new StringBuilder (method.Name + "(");
-                int pindex = 0;
-                foreach (var par in method.GetParameters())
-                {
-                    if (pindex > 0)
-                        sig.Append (',');
-                    var t = par.ParameterType;
-                    AppendType (sig, t);
-                    pindex++;
-                }
-                sig.Append (')');
-                if (icallClass.Icalls.ContainsKey (sig.ToString ()))
-                    icall = icallClass.Icalls [sig.ToString ()];
+                string? methodSig = BuildSignature(method, className);
+                if (methodSig != null && icallClass.Icalls.ContainsKey (methodSig))
+                    icall = icallClass.Icalls [methodSig];
             }
             if (icall == null)
                 // Registered at runtime
@@ -178,6 +175,33 @@ private void ProcessType (Type type)
 
         foreach (var nestedType in type.GetNestedTypes())
             ProcessType(nestedType);
+
+        string? BuildSignature(MethodInfo method, string className)
+        {
+            // Then with signature
+            var sig = new StringBuilder (method.Name + "(");
+            int pindex = 0;
+            foreach (var par in method.GetParameters())
+            {
+                if (pindex > 0)
+                    sig.Append (',');
+                var t = par.ParameterType;
+                try
+                {
+                    AppendType (sig, t);
+                }
+                catch (NotImplementedException nie)
+                {
+                    Log.LogWarning($"Failed to generate icall function for method '[{method.DeclaringType!.Assembly.GetName().Name}] {className}::{method.Name}'" +
+                                    $" because type '{nie.Message}' is not supported for parameter named '{par.Name}'. Ignoring.");
+                    return null;
+                }
+                pindex++;
+            }
+            sig.Append (')');
+
+            return sig.ToString();
+        }
     }
 
     // Append the type name used by the runtime icall tables

diff --git a/src/tasks/WasmAppBuilder/WasmAppBuilder.cs b/src/tasks/WasmAppBuilder/WasmAppBuilder.cs
@@ -348,9 +348,16 @@ private bool FileCopyChecked(string src, string dst, string label)
         }
 
         Log.LogMessage(MessageImportance.Low, $"Copying file from '{src}' to '{dst}'");
-        File.Copy(src, dst, true);
-        _fileWrites.Add(dst);
+        try
+        {
+            File.Copy(src, dst, true);
+            _fileWrites.Add(dst);
 
-        return true;
+            return true;
+        }
+        catch (IOException ioex)
+        {
+            throw new LogAsErrorException($"{label} Failed to copy {src} to {dst} because {ioex.Message}");
+        }
     }
 }
diff --git a/src/tests/BuildWasmApps/Wasm.Build.Tests/BlazorWasmTests.cs b/src/tests/BuildWasmApps/Wasm.Build.Tests/BlazorWasmTests.cs
@@ -79,7 +79,6 @@ private CommandResult PublishForRequiresWorkloadTest(string config, string extra
         [Theory]
         [InlineData("Debug")]
         [InlineData("Release")]
-        [ActiveIssue("https://github.com/dotnet/runtime/issues/59538")]
         public void Net50Projects_NativeReference(string config)
             => BuildNet50Project(config, aot: false, expectError: true, @"<NativeFileReference Include=""native-lib.o"" />");
 
@@ -93,7 +92,6 @@ public void Net50Projects_NativeReference(string config)
 
         [Theory]
         [MemberData(nameof(Net50TestData))]
-        [ActiveIssue("https://github.com/dotnet/runtime/issues/59538")]
         public void Net50Projects_AOT(string config, bool aot, bool expectError)
             => BuildNet50Project(config, aot: aot, expectError: expectError);