Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Conversation

@zanderso
Copy link
Member

@zanderso zanderso commented Nov 21, 2023

This PR adjusts the GN build for a newer Android NDK. It relies on flutter/buildroot#822.

@flutter-dashboard
Copy link

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.

@zanderso zanderso force-pushed the android-34v7 branch 4 times, most recently from 3c6be9f to df834c0 Compare January 11, 2024 16:34
@zanderso zanderso force-pushed the android-34v7 branch 3 times, most recently from 5304452 to 16a4352 Compare January 30, 2024 22:08
@zanderso zanderso marked this pull request as ready for review January 31, 2024 16:00
Copy link
Contributor

@reidbaker reidbaker left a comment

Choose a reason for hiding this comment

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

RSLGTM

-clang-analyzer-nullability.NullablePassedToNonnull,
-clang-analyzer-nullability.NullReturnedFromNonnull,
-clang-analyzer-nullability.NullableReturnedFromNonnull,
-clang-analyzer-nullability.NullableDereferenced,
Copy link
Contributor

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?

Copy link
Member Author

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.

}
if (is_android) {
libs = [
"c++_static", # Note: C++ added by Flutter.
Copy link
Contributor

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?

Copy link
Member Author

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.

Copy link
Member Author

Choose a reason for hiding this comment

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

@zanderso zanderso force-pushed the android-34v7 branch 2 times, most recently from 5a15428 to 150978e Compare February 5, 2024 22:17
}
if (is_android) {
libs = [
"c++_static", # Note: C++ added by Flutter.
Copy link
Member Author

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.

}
if (is_android) {
libs = [
"c++_static", # Note: C++ added by Flutter.
Copy link
Member Author

Choose a reason for hiding this comment

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

-clang-analyzer-nullability.NullablePassedToNonnull,
-clang-analyzer-nullability.NullReturnedFromNonnull,
-clang-analyzer-nullability.NullableReturnedFromNonnull,
-clang-analyzer-nullability.NullableDereferenced,
Copy link
Member Author

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.

zanderso added a commit to flutter/buildroot that referenced this pull request Feb 5, 2024
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.
@zanderso
Copy link
Member Author

zanderso commented Feb 5, 2024

@dnfield ptal

@zanderso zanderso merged commit fafd8e5 into flutter:main Feb 7, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 7, 2024
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Feb 7, 2024
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
@zanderso zanderso deleted the android-34v7 branch February 7, 2024 20:47
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants