Consider warnings-as-errors for plugin CI builds

@blasten

This is something I was thinking about based on this comment, but would apply to iOS as well (and macOS): the way the native code for plugins on some platforms is built, the build environment may change in ways that create warnings (e.g., the way the Flutter tool updates the min deployment version on iOS to match the app's min deployment version frequently creates deprecation warnings that don't exist in our builds), so we generally don't want to make warnings errors to avoid breaking plugin clients. However, that means warnings aren't surfaced in our CI, which in practice means we don't notice when new ones are added by PRs, which makes warnings kind of useless.

I think we should consider either:

modifying the build on disk before building in CI to add -Werror, or
add build output parsing to notice errors, and fail the CI step.

Both have some maintenance complexity, but continuing to ignore warnings on the native side doesn't seem like a good option.

/cc @blasten @jmagman

The Analyze test should already be doing this for iOS and macOS...
https://github.com/flutter/plugins/blob/fb6622092bb8f1a56b701f6c6ede551b6c986d06/script/tool/lib/src/xcode_analyze_command.dart#L100

          'GCC_TREAT_WARNINGS_AS_ERRORS=YES',

      xcode_analyze_script:
        - ./script/tool_runner.sh xcode-analyze --ios

https://github.com/flutter/plugins/blob/176cfb8083bc85eb071599ee8f5ef65b732b8ffd/.cirrus.yml#L280-L281
https://github.com/flutter/plugins/blob/176cfb8083bc85eb071599ee8f5ef65b732b8ffd/.cirrus.yml#L308-L309

Run here:
https://cirrus-ci.com/task/5813760171442176?logs=xcode_analyze#L14

