Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Gradle wrapper upgrade should update more files #3565

Closed
jnizet opened this issue Apr 19, 2019 · 9 comments · Fixed by #5842
Closed

Gradle wrapper upgrade should update more files #3565

jnizet opened this issue Apr 19, 2019 · 9 comments · Fixed by #5842
Assignees
Labels
manager:gradle Gradle package manager priority-3-medium Default priority, "should be done" but isn't prioritised ahead of others type:bug Bug fix of existing functionality

Comments

@jnizet
Copy link

jnizet commented Apr 19, 2019

What Renovate type are you using?
Renovate GitHub App

Describe the bug

The upgrade of gradle is incomplete. It only updates the version in gradle/wrapper/gradle-wrapper.properties (see for example https://github.com/Ninja-Squad/globe42/pull/663/files, which upgrades from 5.3.1 to 5.4).
However, if I do the upgrade manually by following the procedure documented in the Gradle release notes:

./gradlew wrapper --gradle-version=5.4

then not only this URL is changed, but the generated gradlew and gradlew.bat files, and the binary wrapper jar files are being modified. See https://github.com/Ninja-Squad/globe42/pull/665/files for the same upgrade doing it by applying the correct upgrade procedure.

Renovate should apply the correct procedure instead of just changing the version.

Expected behavior

I expect renovate to produce the same changes as if I executed

./gradlew wrapper --gradle-version=5.4
@rarkins rarkins added manager:gradle Gradle package manager type:bug Bug fix of existing functionality priority-2-high Bugs impacting wide number of users or very important features ready and removed type:bug Bug fix of existing functionality ready labels Apr 19, 2019
@rarkins
Copy link
Collaborator

rarkins commented Apr 19, 2019

The difference seems to be:

  • comments (surely entirely optional, and were they really introduced in 5.4?)
  • duplicate/incorrect 64MB RAM limit

I’m not a Gradle user but neither of these seem to be a problem, so will wait to gather more opinions before making any changes.

@jnizet
Copy link
Author

jnizet commented Apr 19, 2019

There is also a difference in the binary jar file used to download the gradle distribution. Maybe it's unimportant, maybe it's a big security fix. And maybe future upgrades will introduce more important changes in the script files or elsewhere.

The fact that this specific upgrade only brings minor, maybe ignorable changes to the files is irrelevant: there is an official procedure to upgrade gradle, that is documented and does everything required to make sure everything is installed as expected, and renovate should follow this procedure instead of taking shortcuts. IMHO of course :-)

@jnizet
Copy link
Author

jnizet commented Apr 19, 2019

Besides, these comments contain copyright/licensing information. If gradle, an open source project that I happily benefit from, thinks this should be in the files that I redistribute in my own projects, then I should comply, IMHO.

@rarkins
Copy link
Collaborator

rarkins commented Apr 19, 2019

  • Shouldn’t the copyright terms have been in your repo already?
  • Or was 5.4 the first release with them in or enforced?

@BigMichi1
Copy link

when doing a gradle upgrade with the command above it can change up to 4 files

  • gradlew - linux command executable
  • gradlew.cmd - windows command executable
  • gradle-wrapper.properties - file with the desired version number of gradle
  • gradle-wrapper.jar - logic for downloading the new distribution

start scripts rarly contain changes which are important, but gradle-wrapper.jar may contain security and bugfixes which are important for the downloading of the dsitribution which will be used when building the project

@rarkins
Copy link
Collaborator

rarkins commented Apr 19, 2019

Thanks. Sounds like we need to perform this command as an “artifacts” get like we do with lock files, checksums, etc.

@rarkins rarkins added type:bug Bug fix of existing functionality ready and removed needs-requirements labels Apr 19, 2019
@jnizet
Copy link
Author

jnizet commented Apr 19, 2019

Shouldn’t the copyright terms have been in your repo already?

I have used renovate for the last few gradle updates, but I used to do the upgrade by myself before, so maybe the copyright terms could have appeared before (if I hadn't used renovate), but they're recent.

@rarkins rarkins changed the title Gradle upgrade is incorrect Gradle wrapper upgrade should update more files Apr 22, 2019
@rarkins rarkins added priority-3-medium Default priority, "should be done" but isn't prioritised ahead of others and removed priority-2-high Bugs impacting wide number of users or very important features labels Apr 29, 2019
BenSartor added a commit to simlar/simlar-server that referenced this issue Mar 1, 2020
   * run
     ./gradlew wrapper --distribution-type all
     git checkout gradle/wrapper/gradle-wrapper.properties

   * see renovatebot issue:
     renovatebot/renovate#3565
@driver733
Copy link
Contributor

@rarkins I can try to fix this issue. Can you assign it to me?

@renovate-release
Copy link
Collaborator

🎉 This issue has been resolved in version 19.192.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 16, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
manager:gradle Gradle package manager priority-3-medium Default priority, "should be done" but isn't prioritised ahead of others type:bug Bug fix of existing functionality
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants