Skip to content
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

Jump to bottom

Move System.Globalization.Native under libraries #875

Merged
merged 1 commit into from
Dec 17, 2019
Merged

Move System.Globalization.Native under libraries #875

merged 1 commit into from
Dec 17, 2019

Conversation

jkotas
Copy link
Member

@jkotas jkotas commented Dec 14, 2019

@jkotas
Copy link
Member Author

jkotas commented Dec 14, 2019

cc @filipnavara

@jkotas jkotas mentioned this pull request Dec 16, 2019
Closed
@jkotas
Move System.Globalization.Native under libraries
9154ddf 
- 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
@jkotas jkotas merged commit 5cc6019 into dotnet:master Dec 17, 2019
@am11
Copy link
Member

am11 commented Dec 18, 2019

@jkotas, thanks for this. two questions related to current structure:

  • can src/coreclr/src/coreclr/hosts be reduced to src/coreclr/src/hosts?
  • what is the purpose of src/coreclr/inc/clr_std/*; can it be replaced by real C++11 std?

i can follow up on these (especially the second one which i am handling via memset fixes). :)

@jkotas
Copy link
Member Author

jkotas commented Dec 19, 2019

can src/coreclr/src/coreclr/hosts be reduced to src/coreclr/src/hosts

I think so. It looks like a cosmetic change only, no change in how things are actually built.

what is the purpose of src/coreclr/inc/clr_std/*; can it be replaced by real C++11 std?

The real C++ 11 std is not compatible with PAL headers. We would need to eliminate all definitions in PAL that conflict with std system headers first (ie make the PAL purely additive). Once that happens, this clr_std can go away.

@jkotas jkotas deleted the glob-native branch December 21, 2019 06:07
@am11
Copy link
Member

am11 commented Dec 26, 2019

Thanks! Yup, bringing hosts directory to one level above was just a structural change i was thinking to remove this one-of-a-kind directory (this PR removed the other one: src/coreclr/src/corefx).

I will try to figure out smooth way to remove stuff from under #ifndef PAL_STDCPP_COMPATs, and include std headers at usage sites.

@ghost ghost locked as resolved and limited conversation to collaborators Dec 11, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@safern safern safern approved these changes

Assignees
No one assigned
Labels
area-System.Globalization
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@jkotas @am11 @safern @Dotnet-GitSync-Bot