The build context is sightly different though, right? For instance, IIRC it didn't flag availability checks that we didn't need after bumping the example app minimum versions. (Or maybe that's just not a warning under our current settings?)

See also: --fatal-warnings in flutter build, and related discussion in flutter-team-archive/plugins#7062

See also: --fatal-warnings in flutter build, and related discussion in flutter/plugins#7062

I spun this out into #121819

For this issue, all that's left is fully enabling Android warnings as errors. flutter/packages#3293 sets up the structure and does it for some packages, but we need to get it enabled for the remaining packages as well. Each of these has the arguments commented out in build.gradle with a reference to this issue:

camera_android - [camera_android] Fix camera android deprecation warning for CamcorderProfile.get() packages#3273
espresso - [espresso] Enable warnings as errors packages#3414
google_maps_flutter_android - [google_maps_flutter] Enable -Werror for Android packages#3399
in_app_purchase - [in_app_purchase] Enable -Werror for Android packages#3403
video_player_android - [video_player_android] Enable -Werror and use dependency override packages#3412
webview_flutter_android - [webview_flutter] Enable warnings-as-errors on Android packages#3356

[ci+various] Partially enable javac warning checks (#3293)

@stuartmorgan

@stuartmorgan So I went ahead and enabled the compiler options for video_player_android, just to see what the fallout would look like.

I get a bunch of errors relating to annotations inside ExoPlayer itself:

Logs

navaronbracke@MacBook-Pro-van-Navaron example % flutter build apk --release     

💪 Building with sound null safety 💪

Warning: unexpected element (uri:"", local:"extension-level"). Expected elements are <{}codename>,<{}layoutlib>,<{}api-level>
Warning: unexpected element (uri:"", local:"base-extension"). Expected elements are <{}codename>,<{}layoutlib>,<{}api-level>
/Users/navaronbracke/.gradle/caches/transforms-3/17d52bfcf76ba92f8a9219102f650808/transformed/jetified-exoplayer-common-2.18.1-api.jar(/com/google/android/exoplayer2/C.class): warning: Cannot find annotation method 'replacement()' in type 'InlineMe': class file for com.google.errorprone.annotations.InlineMe not found
/Users/navaronbracke/.gradle/caches/transforms-3/17d52bfcf76ba92f8a9219102f650808/transformed/jetified-exoplayer-common-2.18.1-api.jar(/com/google/android/exoplayer2/C.class): warning: Cannot find annotation method 'imports()' in type 'InlineMe'
/Users/navaronbracke/.gradle/caches/transforms-3/17d52bfcf76ba92f8a9219102f650808/transformed/jetified-exoplayer-common-2.18.1-api.jar(/com/google/android/exoplayer2/C.class): warning: Cannot find annotation method 'replacement()' in type 'InlineMe'
/Users/navaronbracke/.gradle/caches/transforms-3/17d52bfcf76ba92f8a9219102f650808/transformed/jetified-exoplayer-common-2.18.1-api.jar(/com/google/android/exoplayer2/C.class): warning: Cannot find annotation method 'imports()' in type 'InlineMe'
/Users/navaronbracke/.gradle/caches/transforms-3/17d52bfcf76ba92f8a9219102f650808/transformed/jetified-exoplayer-common-2.18.1-api.jar(/com/google/android/exoplayer2/C.class): warning: Cannot find annotation method 'replacement()' in type 'InlineMe'
/Users/navaronbracke/.gradle/caches/transforms-3/17d52bfcf76ba92f8a9219102f650808/transformed/jetified-exoplayer-common-2.18.1-api.jar(/com/google/android/exoplayer2/C.class): warning: Cannot find annotation method 'imports()' in type 'InlineMe'
/Users/navaronbracke/.gradle/caches/transforms-3/17d52bfcf76ba92f8a9219102f650808/transformed/jetified-exoplayer-common-2.18.1-api.jar(/com/google/android/exoplayer2/C.class): warning: Cannot find annotation method 'replacement()' in type 'InlineMe'
/Users/navaronbracke/.gradle/caches/transforms-3/17d52bfcf76ba92f8a9219102f650808/transformed/jetified-exoplayer-common-2.18.1-api.jar(/com/google/android/exoplayer2/C.class): warning: Cannot find annotation method 'imports()' in type 'InlineMe'
/Users/navaronbracke/.gradle/caches/transforms-3/17d52bfcf76ba92f8a9219102f650808/transformed/jetified-exoplayer-common-2.18.1-api.jar(/com/google/android/exoplayer2/C.class): warning: Cannot find annotation method 'replacement()' in type 'InlineMe'
/Users/navaronbracke/.gradle/caches/transforms-3/17d52bfcf76ba92f8a9219102f650808/transformed/jetified-exoplayer-common-2.18.1-api.jar(/com/google/android/exoplayer2/C.class): warning: Cannot find annotation method 'imports()' in type 'InlineMe'
/Users/navaronbracke/.gradle/caches/transforms-3/17d52bfcf76ba92f8a9219102f650808/transformed/jetified-exoplayer-common-2.18.1-api.jar(/com/google/android/exoplayer2/util/Util.class): warning: Cannot find annotation method 'value()' in type 'EnsuresNonNull': class file for org.checkerframework.checker.nullness.qual.EnsuresNonNull not found
/Users/navaronbracke/.gradle/caches/transforms-3/17d52bfcf76ba92f8a9219102f650808/transformed/jetified-exoplayer-common-2.18.1-api.jar(/com/google/android/exoplayer2/util/Util.class): warning: Cannot find annotation method 'value()' in type 'EnsuresNonNull'
error: warnings found and -Werror specified
1 error
12 warnings

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':video_player_android:compileReleaseJavaWithJavac'.
> Compilation failed; see the compiler error output for details.

* Try:
Run with --stacktrace option to get the stack trace. Run with --info or --debug option to get more log output. Run with --scan to get full insights.

* Get more help at https://help.gradle.org

BUILD FAILED in 36s
Running Gradle task 'assembleRelease'...                           38.0s
Gradle task assembleRelease failed with exit code 1

It builds fine if I remove the compiler args. We might need to bump exoplayer it seems.
I'll see what it gives when I bump to 2.18.2.

I had a similar missing class file issue with the maps plugin, and was able to work around it by injecting a dependency that defined the missing pieces from the example; I'll have that PR up sometime next week (once I fix the warnings that were married by that initial probably).

@navaronbracke

@navaronbracke This is what I did for the similar maps issue. It wasn't the same symbols, but if you can find out what package defines the symbols you are missing the same injection approach would likely work.

Thanks! I'll take a stab at video_player_android with this in mind.

@stuartmorgan

@stuartmorgan Apparently the warnings I get (see the logs in #91868 (comment)) come from two different dependencies:

The InlineMe symbol from com.google.errorprone.annotations
The EnsuresNonNull symbol from org.checkerframework.checker.nullness.qual

Turns out those are excluded here? https://github.com/google/ExoPlayer/blob/release-v2/library/common/build.gradle#L28-L37

The versions of those dependencies are defined here: https://github.com/google/ExoPlayer/blob/r2.18.1/constants.gradle

Edit: Apparently, besides the dependency issue, there were no errors. phew

This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.

Navigation Menu

Repository navigation

Consider warnings-as-errors for plugin CI builds #91868

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Consider warnings-as-errors for plugin CI builds #91868

Description

Activity

jmagman commented on Oct 15, 2021

stuartmorgan-g commented on Oct 15, 2021

stuartmorgan-g commented on Jan 31, 2023

stuartmorgan-g commented on Mar 2, 2023

navaronbracke commented on Mar 5, 2023

stuartmorgan-g commented on Mar 5, 2023

stuartmorgan-g commented on Mar 7, 2023

navaronbracke commented on Mar 7, 2023

navaronbracke commented on Mar 8, 2023

github-actions commented on Mar 25, 2023

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions