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

[Andy Herrick]

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
>>>>>
>>
>