Re: RFR 8009517: Disable fatal compiler warning in the old build

2013-03-09 Thread Chris Hegarty

I agree about warning creeping problems.  This is a temporary solution,
we should soon be fixing the underlying hashcode/equals problems...but...


Your temporary solution, -overrides, is just that. It will enable the 
old build to complete today, but it could fail at any point in the 
future, as the code changes.


For example, java.net is currently warning free, in the old it compiles 
with fatal warnings enabled. Lets say, in a moment of madness, I add a 
dependency from java.net.Socket to say java.awt.RenderingHints.Key ( or 
any class that produces warnings when compiled. I run the new build, all 
is fine. Push the changes. Now someone else sync's up, but need to build 
using the old build. If the new dependent class is not already compiled 
before java.net.Socket gets compiled, it will be compiled implicitly. 
It's warnings will cause the compile to fail, and the old build will 
fail. Or much simpler, anyone could write sloppy code with warnings, the 
new build will suppress them, and they won't notice. Push this code, and 
the old build will fail if is explicitly, or implicitly, compiles this 
code with -Werror enabled.



 We
spent a lot of time cleaning up many directories, seems a shame to start
allowing non-fatal warnings to come back into previously clean code
because people aren't taking the time to fix new warnings as they are
introduced.


I personally spent several weeks over the past number of years fixing 
warnings and reviewing warning cleanup webrevs from others. I took much 
pride in keeping certain areas warnings free.


It is with great regret that I propose to disable fatal warnings in the 
old build, but I felt this the best/safest option. I heard much 
annoyance and frustration from others about hitting seemingly random 
errors with the old build recently. This is the only sure way to avoid that.



The new builds will still warn, but the
old builds will still fail for all but these override problems.  Yes,
you lose the warnings in the old, but seems better than completely
shutting off erroring.


I'm ok with that, if others are. To clarify, I think you are suggesting 
that we keep the old build as it, with -overrides, and use it 
periodically as a way of tracking new warnings being introduced into 
areas that were warning free. That is, if the old build fails because of 
a fatal warning, so be it. File a bug and fix the source code. Then the 
old build will work again. This means that at any point in time the old 
build cannot be guaranteed to be buildable.


Everyone seems to agree, a solution needs to be found to allow us to 
keep certain areas warning free. This issue is too important, and too 
much time was spent, to allow it to regress to the state it was in a few 
years ago.


-Chris.



(Ideally it would be nice to warn but not fail on just this one lint
option, but don't see how that's possible.)

Brad






-Chris.



Mike

On Mar 8 2013, at 05:24 , Chris Hegarty wrote:


Since the new build does not enable -Werror when compiling any java
code, and disables quite a few lint options, new changes my
inadvertently introduce warnings without even realizing. This can
cause problems when building with the old build as many areas do
compile with -Werror set. Since the old build is on life support,
probably best to just completely disable -Werror, so anyone still
needing to use it can.

diff -r 48b7295f02f8 make/common/shared/Defs-java.gmk
--- a/make/common/shared/Defs-java.gmk  Thu Mar 07 10:07:13 2013 +
+++ b/make/common/shared/Defs-java.gmk  Thu Mar 07 11:10:37 2013 +
@@ -122,9 +122,10 @@ ifeq ($(JAVAC_MAX_WARNINGS), true)
ifeq ($(JAVAC_MAX_WARNINGS), true)
   JAVAC_LINT_OPTIONS += -Xlint:all
endif
-ifeq ($(JAVAC_WARNINGS_FATAL), true)
-  JAVACFLAGS  += -Werror
-endif
+# Disable fatal warnings, 8009517
+#ifeq ($(JAVAC_WARNINGS_FATAL), true)
+#  JAVACFLAGS  += -Werror
+#endif

# TODO: Workaround for CR 7063027. Remove -path eventually.
JAVAC_LINT_OPTIONS += -Xlint:-path

-Chris.




Re: RFR 8009517: Disable fatal compiler warning in the old build

2013-03-09 Thread Jonathan Gibbons

On 03/09/2013 12:11 AM, Chris Hegarty wrote:


Everyone seems to agree, a solution needs to be found to allow us to 
keep certain areas warning free. This issue is too important, and too 
much time was spent, to allow it to regress to the state it was in a 
few years ago.


It is true that selective use of -Werror does not play well with the new 
build and the desire to compile as much as possible at any time, 
possibly using sjavac.


I have previously reported on a utility I've written in the past to 
analyze java code and report on the different types of warnings found in 
different packages.  I'm sure we could take something like that utility 
and improve it to the point where it can give errors when entries in the 
matrix which should be empty are not.


This utility could be run as a test meaning it needn't break the build 
but the info is available for those that want to run it.


-- Jon


Re: RFR 8009517: Disable fatal compiler warning in the old build

2013-03-09 Thread Chris Hegarty

On 9 Mar 2013, at 19:01, Jonathan Gibbons jonathan.gibb...@oracle.com wrote:

 On 03/09/2013 12:11 AM, Chris Hegarty wrote:
 
 Everyone seems to agree, a solution needs to be found to allow us to keep 
 certain areas warning free. This issue is too important, and too much time 
 was spent, to allow it to regress to the state it was in a few years ago.
 
 It is true that selective use of -Werror does not play well with the new 
 build and the desire to compile as much as possible at any time, possibly 
 using sjavac.
 
 I have previously reported on a utility I've written in the past to analyze 
 java code and report on the different types of warnings found in different 
 packages.  I'm sure we could take something like that utility and improve it 
 to the point where it can give errors when entries in the matrix which should 
 be empty are not.
 
 This utility could be run as a test meaning it needn't break the build but 
 the info is available for those that want to run it.

Sounds interesting Jon, and certainly worth exploring.

Another idea, is to enable all lint options and -Werror ;-) If the 
SuppressWarnings annotation was allowable on the PACKAGE Target :-^ It may be 
reasonable to add to the packages where applicable.

-Chris.


 
 -- Jon