-
Notifications
You must be signed in to change notification settings - Fork 17
Build fixes for roll #135
Build fixes for roll #135
Conversation
|
Actually I don't think I need this. |
|
The new define isn't essential, but the missing build dep and enum definition will make this landable. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the new define/version check isn't needed, can we remove it? I don't understand it, but I will speculatively approve this PR to unblock the roll. (I slipped the include fix in one of my PRs that still waiting on review BTW)
|
We can remove it when we're certain that no bot will build using Xcode 12. Until then, the bots may be using an older macOS SDK that does not have the enum defined - so we conditionally define it to allow the TU to compile. @jmagman FYI |
|
Argh. I too eagerly merged this. the typedef isn't set up right |
|
|
||
| // TODO(dnfield): remove this declaration when we no longer need to build on | ||
| // machines with lower SDK versions than 11.0.s | ||
| #if !defined(MAC_OS_X_VERSION_11_0) || MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_X_VERSION_11_0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What exactly are you trying to do? Does API_AVAILABLE(macos(11.0)) work? Do you need an iOS availability constraint as MTLCommandEncoderErrorStateToString has API_AVAILABLE(ios(14.0), macos(11.0))
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See more of this continued in #136.
I'm trying to make sure that if we end up on a bot that has an older SDK that does not define this enum, we can still compile.
For flutter/engine#32658