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

#549 Pass the auxiliary classpath using the -auxclasspathFromFile agument #550

Merged
merged 1 commit into from
Feb 25, 2023

Conversation

corebonts
Copy link

The plugin versions before 4.7.3.1 passed the auxiliary classpath via the standard input, which caused a deadlock during parallel builds with non-forked spotbugs. #488

Although this was fixed by removing the -auxclasspathFromInput, this broke the classpath logic.

The solution is to pass the classpath using a file.

…File argument

The plugin versions before 4.7.3.1 passed the auxiliary classpath via the
standard input, which caused a deadlock during parallel builds with
non-forked spotbugs. spotbugs#488

Although this was fixed by removing the -auxclasspathFromInput, this broke the
classpath logic.

The solution is to pass the classpath using a file.
@hazendaz
Copy link
Member

@corebonts bummer not seen in our integration tests. I suspected there was something more missing and had held over a comment related to original to do this.

        args << "-auxclasspath"
        args << "${project.compileClasspathElements}"

I know that was not enough but more of a marker for me that the original change may not have been entirely accurate. Would be nice to have a test confirmation here, is it possible you can get a test together on this and/or point out where we may have been testing this already but not verifying. Not sure if you are available to get back currently but would like to get release out as soon as possible.

@hazendaz
Copy link
Member

accepting as-is, will do more testing separately.

@hazendaz hazendaz merged commit 03a2658 into spotbugs:spotbugs Feb 25, 2023
@corebonts
Copy link
Author

Thank you for adding the tests #554

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

Successfully merging this pull request may close these issues.

2 participants