github / codeql-action

Actions for running CodeQL analysis
MIT License
1.13k stars 310 forks source link

java autobuild overrides maven wrapper only-script distributionType #2373

Open ArloL opened 1 month ago

ArloL commented 1 month ago

When using the maven wrapper distributionType only-script autobuild will override the wrapper:

[2024-07-13 09:29:53] [autobuild] > mvn -N org.apache.maven.plugins:maven-wrapper-plugin:3.3.0:wrapper -Dtype=bin 

See https://github.com/ArloL/chorito/actions/runs/9918944552/job/27404224920 for a full log.

My guess is that autobuild checks for the presence of the maven-wrapper.jar because if I add an empty file back autobuild will not trigger the override. Maybe the check can be changed to check for maven-wrapper.properties or similar?

In the meantime I will revert back to the bin distributionType.

aibaars commented 1 month ago

Looking at the change history of the autobuilder, it looks like this change was on purpose; commit message:

The default kind of wrapper had changed to script-only in plugin 3.3.0. Also pin the plugin version to avoid unexpected surprises in future.

I assume there was an increase in reported failures after the wrapper switching its default behaviour from bin to script-only which was addressed by ensuring bin was used in all cases. The "autobuilder" is just a best effort approach, consisting of many heuristics to try to successfully build as many projects as possible. Note that the default switched to source-only quite recently (April 2024), so using bin is probably still the best choice for most projects. In the future the balance will likely tip in favour of script-only once more and more project start relying on it.

smowton commented 1 month ago

Yes, the autobuilder assumes that if no maven-wrapper.jar is present then this may be because we are expected to acquire the jar ourselves somehow (often because we're working with source-control rules that don't allow storing a jar in-repo). However it doesn't recognise the augmented mvnw script that can do the download itself. It should at least be harmless, if a little annoying to revert what it does when working with a persistent checkout of the repository.