[mono][NativeAOT] System.Globalization.Native build improvements by filipnavara · Pull Request #82393 · dotnet/runtime

filipnavara · 2023-02-20T12:50:39Z

Fixes #82389

Fixes build of System.Globalization.Native for iOS-like, browser, and wasi platforms
Removes ICU shim from static builds of Mono runtime and builds it as part of libs.native subset
Switches browser/wasi to use the static libraries

src/native/libs/System.Globalization.Native/CMakeLists.txt

src/native/libs/System.Globalization.Native/pal_collation.c

ghost · 2023-02-20T13:25:48Z

Tagging subscribers to 'os-ios': @steveisok, @akoeplinger
See info in area-owners.md if you want to be subscribed.

Issue Details

Fixes #82389

Author:	filipnavara
Assignees:	-
Labels:	`os-ios`, `community-contribution`
Milestone:	-

ghost · 2023-02-20T13:25:55Z

Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas
See info in area-owners.md if you want to be subscribed.

Issue Details

Fixes #82389

Author:	filipnavara
Assignees:	-
Labels:	`os-ios`, `community-contribution`, `area-NativeAOT-coreclr`
Milestone:	-

filipnavara · 2023-02-20T13:38:09Z

The build failures are relevant. I'll try to remove the duplicate build from Mono sources.

