[browser][coreCLR] Fix ICU shards#127906
Draft
pavelsavara wants to merge 1 commit intodotnet:mainfrom
Draft
Conversation
pavelsavara
added
arch-wasm
Contributor
|
Tagging subscribers to this area: @dotnet/area-system-globalization |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR targets CoreCLR WASM (browser-wasm) globalization behavior so ICU sharding works in CoreCLR-based Blazor WASM scenarios, and broadens CoreCLR WASM test coverage for ICU/invariant globalization while improving native re-link support via runtime-pack includes.
Changes:
- Default CoreCLR browser-wasm to
InvariantGlobalization=falseand adjust CoreCLR native re-link inputs to prefer runtime-pack headers/includes. - Update runtime-pack/live-build packaging to include additional CoreCLR WASM artifacts (headers, symbols, host assets) needed for app re-link and distribution.
- Enable ICU-related Wasm.Build.Tests coverage on CoreCLR and filter out unsupported CoreCLR+AOT permutations.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/native/corehost/browserhost/libBrowserHost.footer.js | Adjusts explicit dependency handling for JS library linkage. |
| src/native/corehost/browserhost/CMakeLists.txt | Installs additional headers into sharedFramework for app native re-link scenarios. |
| src/mono/wasm/Wasm.Build.Tests/Wasm.Build.Tests.csproj | Updates CoreCLR trait filtering to allow more Wasm.Build.Tests coverage. |
| src/mono/wasm/Wasm.Build.Tests/InvariantGlobalizationTests.cs | Avoids CoreCLR+AOT execution path in invariant globalization tests. |
| src/mono/wasm/Wasm.Build.Tests/IcuTestsBase.cs | Adds CoreCLR-aware AOT option filtering for ICU test matrices. |
| src/mono/wasm/Wasm.Build.Tests/IcuTests.cs | Uses CoreCLR-aware AOT option filtering in ICU test data. |
| src/mono/wasm/Wasm.Build.Tests/IcuShardingTests.cs | Uses CoreCLR-aware AOT option filtering in sharding test data. |
| src/mono/wasm/Wasm.Build.Tests/IcuShardingTests2.cs | Uses CoreCLR-aware AOT option filtering in sharding-from-pack test data. |
| src/mono/browser/build/BrowserWasmApp.CoreCLR.targets | Sets globalization default and refines native re-link include/lib resolution. |
| src/libraries/sendtohelix-browser.targets | Aligns CoreCLR trait filtering with Wasm.Build.Tests changes. |
| src/installer/pkg/sfx/Microsoft.NETCore.App/Microsoft.NETCore.App.Runtime.CoreCLR.sfxproj | Ensures static libraries are included in packs for mobile/browser/wasi builds (TargetsMobile). |
| src/installer/pkg/sfx/Microsoft.NETCore.App/Directory.Build.props | Adds platform manifest entries for additional CoreCLR WASM assets. |
| eng/testing/scenarios/BuildWasmAppsJobsListCoreCLR.txt | Adds ICU/invariant globalization test classes to CoreCLR BuildWasmApps scenario list. |
| eng/liveBuilds.targets | Updates Browser/CoreCLR runtime file collection for live builds (symbols, extern-post JS, headers, .a handling). |
Comment on lines
20
to
24
| // libBrowserHostFn is too complex for acorn-optimizer.mjs to find the dependencies | ||
| // NOTE: wasm_load_icu_data is NOT listed here because it's only available when | ||
| // InvariantGlobalization != true. The loadIcuData() JS code is only called when | ||
| // ICU data is present, so the symbol doesn't need to be a hard link-time dependency. | ||
| let explicitDeps = [ |
Comment on lines
+285
to
+287
| <_EmccCFlags Include="-I"$([MSBuild]::NormalizePath('$(RepositoryEngineeringDir)', '..', 'src', 'coreclr', 'vm', 'wasm'))"" Condition="!Exists('$(MicrosoftNetCoreAppRuntimePackRidNativeDir)include/callhelpers.hpp') and '$(RepoRoot)' == '' and '$(RepositoryEngineeringDir)' != '' and Exists('$([MSBuild]::NormalizePath($(RepositoryEngineeringDir), .., src, coreclr, vm, wasm, callhelpers.hpp))')" /> | ||
| <_EmccCFlags Include="-I"$(RepoRoot)src/native"" Condition="!Exists('$(MicrosoftNetCoreAppRuntimePackRidNativeDir)include/minipal/entrypoints.h') and '$(RepoRoot)' != '' and Exists('$(RepoRoot)src/native/minipal/entrypoints.h')" /> | ||
| <_EmccCFlags Include="-I"$([MSBuild]::NormalizePath('$(RepositoryEngineeringDir)', '..', 'src', 'native'))"" Condition="!Exists('$(MicrosoftNetCoreAppRuntimePackRidNativeDir)include/minipal/entrypoints.h') and '$(RepoRoot)' == '' and '$(RepositoryEngineeringDir)' != '' and Exists('$([MSBuild]::NormalizePath($(RepositoryEngineeringDir), .., src, native, minipal, entrypoints.h))')" /> |
| private async Task TestInvariantGlobalization(Configuration config, bool aot, bool? invariantGlobalization, bool? isNativeBuild = null) | ||
| { | ||
| if (aot && IsCoreClrRuntime) | ||
| return; // CoreCLR WASM doesn't support AOT |
Comment on lines
+283
to
+285
| <PlatformManifestFileEntry Include="callhelpers.hpp" IsNative="true" /> | ||
| <PlatformManifestFileEntry Include="entrypoints.h" IsNative="true" /> | ||
| <PlatformManifestFileEntry Include="utils.h" IsNative="true" /> |
Open
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #127516
Blocked by #127281