-
Notifications
You must be signed in to change notification settings - Fork 5.9k
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
Update NDK to version 26 #48254
Update NDK to version 26 #48254
Conversation
8444c4e
to
7527312
Compare
This pull request has been changed to a draft. The currently pending flutter-gold status will not be able to resolve until a new commit is pushed or the change is marked ready for review again. |
3c6be9f
to
df834c0
Compare
5304452
to
16a4352
Compare
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.
RSLGTM
16a4352
to
4489900
Compare
@@ -22,6 +22,7 @@ Checks: >- | |||
-clang-analyzer-nullability.NullablePassedToNonnull, | |||
-clang-analyzer-nullability.NullReturnedFromNonnull, | |||
-clang-analyzer-nullability.NullableReturnedFromNonnull, | |||
-clang-analyzer-nullability.NullableDereferenced, |
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.
Is this intended for this PR?
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.
Yeah. The sysroot for the newer API level is triggering some false-positives in a few places: https://ci.chromium.org/ui/p/flutter/builders/try/Linux%20linux_clang_tidy/6722/overview.
@@ -464,7 +464,6 @@ foreach(layer_info, layers) { | |||
} | |||
if (is_android) { | |||
libs = [ | |||
"c++_static", # Note: C++ added by Flutter. |
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.
Why isn't this still needed?
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.
It doesn't exist in the sysroot, and seems to be unnecessary when linking against the clangrt libraries instead of libgcc.
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.
Checked in flutter/flutter#142240 by trying the device lab test Aaron added: https://ci.chromium.org/ui/p/flutter/builders/try/Linux_pixel_7pro%20hello_world_impeller/2/overview.
5a15428
to
150978e
Compare
@@ -464,7 +464,6 @@ foreach(layer_info, layers) { | |||
} | |||
if (is_android) { | |||
libs = [ | |||
"c++_static", # Note: C++ added by Flutter. |
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.
It doesn't exist in the sysroot, and seems to be unnecessary when linking against the clangrt libraries instead of libgcc.
@@ -464,7 +464,6 @@ foreach(layer_info, layers) { | |||
} | |||
if (is_android) { | |||
libs = [ | |||
"c++_static", # Note: C++ added by Flutter. |
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.
Checked in flutter/flutter#142240 by trying the device lab test Aaron added: https://ci.chromium.org/ui/p/flutter/builders/try/Linux_pixel_7pro%20hello_world_impeller/2/overview.
@@ -22,6 +22,7 @@ Checks: >- | |||
-clang-analyzer-nullability.NullablePassedToNonnull, | |||
-clang-analyzer-nullability.NullReturnedFromNonnull, | |||
-clang-analyzer-nullability.NullableReturnedFromNonnull, | |||
-clang-analyzer-nullability.NullableDereferenced, |
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.
Yeah. The sysroot for the newer API level is triggering some false-positives in a few places: https://ci.chromium.org/ui/p/flutter/builders/try/Linux%20linux_clang_tidy/6722/overview.
This PR adjusts the buildroot for Android NDK version 26. This NDK version only supports clang, so it contains some cleanups of old non-clang pathways. The companion flutter/engine PR is flutter/engine#48254.
150978e
to
315bcc9
Compare
@dnfield ptal |
…143086) flutter/engine@6807342...fafd8e5 2024-02-07 zanderso@users.noreply.github.com Update NDK to version 26 (flutter/engine#48254) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC chinmaygarde@google.com,rmistry@google.com,zra@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
This PR adjusts the GN build for a newer Android NDK. It relies on flutter/buildroot#822.