ytai / ioio

Software, firmware and hardware of the IOIO - I/O for Android
Apache License 2.0
746 stars 355 forks source link

Add workflow for Update Gradle Wrapper Action. #208

Closed cristiangreco closed 3 years ago

cristiangreco commented 3 years ago

Hey there 👋, first of all thanks for your work on IOIO!

I've got a suggested change: would you be willing to use this GitHub Action to automatically keep Gradle Wrapper updated to latest release?

What does "Update Gradle Wrapper Action" do? It can be configured to run at scheduled intervals (e.g. daily or weekly) and will check whether the Wrapper script in the repo is up-to-date to the latest Gradle release: in case a new Gradle version is available, it will create a PR to update the Wrapper. And that's it!

Why is that a good thing? Well, first of all it alleviates the chore of manually updating the Wrapper, as you got a task that keeps track of new Gradle releases for you! More importantly, it boosts security around the Wrapper update and usage processes: this actions verifies that the gradle-wrapper.jar file has not been tampered with (uses checksum comparison), and it sets the distributionSha256Sum property so that the new Gradle binary itself will be verified locally upon download.

Where can I find more about? The README contains quite detailed information!

In this PR I propose adding a new workflow which runs the action every day at midnight (but feel free to adjust the frequency as you prefer). I've verified it works correctly in my fork of the repo, and you can see here how the generated PR looks like.

The action is under active development, you can have a look at the list of inputs currently supported. There's new features coming up soon and if you'd like to request any particular change just let me know!

I'd love to see the action used by IOIO and I genuinely hope you can find this useful. Would love your feedback! ❤️

hannesa2 commented 3 years ago

I'm not very familiar with coding of Github actions, please can you point me to the code where the gradle command is performed in your code https://github.com/gradle-update/update-gradle-wrapper-action ?

I expected somewhere a gradlew wrapper --gradle-version X.X.X or similar in your repo, but I didn't found it

cristiangreco commented 3 years ago

I'm not very familiar with coding of Github actions, please can you point me to the code where the gradle command is performed in your code https://github.com/gradle-update/update-gradle-wrapper-action ?

I expected somewhere a gradlew wrapper --gradle-version X.X.X or similar in your repo, but I didn't found it

So the action is written in TypeScript to leverage libraries provided by GitHub, and here is the piece of code that runs the actual update. Let me know if you need any further clarification! 🙂

hannesa2 commented 3 years ago

Thank you. Is there something similar for dependencies update ?

cristiangreco commented 3 years ago

Thank you. Is there something similar for dependencies update ?

Sure, you can use Dependabot for keeping your dependencies up to date. It used to be a separate app but it's now owned and developed by GitHub itself. I use it for the UGW action, see here.

The only thing Dependabot doesn't do is updating the Gradle Wrapper itself, but you got covered now 😉

topherbuckley commented 3 years ago

Thanks for your suggestions and work @cristiangreco.

Maybe this is already covered in your action, and I would need to read your code base, but if the updated Gradle Wrapper breaks a build somehow, what is the typical workflow to catch that and revert? Is it standard to run all our other github actions that check for build integrity after each weekly wrapper update or would we just check this after our next PR or push? Or does your action represent a push or PR after it updates the wrapper such that it would trigger our other github actions triggered by pushes and PRs?

It seems like the checks would have to be run at the same time to ensure we don't lose track of what broke what.

hannesa2 commented 3 years ago

This first update happens #209 🍾

Additionally to this question https://github.com/ytai/ioio/pull/209#discussion_r499364530

When I run after this locally ./gradlew wrapper --gradle-version 6.1.1 I observe this changes.

image

It removes the distributionSha256Sum again. Do we need it ?

cristiangreco commented 3 years ago

Thanks for your suggestions and work @cristiangreco.

Maybe this is already covered in your action, and I would need to read your code base, but if the updated Gradle Wrapper breaks a build somehow, what is the typical workflow to catch that and revert? Is it standard to run all our other github actions that check for build integrity after each weekly wrapper update or would we just check this after our next PR or push? Or does your action represent a push or PR after it updates the wrapper such that it would trigger our other github actions triggered by pushes and PRs?

It seems like the checks would have to be run at the same time to ensure we don't lose track of what broke what.

@topherbuckley good question! As you say, it is very good practice to run CI checks on any PR, including those generated by UGW action.

For PRs created automatically by GitHub actions there's a caveat though: if you're using an external CI service (like e.g. Travis) your checks will run normally; if, however, you're using GitHub actions for CI checks (like in your case) then such workflows are not triggered automatically even if you've specified push or pull_request events.

Why is that? This is a limitation enforced by GitHub: to avoid the chance of creating infinite trigger loops, PRs created by actions do not automatically trigger further events.

Can you work around this? Yes! Instead of using the secrets.GITHUB_TOKEN you need to use a Personal Access Token: follow the instructions here to create a PAT with a repo scope and set it as repo-token input.

Let me know if you need any help with that!

cristiangreco commented 3 years ago

This first update happens #209 🍾

Additionally to this question #209 (comment)

When I run after this locally ./gradlew wrapper --gradle-version 6.1.1 I observe this changes.

image

It removes the distributionSha256Sum again. Do we need it ?

@hannesa2 I've replied to the other PR, hope it is clear what that property is used for.

