Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Conversation

hoyosjs
Copy link
Member

@hoyosjs hoyosjs commented Nov 18, 2021

This fixes the build in newer OSX SDKs. @vseanreesermsft reported the following issue under SDK 11.3:

2021-11-17T17:14:20.3631480Z Executing make install -j 4
2021-11-17T17:14:20.4256230Z [  0%] Generating dummy/eventprovdotnetruntime.cpp, dummy/eventprovdotnetruntimerundown.cpp, dummy/eventprovdotnetruntimestress.cpp, dummy/eventprovdotnetruntimeprivate.cpp
2021-11-17T17:14:20.4529460Z [  0%] Building C object src/corefx/System.Globalization.Native/CMakeFiles/System.Globalization.Native.dir/pal_calendarData.c.o
2021-11-17T17:14:20.4707690Z [  0%] Building C object src/corefx/System.Globalization.Native/CMakeFiles/System.Globalization.Native_Static.dir/pal_calendarData.c.o
2021-11-17T17:14:20.4710540Z Scanning dependencies of target coreclrpal
2021-11-17T17:14:20.5081520Z [  0%] Building CXX object src/pal/src/CMakeFiles/coreclrpal.dir/cruntime/file.cpp.o
2021-11-17T17:14:20.7220520Z In file included from /Users/runner/work/1/s/src/pal/src/cruntime/file.cpp:22:
2021-11-17T17:14:20.7322630Z In file included from /Users/runner/work/1/s/src/pal/src/include/pal/palinternal.h:620:
2021-11-17T17:14:20.7422970Z In file included from /Applications/Xcode_13.0.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX11.3.sdk/usr/include/c++/v1/stdlib.h:93:
2021-11-17T17:14:20.7526650Z /Applications/Xcode_13.0.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX11.3.sdk/usr/include/stdlib.h:139:3: error: 'atoll' is missing exception specification 'throw()'
2021-11-17T17:14:20.7625410Z          atoll(const char *);
2021-11-17T17:14:20.7727310Z          ^
2021-11-17T17:14:20.7829290Z /Users/runner/work/1/s/src/pal/inc/pal.h:4224:33: note: previous declaration is here
2021-11-17T17:14:20.7930400Z PALIMPORT long long int __cdecl atoll(const char *) THROW_DECL;

This fixes the build in newer OSX SDKs.
@hoyosjs hoyosjs requested a review from janvorli November 18, 2021 08:09
@hoyosjs hoyosjs changed the title Stop leaking atoll in pal Stop leaking atoll in pal and fix unicode header lookup Nov 18, 2021
@hoyosjs
Copy link
Member Author

hoyosjs commented Nov 18, 2021

@hoyosjs hoyosjs requested a review from jkotas November 18, 2021 20:30
@jkotas jkotas changed the title Stop leaking atoll in pal and fix unicode header lookup [release/3.1] Stop leaking atoll in pal and fix unicode header lookup Nov 18, 2021
@jeffschwMSFT
Copy link
Member

@safern is it safe to merge these now?

Copy link
Member

@jeffschwMSFT jeffschwMSFT left a comment

Choose a reason for hiding this comment

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

Approved. This is an infra change and we should treat as tell mode.

@safern
Copy link
Member

safern commented Nov 18, 2021

Branch is closed till Demeber 17th after the branding changes are made. I will mark as no-merge and will merge whenever the branch is open. Thanks for asking.

@safern safern added the * NO MERGE * The PR is not ready for merge yet (see discussion for detailed reasons) label Nov 18, 2021
@safern
Copy link
Member

safern commented Nov 19, 2021

I didn't realize this was blocking the official builds for 3.1. I will go ahead and merge it.

@safern safern removed the * NO MERGE * The PR is not ready for merge yet (see discussion for detailed reasons) label Nov 19, 2021
@safern safern merged commit fd5f28a into release/3.1 Nov 19, 2021
@safern safern deleted the juhoyosa/fix-atoll-leak branch November 19, 2021 00:12
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants