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

Stylesheet not working in 4.8.4 #2969

Closed
esabol opened this issue Apr 24, 2024 · 13 comments
Closed

Stylesheet not working in 4.8.4 #2969

esabol opened this issue Apr 24, 2024 · 13 comments
Assignees

Comments

@esabol
Copy link

esabol commented Apr 24, 2024

We use stylesheet="fancy-hist.xsl" to generate our SpotBugs reports in our CI/CD pipeline, which builds using ant. Our build.xml has the following:

    <taskdef
        resource="edu/umd/cs/findbugs/anttask/tasks.properties"
        classpath="${spotbugs.home}/lib/spotbugs-ant.jar"/>
 
    <target name="spotbugs">
        <spotbugs home="${spotbugs.home}"
                  output="html"
                  outputFile="${dist.dir}/spotbugs.html"
                  stylesheet="fancy-hist.xsl" >
            <auxclasspath path="${basedir}/../libs" />
            <sourcePath path="${basedir}/src/java" />
            <class location="${build.web.dir}/WEB-INF/classes" />
        </spotbugs>
    </target>

This all worked for us just fine with SpotBugs 4.8.3. After upgrading to 4.8.4, however, the SpotBugs report generated by the same exact build.xml is clearly not using fancy-hist.xml. And, yes, spotbugs.home is defined properly in project.properties (which hasn't changed) and we are using the spotbugs-ant.jar from the 4.8.4 release.

Any idea as to what the problem is or what the solution might be? Thanks!

@gtoison
Copy link
Contributor

gtoison commented Apr 25, 2024

Hello @Vogel612, I'm not sure how this was working but I was wondering if this could be a side effect of #2894

@JuditKnoll
Copy link
Collaborator

Maybe it can be because of #2915. Can you please share the fancy-hist.xml? Is there any error logged about this?

@gtoison
Copy link
Contributor

gtoison commented Apr 25, 2024

I think the xsl is part of SpotBugs: https://github.com/spotbugs/spotbugs/blob/master/spotbugs/src/xsl/fancy-hist.xsl
I was wondering about the version change for the filter xsd too, that might be the problem

@esabol
Copy link
Author

esabol commented Apr 25, 2024

Yes, the fancy-host.xsl we are using is the one provided by SpotBugs, as @gtoison indicated. We haven't modified it or moved it.

I'm not seeing any error messages. Where would they be logged to in this scenario?

@esabol
Copy link
Author

esabol commented Apr 25, 2024

My colleague additionally tested it with SpotBugs 4.8.2, and the stylesheet worked.

Also tested 4.8.4 with stylesheet="${spotbugs.home}/src/xsl/fancy-hist.xsl" instead of stylesheet="fancy-hist.xsl". Still didn't work.

4.8.3 works with either of those two stylesheet attribute values.

@Vogel612
Copy link
Contributor

Hello @Vogel612, I'm not sure how this was working but I was wondering if this could be a side effect of #2894

The changeset you mention should only affect the filepaths, although I did touch the stylesheet assignment so I can't exclude it with any degree of certainty. Judging by the syntax of the options, the code contributed with that changeset should be working as intended and not mess with the stylesheets, assuming generating an XML report withMessages also works as intended...

I haven't checked the release yet, but unfortunately this seems like my changes are a more likely culprit than the filter XSD namespace changes...

@esabol esabol changed the title Stylesheet not working in 4.8.4? Stylesheet not working in 4.8.4 May 13, 2024
@asheldon
Copy link

assuming generating an XML report withMessages also works as intended...

When I diff SpotBugs XML output between 4.8.2 and 4.8.5, having set output to xml:withMessages, the shortMessage and longMessage blocks are missing from the bugInstance.

<     <ShortMessage>Redundant nullcheck of value known to be non-null</ShortMessage>
<     <LongMessage>Redundant nullcheck of maxuptime, which is known to be non-null in new com.example.Test(boolean, String[])</LongMessage>

Additionally, the <SourceLine> blocks, <LocalVariable>, etc blocks are self-closing instead of including a <Message>.

@esabol
Copy link
Author

esabol commented Jun 18, 2024

Is there a developer who can look into this? Maybe try reverting PR #2894?

@gtoison
Copy link
Contributor

gtoison commented Jun 20, 2024

@Vogel612 if the issue is indeed with #2894 I think that the improvement in the PR is not worth the regressions reported here?
Would you be able to look into it? Otherwise I agree with @esabol that reverting it might be reasonable

@Vogel612
Copy link
Contributor

@gtoison I really don't have the capacity to look into it right now. I'm happy to go for a revert, the improvement is only marginal and the regression is quite a heavy thing.

I might be able to look at it at some later point with some more test coverage around this specific element of it, but not in any reasonable time frame. I would like to see the original issue that was supposed to be fixed by that PR reopened if we revert, though :)

@asheldon
Copy link

asheldon commented Jul 10, 2024

I checked out the source for SpotBugs and on commit 6cf7b2c I did "git revert 789d397" and rebuilt the jar locally. The locally built jar appears to work correctly in this scenario.

gtoison added a commit to gtoison/spotbugs that referenced this issue Jul 11, 2024
Keeping track of the files used by bug reporters (to prevent writing the
same file multiple times) seems to prevent the html stylesheets to
apply, as well as breaking xml:withMessages
Revert the changes from spotbugs#2894 as discussed in spotbugs#2969
hazendaz added a commit that referenced this issue Jul 13, 2024
Keeping track of the files used by bug reporters (to prevent writing the
same file multiple times) seems to prevent the html stylesheets to
apply, as well as breaking xml:withMessages
Revert the changes from #2894 as discussed in #2969

Co-authored-by: Jeremy Landis <[email protected]>
@hazendaz
Copy link
Member

Closing as fixed on master, will be released in 4.9.0 as code reverted.

@esabol
Copy link
Author

esabol commented Jul 14, 2024

Thanks, everyone!

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

6 participants