Scaffold Gradle Wrapper differenly

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:

android/gradle/wrapper/gradle-wrapper.properties this is probably the only file we want to commit (if any) as it contains the Gradle version.
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
android/gradlew this is a bash script that has to be updated.
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:

Hide all the changes to the aformentioned files
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:

Ask the user to update only the android/gradle/wrapper/gradle-wrapper.properties
Hide the changes to gradlew, gradlew.bat, and gradle/wrapper/gradle-wrapper.jar from the upgrade helper.
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.

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

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

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

@cortinico

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%

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

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

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

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.

@pvinis

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

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

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

Do we have any updates here? cc. @cortinico @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.

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.

Also 0.73 is going to be affected by this as well

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

@cortinico

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

Navigation Menu

Repository navigation

Scaffold Gradle Wrapper differenly #1837

Describe the Feature

Simple Implementation

More complicated implementation

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Scaffold Gradle Wrapper differenly #1837

Description

Describe the Feature

Simple Implementation

More complicated implementation

Activity

thymikee commented on Feb 17, 2023

cortinico commented on Feb 17, 2023

blakef commented on Feb 24, 2023

cortinico commented on Feb 24, 2023

cortinico commented on Mar 6, 2023

github-actions commented on Jun 5, 2023

cortinico commented on Jun 5, 2023

pvinis commented on Jun 5, 2023

github-actions commented on Sep 4, 2023

szymonrybczak commented on Oct 20, 2023

pvinis commented on Oct 20, 2023

cortinico commented on Oct 20, 2023

cortinico commented on Oct 20, 2023

szymonrybczak commented on Oct 22, 2023

adamTrz commented on Oct 24, 2023

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions