DO NOT REPLY [Bug 49835] Wrong page break with 2 columned region and tables
https://issues.apache.org/bugzilla/show_bug.cgi?id=49835 --- Comment #15 from alpa...@gmail.com 2011-02-21 04:19:14 EST --- (In reply to comment #14) (In reply to comment #13) The minimal test case must be wrong. The output is the same with or without the patch and it behaves normally, ie rows are displayed were they should. Which test case are you referring to exactly? I checked all attachments before and after the patch. It just occurred to me that, if you mean the 'slightly altered' test case, you may want to read comment 5 more closely. It is expected to produce the same result before and after. More a reference sample to be able to see the difference during debugging. I am sorry Andreas, I am not an expert in fop internals, so my response might have been too fast (and wrong) and it created some confusion. I was under the impression, that the minimal test case (by Jeremias) manifested the same behavior as Adam's original sample. The result I got from minimal, with and without, the patch is identical. So, I guess this test case does not expose the problem. I haven't tested with your revised (Slightly Altered) test, and I was not referring to it.. As for the other bug, it is my mistake, I was referring to bug https://issues.apache.org/bugzilla/show_bug.cgi?id=49801, which is still open and is a totally different case. I am sorry again for the misunderstanding. Regards, Alex -- Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug.
DO NOT REPLY [Bug 49835] Wrong page break with 2 columned region and tables
https://issues.apache.org/bugzilla/show_bug.cgi?id=49835 Andreas L. Delmelle adelme...@apache.org changed: What|Removed |Added CC||alpa...@gmail.com --- Comment #16 from Andreas L. Delmelle adelme...@apache.org 2011-02-21 12:28:52 EST --- (In reply to comment #15) Alex, Added you to the CC list so that you are notified directly of updates. Hope you don't mind. I was under the impression, that the minimal test case (by Jeremias) manifested the same behavior as Adam's original sample. The result I got from minimal, with and without, the patch is identical. So, I guess this test case does not expose the problem. I haven't tested with your revised (Slightly Altered) test, and I was not referring to it.. That's strange... I will attach the output before/after the patch here. They definitely look different on my end. After the patch, the entire block is deferred to the second page, where it can be kept together as a whole. As for the other bug, it is my mistake, I was referring to bug https://issues.apache.org/bugzilla/show_bug.cgi?id=49801, which is still open and is a totally different case. OK, no problem! You can add yourself to the CC list of that bug if you want to follow up there. -- Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug.
DO NOT REPLY [Bug 49835] Wrong page break with 2 columned region and tables
https://issues.apache.org/bugzilla/show_bug.cgi?id=49835 --- Comment #17 from Andreas L. Delmelle adelme...@apache.org 2011-02-21 12:30:01 EST --- Created an attachment (id=26686) -- (https://issues.apache.org/bugzilla/attachment.cgi?id=26686) Minimal Test Case - before attached patch -- Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug.
DO NOT REPLY [Bug 49835] Wrong page break with 2 columned region and tables
https://issues.apache.org/bugzilla/show_bug.cgi?id=49835 --- Comment #18 from Andreas L. Delmelle adelme...@apache.org 2011-02-21 12:30:40 EST --- Created an attachment (id=26687) -- (https://issues.apache.org/bugzilla/attachment.cgi?id=26687) Minimal Test Case PDF - after attached patch -- Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug.
Solving FindBugs issues [was: Re: svn commit: r1071912 - in /xmlgraphics/fop/trunk: findbugs-exclude.xml src/java/org/apache/fop/render/pdf/PDFImageHandlerSVG.java]
Hi, If we solve issues raised by FindBugs by listing them in an ignore file, is there still a point to use FindBugs at all? It seems to me that some of those issues deserve to be fixed. They seem to point out genuine problems in the code. Vincent On 18/02/11 08:18, spepp...@apache.org wrote: Author: spepping Date: Fri Feb 18 08:18:04 2011 New Revision: 1071912 URL: http://svn.apache.org/viewvc?rev=1071912view=rev Log: Fixing checkstyle errors and hiding fingbugs errors Modified: xmlgraphics/fop/trunk/findbugs-exclude.xml xmlgraphics/fop/trunk/src/java/org/apache/fop/render/pdf/PDFImageHandlerSVG.java Modified: xmlgraphics/fop/trunk/findbugs-exclude.xml URL: http://svn.apache.org/viewvc/xmlgraphics/fop/trunk/findbugs-exclude.xml?rev=1071912r1=1071911r2=1071912view=diff == --- xmlgraphics/fop/trunk/findbugs-exclude.xml (original) +++ xmlgraphics/fop/trunk/findbugs-exclude.xml Fri Feb 18 08:18:04 2011 @@ -5019,4 +5019,51 @@ Bug pattern=UWF_FIELD_NOT_INITIALIZED_IN_CONSTRUCTOR/ /Match !-- /Automatically generated list of exclusions -- + !-- Automatically generated list of exclusions on 18 February 2011 -- + Match + Class name=org.apache.fop.afp.goca.GraphicsSetProcessColor/ + Method name=writeToStream/ + Bug pattern=OS_OPEN_STREAM/ + /Match + Match + Class name=org.apache.fop.pdf.PDFFactory/ + Method name=makeSeparationColorSpace/ + Bug pattern=DM_FP_NUMBER_CTOR/ + /Match + Match + Class name=org.apache.fop.pdf.PDFFactory/ + Method name=toColorVector/ + Bug pattern=DM_FP_NUMBER_CTOR/ + /Match + Match + Class name=org.apache.fop.fo.expr.PropertyTokenizer/ + Field name=recognizeOperator/ + Bug pattern=URF_UNREAD_FIELD/ + /Match + Match + Class name=org.apache.fop.layoutmgr.BlockLayoutManager/ + Method name=getNextChildElements/ + Bug pattern=BC_UNCONFIRMED_CAST/ + /Match + Match + Class name=org.apache.fop.util.ColorWithFallback/ + !-- Listing the method 'equals' does not work -- + Bug pattern=EQ_DOESNT_OVERRIDE_EQUALS/ + /Match + Match + Class name=org.apache.fop.util.ColorWithFallback/ + Method name=getAlternativeColors/ + Bug pattern=PZLA_PREFER_ZERO_LENGTH_ARRAYS/ + /Match + Match + Class name=org.apache.fop.fo.expr.NamedColorFunction/ + Method name=eval/ + Bug pattern=RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE/ + /Match + Match + Class name=org.apache.fop.util.ColorUtil/ + Method name=parseAsFopRgbNamedColor/ + Bug pattern=REC_CATCH_EXCEPTION/ + /Match + !-- /Automatically generated list of exclusions on 18 February 2011 -- /FindBugsFilter \ No newline at end of file Modified: xmlgraphics/fop/trunk/src/java/org/apache/fop/render/pdf/PDFImageHandlerSVG.java URL: http://svn.apache.org/viewvc/xmlgraphics/fop/trunk/src/java/org/apache/fop/render/pdf/PDFImageHandlerSVG.java?rev=1071912r1=1071911r2=1071912view=diff == --- xmlgraphics/fop/trunk/src/java/org/apache/fop/render/pdf/PDFImageHandlerSVG.java (original) +++ xmlgraphics/fop/trunk/src/java/org/apache/fop/render/pdf/PDFImageHandlerSVG.java Fri Feb 18 08:18:04 2011 @@ -60,7 +60,8 @@ public class PDFImageHandlerSVG implemen private static Log log = LogFactory.getLog(PDFImageHandlerSVG.class); /** {@inheritDoc} */ -public void handleImage(RenderingContext context, Image image, Rectangle pos) +public void handleImage(RenderingContext context,// CSOK: MethodLength +Image image, Rectangle pos) throws IOException { PDFRenderingContext pdfContext = (PDFRenderingContext)context; PDFContentGenerator generator = pdfContext.getGenerator(); - To unsubscribe, e-mail: fop-commits-unsubscr...@xmlgraphics.apache.org For additional commands, e-mail: fop-commits-h...@xmlgraphics.apache.org
Re: Solving FindBugs issues [was: Re: svn commit: r1071912 - in /xmlgraphics/fop/trunk: findbugs-exclude.xml src/java/org/apache/fop/render/pdf/PDFImageHandlerSVG.java]
Yes, there is are good reasons to continue using findbugs: (1) it does find bugs (2) they should not be added the exclude file without careful evaluation (3) when evaluating, real bugs should be fixed; however, there are some cases where findbugs reports a warning or error that is in fact not a bug, but is intended to be that way by design; for example, returning a null vs returning an empty array: findbugs will report the former as a warning, but some designs may prefer that route; Any exclusions added to the findbugs-exclude file should be done carefully and after explicit evaluation. I know that has not been the case recently, due to lack of time, and some exclusions have been added without evaluation. This should be avoided, and any exclusions added automatically should be marked as such for further evaluation and repair. G. On Mon, Feb 21, 2011 at 11:15 AM, Vincent Hennebert vhenneb...@gmail.comwrote: Hi, If we solve issues raised by FindBugs by listing them in an ignore file, is there still a point to use FindBugs at all? It seems to me that some of those issues deserve to be fixed. They seem to point out genuine problems in the code. Vincent On 18/02/11 08:18, spepp...@apache.org wrote: Author: spepping Date: Fri Feb 18 08:18:04 2011 New Revision: 1071912 URL: http://svn.apache.org/viewvc?rev=1071912view=rev Log: Fixing checkstyle errors and hiding fingbugs errors Modified: xmlgraphics/fop/trunk/findbugs-exclude.xml xmlgraphics/fop/trunk/src/java/org/apache/fop/render/pdf/PDFImageHandlerSVG.java Modified: xmlgraphics/fop/trunk/findbugs-exclude.xml URL: http://svn.apache.org/viewvc/xmlgraphics/fop/trunk/findbugs-exclude.xml?rev=1071912r1=1071911r2=1071912view=diff == --- xmlgraphics/fop/trunk/findbugs-exclude.xml (original) +++ xmlgraphics/fop/trunk/findbugs-exclude.xml Fri Feb 18 08:18:04 2011 @@ -5019,4 +5019,51 @@ Bug pattern=UWF_FIELD_NOT_INITIALIZED_IN_CONSTRUCTOR/ /Match !-- /Automatically generated list of exclusions -- + !-- Automatically generated list of exclusions on 18 February 2011 -- + Match + Class name=org.apache.fop.afp.goca.GraphicsSetProcessColor/ + Method name=writeToStream/ + Bug pattern=OS_OPEN_STREAM/ + /Match + Match + Class name=org.apache.fop.pdf.PDFFactory/ + Method name=makeSeparationColorSpace/ + Bug pattern=DM_FP_NUMBER_CTOR/ + /Match + Match + Class name=org.apache.fop.pdf.PDFFactory/ + Method name=toColorVector/ + Bug pattern=DM_FP_NUMBER_CTOR/ + /Match + Match + Class name=org.apache.fop.fo.expr.PropertyTokenizer/ + Field name=recognizeOperator/ + Bug pattern=URF_UNREAD_FIELD/ + /Match + Match + Class name=org.apache.fop.layoutmgr.BlockLayoutManager/ + Method name=getNextChildElements/ + Bug pattern=BC_UNCONFIRMED_CAST/ + /Match + Match + Class name=org.apache.fop.util.ColorWithFallback/ + !-- Listing the method 'equals' does not work -- + Bug pattern=EQ_DOESNT_OVERRIDE_EQUALS/ + /Match + Match + Class name=org.apache.fop.util.ColorWithFallback/ + Method name=getAlternativeColors/ + Bug pattern=PZLA_PREFER_ZERO_LENGTH_ARRAYS/ + /Match + Match + Class name=org.apache.fop.fo.expr.NamedColorFunction/ + Method name=eval/ + Bug pattern=RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE/ + /Match + Match + Class name=org.apache.fop.util.ColorUtil/ + Method name=parseAsFopRgbNamedColor/ + Bug pattern=REC_CATCH_EXCEPTION/ + /Match + !-- /Automatically generated list of exclusions on 18 February 2011 -- /FindBugsFilter \ No newline at end of file Modified: xmlgraphics/fop/trunk/src/java/org/apache/fop/render/pdf/PDFImageHandlerSVG.java URL: http://svn.apache.org/viewvc/xmlgraphics/fop/trunk/src/java/org/apache/fop/render/pdf/PDFImageHandlerSVG.java?rev=1071912r1=1071911r2=1071912view=diff == --- xmlgraphics/fop/trunk/src/java/org/apache/fop/render/pdf/PDFImageHandlerSVG.java (original) +++ xmlgraphics/fop/trunk/src/java/org/apache/fop/render/pdf/PDFImageHandlerSVG.java Fri Feb 18 08:18:04 2011 @@ -60,7 +60,8 @@ public class PDFImageHandlerSVG implemen private static Log log = LogFactory.getLog(PDFImageHandlerSVG.class); /** {@inheritDoc} */ -public void handleImage(RenderingContext context, Image image, Rectangle pos) +public void handleImage(RenderingContext context,// CSOK: MethodLength +Image image, Rectangle pos) throws IOException { PDFRenderingContext pdfContext = (PDFRenderingContext)context; PDFContentGenerator generator =
Re: Solving FindBugs issues [was: Re: svn commit: r1071912 - in /xmlgraphics/fop/trunk: findbugs-exclude.xml src/java/org/apache/fop/render/pdf/PDFImageHandlerSVG.java]
On 21 Feb 2011, at 19:15, Vincent Hennebert wrote: If we solve issues raised by FindBugs by listing them in an ignore file, is there still a point to use FindBugs at all? It seems to me that some of those issues deserve to be fixed. They seem to point out genuine problems in the code. I was about to convey a similar sentiment. If we are only going to ignore potential bugs, the point of the whole exercise seems totally lost. Some of them may be OK to ignore, as Glenn pointed out, but IIUC, one of those potentially introduced bugs (that we are now ignoring) is likely of the same nature as one that Chris fixed in the very same area a while back (potentially unclosed stream, leading to a descriptor leak in the AFP renderer)... Not very encouraging. :( Mea culpa: I saw one exclusion --unconfirmed cast-- that would seem to stem from my recent refactoring in the BlockStackingLMs. Not sure why an exclusion was chosen here, but adding an assert statement in the code avoids the warning as well *and* has the benefit of being visible exactly at the spot in the code where it applies. Seemed like the more proper way to handle this. Just my 2 cents... Regards, Andreas ---
Re: Solving FindBugs issues [was: Re: svn commit: r1071912 - in /xmlgraphics/fop/trunk: findbugs-exclude.xml src/java/org/apache/fop/render/pdf/PDFImageHandlerSVG.java]
On 21 Feb 2011, at 20:28, Andreas Delmelle wrote: On 21 Feb 2011, at 19:15, Vincent Hennebert wrote: If we solve issues raised by FindBugs by listing them in an ignore file, is there still a point to use FindBugs at all? It seems to me that some of those issues deserve to be fixed. They seem to point out genuine problems in the code. I was about to convey a similar sentiment. If we are only going to ignore potential bugs, the point of the whole exercise seems totally lost. Note also that, IIUC, if you define an exclusion for a given bug type in the scope of a given method, future devs are free to introduce many more instances of that very same bug type in that same method, without ever noticing that they are doing something wrong --even if they do their due diligence and run FB before each commit? That just doesn't seem right. :-/ Regards, Andreas ---
Re: Solving FindBugs issues [was: Re: svn commit: r1071912 - in /xmlgraphics/fop/trunk: findbugs-exclude.xml src/java/org/apache/fop/render/pdf/PDFImageHandlerSVG.java]
First, the point of the exclusion file is to bless warnings that, *by design*, do not correspond with findbugs set of default warnings. The presumption is that findbugs actually does identify real or potential bugs, and there should be no argument about whether this is true or not. What we are discussing here is the *automatic* addition of entries to the exclusion file. In general, that is a *bad practice*, however, for expediency purposes that has been done a few times already. Such practice should not be continued. Every exclusion added to the exclude file should be accompanied by a comment in the exclude file, and perhaps a comment in the source file as well, indicating why the exclusion occurred. If folks are uncomfortable with marking an exclusion for an entire method, then they can mark the specific line of source as well; however, that is problematic when line numbers change. I feel it is better to mark it for the method even if there is a risk it will unintentionally apply to another line (in the same method) than was intended. If folks make their methods shorter, then this reduces the likelihood of an unintentional exclusion. My personal criteria is that, in the general case, a method should occupy no more than one screen (on your favorite text editor). G. On Mon, Feb 21, 2011 at 12:35 PM, Andreas Delmelle andreas.delme...@telenet.be wrote: On 21 Feb 2011, at 20:28, Andreas Delmelle wrote: On 21 Feb 2011, at 19:15, Vincent Hennebert wrote: If we solve issues raised by FindBugs by listing them in an ignore file, is there still a point to use FindBugs at all? It seems to me that some of those issues deserve to be fixed. They seem to point out genuine problems in the code. I was about to convey a similar sentiment. If we are only going to ignore potential bugs, the point of the whole exercise seems totally lost. Note also that, IIUC, if you define an exclusion for a given bug type in the scope of a given method, future devs are free to introduce many more instances of that very same bug type in that same method, without ever noticing that they are doing something wrong --even if they do their due diligence and run FB before each commit? That just doesn't seem right. :-/ Regards, Andreas ---
RE: Solving FindBugs issues [was: Re: svn commit: r1071912 - in /xmlgraphics/fop/trunk: findbugs-exclude.xml src/java/org/apache/fop/render/pdf/PDFImageHandlerSVG.java]
I haven't looked at the trunk lately but 1.0 has a ton of warnings, at least in my compile. I don't know how much warnings have changed over the versions. I think it was originally written to compile on Java 1.4 or maybe even 1.3. 1.5 shows thousands of warnings, 1.6 shows more. Some of the warnings are quirky (raw type list?), some are just wasteful (dead code? Local variable never referenced?). If there's code which is actually incorrect logic it needs to be fixed. If there's code which is just incomplete logic, maybe setting something up for a future improvement someone wanted to add, that should be removed and placed in a separate to do list document. -Original Message- From: Andreas Delmelle [mailto:andreas.delme...@telenet.be] Sent: Monday, February 21, 2011 2:29 PM To: fop-dev@xmlgraphics.apache.org Subject: Re: Solving FindBugs issues [was: Re: svn commit: r1071912 - in /xmlgraphics/fop/trunk: findbugs-exclude.xml src/java/org/apache/fop/render/pdf/PDFImageHandlerSVG.java] On 21 Feb 2011, at 19:15, Vincent Hennebert wrote: If we solve issues raised by FindBugs by listing them in an ignore file, is there still a point to use FindBugs at all? It seems to me that some of those issues deserve to be fixed. They seem to point out genuine problems in the code. I was about to convey a similar sentiment. If we are only going to ignore potential bugs, the point of the whole exercise seems totally lost. Some of them may be OK to ignore, as Glenn pointed out, but IIUC, one of those potentially introduced bugs (that we are now ignoring) is likely of the same nature as one that Chris fixed in the very same area a while back (potentially unclosed stream, leading to a descriptor leak in the AFP renderer)... Not very encouraging. :( Mea culpa: I saw one exclusion --unconfirmed cast-- that would seem to stem from my recent refactoring in the BlockStackingLMs. Not sure why an exclusion was chosen here, but adding an assert statement in the code avoids the warning as well *and* has the benefit of being visible exactly at the spot in the code where it applies. Seemed like the more proper way to handle this. Just my 2 cents... Regards, Andreas ---
Re: Solving FindBugs issues [was: Re: svn commit: r1071912 - in /xmlgraphics/fop/trunk: findbugs-exclude.xml src/java/org/apache/fop/render/pdf/PDFImageHandlerSVG.java]
The current trunk shows no warnings during ANT compile. Please make reference to the current trunk/HEAD as 1.0 is published and history at this time. It's a different matter with certain IDEs, e.g., Eclipse, which set their warning levels to a more sensitive level than the ANT build. Although it would be nice to eliminate such additional warnings, it is not as high priority IMO as ensuring that new compile, checkstyle, or findbugs warnings/errors do not appear during ANT builds. At the same time, warnings that do appear should not automatically be excluded without careful, manual review. G. On Mon, Feb 21, 2011 at 12:49 PM, Eric Douglas edoug...@blockhouse.comwrote: I haven't looked at the trunk lately but 1.0 has a ton of warnings, at least in my compile. I don't know how much warnings have changed over the versions. I think it was originally written to compile on Java 1.4 or maybe even 1.3. 1.5 shows thousands of warnings, 1.6 shows more. Some of the warnings are quirky (raw type list?), some are just wasteful (dead code? Local variable never referenced?). If there's code which is actually incorrect logic it needs to be fixed. If there's code which is just incomplete logic, maybe setting something up for a future improvement someone wanted to add, that should be removed and placed in a separate to do list document. -Original Message- From: Andreas Delmelle [mailto:andreas.delme...@telenet.be] Sent: Monday, February 21, 2011 2:29 PM To: fop-dev@xmlgraphics.apache.org Subject: Re: Solving FindBugs issues [was: Re: svn commit: r1071912 - in /xmlgraphics/fop/trunk: findbugs-exclude.xml src/java/org/apache/fop/render/pdf/PDFImageHandlerSVG.java] On 21 Feb 2011, at 19:15, Vincent Hennebert wrote: If we solve issues raised by FindBugs by listing them in an ignore file, is there still a point to use FindBugs at all? It seems to me that some of those issues deserve to be fixed. They seem to point out genuine problems in the code. I was about to convey a similar sentiment. If we are only going to ignore potential bugs, the point of the whole exercise seems totally lost. Some of them may be OK to ignore, as Glenn pointed out, but IIUC, one of those potentially introduced bugs (that we are now ignoring) is likely of the same nature as one that Chris fixed in the very same area a while back (potentially unclosed stream, leading to a descriptor leak in the AFP renderer)... Not very encouraging. :( Mea culpa: I saw one exclusion --unconfirmed cast-- that would seem to stem from my recent refactoring in the BlockStackingLMs. Not sure why an exclusion was chosen here, but adding an assert statement in the code avoids the warning as well *and* has the benefit of being visible exactly at the spot in the code where it applies. Seemed like the more proper way to handle this. Just my 2 cents... Regards, Andreas ---
Re: Solving FindBugs issues [was: Re: svn commit: r1071912 - in /xmlgraphics/fop/trunk: findbugs-exclude.xml src/java/org/apache/fop/render/pdf/PDFImageHandlerSVG.java]
On 21 Feb 2011, at 20:49, Eric Douglas wrote: I haven't looked at the trunk lately but 1.0 has a ton of warnings, at least in my compile. I don't know how much warnings have changed over the versions. I think it was originally written to compile on Java 1.4 or maybe even 1.3. Even lower, IIRC. FOP ultimately dates back to last century. 1.5 shows thousands of warnings, 1.6 shows more. Some of the warnings are quirky (raw type list?), Those are the 'unchecked assignment' type. On the one hand, not much to worry about, as it has posed little problems in the past. On the other, it did sometimes cause nasty ClassCastExceptions... So we definitely try to eliminate those as we go. This number should reduce over time. Regards, Andreas ---
Re: Solving FindBugs issues [was: Re: svn commit: r1071912 - in /xmlgraphics/fop/trunk: findbugs-exclude.xml src/java/org/apache/fop/render/pdf/PDFImageHandlerSVG.java]
On 21 Feb 2011, at 21:18, Eric Douglas wrote: I always had trouble with the ant build. Currently my ant build gets no errors and no warnings and creates fop.jar though it tells me junit and xmlunit are not found. Last time I put jars in my fop lib folder for junit and xmlunit it told me they were found and it wouldn't update the fop jar. If there's already a set of precompiled classes, and none of the sources have changed, there is no need to update the jar. If you just run 'ant compile' or 'ant package', neither JUnit nor XMLUnit are required for a successful build. Compiler warnings are not errors, and as such do not prevent a correct build. How can I get hundreds or even thousands of warnings from trying to use the eclipse build project option with the default settings while the ant build has no errors? The 'ant compile' task does show a hint of those warnings, IIC. Look for Note: some classes use unchecked or unsafe operations. If I were to guess, I'd say your IDE probably compiles with '-Xlint:unchecked', which displays all the gruesome details. Regards, Andreas ---
Re: Solving FindBugs issues [was: Re: svn commit: r1071912 - in /xmlgraphics/fop/trunk: findbugs-exclude.xml src/java/org/apache/fop/render/pdf/PDFImageHandlerSVG.java]
On 21/02/11 19:28, Andreas Delmelle wrote: On 21 Feb 2011, at 19:15, Vincent Hennebert wrote: If we solve issues raised by FindBugs by listing them in an ignore file, is there still a point to use FindBugs at all? It seems to me that some of those issues deserve to be fixed. They seem to point out genuine problems in the code. I was about to convey a similar sentiment. If we are only going to ignore potential bugs, the point of the whole exercise seems totally lost. Some of them may be OK to ignore, as Glenn pointed out, but IIUC, one of those potentially introduced bugs (that we are now ignoring) is likely of the same nature as one that Chris fixed in the very same area a while back (potentially unclosed stream, leading to a descriptor leak in the AFP renderer)... Not very encouraging. :( Mea culpa: I saw one exclusion --unconfirmed cast-- that would seem to stem from my recent refactoring in the BlockStackingLMs. Not sure why an exclusion was chosen here, but adding an assert statement in the code avoids the warning as well *and* has the benefit of being visible exactly at the spot in the code where it applies. Seemed like the more proper way to handle this. Did you remove the corresponding entry from the findbugs-exclude.xml file before running FindBugs again? Just my 2 cents... Regards, Andreas Vincent
Re: Solving FindBugs issues [was: Re: svn commit: r1071912 - in /xmlgraphics/fop/trunk: findbugs-exclude.xml src/java/org/apache/fop/render/pdf/PDFImageHandlerSVG.java]
On 21 Feb 2011, at 22:18, Vincent Hennebert wrote: Did you remove the corresponding entry from the findbugs-exclude.xml file before running FindBugs again? Locally, yes, and confirmed that the assert eliminated the warning. The exclusions file seems to be needing some more general cleanup, so I haven't committed that yet. 1012 exclusions is probably way too many already... :-/ Regards, Andreas ---
Re: Solving FindBugs issues
Not all FOP developers are willing to use findbugs. I hid the findbugs errors as a courtesy to those FOP developers who do use findbugs, so they can check their own code based on a clean slate. FOP's history has left us with a very large number of existing findbugs errors. It makes no sense to comment on that; it is a fact of life. FOP's code first of all does a good job at being a successful, heavily used FO processor. Only after that comes the question of a clean code base. That said, of course every FOP developer and every FOP user is welcome to evaluate and possibly remedy one or more findbugs errors. For precisely that reason I put comments in the exclusion file, so one can see which errors have been simply hidden and which have been truely evaluated. Simon On Mon, Feb 21, 2011 at 06:15:13PM +, Vincent Hennebert wrote: Hi, If we solve issues raised by FindBugs by listing them in an ignore file, is there still a point to use FindBugs at all? It seems to me that some of those issues deserve to be fixed. They seem to point out genuine problems in the code. Vincent On 18/02/11 08:18, spepp...@apache.org wrote: Author: spepping Date: Fri Feb 18 08:18:04 2011 New Revision: 1071912 URL: http://svn.apache.org/viewvc?rev=1071912view=rev Log: Fixing checkstyle errors and hiding fingbugs errors Modified: xmlgraphics/fop/trunk/findbugs-exclude.xml xmlgraphics/fop/trunk/src/java/org/apache/fop/render/pdf/PDFImageHandlerSVG.java Modified: xmlgraphics/fop/trunk/findbugs-exclude.xml URL: http://svn.apache.org/viewvc/xmlgraphics/fop/trunk/findbugs-exclude.xml?rev=1071912r1=1071911r2=1071912view=diff
Re: Solving FindBugs issue
When I build the project or part of it with Eclipse, and run findbugs afterwards (with ant), I get a number of errors. Now I always make a clean compile before running findbugs. I do not understand why Eclipse builds would create findbugs errors where a clean ant build does not. It makes findbugs seem fickle. I just tested it. With 'ant clean findbugs' I get 0 errors and warnings. With eclipse clean and build project, I get 23 low priority warnings. Simon On Mon, Feb 21, 2011 at 12:58:06PM -0700, Glenn Adams wrote: The current trunk shows no warnings during ANT compile. Please make reference to the current trunk/HEAD as 1.0 is published and history at this time. It's a different matter with certain IDEs, e.g., Eclipse, which set their warning levels to a more sensitive level than the ANT build. Although it would be nice to eliminate such additional warnings, it is not as high priority IMO as ensuring that new compile, checkstyle, or findbugs warnings/errors do not appear during ANT builds. At the same time, warnings that do appear should not automatically be excluded without careful, manual review. G. On Mon, Feb 21, 2011 at 12:49 PM, Eric Douglas edoug...@blockhouse.comwrote: I haven't looked at the trunk lately but 1.0 has a ton of warnings, at least in my compile. I don't know how much warnings have changed over the versions. I think it was originally written to compile on Java 1.4 or maybe even 1.3. 1.5 shows thousands of warnings, 1.6 shows more. Some of the warnings are quirky (raw type list?), some are just wasteful (dead code? Local variable never referenced?). If there's code which is actually incorrect logic it needs to be fixed. If there's code which is just incomplete logic, maybe setting something up for a future improvement someone wanted to add, that should be removed and placed in a separate to do list document.
Re: Assertions in junit tests [was: Re: Solving FindBugs issue]
On Mon, Feb 21, 2011 at 08:28:33PM +0100, Andreas Delmelle wrote: I saw one exclusion --unconfirmed cast-- that would seem to stem from my recent refactoring in the BlockStackingLMs. Not sure why an exclusion was chosen here, but adding an assert statement in the code avoids the warning as well *and* has the benefit of being visible exactly at the spot in the code where it applies. Seemed like the more proper way to handle this. It would be nice if assertions were checked during the junit tests. However, when I set: junit ... jvm=java -ea/, I get junit.framework.AssertionFailedError errors. How can this be done? Simon