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.

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

v1.17.0 + requires macOS deployment target >= than 13.3 due to C++ 20 #21033

Closed
ola-ableton opened this issue Jun 13, 2024 · 12 comments · Fixed by #22138
Closed

v1.17.0 + requires macOS deployment target >= than 13.3 due to C++ 20 #21033

ola-ableton opened this issue Jun 13, 2024 · 12 comments · Fixed by #22138
Labels
api issues related to all other APIs: C, C++, Python, etc. stale issues that have not been addressed in a while; categorized by a bot

Comments

@ola-ableton
Copy link

ola-ableton commented Jun 13, 2024

Describe the issue

It seems that moving to C++20 for macOS builds has bumped the minimum macOS that the library can run on. I didn't see this mentioned in the release notes. macOS 13.3 is not that old.

https://stackoverflow.com/questions/76932735/unable-to-use-to-chars-with-clang-on-macos

In file included from /onnxruntime/onnxruntime/include/onnxruntime/core/common/common.h:23:
In file included from /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX14.5.sdk/usr/include/c++/v1/chrono:800:
In file included from /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX14.5.sdk/usr/include/c++/v1/__chrono/formatter.h:23:
In file included from /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX14.5.sdk/usr/include/c++/v1/__chrono/ostream.h:30:
In file included from /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX14.5.sdk/usr/include/c++/v1/__format/format_functions.h:29:
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX14.5.sdk/usr/include/c++/v1/__format/formatter_floating_point.h:73:32: error: 'to_chars' is unavailable: introduced in macOS 13.3
  to_chars_result __r = _VSTD::to_chars(__first, __last, __value, __fmt, __precision);

To reproduce

try and compile explicitly setting a lower deployment target with --apple_deploy_target

Urgency

No response

Platform

Mac

OS Version

11

ONNX Runtime Installation

Built from Source

ONNX Runtime Version or Commit ID

1.17.0

ONNX Runtime API

Python

Architecture

X64

Execution Provider

Default CPU

Execution Provider Library Version

No response

@ola-ableton
Copy link
Author

0141e27

@ola-ableton ola-ableton changed the title v1.18.0 requires macOS deployment target >= than 13.3 due to C++ 20 v1.17.0 + requires macOS deployment target >= than 13.3 due to C++ 20 Jun 13, 2024
@snnn
Copy link
Member

snnn commented Jun 13, 2024

So, you were not using our official packages, you built the binaries from source by yourself and you didn't use the "--apple_deploy_target" flag?

@ola-ableton
Copy link
Author

I am building a custom build but I did use the -apple_deploy_target.

We need to support macOS 11 or higher but if you specify

-apple_deploy_target=11.0

the build will fail with the above error

It's necessary to set at least

-apple_deploy_target=13.3

for the C++ 20 code to compile.

This will be a problem for many other users I imagine. Fortunately we can stay on version 1.16.3 which is the last release that didn't require C++ 20 on macOS.

@snnn
Copy link
Member

snnn commented Jun 13, 2024

As long as you need to deploy the code to a different machine, you always need to set the deploy target's OS version. No matter what ONNX Runtime version you use, or no matter the target OS is Windows or MacOS. Because there is no other way to get this information.

@snnn snnn closed this as not planned Won't fix, can't repro, duplicate, stale Jun 13, 2024
@ola-ableton
Copy link
Author

I understand that you need to set the deploy target to the minimum OS that you want to support. That wasn't the reason for my issue.

Since 0141e27 onnxruntime can only target macOS 13.3 or higher due to the introduction of C++ 20 for macOS. That really limits the machines that are supported since macOS 13.3 is not that old. Many users don't upgrade their operating systems regularly, apple still supports macOS 12 for instance - https://endoflife.date/macos.

This limitation is not mentioned in the commit or the release notes for onnxruntime 1.17.0 or higher.

Perhaps this issue doesn't appear if you compile against a different macOS SDK. I have been compiling against the 14.5 SDK that comes with the latest stable Xcode, but if i am not mistaken I don't think your onnxruntime builds of 1.17.0 + are suitable for macOS < 13.3. If that is intended, then it should be mentioned in the release notes. If it's not intended then maybe it's worth reconsidering if it's necessary to compile with C++20 just on macOS, since the other platforms don't use it yet.

I'm happy to be corrected if I am wrong here. Thanks for your work maintaining onnxruntime.

@snnn snnn reopened this Jun 13, 2024
@snnn
Copy link
Member

snnn commented Jun 13, 2024

Interesting .. I thought it works fine with macOS 11. Because in our pipeline we set MACOSX_DEPLOYMENT_TARGET to '11.0'.
See: https://github.com/microsoft/onnxruntime/blob/main/tools/ci_build/github/azure-pipelines/templates/mac-cpu-packing-jobs.yml#L34

@snnn
Copy link
Member

snnn commented Jun 13, 2024

Our binaries were built on macOS 12 with XCode 14.2.
https://github.com/actions/runner-images/blob/main/images/macos/macos-12-Readme.md
If it doesn't support macOS 12, we wouldn't be able to run the tests.

@snnn
Copy link
Member

snnn commented Jun 13, 2024

Could it because your macOS SDK version is too new?

@ola-ableton
Copy link
Author

It's possible. I will try and get an older SDK tomorrow and see if compiling against that makes any difference

@sophies927 sophies927 added the api issues related to all other APIs: C, C++, Python, etc. label Jun 13, 2024
Copy link
Contributor

This issue has been automatically marked as stale due to inactivity and will be closed in 30 days if no further activity occurs. If further support is needed, please provide an update and/or more details.

@skottmckay
Copy link
Contributor

skottmckay commented Sep 13, 2024

Two things are required to make this work.

This condition needs to be tighter about usage of std::chrono for macOS. The SDK version must be > 14.4 as per the current check, but the min version must also be >= 13.3.

// TODO: When other compilers support std::chrono::operator<<, update this.
// TODO: Check support for other compilers' version before enable C++20 for other compilers.
// Xcode added support for C++20's std::chrono::operator<< in SDK version 14.4.
#if __cplusplus >= 202002L && __MAC_OS_X_VERSION_MAX_ALLOWED >= 140400L
namespace timestamp_ns = std::chrono;
#else
namespace timestamp_ns = ::date;
#endif

Something like this might work for a macOS build. Not sure if additional conditions are required for iOS/mac-catalyst.

// Xcode added support for C++20's std::chrono::operator<< in SDK version 14.4, but the target macOS version must be
// >= 13.3 for it to be used.
#if __cplusplus >= 202002L && \
  (!defined(__MAC_OS_X_VERSION_MAX_ALLOWED) || __MAC_OS_X_VERSION_MAX_ALLOWED >= 140400L) && \
  (!defined(__MAC_OS_X_VERSION_MIN_REQUIRED) || __MAC_OS_X_VERSION_MIN_REQUIRED>= 130300L)
namespace timestamp_ns = std::chrono;
#else
namespace timestamp_ns = ::date;
#endif

Additionally there needs to be disambiguation of date::operator<< and std::chrono::operator<< here and here for a macOS build when C++20 is enabled but we're not using std::chrono.

That can be done by explicitly outputting the timestamp as a first step like this:

  timestamp_ns::operator<<(msg, timestamp);
  msg << " [" << message.SeverityPrefix() << ":" << message.Category() << ":" << logger_id << ", "
      << message.Location().ToString() << "] " << message.Message();

@skottmckay
Copy link
Contributor

It got a little bit more complicated to handle mac-catalyst as well, but hopefully #22138 fixes this completely.

snnn pushed a commit that referenced this issue Sep 20, 2024
### Description
Fix usage of c++ std::chrono::operator<< in mac builds for wider range
of xcode/targets.

### Motivation and Context

#21033
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api issues related to all other APIs: C, C++, Python, etc. stale issues that have not been addressed in a while; categorized by a bot
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants