Skip to content

mobile: fix iOS build for open source contributors #39649

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

Merged
merged 16 commits into from
May 30, 2025

Conversation

fortuna
Copy link
Contributor

@fortuna fortuna commented May 27, 2025

Commit Message: mobile: fix iOS build for open source contributors

Additional Description:
Me and another contributor were unable to build iOS from our machines as external contributors. This fixes the build.

1) XCode version: The main change was to update the XCode version to 16.3 16.1, since 15.3 is too old and no longer runs on modern macOS. Stopped forcing the old XCode version 15.3, allowing the developer to build on modern machines.

2) CI macos version: The macos-14 machine doesn't have XCode 16.3. We could use 16.2, but developers should not be using macOS 14 for security reasons, so migrating to macos-15 better aligns with the developer environment.. Keeping macos-14 for now to make it work with the CI

3) Broken macros: We also had to remove the C++17 override for iOS, since it was causing variadic macros to fail. Adding --cxxopt=-Wno-error=variadic-macro-arguments-omitted fixes the C++17 issue, but using C++20 is more sane and avoids loads of warnings.

4) MINIMUM_IOS_VERSION: We also update MINIMUM_IOS_VERSION from 16.3 to 16.5. Without that the build was failing with:

In file included from external/envoy/source/common/json/json_streamer.h:11:

external/envoy/source/common/buffer/buffer_util.h:46:40: error: 'to_chars' is unavailable: introduced in iOS 16.5 simulator

   46 |     std::to_chars_result result = std::to_chars(buf, buf + sizeof(buf), number);

So the support for 16.3 was already absent.

5) Simulator version: Stopped forcing the simulator version, which prevents running on platforms that don't have the exact version of the simulator required.

Risk Level: Medium

Testing:

Built the xcframework:

./bazelw build ios_dist --config=ios

Ran the Swift app:

./bazelw run //examples/swift/hello_world:app --config=ios
image

Ran the Objective-C app:

./bazelw run //examples/swift/hello_world:app --config=ios

I'm counting on the CI to run the tests.

Docs Changes: None

Release Notes: None

Platform Specific Features: No new features.

[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional API Considerations:]

Copy link

Hi @fortuna, welcome and thank you for your contribution.

We will try to review your Pull Request as quickly as possible.

In the meantime, please take a look at the contribution guidelines if you have not done so already.

🐱

Caused by: #39649 was opened by fortuna.

see: more, trace.

Copy link

As a reminder, PRs marked as draft will not be automatically assigned reviewers,
or be handled by maintainer-oncall triage.

Please mark your PR as ready when you want it to be reviewed!

🐱

Caused by: #39649 was opened by fortuna.

see: more, trace.

@abeyad
Copy link
Contributor

abeyad commented May 27, 2025

cc @phlax

CI breaks b/c the change expects xcode 16.3 which isn't on the CI machines. @phlax can we try this PR when EngFlow upgrades the xcode versions to 16?

@fortuna
Copy link
Contributor Author

fortuna commented May 27, 2025

I updated CI to macos-15. Can you re-run it?

@abeyad
Copy link
Contributor

abeyad commented May 28, 2025

It still failed with Xcode 16.3 not found: https://github.com/envoyproxy/envoy/actions/runs/15286006822/job/42996227244

@phlax
Copy link
Member

phlax commented May 28, 2025

this is not being tested (mostly) - as suggested by the label - once the workers have the updated pool to test with, i will test it properly

@fortuna
Copy link
Contributor Author

fortuna commented May 28, 2025

It looks like some of the test failures are unrelated to my change. For example: https://github.com/envoyproxy/envoy/actions/runs/15286006891/job/42996283719.
Is that a flaky test?

Also, it doesn't seem like it has my last changes. For example, this one is trying to use macos-14, even though the code says macos-15: https://github.com/envoyproxy/envoy/actions/runs/15286006825/job/42996226179. Same for https://github.com/envoyproxy/envoy/actions/runs/15286006823/job/42996225211.

@phlax Can you (or someone else) rerun with the latest code (49c68a1)?

Thanks!

@fortuna fortuna marked this pull request as ready for review May 28, 2025 16:46
@RenjieTang
Copy link
Contributor

/retest

@phlax
Copy link
Member

phlax commented May 28, 2025

Is that a flaky test?

yep fraid so - one of many - in this case, it should be about to be fixed i believe

it doesn't seem like it has my last changes. For example, this one is trying to use macos-14, even though the code says macos-15

yeah, our CI is kinda locked down for this - we test elsewhere for these kinda changes

wrt updating the macos version - we have quite a few different things runnign - envoy and mobile + the rbe workers - so it really makes sense to wait until the workers have been updated (or rather a test pool added) - should be imminent

@fortuna
Copy link
Contributor Author

fortuna commented May 28, 2025

Ok, I changed the PR to use XCode 16.2, which is supported on macos-14. that should circumvent the need to update to macos-15.

/retest

@fortuna
Copy link
Contributor Author

fortuna commented May 28, 2025

@phlax How "imminent" is that? Are we talking about today, this week, a few weeks? Is there a way we can update the pool for you?

My concerning is that this is blocking our external contributions.

BTW, I tried reverting to macoos-14 but it didn't make a difference, because the runners seem to completely ignore the github CI config. However, it does seem your runner supports XCode 16.1, so I'll try that.

image

@fortuna
Copy link
Contributor Author

fortuna commented May 28, 2025

I made progress! Build went farther with XCode 16.1. But now it's not finding the iOS simulator 17.4.

Can someone tell me what simulator versions are available in the runner image?

@phlax
Copy link
Member

phlax commented May 28, 2025

not sure about host vms - but i know the workers have only 17.4 - that is current holdup - the existing workers have additional xcode added, but no simulator update

fortuna added 10 commits May 28, 2025 17:09
Signed-off-by: Vinicius Fortuna <[email protected]>
Signed-off-by: Vinicius Fortuna <[email protected]>
Signed-off-by: Vinicius Fortuna <[email protected]>
Signed-off-by: Vinicius Fortuna <[email protected]>
Signed-off-by: Vinicius Fortuna <[email protected]>
Signed-off-by: Vinicius Fortuna <[email protected]>
Signed-off-by: Vinicius Fortuna <[email protected]>
Signed-off-by: Vinicius Fortuna <[email protected]>
Signed-off-by: Vinicius Fortuna <[email protected]>
Signed-off-by: Vinicius Fortuna <[email protected]>
fortuna added 4 commits May 28, 2025 17:45
Signed-off-by: Vinicius Fortuna <[email protected]>
Signed-off-by: Vinicius Fortuna <[email protected]>
Signed-off-by: Vinicius Fortuna <[email protected]>
@abeyad
Copy link
Contributor

abeyad commented May 30, 2025

#39658 was just merged, so there are some merge conflicts that need resolving

@fortuna
Copy link
Contributor Author

fortuna commented May 30, 2025

Merge conflicts fixed. It looks like the tests are running.

@fortuna
Copy link
Contributor Author

fortuna commented May 30, 2025

@abeyad I want to let you know that I cloned the repo on a second macbook and ios still built successfully and I was also able to run the example app on the simulator using blaze. And it seems all tests passed

@abeyad
Copy link
Contributor

abeyad commented May 30, 2025

awesome, thanks so much @fortuna !

@abeyad abeyad merged commit d504914 into envoyproxy:main May 30, 2025
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants