-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Add Maven Wrapper #2305
base: main
Are you sure you want to change the base?
Add Maven Wrapper #2305
Conversation
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
Disclaimer: I am not a direct project member; they might think differently about this pull request. Thanks, this looks interesting! One issue might be that there is no official "wrapper validation GitHub action" yet (see MNG-6887), so in the future the Gson maintainers would have to be careful when someone creates a pull request claiming to "upgrade the wrapper JAR". It might be good to cache the Maven distribution downloaded by the wrapper because the setup-java action does not do that yet (actions/setup-java#448). I assume the build logs for this pull request did not indicate that the wrapper is downloaded because info log messages are discarded by the wrapper by default. In case this is merged, it might also be useful to use the wrapper for the OSS-Fuzz configuration (probably requires changes to |
There are also variants for the wrapper setup which don't involve any binary files, see https://maven.apache.org/wrapper/index.html#usage-without-binary-jar The question would also be whether the checksums of the downloaded files should be checked, see https://maven.apache.org/wrapper/index.html#checksum-verification-of-downloaded-binaries and also google/guava#6805. |
I was planning to be lazy and not do anything about it... :) Ideally, I would report it upstream (or I guess even propose a fix). I got the impression that the only problem was that we had Windows CI that was using |
It looks like I reproduced it now locally in a test Maven project by running the Linux |
This adds a Maven Wrapper.
Right now, the used Maven version to build this project is unclear and to make reproducible builds, a wrapper should be added.
More info: Maven Wrapper Documentation
Right now, the
check-api-compatibility
Github Action cannot be changed to use the wrapper as it operates on the base commit which does not yet have a Maven Wrapper.