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

Upgrade to NDK r23c #4628

Closed
wants to merge 13 commits into from
Closed

Upgrade to NDK r23c #4628

wants to merge 13 commits into from

Conversation

kneth
Copy link
Contributor

@kneth kneth commented Jun 8, 2022

What, How & Why?

This closes #3905

It simplifies building by using the same CMake and Ninja installation to build for all supported platforms (see contrib/building.md for details).

Note: Using latest version of Ninja with Github Actions leads to a fatal error. An older version works just fine. I observed the same error as in ninja-build/ninja#2048.

☑️ ToDos

  • 📝 Changelog entry
  • [ ] 📝 Compatibility label is updated or copied from previous entry
  • [ ] 🚦 Tests
  • [ ] 🔀 Executed flexible sync tests locally if modifying flexible sync
  • [ ] 📱 Check the React Native/other sample apps work if necessary
  • [ ] 📝 Public documentation PR created or is not necessary
  • [ ] 💥 Breaking label has been applied or is not necessary

If this PR adds or changes public API's:

  • [ ] typescript definitions file is updated
  • [ ] jsdoc files updated
  • [ ] Chrome debug API is updated if API is available on React Native

Copy link
Contributor

@takameyer takameyer left a comment

Choose a reason for hiding this comment

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

👏🏼 That doesn't look trivial! Just a couple things. Other than that, LGTM

contrib/building.md Outdated Show resolved Hide resolved
armhf.Dockerfile Outdated Show resolved Hide resolved
Copy link
Contributor

@tomduncalf tomduncalf left a comment

Choose a reason for hiding this comment

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

Looks good as far as I can tell

armhf.Dockerfile Outdated Show resolved Hide resolved
@kraenhansen
Copy link
Member

My only concern with this, is that as we're shipping a pre-built binary, we need to ensure we're compiling with the same C++ standard library as the developer building their app. It's my understanding that the ABI of the standard library is provided by the NDK, is that correct? If so, how do we ensure these are matching out endusers? (this relates to the comment in the description of #4027)

@kneth kneth closed this Jan 24, 2023
@kneth kneth deleted the kneth/ndk-23 branch January 24, 2023 15:19
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 15, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Upgrade to Android NDK r23
4 participants