Skip to content

Allow the icu native code to be linked out when invariant globalization is enabled. #48120

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

Merged
vargaz merged 10 commits into from
Feb 16, 2021
Merged

Allow the icu native code to be linked out when invariant globalization is enabled. #48120

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
0919a5f
Disable the dynamic loading of icu functions on WASM.
vargaz Feb 10, 2021
740d54f
Preserve GetICUVersion using a DynamicDependency attribute so its onl…
vargaz Feb 10, 2021
c344d28
Add GetICUVersion to .LibraryBuild.xml so its retained during the cla…
vargaz Feb 10, 2021
ff2f2da
Set InvariantGlobalization based on WasmInvariantGlobalization.
vargaz Feb 11, 2021
f142940
Fix the framework build.
vargaz Feb 11, 2021
e8ce84a
Avoid linking in the libicu libraries on wasm in invariant mode.
vargaz Feb 11, 2021
8b2b535
Move ICU tests dependency to libraries build descriptor file
marek-safar Feb 11, 2021
28d8a48
Hook up the shared file in the build
marek-safar Feb 11, 2021
c0e949b
Fix conditionals.
vargaz Feb 11, 2021
8e471a2
Remove WasmInvariantGlobalization msbuild variable.
vargaz Feb 15, 2021
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
4 changes: 0 additions & 4 deletions src/libraries/System.Private.CoreLib/src/ILLink/ILLink.Descriptors.Shared.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,5 @@
<linker>
<assembly fullname="System.Private.CoreLib">
<type fullname="Interop/Globalization">
<!-- Internal API used by tests only. -->
<method name="GetICUVersion" />
</type>
<type fullname="System.Threading.ThreadPoolBoundHandle">
<!-- Workaround to keep .interfaceimpl even though this type
is not instantiated on unix:
Expand Down
8 changes: 8 additions & 0 deletions src/libraries/System.Private.CoreLib/src/ILLink/ILLinkTrim_LibraryBuild.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<linker>
<assembly fullname="System.Private.CoreLib">
<type fullname="Interop/Globalization">
<!-- Internal API used by tests only. -->
<method name="GetICUVersion" />
</type>
</assembly>
</linker>
3 changes: 3 additions & 0 deletions src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,9 @@
<ILLinkSubstitutionsXmls Include="$(ILLinkSharedDirectory)ILLink.Substitutions.NoX86Intrinsics.xml" Condition="'$(SupportsX86Intrinsics)' != 'true'" />
<ILLinkLinkAttributesXmls Include="$(ILLinkSharedDirectory)ILLink.LinkAttributes.Shared.xml" />
</ItemGroup>
<PropertyGroup>
<ILLinkTrimXmlLibraryBuild>$(ILLinkSharedDirectory)ILLinkTrim_LibraryBuild.xml</ILLinkTrimXmlLibraryBuild>
Copy link
Member

@lewing lewing Feb 12, 2021

Choose a reason for hiding this comment

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

Is there some reason we aren't changing the eng/illink tasks to use ILLink/ILLink.Descriptors.LibraryBuild.xml for this sort of thing by default? Does it make sense to generalize the ILLink patterns for runtime?

Copy link
Contributor

@marek-safar marek-safar Feb 12, 2021

Choose a reason for hiding this comment

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

It's already generalized but SPC is special because it has shared part

</PropertyGroup>
<ItemGroup>
<Compile Include="$(MSBuildThisFileDirectory)Internal\AssemblyAttributes.cs" />
<Compile Include="$(MSBuildThisFileDirectory)Internal\Console.cs" />
Expand Down
1 change: 1 addition & 0 deletions src/mono/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,7 @@ elseif(CMAKE_SYSTEM_NAME STREQUAL "Android")
# FIXME: Rest of the flags from configure.ac
elseif(CMAKE_SYSTEM_NAME STREQUAL "Emscripten")
set(HOST_BROWSER 1)
add_definitions(-DNO_GLOBALIZATION_SHIM)
add_definitions(-D_THREAD_SAFE)
set(DISABLE_EXECUTABLES 1)
# FIXME: Is there a cmake option for this ?
Expand Down
12 changes: 6 additions & 6 deletions src/mono/wasm/build/WasmApp.targets
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
- $(WasmProfilers) - Profilers to use
- $(AOTMode) - Defaults to `LLVMOnly`
- $(AOTProfilePath) - profile data file to be used for profile-guided optimization
- $(WasmInvariantGlobalization)
- $(InvariantGlobalization) - Whenever to disable ICU. Defaults to false.

- $(WasmResolveAssembliesBeforeBuild) - Resolve the assembly dependencies. Defaults to false
- $(WasmAssemblySearchPaths) - used for resolving assembly dependencies
Expand Down Expand Up @@ -156,7 +156,7 @@
<Error Condition="'$(WasmMainJSPath)' == ''" Text="%24(WasmMainJSPath) property needs to be set" />

<PropertyGroup>
<WasmIcuDataFileName Condition="'$(WasmInvariantGlobalization)' != 'true'">icudt.dat</WasmIcuDataFileName>
<WasmIcuDataFileName Condition="'$(InvariantGlobalization)' != 'true'">icudt.dat</WasmIcuDataFileName>

<_HasDotnetWasm Condition="'%(WasmNativeAsset.FileName)%(WasmNativeAsset.Extension)' == 'dotnet.wasm'">true</_HasDotnetWasm>
<_HasDotnetJs Condition="'%(WasmNativeAsset.FileName)%(WasmNativeAsset.Extension)' == 'dotnet.js'">true</_HasDotnetJs>
Expand All @@ -167,15 +167,15 @@
<WasmNativeAsset Include="$(MicrosoftNetCoreAppRuntimePackRidDir)native\dotnet.wasm" Condition="'$(_HasDotnetWasm)' != 'true'" />
<WasmNativeAsset Include="$(MicrosoftNetCoreAppRuntimePackRidDir)native\dotnet.js" Condition="'$(_HasDotnetJs)' != 'true'" />

<WasmNativeAsset Include="$(MicrosoftNetCoreAppRuntimePackRidDir)native\$(WasmIcuDataFileName)" Condition="'$(WasmInvariantGlobalization)' != 'true'" />
<WasmNativeAsset Include="$(MicrosoftNetCoreAppRuntimePackRidDir)native\$(WasmIcuDataFileName)" Condition="'$(InvariantGlobalization)' != 'true'" />
<WasmNativeAsset Include="$(MicrosoftNetCoreAppRuntimePackRidDir)native\dotnet.timezones.blat" />
</ItemGroup>

<WasmAppBuilder
AppDir="$(WasmAppDir)"
MainJS="$(WasmMainJSPath)"
Assemblies="@(_WasmAssemblies)"
InvariantGlobalization="$(WasmInvariantGlobalization)"
InvariantGlobalization="$(InvariantGlobalization)"
SatelliteAssemblies="@(WasmSatelliteAssemblies)"
FilesToIncludeInFileSystem="@(WasmFilesToIncludeInFileSystem)"
IcuDataFileName="$(WasmIcuDataFileName)"
Expand Down Expand Up @@ -250,8 +250,8 @@
<_WasmRuntimePackNativeLibs Include="libSystem.Native.a"/>
<_WasmRuntimePackNativeLibs Include="libSystem.IO.Compression.Native.a"/>
<_WasmRuntimePackNativeLibs Include="libmono-profiler-aot.a"/>
<_WasmRuntimePackNativeLibs Include="libicuuc.a"/>
<_WasmRuntimePackNativeLibs Include="libicui18n.a"/>
<_WasmRuntimePackNativeLibs Include="libicuuc.a" Condition="'$(InvariantGlobalization)' != 'true'" />
<_WasmRuntimePackNativeLibs Include="libicui18n.a" Condition="'$(InvariantGlobalization)' != 'true'" />
<_WasmObjects Include="@(_WasmRuntimePackNativeLibs->'$(MicrosoftNetCoreAppRuntimePackRidDir)\native\%(FileName)%(Extension)')" />

<_WasmObjects Include="$(_WasmIntermediateOutputPath)driver.o"/>
Expand Down