Scaffold Gradle Wrapper differenly

[cortinico]

Describe the Feature

I'm not sure how to name this feature, but I just wanted to share my thoughts on a problem which is affecting the CLI and can improve the developer experience somehow. Happy to collect opinion.

The current situation is that we have the following files committed in the new app template and in every project of users:

android/gradle/wrapper/gradle-wrapper.jar
android/gradle/wrapper/gradle-wrapper.properties
android/gradlew
android/gradlew.bat

Those files are The Gradle Wrapper, essentially scripts that download and execute the correct Gradle version.

The problem is that those files are committed in the repo of users, and create noise in the upgrade helper. For example: https://react-native-community.github.io/upgrade-helper/?from=0.67.5&to=0.68.6

With the upcoming version of React Native, we'll be having similar problems as the version of Gradle got updated and the Gradle Wrapper was also updated.

Binary files 0.71.1/RNDiffProj/android/gradle/wrapper/gradle-wrapper.jar and nightly/RNDiffProj/android/gradle/wrapper/gradle-wrapper.jar differ
diff --color -r 0.71.1/RNDiffProj/android/gradle/wrapper/gradle-wrapper.properties nightly/RNDiffProj/android/gradle/wrapper/gradle-wrapper.properties
3c3
< distributionUrl=https\://services.gradle.org/distributions/gradle-7.5.1-all.zip
---
> distributionUrl=https\://services.gradle.org/distributions/gradle-7.6-bin.zip
diff --color -r 0.71.1/RNDiffProj/android/gradlew nightly/RNDiffProj/android/gradlew
207a208,213
> # Stop when "xargs" is not available.
> if ! command -v xargs >/dev/null 2>&1
> then
>     die "xargs is not available"
> fi
>
diff --color -r 0.71.1/RNDiffProj/android/gradlew.bat nightly/RNDiffProj/android/gradlew.bat
17c17
< @if "%DEBUG%" == "" @echo off
---
> @if "%DEBUG%"=="" @echo off
28c28
< if "%DIRNAME%" == "" set DIRNAME=.
---
> if "%DIRNAME%"=="" set DIRNAME=.
43c43
< if "%ERRORLEVEL%" == "0" goto execute
---
> if %ERRORLEVEL% equ 0 goto execute
78c78
< if "%ERRORLEVEL%"=="0" goto mainEnd
---
> if %ERRORLEVEL% equ 0 goto mainEnd
83,84c83,86
< if  not "" == "%GRADLE_EXIT_CONSOLE%" exit 1
< exit /b 1
---
> set EXIT_CODE=%ERRORLEVEL%
> if %EXIT_CODE% equ 0 set EXIT_CODE=1
> if not ""=="%GRADLE_EXIT_CONSOLE%" exit %EXIT_CODE%
> exit /b %EXIT_CODE%

The problem is that we can't expect users to go inside the ./gradlew.bat (which is a Windows only file) to replicate the changes that the Gradle Wrapper brings over.

The problems are the following:

1. android/gradle/wrapper/gradle-wrapper.properties this is probably the only file we want to commit (if any) as it contains the Gradle version.
2. android/gradle/wrapper/gradle-wrapper.jar this is a binary file. We can't expect users to go to the react-native repo to download the corresponding binary
3. android/gradlew this is a bash script that has to be updated.
4. android/gradlew.bat this is a windows bat script that has to be updated.

Simple Implementation

The updates to those mentioned files can be achieved easily by the user if they run

cd android && ./gradlew wrapper --gradle-version=<VERSION> --distribution-type=all

So we could simply update the upgrade helper to:

1. Hide all the changes to the aformentioned files
2. Show a popup to run the command mentioned above (or have some sort of util inside the CLI that runs that).

Still users could miss the popup on top and never update their Gradle Wrapper version (which could cause build failures).

More complicated implementation

Potentially we can:

1. Ask the user to update only the android/gradle/wrapper/gradle-wrapper.properties
2. Hide the changes to gradlew, gradlew.bat, and gradle/wrapper/gradle-wrapper.jar from the upgrade helper.
3. Have checks on the run-android and build-android commands that check if the wrapper is up to date (i.e. by diffing the files against local versions of the wrapper), if not, run the command above that updates the wrapper for the user.

[thymikee]

Ok, so I ran:

./gradlew wrapper --gradle-version=7.6 --distribution-type=all

from a 0.71 app and it updated the version in gradle-wrapper.properties. Then I ran it second time and it downloaded the binaries (took 55s on my machine):

When I ran it again, it bails after 2s with no updates. To me it makes sense to run it before build-android either every time or only when we detect changes (which I hope there's a non-hacky way of doing).

[cortinico]

(which I hope there's a non-hacky way of doing).

I believe the CLI can hash those files (as you know beforehand which version of Gradle we ship in a given version of RN), and if the hash doesn't match:

1. Heads up to the user on build-android/run-android that their version is not the correct one, y/n
2. If y run ./gradlew wrapper twice.

[blakef]

Running it twice seems to be a known issue for the last 10yrs. I don't think there's a programatic way to determine if we need to run this once or twice?

The first time I run it, it produces the diff that @cortinico showed. After running for the 2nd time it made these changes (not that it matters, because what you're proposing is significantly more robust and simplifies upgrades):

diff --git a/android/gradle/wrapper/gradle-wrapper.jar b/android/gradle/wrapper/gradle-wrapper.jar
index 249e583..943f0cb 100644
Binary files a/android/gradle/wrapper/gradle-wrapper.jar and b/android/gradle/wrapper/gradle-wrapper.jar differ
diff --git a/android/gradle/wrapper/gradle-wrapper.properties b/android/gradle/wrapper/gradle-wrapper.properties
index f42e62f..2b22d05 100644
--- a/android/gradle/wrapper/gradle-wrapper.properties
+++ b/android/gradle/wrapper/gradle-wrapper.properties
@@ -1,5 +1,6 @@
 distributionBase=GRADLE_USER_HOME
 distributionPath=wrapper/dists
 distributionUrl=https\://services.gradle.org/distributions/gradle-7.6-all.zip
+networkTimeout=10000
 zipStoreBase=GRADLE_USER_HOME
 zipStorePath=wrapper/dists
diff --git a/android/gradlew b/android/gradlew
index a69d9cb..65dcd68 100755
--- a/android/gradlew
+++ b/android/gradlew
@@ -55,7 +55,7 @@
 #       Darwin, MinGW, and NonStop.
 #
 #   (3) This script is generated from the Groovy template
-#       https://github.com/gradle/gradle/blob/master/subprojects/plugins/src/main/resources/org/gradle/api/internal/plugins/unixStartScript.txt
+#       https://github.com/gradle/gradle/blob/HEAD/subprojects/plugins/src/main/resources/org/gradle/api/internal/plugins/unixStartScript.txt
 #       within the Gradle project.
 #
 #       You can find Gradle at https://github.com/gradle/gradle/.
@@ -80,10 +80,10 @@ do
     esac
 done
 
-APP_HOME=$( cd "${APP_HOME:-./}" && pwd -P ) || exit
-
-APP_NAME="Gradle"
+# This is normally unused
+# shellcheck disable=SC2034
 APP_BASE_NAME=${0##*/}
+APP_HOME=$( cd "${APP_HOME:-./}" && pwd -P ) || exit
 
 # Add default JVM options here. You can also use JAVA_OPTS and GRADLE_OPTS to pass JVM options to this script.
 DEFAULT_JVM_OPTS='"-Xmx64m" "-Xms64m"'
@@ -143,12 +143,16 @@ fi
 if ! "$cygwin" && ! "$darwin" && ! "$nonstop" ; then
     case $MAX_FD in #(
       max*)
+        # In POSIX sh, ulimit -H is undefined. That's why the result is checked to see if it worked.
+        # shellcheck disable=SC3045 
         MAX_FD=$( ulimit -H -n ) ||
             warn "Could not query maximum file descriptor limit"
     esac
     case $MAX_FD in  #(
       '' | soft) :;; #(
       *)
+        # In POSIX sh, ulimit -n is undefined. That's why the result is checked to see if it worked.
+        # shellcheck disable=SC3045 
         ulimit -n "$MAX_FD" ||
             warn "Could not set maximum file descriptor limit to $MAX_FD"
     esac
diff --git a/android/gradlew.bat b/android/gradlew.bat
index 53a6b23..6689b85 100644
--- a/android/gradlew.bat
+++ b/android/gradlew.bat
@@ -26,6 +26,7 @@ if "%OS%"=="Windows_NT" setlocal
 
 set DIRNAME=%~dp0
 if "%DIRNAME%"=="" set DIRNAME=.
+@rem This is normally unused
 set APP_BASE_NAME=%~n0
 set APP_HOME=%DIRNAME%
 

[cortinico]

I don't think there's a programatic way to determine if we need to run this once or twice?

It should be safe to always run it twice

[cortinico]

Another related issue but for iOS is about the project.pbxproj here:

• Unclear Upgrade Documentation for project.pbxproj changes on iOS facebook/react-native-website#3607

[github-actions]

There hasn't been any activity on this issue in the past 3 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 7 days.

[cortinico]

This is still a thing. I've asked @pvinis to take a stance and add a banner to the upgrade helper at least to mitigate the impact of those files appearing in the upgrade helper

[pvinis]

oh god I'm sorry I haven't done that yet! it's been busy. I'll try to take a look this week 🙇🏼

[github-actions]

There hasn't been any activity on this issue in the past 3 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 7 days.

[szymonrybczak]

Do we have any updates here? cc. @cortinico @pvinis

[pvinis]

I have not had time to check this, sorry for that. To be honest, it's kinda hard to find the motivation, when I'm not using, thanks to expo. I still have a reminder to check this issue out, but have not found the time. I feel like the upgrades even without that tweak are not hard, just less readable.

[cortinico]

I think the minimal change we could do is just to "hide" those changes in the diff helper and mention what is the command to invoke to update those files.

[cortinico]

Also 0.73 is going to be affected by this as well

[szymonrybczak]

Alright, I will try to tackle this. Hopefully before React Native 0.73 release.

[adamTrz]

@cortinico - did small PR with proposed solution. Mind taking a look please?