Update Gradle Wrapper to 6.6

[friederbluemle]

Summary

Gradle Wrapper to 6.6

https://docs.gradle.org/6.6/release-notes.html

Changelog

[Android] [Changed] - Update Gradle Wrapper to 6.6

Test Plan

Build project

Closes #28914

[analysis-bot]

Platform	Engine	Arch	Size (bytes)	Diff
ios	-	universal	n/a	--

Base commit: eecb930

[analysis-bot]

Platform	Engine	Arch	Size (bytes)	Diff
android	hermes	arm64-v8a	7,003,030	-28
android	hermes	armeabi-v7a	6,666,643	-18
android	hermes	x86	7,423,355	-28
android	hermes	x86_64	7,314,247	-26
android	jsc	arm64-v8a	9,162,798	-128
android	jsc	armeabi-v7a	8,818,528	-122
android	jsc	x86	9,011,190	-134
android	jsc	x86_64	9,588,283	-131

Base commit: eecb930

[mdvacca]

CC @kgozali

[fkgozali]

we upgraded to 6.5 not too long ago, any specific benefit for this upgrade to 6.6?

[fkgozali]

@friederbluemle @dulmandakh, thoughts on my question?

we upgraded to 6.5 not too long ago, any specific benefit for this upgrade to 6.6?

[friederbluemle]

@fkgozali - Thanks for the review/question.

There's no "specific benefit" for this update (i.e. it does not fix any major/blocking issue). However, the same question can be asked for the previous update to 6.5 (or other updates before that). I'm only following what seems to have been standard practice in this repo to always update Gradle Wrapper to the latest version.

Gradle 6.6 comes with some potential performance improvements (hard to measure those, and maybe negligible/non-existent in the React Native repo). Perhaps more importantly, Gradle 6.6 fixes a whitespace error in the gradlew script on line 133 that was introduced with Gradle 6.5 - Because this whitespace error is also present in the template/ folder, it would be carried over into every single new React Native app created with react-native init.

All that said, if it was me, we should simply keep the version of the Gradle Wrapper aligned with the version of the Android Gradle plugin (update less often), unless there are specific reasons for a newer, out-of-sync version. Thoughts?

[friederbluemle]

Rebased onto latest master branch.

[fkgozali]

OK. The reason I asked is that, each upgrade is not as simple as just landing this PR. We have a bunch of offline caching in our internal CI that needs to be updated, and a bunch of other scripts as well. I wasn't involved in the upgrades pre-6.5 so I wasn't sure what the thoughts back then was, but moving forward we do want to be intentional when upgrading the build system, not just because it's the latest and the greatest.

That said, we may just take in this 6.6 soon (maybe in a week or so). Though further upgrades should probably be discussed briefly in the future.

[facebook-github-bot]

@fkgozali has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

[friederbluemle]

@fkgozali - Sounds good, and makes sense! Thanks for the additional explanation 👍

Like I said, in the future we could also consider simply keeping the version of Gradle Wrapper aligned with the default version used by the Android Gradle plugin, unless there is a specific reason to upgrade.
Yes, this might involve a one-time downgrade of Gradle Wrapper, since it will probably be quite a while until AGP defaults to Gradle 6.6+. In order to avoid a major version downgrade, a good potential opportunity would be once we move from AGP 3 to AGP 4 (#29013), since AGP 4.0.1 uses Gradle Wrapper 6.1.1.

[react-native-bot]

This pull request was successfully merged by @friederbluemle in 5bc67b6.

^{When will my fix make it into a release? | Upcoming Releases}

[friederbluemle]

Hi @fkgozali - Quick question: I noticed the rewritten commit from this PR that ended up on the master branch (5bc67b6) has the -bin variant of Gradle Wrapper. This PR has -all (like before, and also what's the default for Android Studio). Do you happen to know why?

[fkgozali]

Hi @fkgozali - Quick question: I noticed the rewritten commit from this PR that ended up on the master branch (5bc67b6) has the -bin variant of Gradle Wrapper. This PR has -all (like before, and also what's the default for Android Studio). Do you happen to know why?

I'm not too sure, I think I needed to run gradlew wrapper (to upgrade other scripts) and it might have produced the change. Regardless, it seems reasonable to use -bin (for less build-time download)? I noticed some gradlew used -all, some used -bin.

[fkgozali]

According to https://docs.gradle.org/current/userguide/gradle_wrapper.html:

The type of Gradle distribution. By default that’s the -bin distribution containing only the runtime but no sample code and documentation.

So it seems ok to rely on -bin?

[friederbluemle]

Ooh, okay, so I guess you ran ./gradlew wrapper without the --distribution-type all flag. It also happened to me before, which is why I defined an alias in my shell to make this easier to update Gradle Wrapper across many repositories that involve Android.
-all is actually the default as generated by Android Studio, and it also provides additional sources helpful for debugging. It's also much more likely to already exist on a developer's machine (which has built other Android projects), avoiding additional downloads and saving disk space. Therefore, the -all variant is preferred.
I opened a PR here: #29793

[fkgozali]

I see, I'll merge that tomorrow/early next week On Aug 27, 2020, at 5:04 PM, Frieder Bluemle <notifications@github.com> wrote:  Ooh, okay, so I guess you ran ./gradlew wrapper without the --distribution-type all flag. It also happened to me before, which is why I defined an alias in my shell to make this easier to update Gradle Wrapper across many repositories that involve Android. -all is actually the default as generated by Android Studio, and it also provides additional sources helpful for debugging. It's also much more likely to already exist on a developer's machine (which has built other Android projects), avoiding additional downloads and saving disk space. Therefore, the -all variant is preferred. I opened a PR here: #29793<#29793> — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub<#29613 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AAILROTLKVFIEYQ3CGYSZLDSC3YBTANCNFSM4P2VKDKA>.

[friederbluemle]

Sounds good, thanks for the fast replies @fkgozali 👍

[mikehardy]

When my clone of react-native includes this PR, I have a persistent line ending issue with one of the gradlew.bat files, it's resistant to all my attempts to fix so far, on ubuntu 20

mike@isabela:~/work/react-random/react-native (master *) % git diff |head -10
diff --git a/packages/react-native-codegen/android/gradlew.bat b/packages/react-native-codegen/android/gradlew.bat
index ac1b06f938..107acd32c4 100644
--- a/packages/react-native-codegen/android/gradlew.bat
+++ b/packages/react-native-codegen/android/gradlew.bat
@@ -1,89 +1,89 @@
-@rem
-@rem Copyright 2015 the original author or authors.
-@rem
-@rem Licensed under the Apache License, Version 2.0 (the "License");
-@rem you may not use this file except in compliance with the License.
mike@isabela:~/work/react-random/react-native (master *) % git diff -w
mike@isabela:~/work/react-random/react-native (master *) % 
mike@isabela:~/work/react-random/react-native (master *) % git log packages/react-native-codegen/android/gradlew.bat|head -3
commit 5bc67b658e581e0176deb7ed95b51a5c1cbe65c2
Author: Frieder Bluemle <frieder.bluemle@gmail.com>
Date:   Wed Aug 26 16:32:53 2020 -0700

[friederbluemle]

Hi @mikehardy - Yeah, unfortunately that is a problem for me as well (I'm surprised not more people have pointed this out)..

I've already opened a PR with a brief description more than a week ago: #29792
Hopefully it can get merged soon.