RFR: 8248261: Add timestamps to jpackage and jpackage tests verbose output

[Alexey Semenyuk]

+1

- Alexey

On 7/10/2020 6:29 PM, Andy Herrick wrote:
> looks good.
>
> /Andy
>
> On 7/9/2020 12:02 AM, alexander.matveev at oracle.com wrote:
>> Hi Alexey,
>>
>> http://cr.openjdk.java.net/~almatvee/8248261/webrev.01/
>>
>> - Added fatalError() to log fatal errors without timestamp.
>> - Added missing timestamp to Log.verbose(Throwable t).
>>
>> Thanks,
>> Alexander
>>
>> On 7/8/20 9:34 AM, Alexey Semenyuk wrote:
>>> I still think it would be good to create dedicated method for final 
>>> error reporting in Main and Arguments classes without timestamps.
>>>
>>> - Alexey
>>>
>>> On 7/8/2020 9:59 AM, Andy Herrick wrote:
>>>> After further discussion, I approve this change as is.
>>>>
>>>> The main fatal error in Arguments.processArguments() only calls 
>>>> Log.error if not verbose , so guarding it from having a timestamp 
>>>> when verbose is not an issue.
>>>>
>>>> /Andy
>>>>
>>>> On 7/7/2020 7:20 PM, Andy Herrick wrote:
>>>>> I agree - but I would rather then that Log.error() never show 
>>>>> timestamp.
>>>>>
>>>>> It would be better still if we could differentiate fatal error 
>>>>> messages from warning messages - right now we are using Log.error 
>>>>> for both.
>>>>>
>>>>> /Andy
>>>>>
>>>>>
>>>>> On 7/7/2020 6:51 PM, alexander.matveev at oracle.com wrote:
>>>>>> Hi Andy,
>>>>>>
>>>>>> Timestamps for error message without verbose output are 
>>>>>> meaningless in my opinion. This is why I did not add them. Also, 
>>>>>> in some cases output does not look right. For example when 
>>>>>> timestamp is added to error message always:
>>>>>> jpackage --someoption
>>>>>> WARNING: Using incubator modules: jdk.incubator.jpackage
>>>>>> [15:49:23.798] Error: Invalid Option: [--someoption]
>>>>>>
>>>>>> In example above I do not see any point to have timestamp.
>>>>>>
>>>>>> Thanks,
>>>>>> Alexander
>>>>>>
>>>>>> On 7/7/20 6:18 AM, Andy Herrick wrote:
>>>>>>> All looks good, except maybe Log.error().
>>>>>>>
>>>>>>> I think Log.error() should have the same output whether verbose 
>>>>>>> or not, adding timestamp to the message only if verbose does not 
>>>>>>> look right.
>>>>>>>
>>>>>>> Problem is that Log.error() is used for two fundamentally 
>>>>>>> different purposes:
>>>>>>>
>>>>>>> 1.) by Main and Arguments to display the final error message 
>>>>>>> when jpackage is failing.
>>>>>>>
>>>>>>> 2.) by everyone else to display a serious warning message when 
>>>>>>> jpackage is continuing (sometimes failing further on, and 
>>>>>>> sometimes not).
>>>>>>>
>>>>>>> I wouldn't mind adding timestamps only when verbose in case (2), 
>>>>>>> but I don't think that's appropriate for case (1).
>>>>>>>
>>>>>>> /ANdy
>>>>>>>
>>>>>>> On 7/6/2020 8:27 PM, alexander.matveev at oracle.com wrote:
>>>>>>>> Please review the jpackage fix for bug [1] at [2].
>>>>>>>>
>>>>>>>> Added timestamp to verbose and test output in form of 
>>>>>>>> [HH:mm:ss.SSS].
>>>>>>>>
>>>>>>>> [1] https://bugs.openjdk.java.net/browse/JDK-8248261
>>>>>>>> [2] http://cr.openjdk.java.net/~almatvee/8248261/webrev.00/
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Alexander
>>>>>>
>>>
>>