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

WIP custom ant task to produce html report from Rat report and NetBeans clusters #70

Closed
wants to merge 1 commit into from

Conversation

ebarboni
Copy link
Contributor

@ebarboni ebarboni commented Oct 6, 2017

Hi,
this is a more deep integration with Netbeans of the rat report.

It filter the report using cluster.properties and check for local folder.

The filtering pattern is now a variable to get same result for txt and html report to keep the current txt report intact.

Regards

@matthiasblaesing
Copy link
Contributor

Could you please have a look at the diff of build.xml? It looks as if it got a complete make over.

@matthiasblaesing
Copy link
Contributor

And it should be noted, that merging this, will cause merge conflicts for anyone adding exclusions to the rat report.

@ebarboni
Copy link
Contributor Author

ebarboni commented Oct 6, 2017

build.xml revised. I do a full file xml formating within Netbeans, that was not a good idea. I do a partial formatting and add the exclusion. If merged, the ant task may be updated separatly later. Build.xml should not be impacted any more.

* @author skygo
*/
class ModuleInfo {
// approuved resource list

Choose a reason for hiding this comment

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

I guess this is a typo: "approved".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks

ModuleInfo get = unbymodule.get(mm);
for (String mmu : get.getUnapprouved()) {
bw.write("<li class=\"unapprouved\">");
bw.write("U:<a href=\"https://github.com/apache/incubator-netbeans/tree/master/" + mmu + "\">" + mmu + "</a>");
Copy link

@JaroslavTulach JaroslavTulach Oct 7, 2017

Choose a reason for hiding this comment

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

Hardcoding repository URL is probably not good idea.

Either make the URL an attribute of the task or (preferrably) obtain it from the .git/config file. There already are places where the build uses hg command to obtain the commit id (that should be changed to git one day), so precedent of using the repo information from builds exists.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

nice addition thanks. will look at it

</fileset>
</rat:report>
<taskdef name="rattohtml" classname="org.netbeans.nbbuild.reporting.RatHtmlReportTask" classpath="${build.ant.classes.dir}"/>
<loadproperties srcFile="cluster.properties" />
<rattohtml source="build/rat-report.xml" target="build/rat-report.html"/>
Copy link

@JaroslavTulach JaroslavTulach Oct 7, 2017

Choose a reason for hiding this comment

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

I am not sure whether HTML is that much better format than txt. Probably the direct hyperlinks to the Git repo are the biggest improvement, right?

In any case I was looking for a way to track progress of our rat related efforts. Instead of HTML the task should generate JUnit XML report files. If the output used JUnitReportWritter just like the other classes in the nbbuild/antsrc, we could set the continuous job to show us how many violations we still have and track the progress.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry but the last commit with refactoring kill the converstation flow.

Maybe hyperlink is for now the only improvement for having HTML report. By cluster information can be done in txt file.

Helping tooling continuous job is a big point. But we should also think on people who will review and vote for release.
for example http://commons.apache.org/proper/commons-bcel/ has a project information section including a rat report.

* specific language governing permissions and limitations
* under the License.
*/
package org.netbeans.nbbuild.reporting;

Choose a reason for hiding this comment

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

Does this really deserve a separate package? There is a lot of reporting tasks already present in org.netbeans.nbbuild... Moving into org.netbeans.nbbuild and merging ModuleInfo to be static inner class would, in my opinion, be more consistent.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This sounds good for me

bw.write("<h3>Module::" + mm + "</h3>");
bw.write("<ul>");
ModuleInfo get = unbymodule.get(mm);
for (String mmu : get.getUnapprouved()) {

Choose a reason for hiding this comment

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

approuved typos are everywhere...

@ebarboni
Copy link
Contributor Author

I had to check on personal jenkins to see what I get from junit report. I hope the mutliple commit and rebase were not notified.

The report are by clusters in a rat folder in case we want to filter them later.

nbbuild/build/rat/**
by putting url in the report jenkins build hyperlink which make it equivalent of a html report that is good enough and make pure html maybe useless for development point of view.

@ebarboni
Copy link
Contributor Author

@JaroslavTulach I think the WIP is in now in a stable state.
Unit report
are done for unaprouverd license
external library with no maven pattern

rebasing breaks in place review sorry for that :/
regards

Copy link
Contributor

@jtulach jtulach left a comment

Choose a reason for hiding this comment

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

Once integrated we have to use this in our Jenkins jobs. I can administered them...

}

@Override
public void execute() throws BuildException {
Copy link
Contributor

Choose a reason for hiding this comment

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

Super.execute is probably useless.

commandAndArgs.add("config");
commandAndArgs.add("--get");
commandAndArgs.add("remote.origin.url");
Process p = new ProcessBuilder(commandAndArgs).directory(root).start();
Copy link
Contributor

Choose a reason for hiding this comment

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

R is never closed

// XXx if amoduleInfo is null folder is not present source base incomplete
}

}
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice.

@ebarboni
Copy link
Contributor Author

@jtulach rebased again but rat task have more than only fileset from a PR by @matthiasblaesing

get git information from git process

add scan for external
@ebarboni
Copy link
Contributor Author

@jtulach or @JaroslavTulach , rebasing of build.xml is annoying.
I you prefer I can do a PR only for the ant task will let you put the configuration in build.xml later on ?
Let me know.

@JaroslavTulach
Copy link

#1 - I don't understand why rebase at all.
#2 - I've already approved your changes. Feel free to merge.

@ebarboni
Copy link
Contributor Author

@JaroslavTulach
#1 I want to merge the exclude from license review otherwise it's conflicting
#2 I'm not commiter, I can only propose PR. (Commiter on other Apache Archiva project)

@JaroslavTulach
Copy link

OK, I try to merge.

@JaroslavTulach
Copy link

Integrated as 162850d - builds modified to pick the generated .xml files up.

@JaroslavTulach
Copy link

The build is failing: https://builds.apache.org/job/incubator-netbeans-linux/114/console
I am not sure why right now.

@ebarboni
Copy link
Contributor Author

Strange,
I wipe and recreate a build base on apache/incubator-netbeans, on a private jenkins (it works). A difference I notice is that encoding on the apache H21,h24 node are iso while my jenkins os or docker image are UTF-8

@ebarboni
Copy link
Contributor Author

parsing xml with file seems to be dangerous for encoding. #144 use InputSource. Seems to fix the issue on my new non UTF-8 image.

@matthiasblaesing
Copy link
Contributor

I think the problem is in rat - with an output file this is what happens:

  1. the Report Task creates an FileWriter that writes to the target file and wraps a PrintWriter to it (Report Task, line 196)
  2. this PrintWriter is passed to the core Report report method, where an XmlWriter is created (Report Task, line 268, Report Class, line 411)
  3. The XML prolog is written in org.apache.rat.report.xml.writer.impl.base.XmlWriter#startDocument XmlWriter
            if (reportFile == null) {
                out = new PrintWriter(
                          new OutputStreamWriter(
                              new LogOutputStream(this, Project.MSG_INFO)
                              )
                          );
            } else {
                out = new PrintWriter(new FileWriter(reportFile));
            }
            createReport(o

The prolog written in step 3 would be wrong in many cases as it does not write an encoding info. This means the encoding of the resulting document must be UTF-8. The FileWriter created in step 1 uses the default charset and if that is not UTF-8 a broken stream will result.

The Report Task would need to be changed from (line 196):

out = new PrintWriter(new FileWriter(reportFile));

to

out = new PrintWriter(new OutputStreamWriter(new FileOutputStream(reportFile), Charset.forName("UTF-8")));

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.

4 participants