Just out of curiosity, why are you running gradle wrapper locally? Is it to verify that the outcome matches with the content of the PR? In case you just wanted to download the new Gradle 6.6.1 version, executing ./gradlew build would do it for you!

topherbuckley commented 3 years ago

@cristiangreco thanks for the detailed explanation! That makes sense. I guess the personal access token has to be generated and saved as a secret by @hannesa2 as he is the only one with appropriate access rights?

hannesa2 commented 3 years ago

Can you work around this? Yes! Instead of using the secrets.GITHUB_TOKEN you need to use a Personal Access Token: follow the instructions here to create a PAT with a repo scope and set it as repo-token input.

Hm, I'm not sure if we already did this (I would say we did it). I use here my own token as secret to make release work. This token has repo scope

But it's not that easy to change this, because I've not to rights to change this

hannesa2 commented 3 years ago

Just out of curiosity, why are you running gradle wrapper locally? Is it to verify that the outcome matches with the content of the PR? In case you just wanted to download the new Gradle 6.6.1 version, executing ./gradlew build would do it for you!

To verify what this action did.

But I never observed a ./gradlew build which change

hannesa2 commented 3 years ago

I guess the personal access token has to be generated and saved as a secret by @hannesa2 as he is the only one with appropriate access rights?

Sorry, I've not the power to go into settings. Only @ytai can do this. He applied my personal access token here (which has repo scope) that's why the release action is working. He declined to generate an own token, that's why we use my one

hannesa2 commented 3 years ago

I will continue to investigate how to make this update action triggers new github action here https://github.com/hannesa2/LiveEdgeDetection because there, I've to power to change everything

hannesa2 commented 3 years ago

To be honest, I'm somehow disappointed, now on open project I see this.

image

I want to have a error and message free repo on first clone, open and start

hannesa2 commented 3 years ago

I will continue to investigate how to make this update action triggers new github action here https://github.com/hannesa2/LiveEdgeDetection because there, I've to power to change everything

Hmm, there https://github.com/hannesa2/LiveEdgeDetection/pull/87 is starts other github action with the same token. I've no clue, why this here didn't work

cristiangreco commented 3 years ago

I will continue to investigate how to make this update action triggers new github action here https://github.com/hannesa2/LiveEdgeDetection because there, I've to power to change everything

Hmm, there hannesa2/LiveEdgeDetection#87 is starts other github action with the same token. I've no clue, why this here didn't work

Hmm I see you're using ${{ secrets.GITHUBTOKEN }}, is this a personal access token you created? Saying this because the pre-installed secret would be GITHUB_TOKEN rather than GITHUBTOKEN.

cristiangreco commented 3 years ago

To be honest, I'm somehow disappointed, now on open project I see this.

image

I want to have a error and message free repo on first clone, open and start

This is AndroidStudio unnecessarily complaining. You can get rid of the warning by just clicking the first option ("Use ... as checksum for https://.../gradle-6.6.1-bin.zip and sync project"). I appreciate this might be an annoying warning and might confuse contributors that open the project for the first time. If you're concerned about it you can just remove the distributionSha256Sum property and then set the action to not re-introduce it anymore (see set-distribution-checksum input).

cristiangreco commented 3 years ago

Just out of curiosity, why are you running gradle wrapper locally? Is it to verify that the outcome matches with the content of the PR? In case you just wanted to download the new Gradle 6.6.1 version, executing ./gradlew build would do it for you!

To verify what this action did.

But I never observed a ./gradlew build which change

* `gradle-wrapper.jar`

* `gradlew.bat`

* `gradlew`

The gradlew and gradlew.batscript might change depending on which Gradle version you use to generate the wrapper. If you run ./gradlew wrapper --gradle-version= ... you're running the upgrade with the Gradle version currently specified in your wrapper file. The action uses an external, up-to-date Gradle binary instead, so that's why those files might get updated as well.

hannesa2 commented 3 years ago

Hmm I see you're using ${{ secrets.GITHUBTOKEN }}, is this a personal access token you created? Saying this because the pre-installed secret would be GITHUB_TOKEN rather than GITHUBTOKEN.

Yes, GITHUBTOKEN is a personal access token I created

Here it has the name GITHUB_TOKEN https://github.com/ytai/ioio/blob/master/.github/workflows/Android-CI-release.yml#L34 but I can't verify it in settings. But it works

cristiangreco commented 3 years ago

Hmm I see you're using ${{ secrets.GITHUBTOKEN }}, is this a personal access token you created? Saying this because the pre-installed secret would be GITHUB_TOKEN rather than GITHUBTOKEN.

Yes, GITHUBTOKEN is a personal access token I created

Here it has the name GITHUB_TOKEN https://github.com/ytai/ioio/blob/master/.github/workflows/Android-CI-release.yml#L34 but I can't verify it in settings. But it works

So the problem with the default GITHUB_TOKEN is that if an action creates a PR, other workflows won't run in this PR. This is what's happening with the Update Gradle Wrapper action, which creates a PR automatically but your usul workflow don't run in this PR.

The Android-CI-release.yml workflow is different, as the commands you run in that workflow don't create a PR.

If GITHUBTOKEN is a personal access token you're already using across your repositories, can you just re-use it for the Update Gradle Wrapper action as well?

hannesa2 commented 3 years ago

If GITHUBTOKEN is a personal access token you're already using across your repositories, can you just re-use it for the Update Gradle Wrapper action as well?

Yes, but I can't change anything in settings. That means, it's like it is