-
Notifications
You must be signed in to change notification settings - Fork 25k
feat: update cmake to 3.31.6 version #54520
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
feat: update cmake to 3.31.6 version #54520
Conversation
cipolleschi
left a comment
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.
Thanks for fixing this!
|
@cipolleschi has imported this pull request. If you are a Meta employee, you can view this in D86873726. |
cortinico
left a comment
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.
Review automatically exported from Phabricator review in Meta.
This is a minor bump of CMake to reflect: - facebook/react-native#54520
|
I think we could merge this but I dislike that we will now have a bunch of extra warnings during the build: |
Because of this, I'd rather bump CMake version once we also bump to NDK 28 version. |
|
@cortinico do you have any eta for NDK 28 bump? 👀
Yeah, I can, but I'm constantly getting complains that my lib breaks compilation on Windows machine, and even if you tell developers how to overcome the problem they will still claim, that the issue should be fixed in the lib itself, so I thought that a better approach would be to bump the cmake version in RN itself... So yeah, out of curiosity decided to ask about NDK 28 plan 🙃 |
Why is your library creating this failure? Is it because the name of the library is too long? |
I don't have an ETA but I suspect this would land sometime next year, because it comes by default with AGP 9.x (that's when we generally bump the NDK version) which has several breaking changes in it. One thing you can try to do, is to open an issue on https://github.com/android/ndk and report the warning once we use CMake 3.31.6 + NDK 27 |
Eventually yes, it wasn't a problem when I used autogenerated code. But when I started to use custom C++ ShadowNodes the issue started to appear on Windows machines 🤷♂️
But that warning says that underlying cmake version will be removed. And it looks like |
Nope because the warning is happening inside the Android NDK toolchain files:
|
Summary:
On version
3.30.5and lower theninjaversion is1.10.2or lower:With this version in place we can not use long pathnames (that exceeds 260 symbols). The issue has been fixed in
ninjaversion1.12.+see ninja-build/ninja#1900In this PR I'm proposing to bump cmake to next latest release (i. e. we were on
.30and move to.31). In this PR the ninja version is1.12.1and it should fix the problem:This update should simplify a life for maintainers of projects that use c++ code, such as:
Current workaround for Windows users is to manually download a new CMake version and use it, but they don't like this approach and keep opening issues in various 3rd party libraries.
Let me know if you have any concerns 🙏
Changelog:
[GENERAL] [CHANGED] - Update cmake to 3.31.6
Test Plan:
Run CI and make sure it's not failed due to dependency update 😅