RFR 8009517: Disable fatal compiler warning in the old build

[Brad Wetmore]

David,

Plugin's integration next week is now also impacted since they also have 
to use the old build environment, so it's time to resolve this.  So 
here's an update/proposal/webrev.

The original bug filed by David Katleman was:

     8009517: build-infra: jdk8: -Werror not being applied to nio builds

and I filed:

     8010434: Old build environment no longer builds.

I've closed mine, and will integrate against the former.

In my partial (JDK) build using the old mechanism and the current TL 
JDK/langtools gate, only two changes are now necessary.

I propose we disable -deprecation in jdk/makejavax/others, and 
-overrides in  make/com/sun/org/apache/xml.

The first *LIKELY* crept in due to a recent change in the Base64 code 
which no longer implicitly compiles sun/misc/CharacterDecoder.java 
without -Werror active.  The original deprecation issue needs to be 
addressed sometime by the responsible team, but that will avoid RE 
having to hand edit to the Makefiles just to build JCE.

The second change is due to the compiler change with hashCode/equals.

The codereview is here:

     http://cr.openjdk.java.net/~wetmore/8009517/webrev.00/

I plan to push through the deploy gate, as they have an integration next 
week.  Thomas Ng will do the push for us.

Any objections, please speak now.

Brad



On 3/18/2013 6:29 PM, Brad Wetmore wrote:
> Sorry for the delay in response, I've been pulled in yet another
> direction, and this has come back up in priority.
>
> On 3/9/2013 12:11 AM, Chris Hegarty wrote:
>>> 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.
>
> Correct.  As it stands today, a recent change now requires *BOTH*
> overrides/deprecation in order to get a complete MASTER build using the
> old build system.
>
> [brwetmor at flicker-vm1] 222 >hg diff common/shared/Defs-java.gmk
> diff --git a/make/common/shared/Defs-java.gmk
> b/make/common/shared/Defs-java.gmk
> --- a/make/common/shared/Defs-java.gmk
> +++ b/make/common/shared/Defs-java.gmk
> @@ -127,7 +127,7 @@
>   endif
>
>   # TODO: Workaround for CR 7063027. Remove -path eventually.
> -JAVAC_LINT_OPTIONS += -Xlint:-path
> +JAVAC_LINT_OPTIONS += -Xlint:-path,-overrides,-deprecation
>
>   JAVACFLAGS += $(JAVAC_LINT_OPTIONS)
>
>> 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.
>
> Exactly.  Our formerly clean code now requires disabling of two Lint
> options, but the new build is happy just to report the warning.  The old
> build crashes on the warning.
>
> Our options for the old build system are:
>
> 1.  disable the warning for overrides/deprecation, keep -Werror (my
> preferred since these are minor warnings.)
> 2.  Somehow disable -Werror on these new directories that are now
> failing.  (more work to figure out, but also acceptable)
> 3.  Fix the warnings.  (I don't have cycles to drive a rewrite of use of
> deprecated code and/or add missing equals/hashcode that the recent javac
> changes exposed.)
>
>>>  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 now ",-deprecation"  :(
>
>>  and use it
>> periodically as a way of tracking new warnings being introduced into
>> areas that were warning free.
>
> That would be a side-effect, as someone would occasionally need to
> figure out what's changed.
>
> The main issue we're hitting right now is that RE has to make several
> source code changes in order to build JCE jar files without errors.  I
> was able to change the individual LINT options globally and reduce it
> down to one change, but that's still one change that RE has to make.  I
> feel that RE should not be making any changes, but that ship has already
> sailed and we're stuck with the results now.
>
>   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.
>
> It's already started.
>
> Brad
>
>
>> -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
>>>>>> +0000
>>>>>> +++ b/make/common/shared/Defs-java.gmk  Thu Mar 07 11:10:37 2013
>>>>>> +0000
>>>>>> @@ -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.
>>>>>