RFR: JDK-7184195 - java.util.logging.Logger.getGlobal().info() doesn't log without configuration

[Daniel Fuchs]

Hi Mandy,

Please find an updated webrev.

I have refactored the tests a bit:

renamed "TestGetGlobal2" into "TestGetGlobalByName"
put the auxiliary classes in a sub directory
removed the 'Test' prefix from the auxiliary classes,
added comments in the various LogManagerImpl* classes
updated the 3 tests summary.
added "." as default value for test.src.

<http://cr.openjdk.java.net/~dfuchs/JDK-7184195/webrev.02/>

Hope you find it more to your liking!

Best regards,

-- daniel

On 7/1/13 7:57 PM, Mandy Chung wrote:
> On 7/1/13 6:25 AM, Daniel Fuchs wrote:
>>
>>> Looks good in general.  Some comments:
>>>
>>> Logger.global is deprecated.  In LogManager static initializer, we
>>> should have @SuppressWarnings("deprecation") on Logger.global (probably
>>> need to define a local variable) to suppress the warning and together
>>> with your comment it will make it clear that accessing this deprecated
>>> field is intentional.
>>
>> Right. I can do that. But I don't see any warnings when compiling
>> (with make) - maybe they're turned out by default?
>
> Deprecation and a few other javac warnings are disabled (see
> jdk/makefiles/Setup.gmk).
>>
>>>
>>> As you pointed out, the same pattern is used in the checkPermission()
>>> method.  Might be worth having a private getManager() method to replace
>>> direct access to the manager field in the Logger class - just a thought
>>> to prevent similar bug from happening again.
>>
>> Well - hmm - I'd prefer to do that in a later cleanup.
>> I'd want first to analyze all the places where manager is used.
>> Plus there is Peter's suggestion about changing the dynamics
>> between LogManager & Logger.
>> Both are excellent suggestions but I think I'd like to dedicate
>> a whole changeset for this kind of cleanup, without mixing it
>> with bug fix.
>
> Agree.  It's good to do it in a separate fix especially it may involve
> untangling the Logger and LogManager initialization.
>
> Mandy