(For posterity, took me a while to trace the Mono ICU build all the way to #35790. I think there's no reason to link ICU shim directly into the runtime anymore. This is likely a relict of the same thing being done in CoreCLR in the past.)

filipnavara · 2023-02-22T20:54:47Z

The WASI part has overlap with #82444.

src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.Unix.targets

kotlarmilos · 2023-04-10T17:49:21Z

@filipnavara is there anything for this PR or is it waiting for a review?

filipnavara · 2023-04-10T18:03:42Z

@filipnavara is there anything for this PR or is it waiting for a review?

Waiting for review.

kotlarmilos · 2023-04-11T09:02:43Z

/cc: @akoeplinger @MichalStrehovsky @ivanpovazan

ivanpovazan

Minor note: The title seems outdated as the PR changes now affect mono builds and other platforms as well. I don't have a strong opinion but something like [mono][NativeAOT] System.Globalization.Native build improvements might be more suitable.

Other than that, LGTM.
Thank you very much!

PS I would leave this to @akoeplinger for final approval.

The coreclr CMake "includes" the src/native directory so it doesn't go through build-native.sh.

akoeplinger · 2023-04-25T18:46:32Z

Failures are unrelated.

lewing · 2023-04-25T19:22:13Z

did anyone check the final post linking binary sizes on this?

filipnavara · 2023-04-25T19:27:49Z

did anyone check the final post linking binary sizes on this?

Any specific configuration you are concerned about?

For NativeAOT/iOS there's nothing to compare against since it didn't work previously. For everything else it shuffles a bit where/how the globalization shim is built but it doesn't really add or remove any code, or at least not intentionally.

steveisok · 2023-04-25T19:44:46Z

did anyone check the final post linking binary sizes on this?

Quick spot check on one of Alex's test official builds show no change. I'll check again when a real one gets through.

Changes: dotnet/installer@f876fb5...0ce8918 Changes: dotnet/runtime@ab2b80d...9a7db55 Changes: dotnet/emsdk@2327f6b...31a4a87 Changes: dotnet/cecil@9a7551f...80d3f38 Updates: * Microsoft.Dotnet.Sdk.Internal: from 8.0.100-preview.4.23224.14 to 8.0.100-preview.5.23228.7 * Microsoft.NETCore.App.Ref: from 8.0.0-preview.4.23221.1 to 8.0.0-preview.4.23225.14 * Microsoft.NET.Workload.Emscripten.Current.Manifest-8.0.100.Transport: from 8.0.0-preview.4.23218.1 to 8.0.0-preview.4.23219.1 * Microsoft.NET.ILLink.Tasks: from 8.0.0-preview.4.23221.1 to 8.0.0-preview.4.23225.14 * Microsoft.DotNet.Cecil: from 0.11.4-alpha.23178.1 to 0.11.4-alpha.23218.2 Other changes: A new `libSystem.Globalization.Native.so` file was introduced in the Mono runtime pack in: dotnet/runtime#82393 Android doesn't actually need this file. * Introduce a new private `@(_MonoExcludedLibraries)` item group * Add exclusion for `libSystem.Globalization.Native` Co-authored-by: Jonathan Peppers <[email protected]>

ghost added the community-contribution Indicates that the PR has been added by a community member label Feb 20, 2023

filipnavara commented Feb 20, 2023

View reviewed changes

src/native/libs/System.Globalization.Native/CMakeLists.txt Outdated Show resolved Hide resolved

filipnavara commented Feb 20, 2023

View reviewed changes

src/native/libs/System.Globalization.Native/pal_collation.c Outdated Show resolved Hide resolved

filipnavara requested review from akoeplinger and ivanpovazan February 20, 2023 12:52

ivanpovazan added the os-ios Apple iOS label Feb 20, 2023

ivanpovazan added the area-NativeAOT-coreclr label Feb 20, 2023

ivanpovazan changed the title ~~Add System.Globalization.Native builds for iOS-like platforms~~ [NativeAOT] Add System.Globalization.Native builds for iOS-like platforms Feb 20, 2023

This comment was marked as outdated.

Sign in to view

filipnavara closed this Feb 20, 2023

build-analysis bot mentioned this pull request Feb 20, 2023

Roslyn source generator crash on mono/linux/arm64 #81123

Closed

filipnavara reopened this Feb 20, 2023

filipnavara force-pushed the icu-ios branch from 532c139 to 6166520 Compare February 20, 2023 21:44

build-analysis bot mentioned this pull request Feb 21, 2023

Tracking issue for CI build timeouts #76454

Closed

filipnavara force-pushed the icu-ios branch 2 times, most recently from d59baf0 to bc07402 Compare February 22, 2023 08:44

am11 reviewed Feb 23, 2023

View reviewed changes

src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.Unix.targets Outdated Show resolved Hide resolved

filipnavara added 9 commits February 23, 2023 21:36

Add System.Globalization.Native builds for iOS-like platforms

e9805a2

Update package manifest

9092d1c

Move copying of ICU static libs and data files from Mono to libs.native

e78a32d

Mono: Link static ICU shim only into Mono shared libraries

c83a2dc

Include ICU .dat files in LibrariesRuntimeFiles

628cb70

Build System.Globalization.Native on Browser/WASI

3689be7

Fix configuration options for WASM/WASI builds

2edeebd

Update src/libraries/native-binplace.proj to include ICU .dat files

5ec149f

List correct libraries in link_libraries

3156ff7

filipnavara added 2 commits March 4, 2023 11:54

Remove unnecessary empty line

5d0700a

Wrap icu_trace_data in #if defined(ICU_TRACING)

8e07a43

build-analysis bot mentioned this pull request Mar 4, 2023

IOException running NuGet-Migrations during tests in dotnet CLI first run #80619

Closed

filipnavara requested a review from akoeplinger March 7, 2023 07:24

kotlarmilos requested review from am11 and pavelsavara April 11, 2023 09:02

ivanpovazan approved these changes Apr 11, 2023

View reviewed changes

akoeplinger added 2 commits April 24, 2023 13:34

Merge branch 'main' into icu-ios

80cc2dc

Fix "unused parameter 'symbolVersion'" error

3c823b8

akoeplinger changed the title ~~[NativeAOT] Add System.Globalization.Native builds for iOS-like platforms~~ [mono][NativeAOT] System.Globalization.Native build improvements Apr 24, 2023

akoeplinger approved these changes Apr 25, 2023

View reviewed changes

Fix NativeAOT build for Apple Mobile

42ae535

The coreclr CMake "includes" the src/native directory so it doesn't go through build-native.sh.

akoeplinger merged commit 0fc8978 into dotnet:main Apr 25, 2023

filipnavara deleted the icu-ios branch April 25, 2023 18:51

jonathanpeppers mentioned this pull request Apr 28, 2023

[main] Update dependencies from dotnet/installer dotnet/android#7994

Merged

SingleAccretion mentioned this pull request May 19, 2023

NativeAOT-LLVM: Merge runtime to 17 May 2023 dotnet/runtimelab#2283

Merged

ghost locked as resolved and limited conversation to collaborators May 26, 2023

Conversation

filipnavara commented Feb 20, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ghost commented Feb 20, 2023

Uh oh!

ghost commented Feb 20, 2023

Uh oh!

filipnavara commented Feb 20, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as outdated.

filipnavara commented Feb 22, 2023

Uh oh!

Uh oh!

kotlarmilos commented Apr 10, 2023

Uh oh!

filipnavara commented Apr 10, 2023

Uh oh!

kotlarmilos commented Apr 11, 2023

Uh oh!

ivanpovazan left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

akoeplinger commented Apr 25, 2023

Uh oh!

lewing commented Apr 25, 2023

Uh oh!

filipnavara commented Apr 25, 2023

Uh oh!

steveisok commented Apr 25, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

filipnavara commented Feb 20, 2023 •

edited

Loading

filipnavara commented Feb 20, 2023 •

edited

Loading

ivanpovazan left a comment •

edited

Loading