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

Changes needed for gradle 3.0.1 compatibility #581

Closed
pacamara opened this issue Jan 19, 2018 · 28 comments
Closed

Changes needed for gradle 3.0.1 compatibility #581

pacamara opened this issue Jan 19, 2018 · 28 comments

Comments

@pacamara
Copy link

Some minor changes are needed for gradle 3.0.1 compatibility: in android/build.gradle:

We are currently hacking these changes with a script in status.im, and would rather it is fixed upstream. Thanks!

Parent issue: status-im/status-mobile#3037

@msand
Copy link
Collaborator

msand commented Jan 19, 2018

How about this? #582

@pacamara
Copy link
Author

@msand Thanks! #582 looks good.

@msand
Copy link
Collaborator

msand commented Jan 19, 2018

Published 6.0.1-rc.3 now. Please try it out.

@pacamara
Copy link
Author

pacamara commented Jan 19, 2018

It works!

  • git clone https://github.com/status-im/status-react.git
  • in package.json change react-native-svg entry to: "react-native-svg": "6.0.1-rc.3",
  • also upgrade status-react to compileSdkVersion 27
  • build as usual
  • build passes, and there are no warnings relating to react-native-svg in the logs
    🍻

@jskidd3
Copy link

jskidd3 commented Jan 20, 2018

FAILURE: Build failed with an exception.

* Where:
Build file '..\node_modules\react-native-svg\android\build.gradle' line: 4

* What went wrong:
A problem occurred evaluating project ':react-native-svg'.
> Could not find method google() for arguments [] on repository container.

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

BUILD FAILED

Any ideas, guys?

@msand
Copy link
Collaborator

msand commented Jan 20, 2018

Hmm, what version of Android studio are you using? Can you try building using the latest?

@jskidd3
Copy link

jskidd3 commented Jan 20, 2018

@msand Built using command line, used latest version with a fresh RN project and can't get past this

{
  "name": "",
  "version": "0.0.1",
  "private": true,
  "scripts": {
    "start": "node node_modules/react-native/local-cli/cli.js start",
    "test": "jest",
  },
  "dependencies": {
    "react": "16.2.0",
    "react-native": "0.52.0",
    "react-native-maps": "^0.19.0",
    "react-native-vector-icons": "^4.5.0"
  },
  "devDependencies": {
    "babel-jest": "22.1.0",
    "babel-preset-react-native": "4.0.0",
    "jest": "22.1.4",
    "react-test-renderer": "16.2.0"
  },
  "jest": {
    "preset": "react-native"
  }
}

@msand
Copy link
Collaborator

msand commented Jan 20, 2018

Do you have Android Studio installed? Try installing the latest of that: https://developer.android.com/studio/index.html

@jskidd3
Copy link

jskidd3 commented Jan 20, 2018

@msand Just done a fresh install of Android Studio version 1.5.1 still the same

@pacamara
Copy link
Author

pacamara commented Jan 20, 2018

Hi guys, I just checked and a RN project freshly generated with react-native init FooProj has

        classpath 'com.android.tools.build:gradle:2.2.3'

in FooProj/android/build.gradle. So that's the problem I guess.

@jskidd3
Copy link

jskidd3 commented Jan 20, 2018

@pacamara Yes mine has that same version. Could you let me know what it needs to be?

@pacamara
Copy link
Author

pacamara commented Jan 20, 2018

@jskidd3 Sure, here's the diff of the changes I needed for a fresh project:

FooProj $ git diff
diff --git a/android/build.gradle b/android/build.gradle
index eed9972..965bebb 100644
--- a/android/build.gradle
+++ b/android/build.gradle
@@ -2,10 +2,11 @@
 
 buildscript {
     repositories {
+        google()
         jcenter()
     }
     dependencies {
-        classpath 'com.android.tools.build:gradle:2.2.3'
+        classpath 'com.android.tools.build:gradle:3.0.1'
 
         // NOTE: Do not place your application dependencies here; they belong
         // in the individual module build.gradle files
diff --git a/android/gradle/wrapper/gradle-wrapper.properties b/android/gradle/wrapper/gradle-wrapper.properties
index dbdc05d..bf1b63c 100644
--- a/android/gradle/wrapper/gradle-wrapper.properties
+++ b/android/gradle/wrapper/gradle-wrapper.properties
@@ -2,4 +2,4 @@ distributionBase=GRADLE_USER_HOME
 distributionPath=wrapper/dists
 zipStoreBase=GRADLE_USER_HOME
 zipStorePath=wrapper/dists
-distributionUrl=https\://services.gradle.org/distributions/gradle-2.14.1-all.zip
+distributionUrl=https\://services.gradle.org/distributions/gradle-4.1-all.zip

Installed and ran ok after I also manually created the assets bundle (don't believe this step has anything to do with gradle upgrade).

@jskidd3
Copy link

jskidd3 commented Jan 20, 2018

@pacamara Thanks so much!

@pacamara
Copy link
Author

@jskidd3 You're welcome! 🍻

@jskidd3
Copy link

jskidd3 commented Jan 21, 2018

@pacamara Experiencing a similar issue in a fresh project on OSX. Mind lending a hand? #584

@msand
Copy link
Collaborator

msand commented Jan 21, 2018

@jskidd3 Have you tried these changes? #581 (comment)

@jskidd3
Copy link

jskidd3 commented Jan 21, 2018

@msand Yes I implemented them without any problem yesterday on Windows but am just having the same problem on Mac now. The difference is this time that my build.gradle already looks like what @pacamara suggested. This is the current one:

buildscript {
    repositories {
        jcenter()
        google()
    }

    dependencies {
        classpath 'com.android.tools.build:gradle:3.0.1'
    }
}

apply plugin: 'com.android.library'

android {
    compileSdkVersion 27

    defaultConfig {
        minSdkVersion 16
        targetSdkVersion 27
        versionCode 1
        versionName "1.0"
    }
    lintOptions {
        abortOnError false
    }
}

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

dependencies {
    api "com.android.support:appcompat-v7:27.0.2"
    //noinspection GradleDynamicVersion
    api 'com.facebook.react:react-native:+'
}

react-native run-android produces:

A problem occurred evaluating project ':react-native-svg'.
> Could not find method google() for arguments [] on repository container

@msand
Copy link
Collaborator

msand commented Jan 21, 2018

@jskidd3 I think you're looking at appName/node_modules/react-native-svg/android/build.gradle rather than appName/android/build.gradle

@sibelius
Copy link

you can do this on user land, no need to do this right now, as react native is not using gradle 3 yet

@pacamara
Copy link
Author

Agree with @sibelius , better to wait for react native itself to upgrade. Apologies @msand @jskidd3 for causing you extra work here

@msand
Copy link
Collaborator

msand commented Jan 25, 2018

@dlimx @lepouya How come FBSDK uses SDK 26 and appcompat 27.0.2
https://github.com/facebook/react-native-fbsdk/blame/master/android/build.gradle

While react-native uses these? @jpshelley @grabbou
compileSdkVersion 23
minSdkVersion 16
targetSdkVersion 22
appcompat 23.0.1
and old gradle versions?

Perhaps I should make a PR upgrading them all (except min sdk) to the latest stable versions?

Google has this to say about targetSdkVersion:

To maintain your application along with each Android release, you should increase the value of this attribute to match the latest API level, then thoroughly test your application on the corresponding platform version.
https://developer.android.com/guide/topics/manifest/uses-sdk-element.html

https://medium.com/google-developers/picking-your-compilesdkversion-minsdkversion-targetsdkversion-a098a0341ebd

https://developer.android.com/training/basics/supporting-devices/platforms.html#sdk-versions

@msand
Copy link
Collaborator

msand commented Jan 26, 2018

Made a PR facebook/react-native#17747

@lepouya
Copy link

lepouya commented Jan 26, 2018

@msand It's the same versions as what facebook-android-sdk uses

@msand
Copy link
Collaborator

msand commented Jan 26, 2018

I think I just found a more than good enough reason for upgrade: Path Traversal Vulnerability

@msand
Copy link
Collaborator

msand commented Jan 26, 2018

@pacamara @sibelius I guess meanwhile the PR is open, we could revert these changes. But, should we change them to align with the latest stable react-native init, or revert to the previous versions, what do you think?

@msand
Copy link
Collaborator

msand commented Jan 26, 2018

Although, now that I think about it. The latest changes have semver pre-release identifiers, and the other changes have been there for some time already. Perhaps best to wait and see for a bit more response on the PR.

https://semver.org/#spec-item-9

A pre-release version MAY be denoted by appending a hyphen and a series of dot separated identifiers immediately following the patch version. Identifiers MUST comprise only ASCII alphanumerics and hyphen [0-9A-Za-z-]. Identifiers MUST NOT be empty. Numeric identifiers MUST NOT include leading zeroes.

@pacamara
Copy link
Author

@msand Thanks for filing facebook/react-native#17747!

should we change them to align with the latest stable react-native init, or revert to the previous versions,

The only argument I can think of for aligning them with current react-init values is users wouldn't have to install as many versions of SDK Build Tools. Seems like a slim advantage, so reverting to the previous versions seems safer.

@msand
Copy link
Collaborator

msand commented Feb 3, 2018

@pacamara @sibelius I've released a new version, which aligns with react-native init, to make it as easy as possible for newcomers. Anyone with needs to upgrade any versions to anything different from that, will hopefully learn how to change them in their dependencies in the process as well. Lets see if they upgrade any versions, now that a PR exists. Can release a new version to align with that when they do.

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

No branches or pull requests

5 participants