Skip to content
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

Updated gradle configuration for gradle 3.0.0+ #2096

Merged
merged 2 commits into from
Mar 26, 2018

Conversation

MarkOSullivan94
Copy link
Contributor

Further details on Gradle 3.0.0+ https://developer.android.com/studio/build/gradle-plugin-3-0-0-migration.html#new_configurations

Is there any other PR opened that does the same ?

No.

What issue is this PR fixing

#2095

Also fixing installation.md which had the old gradle configuration and not the new one.

@rborn
Copy link
Collaborator

rborn commented Mar 14, 2018

LGTM @alvelig 🐽

@MarkOSullivan94
Copy link
Contributor Author

@rborn will this be updated in the next React Native Maps update? 0.20.2? How can I tell when it's included in a release version?

@rborn
Copy link
Collaborator

rborn commented Mar 14, 2018

@MarkOSullivan94 usually @christopherdro includes a change log when he does a release.

@MarkOSullivan94
Copy link
Contributor Author

@rborn can this PR be merged in?

@rborn
Copy link
Collaborator

rborn commented Mar 26, 2018

@christocracy how does this influences/compares with your PR #2047

@christocracy
Copy link
Contributor

christocracy commented Mar 26, 2018

As long as it uses the new $googlePlayServicesVersion property, it's fine.

@alvelig
Copy link
Contributor

alvelig commented Mar 26, 2018

LGTM

@rborn
Copy link
Collaborator

rborn commented Mar 26, 2018

@christocracy looks like it does, thank you
@alvelig thnx
@MarkOSullivan94 🎉

@rborn rborn merged commit 8cb158e into react-native-maps:master Mar 26, 2018
Copy link
Contributor

@todorone todorone left a comment

Choose a reason for hiding this comment

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

Seems this PR has broken building react-native-maps master branch on Android...

@rborn
Copy link
Collaborator

rborn commented Mar 28, 2018

@todorone how? what's wrong, error logs?

@todorone
Copy link
Contributor

todorone commented Mar 28, 2018

@rborn It can be simply reproduced:

  • install react-native-maps master branch over clean react-native 0.54.3 repository
  • set up react-native-maps for Android according to latest instructions
  • build result in gradle error:
A problem occurred evaluating project ':app'.
> Could not find method implementation() for arguments [project ':react-native-maps'] on object of type org.gradle.api.internal.artifacts.dsl.dependencies.DefaultDependencyHandler.

The solution was to reverse this commit and install according to old instructions. Seems that react-native built-in gradle version is not compatible.

PS: Sorry for the message here rather than in issues.

@christocracy
Copy link
Contributor

@todorone post your android/build.gradle

@todorone
Copy link
Contributor

@christocracy It's default react-native one with only change of react-native-maps:

// Top-level build file where you can add configuration options common to all sub-projects/modules.

buildscript {
    repositories {
        jcenter()
    }
    dependencies {
        classpath 'com.android.tools.build:gradle:2.2.3'

        // NOTE: Do not place your application dependencies here; they belong
        // in the individual module build.gradle files
    }
}

allprojects {
    repositories {
        mavenLocal()
        jcenter()
        maven {
            // All of React Native (JS, Obj-C sources, Android binaries) is installed from npm
            url "$rootDir/../node_modules/react-native/android"
        }
    }
}

ext {
    compileSdkVersion   = 26
    targetSdkVersion    = 26
    buildToolsVersion   = "26.0.2"
    supportLibVersion   = "26.1.0"
    googlePlayServicesVersion = "11.8.0"
    androidMapsUtilsVersion = "0.5+"
}

@christocracy
Copy link
Contributor

christocracy commented Mar 28, 2018

gradle implementation is only available in Gradle 3.

buildscript {
    repositories {
        jcenter()
+        google()
    }
    dependencies {
+        classpath 'com.android.tools.build:gradle:3.0.1'  
    }
}
``` @

@rborn
Copy link
Collaborator

rborn commented Mar 28, 2018

@christocracy maybe we should add a note in the docs (if it's not there already) about using gradle 3?

@todorone
Copy link
Contributor

@christocracy Thanks for the hint, but it definitely needs to be added to installation instructions...

@rborn
Copy link
Collaborator

rborn commented Mar 28, 2018

@todorone that would be a good PR 🤪

@todorone
Copy link
Contributor

@rborn I've tried to use latest gradle version for building, but experienced that it somehow incompatible with another native dependency - react-native-vector-icons. I'll be glad to help you guys in testing or whatever but I'm not experienced enough here to make consistent PR adressing this issue. Sorry. 😊
BTW: Thanks for an amazing work of supporting react-native-maps!

@christocracy
Copy link
Contributor

react-native-vector-icons has an unusual build.gradle. I don't think they need to specify the buildscript block.

@christocracy
Copy link
Contributor

christocracy commented Mar 28, 2018

@todorone, If you have your project open in Android Studio, edit react-native-vector-icons/build.gradle and delete the entire buildscript block and see if it builds.

@todorone
Copy link
Contributor

@christocracy So the latest news. Everything started to work, even while not touching react-native-vector-icons.

That's the section of build.gradle:

buildscript {
    repositories {
        jcenter()
        // google() must be added to default react-native build.gradle
        google()
    }
    dependencies {
        classpath 'com.android.tools.build:gradle:3.1.0' // changed version to 3.1.0
    }
}

And also adjusted in gradle-wrapper.properties:
distributionUrl=https\://services.gradle.org/distributions/gradle-4.4-all.zip

@mauron85
Copy link

mauron85 commented Apr 11, 2018

This PR was completely unnecessary and it goes against default react-native template which is still using gradle 2. As result it makes this project incompatible with some rn plugins which use gradle 2. If you need one - mauron85/react-native-background-geolocation#176. Also PR is incomplete without updating installation instructions how to update your project to gradle 3. Something like - https://medium.com/bam-tech/boost-compilation-time-of-your-react-native-or-android-app-with-the-latest-build-tools-a3d5d398ed33

I have also created new issue with more details #2180

Please revert this PR.

@MarkOSullivan94
Copy link
Contributor Author

No it was completely necessary for anyone wanting to use RN-Maps and is using Android Studio 3.x (the stable version is 3.1.1) otherwise they'll be unable to build the project successfully.

Perhaps more instructions will be required, I just changed what I changed within my project to get RN Maps to work on Gradle 3.x and updated the docs with the steps I had to do.

@mauron85
Copy link

@MarkOSullivan94 I open all my RN gradle2 projects in Android Studio 3.1. Not sure what are you talking about.

@donmesserli
Copy link

Got the following build error after changing build.gradle. I've upgraded Android Studio to 3.1.1 and the Gradle plugin.

A problem occurred configuring root project 'xxxxxxx'.

Could not resolve all dependencies for configuration ':classpath'.
Could not find com.android.tools.build:gradle:3.1.0.
Searched in the following locations:
https://jcenter.bintray.com/com/android/tools/build/gradle/3.1.0/gradle-3.1.0.pom
https://jcenter.bintray.com/com/android/tools/build/gradle/3.1.0/gradle-3.1.0.jar
Required by:
:xxxxxx:unspecified

@christocracy
Copy link
Contributor

Post your android/build.gradle

@donmesserli
Copy link

@christocracy

// Top-level build file where you can add configuration options common to all sub-projects/modules.

buildscript {
    repositories {
        jcenter()
        
    }
    dependencies {
        classpath 'com.android.tools.build:gradle:3.1.0' // changed version to 3.1.0

        // NOTE: Do not place your application dependencies here; they belong
        // in the individual module build.gradle files
    }
}

allprojects {
    repositories {
        mavenLocal()
        jcenter()
        maven {
            // All of React Native (JS, Obj-C sources, Android binaries) is installed from npm
            url "$rootDir/../node_modules/react-native/android"
        }
    }
}

@christocracy
Copy link
Contributor

To your repositories, add google()

@donmesserli
Copy link

@christocracy
That gives me this error...

  • Where:
    Build file '/Users/dmesser/Develop/React-Native/xxxxxx/android/build.gradle' line: 6

  • What went wrong:
    A problem occurred evaluating root project 'xxxxxx'.

Could not find method google() for arguments [] on repository container.

@christocracy
Copy link
Contributor

Replace it with:

maven {
          url 'https://maven.google.com'       
}

@donmesserli
Copy link

@christocracy

Got me further. Now getting...

> Task :app:compileDebugJavaWithJavac FAILED
/Users/dmesser/Develop/React-Native/RetailApp/android/app/src/main/java/com/lifeway/christianstores/MainApplication.java:13: error: package com.airbnb.android.react.maps does not exist
import com.airbnb.android.react.maps.MapsPackage;
                                    ^
/Users/dmesser/Develop/React-Native/RetailApp/android/app/src/main/java/com/lifeway/christianstores/MainApplication.java:33: error: cannot find symbol
            new MapsPackage()
                ^
  symbol: class MapsPackage
2 errors

@christocracy
Copy link
Contributor

Now you circle back and verify your setup steps.

@donmesserli
Copy link

@christocracy
I followed all the steps. The only one that looks like it might be causing this error is...

include ':react-native-maps'
project(':react-native-maps').projectDir = new File(rootProject.projectDir, '../node_modules/react-native-maps/lib/android')

But I did that.

@christocracy
Copy link
Contributor

what version do you have installed?

Does this path exist on your app?

$ ls node_modules/react-native-maps/lib/android

@donmesserli
Copy link

@christocracy
0.21.0
Yes, the files exist

@christocracy
Copy link
Contributor

christocracy commented Apr 12, 2018

No idea. Load your app in Android Studio and look errors / warnings.

Kammeh added a commit to Kammeh/react-native-maps that referenced this pull request Apr 12, 2018
maslhiro added a commit to maslhiro/ATMFinder_RN that referenced this pull request Apr 13, 2018
- Cập nhật react 0.44.0-rc-4
- Cần cập nhật Gradle 3 (react-native-maps/react-native-maps#2096 (comment))
@MarkOSullivan94
Copy link
Contributor Author

Just had a look over the React Native PRs, it looks like the RN project will be updated to Gradle 3.x soon: facebook/react-native#17747

@lachenmayer
Copy link
Contributor

Hi @MarkOSullivan94, could you please add documentation for this change?
The project does not work out of the box with the current documentation.
Thanks!

@MarkOSullivan94
Copy link
Contributor Author

@lachenmayer are you having issues with getting it working? I'd look over your project setup and see if I can spot anything

@lachenmayer
Copy link
Contributor

Thanks a lot Mark, I have created a repro project here: https://github.com/lachenmayer/react-native-maps-gradle-repro

The repro

The repro contains detailed instructions for how to reproduce every step, and you can just click on every line item to verify the file changes. Note that I have left out all of the steps that I believe are irrelevant to this discussion, eg. setting up Google Maps API keys etc.

I did use react-native link contrary to the current documentation. However looking at the file changes, there should be no reason why that should not work - it adds all of the correct rules to app/build.gradle and settings.gradle, and adds MapsPackage to the main application file. I may have missed something here, please let me know if so.

You should hopefully be able to see that when the changes from this PR are removed, the project builds as expected.

The problem/goal

react-native-maps is currently extremely difficult to install, especially for developers who are not very experienced with native iOS and/or Android development.

The goal I would like us to strive towards is that the library can be installed just using a simple react-native link, like any other RN library with native dependencies. Whether that's achievable on iOS is debatable, but on Android this definitely should not be a problem.

This change has added significant complexity to the installation procedures when starting from create-react-native-app - a search for "compileOnly" in this repo's issues should confirm that our users are having a lot of trouble with this change.

Now, I am aware of the discussion in #2180.

I understand your frustration that you are not able to use Gradle 3 by default - it is clearly superior, and I agree that React Native should be using it by default. However, at this point in time Gradle 3 is clearly a "power-user" feature which is mainly used by teams/individuals who are more likely to understand the intricacies of Android dependency management. I believe that the library should work out of the box for "everyone else" by default. In particular, we should not force people to have to upgrade the underlying tooling of their project just to get this particular dependency to build.

Since react-native-maps relies on React Native, we are unfortunately tied to their upstream decisions & release cycles. I am aware that you are contributing on the PR to upgrade the default React Native project to Gradle 3 (facebook/react-native#17747), thanks a lot for that. Once that change lands, we will be able to safely upgrade to Gradle 3 without necessarily maintaining backwards compatibility, since the policy in this repo is to only maintain compatibility with the latest version of React Native.

Most importantly... The original bug report you submitted #2095 - which this PR does fix, in a sense - is not a bug. In your screenshot, you show the deprecation warnings that get triggered when you have Gradle >3 installed. These are just warnings, the project will still compile just fine, even with these warnings. If you are not convinced of this, I can update the repro project above to use Gradle >3.

Possible solutions

  1. Revert this PR and wait for Upgrade Gradle and Android SDK versions. facebook/react-native#17747 to land.

    This would allow us to massively simplify the installation documentation & to allow an easy installation experience for Android users. If you need Gradle 3 support in this scenario, I recommend using patch-package. This will let you easily make the required changes to react-native-maps to make it compatible with your own projects, as well as letting you keep up with the latest changes in the library without having to maintain a separate fork. Once Upgrade Gradle and Android SDK versions. facebook/react-native#17747 lands, we can revisit the Gradle 3 change & break backwards compatibility.

  2. Find a different backwards-compatible solution

    I have noticed that (eg.) this PR Android Gradle Plugin 3.x integration microsoft/react-native-code-push#1219 claims to have a backwards-compatible solution for using Gradle 3. I don't really understand how this works, but this may also be a useful way to go.

Option 1 is obviously the least effort, but if you have the time to look into option 2 that would definitely be appreciated.

It would be great if some of the maintainers could chime in here, so that we can get the project building out of the box again. Thanks folks!

@MarkOSullivan94
Copy link
Contributor Author

@lachenmayer absolutely fantastic work with that repo, so easy and clear to follow. I've forked your repo and made the necessary changes to get a successful build. The files changed are a bit misleading as the majority of the changes are done automatically within Android Studio whenever you update the Android projects gradle files.

https://github.com/MarkOSullivan94/react-native-maps-gradle-repro/blob/master/android/build.gradle#L11

This is were you specify that wish to use gradle 3.0.1 for the project. This will result on a knock on affect within Android Studio recommending you to upgrade other dependencies which are old and outdated in React Native projects. Everything is suggested in the errors / warning messages.

For example

* What went wrong:
A problem occurred evaluating project ':react-native-maps'.
> Could not find method compileOnly() for arguments [com.facebook.react:react-native:+] on object of type org.gradle.api.internal.artifacts.dsl.dependencies.DefaultDependencyHandler.

This error message would indicate that your Android project doesn't have Gradle 3.0.

If you would like to try and upgrade it yourself, focus on the build.gradle files:

https://github.com/MarkOSullivan94/react-native-maps-gradle-repro/blob/master/android/build.gradle

https://github.com/MarkOSullivan94/react-native-maps-gradle-repro/blob/master/android/app/build.gradle

This is where you'll update dependencies until it will work with Gradle 3.0. Like I've said previously, Android Studio helps with recommendations on what to use, so I would recommend using it.

Most importantly... The original bug report you submitted #2095 - which this PR does fix, in a sense - is not a bug. In your screenshot, you show the deprecation warnings that get triggered when you have Gradle 3 installed. These are just warnings, the project will still compile just fine, even with these warnings. If you are not convinced of this, I can update the repro project above to use Gradle 3

Perhaps I didn't include it in the original issue report but I would try and build our project using Android Studio it would not build because our existing Android project was already using Gradle 3.0+.

It will produce a warning about the compile and provided keywords as shown by my screenshot in #2095 and unlike most other warnings in Android Studio, this one will cause it not to successfully build which is a big problem for anyone having similar issues.

I know some might suggest not to use Android Studio however that is impossible whenever you consider Android applications which have React Native components within them (which is what we do).

@niralivasoya
Copy link

@todorone, Thanks man, it really works.

buildscript {
repositories {
jcenter()
// google() must be added to default react-native build.gradle
google()
}
dependencies {
classpath 'com.android.tools.build:gradle:3.1.0' // changed version to 3.1.0
}
}


And also adjusted in `gradle-wrapper.properties`:
`distributionUrl=https\://services.gradle.org/distributions/gradle-4.4-all.zip`

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants