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

[Stuart Marks]

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.

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.

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