Commit
This is helpful when, on Xcode projects, you want to exclude from the report bugs generated by 3rd parties libraries included in the project
- Loading branch information
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -39,16 +39,20 @@ public class ClangScanBuildPublisher extends Recorder{ | |
private static final Pattern BUGCATEGORY_PATTERN = Pattern.compile( "<!--\\sBUGCATEGORY\\s(.*)\\s-->" ); | ||
|
||
private int bugThreshold; | ||
private String clangexcludedpaths; | ||
This comment has been minimized.
Sorry, something went wrong. |
||
|
||
private boolean markBuildUnstableWhenThresholdIsExceeded; | ||
|
||
public ClangScanBuildPublisher( | ||
boolean markBuildUnstableWhenThresholdIsExceeded, | ||
int bugThreshold | ||
){ | ||
int bugThreshold, | ||
String clangexcludedpaths | ||
This comment has been minimized.
Sorry, something went wrong. |
||
){ | ||
|
||
super(); | ||
this.markBuildUnstableWhenThresholdIsExceeded = markBuildUnstableWhenThresholdIsExceeded; | ||
this.bugThreshold = bugThreshold; | ||
this.clangexcludedpaths = clangexcludedpaths; | ||
} | ||
|
||
public int getBugThreshold() { | ||
|
@@ -63,6 +67,10 @@ public void setBugThreshold(int bugThreshold) { | |
this.bugThreshold = bugThreshold; | ||
} | ||
|
||
public void setClangexcludedpaths(String clangExcludePaths){ | ||
this.clangexcludedpaths = clangExcludePaths; | ||
} | ||
|
||
@Override | ||
public Action getProjectAction( AbstractProject<?, ?> project ){ | ||
return new ClangScanBuildProjectAction( project ); | ||
|
@@ -77,6 +85,10 @@ public BuildStepMonitor getRequiredMonitorService() { | |
return BuildStepMonitor.NONE; | ||
} | ||
|
||
public String getClangexcludedpaths(){ | ||
return clangexcludedpaths; | ||
} | ||
|
||
@Override | ||
public boolean perform( AbstractBuild<?, ?> build, Launcher launcher, BuildListener listener ) throws InterruptedException, IOException { | ||
|
||
|
@@ -99,11 +111,28 @@ public boolean perform( AbstractBuild<?, ?> build, Launcher launcher, BuildListe | |
|
||
// this builds and new bug summary and populates it with bugs | ||
ClangScanBuildBugSummary newBugSummary = new ClangScanBuildBugSummary( build.number ); | ||
for( FilePath report : clangReports ){ | ||
// bugs are parsed inside this method: | ||
ClangScanBuildBug bug = createBugFromClangScanBuildHtml( build.getProject().getName(), report, previousBugSummary, build.getWorkspace().getRemote() ); | ||
newBugSummary.add( bug ); | ||
|
||
String[] tokens = new String[0]; | ||
This comment has been minimized.
Sorry, something went wrong. |
||
if(this.getClangexcludedpaths().length() > 0){ | ||
This comment has been minimized.
Sorry, something went wrong.
xtofl
|
||
tokens = this.getClangexcludedpaths().split(","); | ||
} | ||
This comment has been minimized.
Sorry, something went wrong.
orrc
Member
|
||
|
||
for( FilePath report : clangReports ){ | ||
// bugs are parsed inside this method: | ||
ClangScanBuildBug bug = createBugFromClangScanBuildHtml( build.getProject().getName(), report, previousBugSummary, build.getWorkspace().getRemote() ); | ||
boolean validBug = true; | ||
for(String token:tokens){ | ||
This comment has been minimized.
Sorry, something went wrong.
xtofl
|
||
String trimmedToken = token.trim().toLowerCase(); | ||
if(bug.sourceFile.toLowerCase().contains(trimmedToken)){ | ||
listener.getLogger().println( "Skipping file: " + bug.sourceFile + " because it matches exclusion pattern: " + trimmedToken ); | ||
validBug = false; | ||
break; | ||
} | ||
} | ||
if(validBug) { | ||
newBugSummary.add( bug ); | ||
} | ||
} | ||
|
||
// this line dumps a bugSummary.xml file to the build artifacts. did this instead of using job config xml for performance | ||
//FilePath bugSummaryXMLFile = new FilePath( reportOutputFolder, "bugSummary.xml" ); | ||
|
@@ -128,7 +157,7 @@ private ClangScanBuildBug createBugFromClangScanBuildHtml( String projectName, F | |
ClangScanBuildBug bug = createBugInstance( projectName, report, workspacePath ); | ||
|
||
// this checks to see if the bug is new since the last build | ||
if( previousBugSummary != null ){ | ||
if(bug != null && previousBugSummary != null ){ | ||
// This marks bugs as new if they did not exist in the last build report | ||
bug.setNewBug( !previousBugSummary.contains( bug ) ); | ||
} | ||
|
@@ -182,7 +211,7 @@ private void copyClangReportsToMaster( FilePath reportsFolder, FilePath materPat | |
* scan-build. If scan-build ever adds an XML option, this functionality can be replaced with an XML parsing | ||
* routine. | ||
*/ | ||
private ClangScanBuildBug createBugInstance( String projectName, FilePath report, String workspacePath ){ | ||
private ClangScanBuildBug createBugInstance( String projectName, FilePath report, String workspacePath){ | ||
This comment has been minimized.
Sorry, something went wrong. |
||
// the report parameter is the file the points to an HTML report generated by clang | ||
ClangScanBuildBug instance = new ClangScanBuildBug(); | ||
String basePath = "StaticAnalyzer/"; | ||
|
@@ -205,6 +234,7 @@ private ClangScanBuildBug createBugInstance( String projectName, FilePath report | |
instance.setBugType( getMatch( BUG_TYPE_PATTERN, contents ) ); | ||
instance.setBugCategory( getMatch( BUGCATEGORY_PATTERN, contents ) ); | ||
String sourceFile = getMatch( BUGFILE_PATTERN, contents ); | ||
|
||
This comment has been minimized.
Sorry, something went wrong. |
||
|
||
// This attempts to shorten the file path by removing the workspace path and | ||
// leaving only the path relative to the workspace. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,14 +22,16 @@ public Publisher newInstance(StaplerRequest arg0, JSONObject json ) throws hudso | |
|
||
boolean markBuildUnstable = false; | ||
int bugThreshold = 0; | ||
String excludedPaths = ""; | ||
|
||
JSONObject failWhenThresholdExceeded = json.optJSONObject( "failWhenThresholdExceeded" ); | ||
if( failWhenThresholdExceeded != null ){ | ||
markBuildUnstable = true; | ||
bugThreshold = failWhenThresholdExceeded.getInt( "bugThreshold" ); | ||
excludedPaths = failWhenThresholdExceeded.getString( "clangexcludedpaths" ); | ||
This comment has been minimized.
Sorry, something went wrong.
xtofl
|
||
} | ||
|
||
return new ClangScanBuildPublisher( markBuildUnstable, bugThreshold ); | ||
return new ClangScanBuildPublisher( markBuildUnstable, bugThreshold, excludedPaths ); | ||
} | ||
|
||
@Override | ||
|
@@ -42,7 +44,7 @@ public boolean isApplicable( @SuppressWarnings("rawtypes") Class<? extends Abstr | |
if( !FreeStyleProject.class.isAssignableFrom( jobType ) ){ | ||
System.err.println( "Clang scan-build ERROR: Expected FreeStyleProject but was: " + jobType + " at Publisher Descriptor" ); | ||
} | ||
return FreeStyleProject.class.isAssignableFrom( jobType ); | ||
return AbstractProject.class.isAssignableFrom( jobType ); | ||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
Comma separated path to exclude from the clang scan-build report. This setting is particularly useful to exclude third party libraries bugs from the report. | ||
This comment has been minimized.
Sorry, something went wrong. |
3 comments
on commit 0fa4d64
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When judging changes, I always look for unit test changes corresponding to the functional changes. Here, the unit test doesn't even contain a functional test (i.e. the perform(build, launcher, listener)
method) - only a roundtrip persistency test, so you can't be blamed for not extending it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@xtofl This code was written 1.5 years ago by somebody who presumably sent in one pull request to this plugin.
I'm just wondering what you're wanting to achieve with these comments last week and now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@orcc I forked this repo about a month ago because we started using the scan build plugin. Somehow I got notified of this pull request, and started looking at it.
I must admit
- I'm somewhat lost in understanding the workflow (didn't note that this is so old already)
- My wordings may be a bit terse - I'm used to communicating the bare essence when reviewing - I'll try harder to write nicer proze
When reviewing code changes where persistent data is involved, I am quite concerned with terminology and consistency, especially in the user visible interface. This casing issue will for sure cause much cursing since it violates the principle of the least surprise.
That's why I thought it very important to not just sit back, and spit it out.
I understand now that it's too late after the facts...
consistency: pascal casing used for other properties: name should be
clangExcludedPaths
.