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

Spotbugs task can cache invalid outputs when ignoreFailures is set to true #826

Open
BrianM5707 opened this issue Jan 5, 2023 · 0 comments

Comments

@BrianM5707
Copy link

My team uses a remote build cache to speed up our CI builds. We also set ignoreFailures=true in our CI scripts: we want the build to always report all Spotbugs violations, not fail fast. We rely on a Jenkins plugin to process the XML report after the fact and fail the build if there are any violations.

Currently, ignoreFailures=true causes the Spotbugs task to succeed regardless of how the Spotbugs process exits. And because of the nature of the Gradle build cache, the XML report will be cached whenever the task succeeds. Therefore it's possible to cache outputs from a failed Spotbugs run.

This makes perfect sense if the failure is a simple verification failure (e.g. there were Spotbugs violations), as this result will be stable from build to build. But, rarely, it is possible for the Spotbugs process to fail for some transient reason. This happens, for example, when a build is aborted in our CI pipeline while Spotbugs is running. The Spotbugs process receives a SIGTERM and exits with code 143. If it survives long enough, the Gradle daemon ignores the Spotbugs exit code and caches the current (usually empty or otherwise invalid) XML report. This is just one example we ran in to, but conceivably it could happen for other reasons (i.e. Spotbugs being killed by OOMKiller).

Although these transient failures are rare, once the invalid XML is cached, the problem sticks around until the task inputs are changed or until we manually intervene, which can be pretty disruptive depending on how infrequently the code changes.

This issue can be reproduced with a local build cache by doing the following:

  1. Have a java project with the spotbugs plugin applied. I reproduced this with version 5.0.13, but older versions will likely work as well.
  2. Enable the Gradle build cache
  3. Configure an XML report and set ignoreFailures=true on the spotbugs task
  4. Run the spotbugs task
  5. Before spotbugs completes, kill the forked spotbugs process (e.g. using kill/pkill). It helps if the project has enough class files for spotbugs to run a while, making it easier to time the kill signal.
  6. Clean and re-run the spotbugs task

Desired Behavior: Spotbugs should re-run and produce a valid XML report
Actual Behavior: Spotbugs will retrieve the invalid XML report from the build cache and skip execution, and will continue to do so until the task inputs change or the cache entry is deleted

We can work around this by not using ignoreFailures, but this is not ideal for us since it means we can only discover Spotbugs violations in our pipeline one module at a time. We can also disable the build cache for Spotbugs tasks, but this increases our build times.

It would be great if the "ignoreFailures" behavior only applied when Spotbugs fails for a verification reason. I think a good analogy is the Test task in the builtin Gradle java plugin. Like Spotbugs, it runs in a forked JVM and has an ignoreFailures property. Unlike Spotbugs, the Test task only ignores verification failures (e.g. junit test failures), but will still fail in other cases, such as when the test JVM terminates unexpectedly. This prevents the outputs from being cached.

I'm open to other suggestions as well. Thanks in advance!

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