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

Projects
None yet
9 participants
@MarkOSullivan94
Copy link
Contributor

MarkOSullivan94 commented Mar 14, 2018

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.

MarkOSullivan94 added some commits Mar 14, 2018

@rborn

This comment has been minimized.

Copy link
Collaborator

rborn commented Mar 14, 2018

LGTM @alvelig 🐽

@MarkOSullivan94

This comment has been minimized.

Copy link
Contributor Author

MarkOSullivan94 commented Mar 14, 2018

@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

This comment has been minimized.

Copy link
Collaborator

rborn commented Mar 14, 2018

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

@MarkOSullivan94

This comment has been minimized.

Copy link
Contributor Author

MarkOSullivan94 commented Mar 26, 2018

@rborn can this PR be merged in?

@rborn

This comment has been minimized.

Copy link
Collaborator

rborn commented Mar 26, 2018

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

@christocracy

This comment has been minimized.

Copy link
Contributor

christocracy commented Mar 26, 2018

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

@alvelig

This comment has been minimized.

Copy link
Collaborator

alvelig commented Mar 26, 2018

LGTM

@rborn

This comment has been minimized.

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-community:master Mar 26, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@todorone
Copy link
Contributor

todorone left a comment

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

@rborn

This comment has been minimized.

Copy link
Collaborator

rborn commented Mar 28, 2018

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

@todorone

This comment has been minimized.

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

This comment has been minimized.

Copy link
Contributor

christocracy commented Mar 28, 2018

@todorone post your android/build.gradle

@todorone

This comment has been minimized.

Copy link
Contributor

todorone commented Mar 28, 2018

@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

This comment has been minimized.

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

This comment has been minimized.

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

This comment has been minimized.

Copy link
Contributor

todorone commented Mar 28, 2018

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

@rborn

This comment has been minimized.

Copy link
Collaborator

rborn commented Mar 28, 2018

@todorone that would be a good PR 🤪

@todorone

This comment has been minimized.

Copy link
Contributor

todorone commented Mar 28, 2018

@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

This comment has been minimized.

Copy link
Contributor

christocracy commented Mar 28, 2018

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

@christocracy

This comment has been minimized.

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

This comment has been minimized.

Copy link
Contributor

todorone commented Mar 28, 2018

@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

@MarkOSullivan94

This comment has been minimized.

Copy link
Contributor Author

MarkOSullivan94 commented Apr 12, 2018

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

This comment has been minimized.

Copy link

mauron85 commented Apr 12, 2018

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

@donmesserli

This comment has been minimized.

Copy link

donmesserli commented Apr 12, 2018

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

This comment has been minimized.

Copy link
Contributor

christocracy commented Apr 12, 2018

Post your android/build.gradle

@donmesserli

This comment has been minimized.

Copy link

donmesserli commented Apr 12, 2018

@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

This comment has been minimized.

Copy link
Contributor

christocracy commented Apr 12, 2018

To your repositories, add google()

@donmesserli

This comment has been minimized.

Copy link

donmesserli commented Apr 12, 2018

@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

This comment has been minimized.

Copy link
Contributor

christocracy commented Apr 12, 2018

Replace it with:

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

This comment has been minimized.

Copy link

donmesserli commented Apr 12, 2018

@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

This comment has been minimized.

Copy link
Contributor

christocracy commented Apr 12, 2018

Now you circle back and verify your setup steps.

@donmesserli

This comment has been minimized.

Copy link

donmesserli commented Apr 12, 2018

@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

This comment has been minimized.

Copy link
Contributor

christocracy commented Apr 12, 2018

what version do you have installed?

Does this path exist on your app?

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

This comment has been minimized.

Copy link

donmesserli commented Apr 12, 2018

@christocracy
0.21.0
Yes, the files exist

@christocracy

This comment has been minimized.

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

Fix React-native-maps 0.21.0
- Cập nhật react 0.44.0-rc-4
- Cần cập nhật Gradle 3 (react-native-community/react-native-maps#2096 (comment))
@MarkOSullivan94

This comment has been minimized.

Copy link
Contributor Author

MarkOSullivan94 commented Apr 13, 2018

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

mlisik pushed a commit to mlisik/react-native-maps that referenced this pull request Apr 26, 2018

@lachenmayer

This comment has been minimized.

Copy link
Contributor

lachenmayer commented May 29, 2018

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

This comment has been minimized.

Copy link
Contributor Author

MarkOSullivan94 commented May 29, 2018

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

@lachenmayer

This comment has been minimized.

Copy link
Contributor

lachenmayer commented May 29, 2018

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 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 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 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

This comment has been minimized.

Copy link
Contributor Author

MarkOSullivan94 commented May 29, 2018

@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).

lachenmayer added a commit to lachenmayer/react-native-maps that referenced this pull request Jun 14, 2018

sdovenor added a commit to sdovenor/react-native-maps that referenced this pull request Jul 18, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.