Review Request: 7193406 - Clean-up JDK Build Warnings in java.util, java.io

[Andrew Hughes]

----- Original Message -----
> On 8/23/12 5:12 PM, Andrew Hughes wrote:
> > Dan Xu wrote:
> >> Please review the fix of CR 7193406 at
> >> http://cr.openjdk.java.net/~dxu/7193406/webrev/.
> >> And it enables fatal warning flag in the following make file.
> >>
> >>      make/java/jar/Makefile
> >
> > Please don't do this, at least not unconditionally.
> 
> Right, we had been enabling fatal warnings in the various makefiles
> but we've
> stopped doing so since it has started to cause problems. The intent
> was to
> enable fatal warnings by default in individual makefiles, as those
> areas of the
> build were cleared of warnings. That way, the introduction of a new
> warning
> into a warnings-free area **would** break the build.

I understand the motivation and why the current setup would be needed
in transition.

However, once the whole build is warning free, would it not be preferable
to remove these and just set JAVAC_WARNINGS_FATAL=true when doing development
builds?

The problem I see is someone new coming to OpenJDK and not being able to
simply build it (with no changes) because a new warnings has appeared
and is being treated as an error.  This is less of a problem with javac
as we control its development, and the JDK will use the javac built in the
langtools step in most cases.  But, generally, -Werror is something you
should choose to enable, with the intention of fixing failures, not something
that should be forced on everyone building the code.

On a related topic, it would be nice if javadoc could also support -Werror
as I constantly see warnings reappearing in the documentation.

> 
> The problem is that because of implicit compilation, as code is
> modified, files
> can shift around being compiled by different Makefiles. Thus an
> apparently
> innocuous change might cause a file with warnings to be built by a
> different
> javac run from a makefile that has fatal warnings enabled, causing an
> unexpected build breakage.
> 
> Anyway, Dan, please don't enable this flag in this (or any other)
> Makefile.
> Sorry, I thought I had mentioned this to you before.
> 
> > At the very least, these should not be forced on if the user
> > has explicitly built with them set to false.  If I set
> > JAVAC_WARNINGS_FATAL=false, I don't expect part of the build
> > to ignore this and use -Werror, possibly then causing build
> > failures.
> 
> Yes, it should always be possible to override this by specifying
> JAVAC_WARNINGS_FATAL=false on the make command line. This overriding
> should
> work if the value is specified directly in a Makefile, e.g.
> 
>      JAVAC_WARNINGS_FATAL = true
> 
> However, there are several cases where the following occurs:
> 
>      SUBDIRS_MAKEFLAGS += JAVAC_WARNINGS_FATAL=true
> 
> and this is **not** overridable on the command line. That's wrong. If
> these are
> causing problems for you, please do submit patches.

Yes, that's what we have in java/tools and is why JAVAC_WARNINGS_FATAL=false
doesn't completely remove -Werror at present.  I'll post a webrev of the change
I have for this.

> 
> Although we still occasionally run into problems with implicit
> compilation
> causing unexpected build failures, I don't want to remove the fatal
> warnings
> settings wholesale. The settings in place now do seem to be keeping
> at least
> some parts of the build warning-free.
> 
> The new build system will change all of this completely, of course. I
> don't
> think they have a solution yet for applying different flags to
> different parts
> of the build.
> 
> >> In FilePermission.java file, I make one change to its method
> >> signature,
> >>
> >>      public Enumeration elements()  ==>  public
> >>      Enumeration<Permission>
> >>      elements()
> >
> > It's in a package-private class so I doubt it.
> >
> > Even if it wasn't, a new major release is the perfect time to fix
> > these issues.
> 
> Yes, this one is fine because it's a private class.
> 
> For warnings fixes we're trying to avoid making any API changes,
> since those
> have to go through some additional process steps.
> 
> s'marks
> 

-- 
Andrew :)

Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)

PGP Key: 248BDC07 (https://keys.indymedia.org/)
Fingerprint = EC5A 1F5E C0AD 1D15 8F1F  8F91 3B96 A578 248B DC07