RFR: 8191033: Regression in logging.properties: specifying .handlers= for root logger (instead of handlers=) no longer works

classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

RFR: 8191033: Regression in logging.properties: specifying .handlers= for root logger (instead of handlers=) no longer works

Daniel Fuchs
Hi,

Please find below a patch for:

8191033 Regression in logging.properties: specifying .handlers= for
         root logger (instead of handlers=) no longer works
https://bugs.openjdk.java.net/browse/JDK-8191033

webrev:
http://cr.openjdk.java.net/~dfuchs/webrev_8191033/webrev.00/

The patch restores the behavior that was observed for JDK 8.

The test fails with jdk 9.0.1, but passes with jdk1.8.0_131 and
with this patch applied to a clone of http://hg.openjdk.java.net/jdk/jdk

best regards,

-- daniel
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8191033: Regression in logging.properties: specifying .handlers= for root logger (instead of handlers=) no longer works

Martin Buchholz-3
configure => configured
+                        // For backward compatibility: add any handlers
configure using
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8191033: Regression in logging.properties: specifying .handlers= for root logger (instead of handlers=) no longer works

Martin Buchholz-3
In reply to this post by Daniel Fuchs
You went to the trouble of checking test.src.  Shouldn't  CONFIG_FILE be
found relative to SRC_DIR?

+    public static final Path SRC_DIR =
+            Paths.get(System.getProperty("test.src", "src"));
+    public static final Path USER_DIR =
+            Paths.get(System.getProperty("user.dir", "."));
+    public static final Path CONFIG_FILE = Paths.get("logging.properties");
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8191033: Regression in logging.properties: specifying .handlers= for root logger (instead of handlers=) no longer works

Martin Buchholz-3
On Thu, Dec 7, 2017 at 11:00 AM, Martin Buchholz <[hidden email]>
wrote:

> You went to the trouble of checking test.src.  Shouldn't  CONFIG_FILE be
> found relative to SRC_DIR?
>
>
Oh never mind, I now see you did

+        Path initialProps = SRC_DIR.resolve(CONFIG_FILE);
+        Path loggingProps = USER_DIR.resolve(CONFIG_FILE);



> +    public static final Path SRC_DIR =
> +            Paths.get(System.getProperty("test.src", "src"));
> +    public static final Path USER_DIR =
> +            Paths.get(System.getProperty("user.dir", "."));
> +    public static final Path CONFIG_FILE = Paths.get("logging.properties"
> );
>
>
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8191033: Regression in logging.properties: specifying .handlers= for root logger (instead of handlers=) no longer works

Martin Buchholz-3
In reply to this post by Daniel Fuchs
I'm not a logging expert, but this change Looks Good To Me.

---

A bunch of "handler" should be changed to "handlers"

+        // Verify that exactly one of the two handler is a custom.Handler
+        // Verify that exactly one of the two handler is a
custom.DotHandler
+        // Verify that the two handler have an id of '1'

--

This code makes me think you use the identity of the Longs, but it seems
you don't, so I would just get rid of these and use e.g. 3L instead of
THREE.

+    public static final Long ONE = 1L;
+    public static final Long TWO = 2L;
+    public static final Long THREE = 3L;
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8191033: Regression in logging.properties: specifying .handlers= for root logger (instead of handlers=) no longer works

Mandy Chung
In reply to this post by Daniel Fuchs


On 12/7/17 6:38 AM, Daniel Fuchs wrote:

> Hi,
>
> Please find below a patch for:
>
> 8191033 Regression in logging.properties: specifying .handlers= for
>         root logger (instead of handlers=) no longer works
> https://bugs.openjdk.java.net/browse/JDK-8191033
>
> webrev:
> http://cr.openjdk.java.net/~dfuchs/webrev_8191033/webrev.00/

Looks good to me.

Mandy
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8191033: Regression in logging.properties: specifying .handlers= for root logger (instead of handlers=) no longer works

Daniel Fuchs
In reply to this post by Martin Buchholz-3
On 07/12/2017 19:15, Martin Buchholz wrote:
> I'm not a logging expert, but this change Looks Good To Me.

Thanks Martin!


>  > configure => configured
> +                        // For backward compatibility: add any handlers configure using
>

Fixed. Thanks for spotting that!

> ---
>
> A bunch of "handler" should be changed to "handlers"

Oh :-( I am ashamed. I should have proof read it. Fixed.


> +        // Verify that exactly one of the two handler is a custom.Handler
> +        // Verify that exactly one of the two handler is a
> custom.DotHandler
> +        // Verify that the two handler have an id of '1'
>
> --
>
> This code makes me think you use the identity of the Longs, but it seems
> you don't, so I would just get rid of these and use e.g. 3L instead of
> THREE.

Done.

>
> +    public static final Long ONE = 1L;
> +    public static final Long TWO = 2L;
> +    public static final Long THREE = 3L;
>

I will push shortly - but for the record I have copied the final
webrev here:
http://cr.openjdk.java.net/~dfuchs/webrev_8191033/webrev.01

best regards,

-- daniel
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8191033: Regression in logging.properties: specifying .handlers= for root logger (instead of handlers=) no longer works

Daniel Fuchs
In reply to this post by Mandy Chung
Thanks Mandy!

-- daniel

On 07/12/2017 23:59, mandy chung wrote:

>
>
> On 12/7/17 6:38 AM, Daniel Fuchs wrote:
>> Hi,
>>
>> Please find below a patch for:
>>
>> 8191033 Regression in logging.properties: specifying .handlers= for
>>         root logger (instead of handlers=) no longer works
>> https://bugs.openjdk.java.net/browse/JDK-8191033
>>
>> webrev:
>> http://cr.openjdk.java.net/~dfuchs/webrev_8191033/webrev.00/
>
> Looks good to me.
>
> Mandy

Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8191033: Regression in logging.properties: specifying .handlers= for root logger (instead of handlers=) no longer works

Jeremy Manson-4
Thanks for looking at this, Daniel!

Jeremy

On Fri, Dec 8, 2017 at 2:30 AM, Daniel Fuchs <[hidden email]>
wrote:

> Thanks Mandy!
>
> -- daniel
>
>
> On 07/12/2017 23:59, mandy chung wrote:
>
>>
>>
>> On 12/7/17 6:38 AM, Daniel Fuchs wrote:
>>
>>> Hi,
>>>
>>> Please find below a patch for:
>>>
>>> 8191033 Regression in logging.properties: specifying .handlers= for
>>>         root logger (instead of handlers=) no longer works
>>> https://bugs.openjdk.java.net/browse/JDK-8191033
>>>
>>> webrev:
>>> http://cr.openjdk.java.net/~dfuchs/webrev_8191033/webrev.00/
>>>
>>
>> Looks good to me.
>>
>> Mandy
>>
>
>