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

fix: Use Classpath normalization for auxClassPaths #314

Merged
merged 1 commit into from
Jul 30, 2020

Conversation

bigdaz
Copy link
Contributor

@bigdaz bigdaz commented Jul 22, 2020

Declaring auxClassPaths as a Classpath property allows Gradle to perform additional normalization,
so that functionally-identical jar files do not trigger a re-execution of the SpotBugsTask.

A re-generated jar containing identical classes will have different entry timestamps for each class entry,
and as such is not bitwise identical even though it is functionally identical.
Without this fix, changing an input jar file will change the input cache key, leading to consistent misses from the local build cache.

See https://docs.gradle.org/6.0/userguide/more_about_tasks.html#sec:task_input_using_classpath_annotations

Note that I have tested this change locally, but not added any specific test coverage for this use case.

@bigdaz bigdaz changed the title Use Classpath normalization for auxClassPaths fix: Use Classpath normalization for auxClassPaths Jul 22, 2020
Declaring `auxClassPaths` as a `Classpath` property allows Gradle to perform additional normalization,
so that functionally-identical jar files do not trigger a re-execution of the SpotBugsTask.

A re-generated jar containing identical classes will have different entry timestamps for each class entry,
and as such is not bitwise identical even though it is functionally identical.
Without this fix, changing an input jar file will change the input cache key, leading to consistent misses from the local build cache.

See https://docs.gradle.org/6.0/userguide/more_about_tasks.html#sec:task_input_using_classpath_annotations
Copy link
Contributor

@jscancella jscancella left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@KengoTODA
Copy link
Member

It seems that #279 is similar to this fix. @DPUkyle could you review this change?
If you know something to consider, please share it with us. :)

@DPUkyle
Copy link
Contributor

DPUkyle commented Jul 30, 2020

Yes, this is great. Thanks @bigdaz for fixing it!

@@ -235,7 +236,7 @@ class SpotBugsTask extends DefaultTask implements VerificationTask {
* Default value is the compile-scope dependencies of the target sourceSet.
*/
@InputFiles
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: we can remove @InputFiles as it's replaced by @Classpath

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@KengoTODA KengoTODA merged commit 76ed3a9 into spotbugs:master Jul 30, 2020
@github-actions
Copy link

🎉 This PR is included in version 4.4.5 🎉

The release is available on:

Your semantic-release bot 📦🚀

@foto-andreas
Copy link

Is it correct, that v4.5.0 has both annotations (@InputFiles AND @classpath)? Currently I am searching for changes of auxClassPaths fingerprints in my project...

@bigdaz
Copy link
Contributor Author

bigdaz commented Sep 29, 2020

Is it correct, that v4.5.0 has both annotations (@InputFiles AND @classpath)?

It's safe to remove the @InputFiles annotation, unless there is a goal to remain compatible with Gradle < 3.2. I'll leave that to the plugin maintainers to decide.

@KengoTODA
Copy link
Member

This plugin does not support Gradle < 3.2, so we can remove @InputFiles in our case.

KengoTODA added a commit that referenced this pull request Sep 30, 2020
we don't support Gradle < 3.2, so the InputFiles annotation is needless 
for fields which is already annotated with the Classpath annotation.

refs #314
KengoTODA added a commit that referenced this pull request Sep 30, 2020
we don't support Gradle < 3.2, so the InputFiles annotation is needless 
for fields which is already annotated with the Classpath annotation.

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

Successfully merging this pull request may close these issues.

5 participants