DO NOT REPLY [Bug 49835] Wrong page break with 2 columned region and tables

2011-02-21 Thread bugzilla
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

2011-02-21 Thread bugzilla
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

2011-02-21 Thread bugzilla
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

2011-02-21 Thread bugzilla
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]

2011-02-21 Thread Vincent Hennebert
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]

2011-02-21 Thread Glenn Adams
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]

2011-02-21 Thread Andreas Delmelle
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]

2011-02-21 Thread Andreas Delmelle
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]

2011-02-21 Thread Glenn Adams
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]

2011-02-21 Thread Eric Douglas
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]

2011-02-21 Thread Glenn Adams
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]

2011-02-21 Thread Andreas Delmelle
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]

2011-02-21 Thread Andreas Delmelle
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]

2011-02-21 Thread Vincent Hennebert
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]

2011-02-21 Thread Andreas Delmelle
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

2011-02-21 Thread Simon Pepping
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

2011-02-21 Thread Simon Pepping
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]

2011-02-21 Thread Simon Pepping
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