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

checkLicense task and generateLicenseReport task have overlapping outputs => very bad #321

Open
Vampire opened this issue Sep 23, 2024 · 0 comments

Comments

@Vampire
Copy link
Contributor

Vampire commented Sep 23, 2024

  • generateLicenseReport has hard-coded config.absoluteOutputDir as @OutputDirectory
  • checkLicense has hard-coded "${config.absoluteOutputDir}/$NOT_PASSED_DEPENDENCIES_FILE" as @OutputFile

This is very bad.
This means that the checkLicense file is also considered output of the generateLicenseReport task and such overlapping outputs are always very bad and highly discouraged bad practice.

For example I have a Zip task that packages up the generateLicenseReport output and publishes it as a build artifact.
But if now my Zip task runs and after that the checkLicense task runs, Gradle 7 detects that my Zip task is using the checkLicense output without having any dependency or ordering constraint and disables certain optimizations. Gradle 8 finally fails the build on such situations.

Also, as generateLicenseReport is a cacheable task, the stored cache entries are incorrect and inconsistent.
The cache entry for generateLicenseReport could include no file of checkLicense if none happens to be present, it could contain a stale file if one was laying around, or it could contain an up-to-date file. And on cache reuse, this file will be restored along with the real outputs, so actually currently the task is not cleanly cacheable.

Unfortunately the task classes of this plugin are also extremely inflexible and do not allow to configure the output locations so that a consuming build could fix it properly that way. Only maybe by subclassing one of the tasks, overriding one of those getters, registering an own instance of that custom task, and disabling the instance added by the plugin, or by monkey-patching the task class it could currently be properly fixed,

So it should also be considered to follow best practices, making all the properties Property instances that can be wired together and customized with only convention values coming from wired extension properties, instead of the current hard-coding.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant