mistriggered "error: warnings found and -Werror specified" for java warnings

Fri Apr 24 08:51:43 UTC 2020

On 2020-04-24 10:29, Matthias Klose wrote:
> On 4/23/20 6:59 PM, Magnus Ihse Bursie wrote:
>>
>> On 2020-04-23 17:17, Igor Ignatyev wrote:
>>> Hi Matthias,
>>>
>>>> jtharness 6.0-b10 and jtreg 5.0-b1.
>>> jtreg 5.0-b1 "officially" depends on jt6.0-b08, and the bundle which we use
>>> internally has jt6.0-b08 within it, and AFAICT jt6.0-b08 hasn't deprecated
>>> c.s.j.H.Observer::finishedTesting yet. so as a temp. solution to allow
>>> configurations like yours, we need to suppress deprecation warning in failure
>>> handler, that's if we decide that such configurations are "supported", which
>>> isn't that obvious as you might encounter some deviations in jtreg behavior
>>> b/c another version of its dependency is used, so I'd encourage you to use the
>>> exact same version as used by jtreg build script[1].
>>  From my point of view, I think we should have a very narrow range of supported
>> jtreg versions. It's not like when building with the system zlib; this is a
>> framework solely used for testing the JDK, and I think we can (and should) be
>> quite strict as to which versions the current code base is designed to work with.
> well, you are not that strict when it comes to the compiler used, and try to
> support a wide range of compilers.  You bundle some third party libraries where
> you have stricter version requirements.
You are absolutely correct. My point here is that jtreg is a specialized 
tool, created solely for running tests in the JDK. This is not the case 
with compilers and external libraries. I think the main reason that 
jtreg is not part of the JDK repo is ... well... maybe not to "pollute" 
it with things that might strictly speaking not be needed, and could be 
placed elsewhere. Or just legacy reasons.
>
>> Things get a little bit muddier if the version of jtreg matches, but not an
>> upstream dependency for jtreg. I still tend to lean towards Igor's suggestion
>> here, that a properly setup jtreg environment is the one officially built by jtreg.
> I am trying to run the tests as part of the package builds in Debian/Ubuntu.
> Now you can argue that packaging jtreg 5.0-b1 depending on jtharness 6.0-b10 is
> a packaging mistake, otoh there is no upper dependency on the jtharness version.
>
> Going forward with the suggestion would mean to track all dependencies of jtreg,
> package those, hoping that unrelated packages in the distro archives can work
> with these versions, and do this for every jtreg version that is required by any
> openjdk version you want to build in a distro context. Not very appealing. The
> alternative would be to disable testing. Not appealing either.
>
> I'll see to revert that deprecation in jtharness 6.0-b10 for now, or patching
> the openjdk build process to ignore that warning.
I think you missed the third option: updating the failure handler code 
so it does not use deprecated, or soon-to-be-deprecated, methods. Even 
if builds of jtreg using jtharness 6.0-b08 works fine, there will 
apparently come a time when the current code will trigger a warning. If 
there exists non-deprecated replacement methods in 6.0-b08, then we 
should perhaps switch to using these right now?

(And also, what kind of madman versioning scheme is used if you 
introduce deprecations between "6.0-b08" and "6.0-b10"?!? :-o I would 
have assumed API changes would have caused a version bumb to something 
like 6.1 at the very least.)

But if this approach is not feasible, I'm going to accept to re-disable 
the deprecation warning for the failure handler. I'm just noting that if 
we do that, it will basically have to stay there forever. If an engineer 
ever wanted to clear up the remaining warnings for the failure handler, 
how would they go ahead to make sure they don't break anything? A build 
with the recommended libraries will produce no warnings, so how can he 
know that he needs to keep it? Download and try a wide range of 
combination of libraries and their dependencies?

/Magnus

>
> Matthias
>
>> /Magnus
>>
>>
>>> a proper solution would include switching jtreg to newer version of jt-harness
>>> (which implies adjustment of jtreg and subsequently testing),
>>> promotion/tagging of newer jtreg build, switching to newer jtreg in jdk and
>>> updating in failurehandler.
>>>
>>> Thanks,
>>> -- Igor
>>>
>>> [1]
>>> http://hg.openjdk.java.net/code-tools/jtreg/file/fc37a1d7f0ea/make/build-all.sh#l129
>>>
>>>
>>>> On Apr 23, 2020, at 7:48 AM, Matthias Klose <doko at ubuntu.com
>>>> <mailto:doko at ubuntu.com>> wrote:
>>>>
>>>> On 4/23/20 4:05 PM, Magnus Ihse Bursie wrote:
>>>>>> 23 apr. 2020 kl. 15:50 skrev Igor Ignatyev <igor.ignatyev at oracle.com
>>>>>> <mailto:igor.ignatyev at oracle.com>>:
>>>>>>
>>>>>>
>>>>>>
>>>>>>> On Apr 23, 2020, at 6:12 AM, Erik Joelsson <erik.joelsson at oracle.com
>>>>>>> <mailto:erik.joelsson at oracle.com>> wrote:
>>>>>>>
>>>>>>> Hello Matthias,
>>>>>>>
>>>>>>>> On 2020-04-23 05:51, Matthias Klose wrote:
>>>>>>>> jdk-15+20 fails to build with
>>>>>>>>
>>>>>>>> * For target
>>>>>>>> support_test_failure_handler_classes__the.BUILD_FAILURE_HANDLER_batch:
>>>>>>>> /packages/openjdk/15/openjdk-15-15~20/test/failure_handler/src/share/classes/jdk/test/failurehandler/jtreg/GatherDiagnosticInfoObserver.java:136:
>>>>>>>>
>>>>>>>> warning: [deprecation] finishedTesting() in Observer has been deprecated
>>>>>>>>    public void finishedTesting() {
>>>>>>>>                ^
>>>>>>>> error: warnings found and -Werror specified
>>>>>>>> 1 error
>>>>>>>> 1 warning
>>>>>>> That's strange. I assume this tool is built with the boot JDK, so that
>>>>>>> makes me wonder what boot JDK you are using as we have not seen this
>>>>>>> warning/error?
>>>> that's with 14.0.1+7.
>>>>
>>>>>> I guess version of jtreg/jt-harness is more relevant here as deprecated
>>>>>> finishedTesting is from com.sun.javatest.Harness.Observer.
>>>>> Aha, that’s probably the explanation. I recently removed deprecation as a
>>>>> disabled warning for the failure handler. If it depends on jtreg, and it has
>>>>> changed deprecation status, that might trigger a compilation warning.
>>>>>
>>>>> I’m on mobile now and can’t check how this should be resolved.
>>>>>
>>>>> If a newer version of jtreg introduced the depreciation, we should fix our
>>>>> sources. If this is something only present in older sources (?) we might
>>>>> need to raise the minimum jtreg version.
>>>> jtharness 6.0-b10 and jtreg 5.0-b1.
>>>>
>>>>> /Magnus
>>>>>
>>>>>> --Igor
>>>>>>
>>>>>>>> Apparently --disable-warnings-as-errors only has an effect on C/C++ files,
>>>>>>>> however the build diagnostics trigger on java warnings as well, and
>>>>>>>> apparently
>>>>>>>> -Werror is hard-coded in various places for java options. Should the
>>>>>>>> documentation for this configure option be clarified, or should it
>>>>>>>> trigger for
>>>>>>>> java warnings as well?
>>>>>>> Correct. The reasoning is that OpenJDK is built on a wide variety of
>>>>>>> environments with different compiler versions, so keeping the build
>>>>>>> warning free on all of them isn't feasible. This option makes it possible
>>>>>>> to build with all those different compiler versions while still
>>>>>>> maintaining a warning free source for a core set of compiler versions. In
>>>>>>> contrast, the Java code should only be compiled with a very small set of
>>>>>>> javac versions, which should be easily controlled. The majority of the
>>>>>>> code is even compiled with the Javac we are building. We have contemplated
>>>>>>> a similar option for Java code, but concluded that it doesn't serve any
>>>>>>> purpose. The Java source should just always be warning free.
>>>>>>>
>>>>>>> /Erik
>>