RFR 8009517: Disable fatal compiler warning in the old build

[Chris Hegarty]

Brad,

I do not build using the old build anymore. This is clearly a blocker 
for your work. If you want to suppress the warnings for 
overrides/deprecation, then please push the change ( your patch ). We 
can revisit this in the future, when it is necessary.

-Chris.

On 03/19/2013 01:29 AM, 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.
>>>>>