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

[connectivity] Android Code Inspection and Clean up #3051

Merged
merged 21 commits into from
Oct 2, 2020
Merged

[connectivity] Android Code Inspection and Clean up #3051

merged 21 commits into from
Oct 2, 2020

Conversation

hamdikahloun
Copy link
Member

Description

  • Handle deprecation & unchecked warning as error

  • Avoiding uses or overrides a deprecated API

  • Update README with the updated information about WifiInfo on Android O or higher.

Related Issues

flutter/flutter#65970

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • My PR includes unit or integration tests for all changed/updated/fixed behaviors (See Contributor Guide).
  • All existing and new tests are passing.
  • I updated/added relevant documentation (doc comments with ///).
  • The analyzer (flutter analyze) does not report any problems on my PR.
  • I read and followed the Flutter Style Guide.
  • The title of the PR starts with the name of the plugin surrounded by square brackets, e.g. [shared_preferences]
  • I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy.
  • I updated CHANGELOG.md to add a description of the change.
  • I signed the CLA.
  • I am willing to follow-up on review comments in a timely manner.

Breaking Change

Does your PR require plugin users to manually update their apps to accommodate your change?

  • Yes, this is a breaking change (please indicate a breaking change in CHANGELOG.md and increment major revision).
  • No, this is not a breaking change.

Copy link
Contributor

@dnfield dnfield left a comment

Choose a reason for hiding this comment

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

LGTM

@dnfield
Copy link
Contributor

dnfield commented Sep 25, 2020

Can you fix up the merge conflicts?

@xster - will this cause any problems with the v1 deprecation?

@hamdikahloun
Copy link
Member Author

Can you fix up the merge conflicts?
Thanks @dnfield .
Ofcorse I will fix it

@xster
Copy link
Member

xster commented Sep 26, 2020

LGTM

@hamdikahloun
Copy link
Member Author

@dnfield
merge conflict(s) fixed .
Thank you

@dnfield dnfield merged commit c696333 into flutter:master Oct 2, 2020
yasargil added a commit to yasargil/plugins that referenced this pull request Oct 8, 2020
* master:
  [in_app_purchase] Android Code Inspection and Clean up (flutter#3120)
  Android Code Inspection and Clean up (flutter#3117)
  [in_app_purchase] Fix finishing purchases upon payment cancellation (flutter#3106)
  [google_maps_flutter_web] Fix convert.dart issues (flutter#3093)
  [multiple] Opt-out tests of null-safety (flutter#3113)
  [webview_flutter] add public documentation. (flutter#3114)
  in_app_purchase: started supported null as a parameter for the sandbox arguement (flutter#3110)
  [connectivity] Android Code Inspection and Clean up (flutter#3051)
  [android_intent] Android Code Inspection and Clean up (flutter#3043)
  Remove `io.flutter.embedded_views_preview` from README
  [google_maps_flutter] Fix headline in the readme (flutter#3100)
  [webview_flutter] Add new entrypoint that uses hybrid composition on Android (flutter#3067)
  [google_maps_flutter] Out of developers preview, bump to 1.0.0 (flutter#3091)
  [url_launcher_web] Move third_party under src. (flutter#3080)
  [plugin_platform_interface] Fix homepage in pubspec.yaml (flutter#3088)
  [connectivity_for_web] Fix homepage in pubspec.yaml (flutter#3089)
  [in_app_purchase] Update typo in example main.dart (flutter#3073)
jorgefspereira pushed a commit to jorgefspereira/plugins_flutter that referenced this pull request Oct 10, 2020
@hacker1024
Copy link

hacker1024 commented Oct 19, 2020

This seems to break building for me on the Flutter dev branch with connectivity version 0.4.9+5:

warning: [options] source value 7 is obsolete and will be removed in a future release
warning: [options] target value 7 is obsolete and will be removed in a future release
warning: [options] To suppress warnings about obsolete options, use -Xlint:-options.
error: warnings found and -Werror specified

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':connectivity:compileDebugJavaWithJavac'.
> 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 32s
Exception: Gradle task assembleDebug failed with exit code 1

Rolling back to 0.4.9+3 fixes the issue.

@hamdikahloun
Copy link
Member Author

Hi @hacker1024

Can you try with JDK 8 please .

Thank you

@hacker1024
Copy link

I added the following to android/app/build.gradle, but it doesn't help.

android {
    compileOptions {
        sourceCompatibility JavaVersion.VERSION_1_8
        targetCompatibility JavaVersion.VERSION_1_8
    }

    kotlinOptions {
        jvmTarget = "1.8"
    }
}

@hamdikahloun
Copy link
Member Author

Can you please provide your flutter doctor -v, your android/build.gradle, your android/gradle/wrapper/gradle-wrapper.properties and the JDK version that installed on your computer?
Thank you

@hacker1024
Copy link

$ flutter doctor -v

[✓] Flutter (Channel dev, 1.23.0-18.0.pre, on Microsoft Windows [Version 10.0.20236.1005], locale en-AU)
    • Flutter version 1.23.0-18.0.pre at C:\Users\<USER>\AppData\Local\Flutter
    • Framework revision 37ebe3d82a (6 days ago), 2020-10-13 10:52:23 -0700
    • Engine revision 6634406889
    • Dart version 2.11.0 (build 2.11.0-213.0.dev)


[✓] Android toolchain - develop for Android devices (Android SDK version 30.0.2)
    • Android SDK at C:\Android\android-sdk
    • Platform android-30, build-tools 30.0.2
    • ANDROID_HOME = C:\Android\android-sdk
    • Java binary at: C:\Program Files\Common Files\Oracle\Java\javapath\java.exe
    • Java version Java(TM) SE Runtime Environment (build 15+36-1562)
    • All Android licenses accepted.

[✓] Chrome - develop for the web
    • Chrome at C:\Users\<USER>\AppData\Local\Google\Chrome\Application\chrome.exe

[✓] Visual Studio - develop for Windows (Visual Studio Community 2019 16.7.6)
    • Visual Studio at C:\Program Files (x86)\Microsoft Visual Studio\2019\Community
    • Visual Studio Community 2019 version 16.7.30611.23
    • Windows 10 SDK version 10.0.19041.0

[!] Android Studio (not installed)
    • Android Studio not found; download from https://developer.android.com/studio/index.html
      (or visit https://flutter.dev/docs/get-started/install/windows#android-setup for detailed instructions).

[✓] IntelliJ IDEA Ultimate Edition (version 2020.3)
    • IntelliJ at C:\Users\<USER>\AppData\Local\JetBrains\Toolbox\apps\IDEA-U\ch-0\203.4818.26
    • Flutter plugin installed
    • Dart plugin version 203.4818.26

[✓] VS Code, 64-bit edition (version 1.50.1)
    • VS Code at C:\Program Files\Microsoft VS Code
    • Flutter extension version 3.15.1

[✓] Connected device (4 available)
    • Redmi K20 Pro (mobile) • <SERIAL>   • android-arm64  • Android 11 (API 30)
    • Windows (desktop)      • windows    • windows-x64    • Microsoft Windows [Version 10.0.20236.1005]
    • Web Server (web)       • web-server • web-javascript • Flutter Tools
    • Chrome (web)           • chrome     • web-javascript • Google Chrome 88.0.4292.2

! Doctor found issues in 1 category.

android/build.gradle (note: I've tried with the default Kotlin 1.3.50 as well)

buildscript {
    ext.kotlin_version = '1.4.10'
    repositories {
        google()
        jcenter()
    }

    dependencies {
        classpath 'com.android.tools.build:gradle:3.5.0'
        classpath "org.jetbrains.kotlin:kotlin-gradle-plugin:$kotlin_version"
    }
}

allprojects {
    repositories {
        google()
        jcenter()
    }
}

rootProject.buildDir = '../build'
subprojects {
    project.buildDir = "${rootProject.buildDir}/${project.name}"
}
subprojects {
    project.evaluationDependsOn(':app')
}

task clean(type: Delete) {
    delete rootProject.buildDir
}

gradle-wrapper.properties

distributionBase=GRADLE_USER_HOME
distributionPath=wrapper/dists
zipStoreBase=GRADLE_USER_HOME
zipStorePath=wrapper/dists
distributionUrl=https\://services.gradle.org/distributions/gradle-6.7-all.zip

@hamdikahloun
Copy link
Member Author

Can you install JDK 8, Set JAVA_HOME and update gradle-wrapper.properties to :

distributionUrl=https\://services.gradle.org/distributions/gradle-5.6.2-all.zip

Set JAVA_HOME:

  • Right click My Computer and select Properties.

  • On the Advanced tab, select Environment Variables, and then edit JAVA_HOME to point to where the JDK software is located, for example, C:\Program Files\Java\jdk1.8.0_211.

@hacker1024
Copy link

hacker1024 commented Oct 19, 2020

It looks like switching to JDK 8 on my end did actually fix it - odd.
Sorry - I misinterpreted your original request.

@hamdikahloun
Copy link
Member Author

@hacker1024 you are welcome

brunobowden added a commit to brunobowden/plugins that referenced this pull request Oct 28, 2020
PROBLEM:
Upgrading to Java 11 will cause the following innocuous warnings
to become build failures due to the "-Werror" flag:
```
> Task :connectivity:compileReleaseJavaWithJavac FAILED
warning: [options] source value 7 is obsolete and will be removed in a future release
warning: [options] target value 7 is obsolete and will be removed in a future release
warning: [options] To suppress warnings about obsolete options, use -Xlint:-options.
error: warnings found and -Werror specified
FAILURE: Build failed with an exception.
```
Example: https://github.com/WorldHealthOrganization/app/runs/1318596280

FIX:
Target Java 8 for all plugins that use the `-Werror` compiler argument:
```
android {
    ...
    compileOptions {
        sourceCompatibility JavaVersion.VERSION_1_8
        targetCompatibility JavaVersion.VERSION_1_8
    }
```

The plugins were all found with:
```
$ find . -name build.gradle | xargs grep Werror
./packages/connectivity/connectivity/android/build.gradle:def args = ["-Xlint:deprecation","-Xlint:unchecked","-Werror"]
./packages/camera/android/build.gradle:def args = ["-Xlint:deprecation","-Xlint:unchecked","-Werror"]
./packages/video_player/video_player/android/build.gradle:def args = ["-Xlint:deprecation","-Xlint:unchecked","-Werror"]
./packages/android_alarm_manager/android/build.gradle:def args = ["-Xlint:deprecation","-Xlint:unchecked","-Werror"]
./packages/android_intent/android/build.gradle:def args = ["-Xlint:deprecation","-Xlint:unchecked","-Werror"]
```
I think the config was broken for video_player. Also updated a few example apps too.

WORKAROUND:
As a temporary workaround, developers can downgrade to Java 11 which
doesn't warn that Java 7 is obsolete. Example:
https://github.com/WorldHealthOrganization/app/pull/1669/files#diff-710a4407aba47ffddf61ee9be9490428d5d0883759b18e490858db28e1ace4afR86

REGRESSION:
@hamdikahloun - thanks for your work on the Connectivity package. Adding `-Werror`
flags is a positive thing but I think is best combined with Java 8 to ensure that
the warning don't become error. Your PR: flutter#3051

FYI @mehmetf, @Salakar, @bkonyi, @ened, @dnfield as you've all edited Java 8 targets
brunobowden added a commit to brunobowden/plugins that referenced this pull request Oct 28, 2020
PROBLEM:
Upgrading to Java 12 will cause the following innocuous warnings
to become build failures due to the "-Werror" flag:
```
> Task :connectivity:compileReleaseJavaWithJavac FAILED
warning: [options] source value 7 is obsolete and will be removed in a future release
warning: [options] target value 7 is obsolete and will be removed in a future release
warning: [options] To suppress warnings about obsolete options, use -Xlint:-options.
error: warnings found and -Werror specified
FAILURE: Build failed with an exception.
```
Example: https://github.com/WorldHealthOrganization/app/runs/1318596280

FIX:
Target Java 8 for all plugins that use the `-Werror` compiler argument:
```
android {
    ...
    compileOptions {
        sourceCompatibility JavaVersion.VERSION_1_8
        targetCompatibility JavaVersion.VERSION_1_8
    }
```

The plugins were all found with:
```
$ find . -name build.gradle | xargs grep Werror
./packages/connectivity/connectivity/android/build.gradle:def args = ["-Xlint:deprecation","-Xlint:unchecked","-Werror"]
./packages/camera/android/build.gradle:def args = ["-Xlint:deprecation","-Xlint:unchecked","-Werror"]
./packages/video_player/video_player/android/build.gradle:def args = ["-Xlint:deprecation","-Xlint:unchecked","-Werror"]
./packages/android_alarm_manager/android/build.gradle:def args = ["-Xlint:deprecation","-Xlint:unchecked","-Werror"]
./packages/android_intent/android/build.gradle:def args = ["-Xlint:deprecation","-Xlint:unchecked","-Werror"]
```
I think the config was broken for video_player. Also updated a few example apps too.

WORKAROUND:
As a temporary workaround, developers can downgrade to Java 11 which
doesn't warn that Java 7 is obsolete. Example:
https://github.com/WorldHealthOrganization/app/pull/1669/files#diff-710a4407aba47ffddf61ee9be9490428d5d0883759b18e490858db28e1ace4afR86

REGRESSION:
@hamdikahloun - thanks for your work on the Connectivity package. Adding `-Werror`
flags is a positive thing but I think is best combined with Java 8 to ensure that
the warning don't become error. Your PR: flutter#3051

FYI @mehmetf, @Salakar, @bkonyi, @ened, @dnfield as you've all edited Java 8 targets
FlutterSu pushed a commit to FlutterSu/flutter-plugins that referenced this pull request Nov 20, 2020
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.

5 participants