You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@thrift.apache.org by "ctubbsii (via GitHub)" <gi...@apache.org> on 2024/08/14 21:52:36 UTC
[PR] Simplify Gradle installation in GitHub Actions [thrift]
ctubbsii opened a new pull request, #3022:
URL: https://github.com/apache/thrift/pull/3022
* Make use of gradle wrapper (gradlew) without installing it in the source repo by adding it to GitHub Actions
* Rely on gradle wrapper to install the correct version (currently 8.4); gradlew is installed using `gradle wrapper` using the default version of gradle available in GitHub Actions (which I believe is 8.9 currently)
* Modify build steps to use gradlew instead of gradle
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Simplify Gradle installation in GitHub Actions [thrift]
Posted by "ctubbsii (via GitHub)" <gi...@apache.org>.
ctubbsii commented on PR #3022:
URL: https://github.com/apache/thrift/pull/3022#issuecomment-2290001040
@jimexist I think you know the Java/Kotlin build stuff better than I do. I did this to attempt to simplify the GitHub Actions. However, with this change, it's easier to set the version and use gradlew for the spotless and ktfmt stuff, but the configure script still uses the version 8.9 from the GitHub Actions image because it doesn't know about gradlew, only the gradle on the path. Do you know of an easy way for the configure script to pick up and use gradlew? I was thinking it might be easier to contribute if users could use `gradle wrapper` to easily install the correct version of gradle using gradlew, but that won't work if the configure script ignores it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Simplify Gradle installation in GitHub Actions [thrift]
Posted by "ctubbsii (via GitHub)" <gi...@apache.org>.
ctubbsii commented on PR #3022:
URL: https://github.com/apache/thrift/pull/3022#issuecomment-2292301353
> in my opinion the easiest way is to just use https://github.com/gradle/wrapper-validation-action but not sure if this 3rd party (in the context of GitHub action) action is allowed?
I think you mean [setup-gradle](https://github.com/gradle/actions?tab=readme-ov-file#the-setup-gradle-action)? I think it would be easy to get ASF INFRA to approve it if they haven't already.
However, my bigger concerns are that they won't work correctly:
1. the `setup-gradle` action sets up the gradle wrapper in the root of the project. We only use it in `lib/java` (or `lib/kotlin`) and expect it to be set up there. I spent quite a bit of time trying to set up the gradle wrapper outside of the project tree, so it could be used only as version control (like `rvm` or `rbenv` are used to manage ruby installations), but it wouldn't work because the gradle wrapper very strictly requires being installed inside an existing project and cannot be easily used to manage versions outside of a project. Since the `setup-gradle` action doesn't have the option to specify the target directory (that I could find), we'd have to substantially change things around to have gradlew installed at the root of the project to work with this action.
2. even if we use `setup-gradle` action, I don't think `./configure` would find it without additional changes to `configure.ac`, and it would still use `gradle` from the environment when building with `make` instead of `./gradlew`. Perhaps there's a way to specify the gradle command directly using a `./configure` flag that I don't know about? If so, then perhaps we can force it to use `./gradlew` instead of `gradle`, but I don't know how to do that.
My conclusion is that it's probably not worth changing right now. However, perhaps it might be possible to just simplify the installation of our preferred gradle version by installing it in local directory and adding it to the PATH, rather than trying to clobber the system-provided gradle using `sudo`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Simplify Gradle installation in GitHub Actions [thrift]
Posted by "jimexist (via GitHub)" <gi...@apache.org>.
jimexist commented on PR #3022:
URL: https://github.com/apache/thrift/pull/3022#issuecomment-2290527745
in my opinion the easiest way is to just use https://github.com/gradle/wrapper-validation-action but not sure if this 3rd party (in the context of GitHub action) action is allowed?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Simplify Gradle installation in GitHub Actions [thrift]
Posted by "fishy (via GitHub)" <gi...@apache.org>.
fishy commented on code in PR #3022:
URL: https://github.com/apache/thrift/pull/3022#discussion_r1719011713
##########
.github/workflows/build.yml:
##########
@@ -209,24 +209,21 @@ jobs:
sudo apt-get install -y --no-install-recommends $BUILD_DEPS
sudo apt-get install -y wget unzip ant maven
- - name: Setup gradle
+ - name: Setup gradle wrapper
run: |
- wget https://services.gradle.org/distributions/gradle-$GRADLE_VERSION-bin.zip -q -O /tmp/gradle-$GRADLE_VERSION-bin.zip
- (echo "3e1af3ae886920c3ac87f7a91f816c0c7c436f276a6eefdb3da152100fef72ae /tmp/gradle-$GRADLE_VERSION-bin.zip" | sha256sum -c -)
- unzip -d /tmp /tmp/gradle-$GRADLE_VERSION-bin.zip
- sudo mv /tmp/gradle-$GRADLE_VERSION /usr/local/gradle
- sudo ln -s /usr/local/gradle/bin/gradle /usr/local/bin
- gradle --version
+ cd lib/java
+ gradle wrapper --gradle-version $GRADLE_VERSION
Review Comment:
hmm this would require `gradle` to be already in `$PATH` at this point, right? I guess that's handled by `actions/setup-java@v4` above, as the this step passes? I wonder how this is handled/expected from developers' computers.
##########
.github/workflows/build.yml:
##########
@@ -209,24 +209,21 @@ jobs:
sudo apt-get install -y --no-install-recommends $BUILD_DEPS
sudo apt-get install -y wget unzip ant maven
- - name: Setup gradle
+ - name: Setup gradle wrapper
run: |
- wget https://services.gradle.org/distributions/gradle-$GRADLE_VERSION-bin.zip -q -O /tmp/gradle-$GRADLE_VERSION-bin.zip
- (echo "3e1af3ae886920c3ac87f7a91f816c0c7c436f276a6eefdb3da152100fef72ae /tmp/gradle-$GRADLE_VERSION-bin.zip" | sha256sum -c -)
- unzip -d /tmp /tmp/gradle-$GRADLE_VERSION-bin.zip
- sudo mv /tmp/gradle-$GRADLE_VERSION /usr/local/gradle
- sudo ln -s /usr/local/gradle/bin/gradle /usr/local/bin
- gradle --version
+ cd lib/java
+ gradle wrapper --gradle-version $GRADLE_VERSION
+ ./gradlew --version
Review Comment:
disclaimer: I don't really understand gradle much, but I think the `gradle wrapper ...` step above generates `./gradlew` script that's used in this step, is that correct?
can we do that locally and commit `gradlew` into git instead? that seems to be the way android studio generating android projects.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Simplify Gradle installation in GitHub Actions [thrift]
Posted by "ctubbsii (via GitHub)" <gi...@apache.org>.
ctubbsii closed pull request #3022: Simplify Gradle installation in GitHub Actions
URL: https://github.com/apache/thrift/pull/3022
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org