-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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.
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
Move System.Globalization.Native to CoreFX repository? #11966
Comments
@marek-safar Do you plan to use System.Globalization.Native in Mono? |
I believe it is already used in Mono since it's referenced from the shared part of CoreLib and GlobalizationMode implementation in Mono. |
@jkotas yes, we do but cannot say in which form exactly right now but if we are going to customize it for a specific platform I'd prefer to do it in a shared location as well. |
Just making sure - does corefx/src/Native count as shared location? |
Yes, I consider corefx to be fully shared location (all flavours to be built there) |
I think we should sit on this one for a bit until we have a better idea about the form that it will be used by Mono. |
Another advantage of this move would be that we can clean up some helpers (such as https://github.com/dotnet/coreclr/blob/af83b8ad6f/src/corefx/System.Globalization.Native/pal_compiler.h) and utilize others for safe math in the CoreFX repo. |
We plan to look at adjusting repo boundaries for .NET 5. |
- Delete Microsoft.NETCore.Native package - Unify CMake files with the rest of libraries/native - Fix pedantic warnings - Comment out assert (#946) - Delete dlclose during shutdown. It is not safe to unload any libraries that the runtime depends on. - Install ICU for libraries build in the CI Fixes https://github.com/dotnet/coreclr/issues/22391
According to Git history the original rationale for keeping it in CoreCLR repository is that System.Private.CoreLib depends on it. That's the case now even for System.Native that resides in CoreFX, so that reason seems to be passé. Furthermore it's shared by CoreRT so it adds one more dependency to publish and track.
The text was updated successfully, but these errors were encountered: