[10] Request for review and approval: JDK-8177770: Need more precise control on build system logging

classic Classic list List threaded Threaded
10 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[10] Request for review and approval: JDK-8177770: Need more precise control on build system logging

alexey semenyuk
Please review and approve enhancement for JDK10.

Bug: https://bugs.openjdk.java.net/browse/JDK-8177770
Webrev: http://cr.openjdk.java.net/~asemenyuk/8177770/webrev.00/

Thanks,
Alexey

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [10] Request for review and approval: JDK-8177770: Need more precise control on build system logging

David Holmes
Hi Alexey,

Can't really review the details of this but wanted to ensure that this
gets tested on all platforms before being pushed. Internally use JPRT
and "-testset hotspot", but you should also reach out to the AIX-port
folk to ensure it works as expected there too.

Thanks,
David

On 30/03/2017 2:21 AM, alexey semenyuk wrote:
> Please review and approve enhancement for JDK10.
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8177770
> Webrev: http://cr.openjdk.java.net/~asemenyuk/8177770/webrev.00/
>
> Thanks,
> Alexey
>
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [10] Request for review and approval: JDK-8177770: Need more precise control on build system logging

Erik Joelsson
In reply to this post by alexey semenyuk
Looks good Alexey, thanks for doing this!

/Erik


On 2017-03-29 18:21, alexey semenyuk wrote:
> Please review and approve enhancement for JDK10.
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8177770
> Webrev: http://cr.openjdk.java.net/~asemenyuk/8177770/webrev.00/
>
> Thanks,
> Alexey
>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [10] Request for review and approval: JDK-8177770: Need more precise control on build system logging

Magnus Ihse Bursie
In reply to this post by alexey semenyuk
On 2017-03-29 18:21, alexey semenyuk wrote:
> Please review and approve enhancement for JDK10.
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8177770
> Webrev: http://cr.openjdk.java.net/~asemenyuk/8177770/webrev.00/
>
> Thanks,
> Alexey
>

Looks great! :) Thank you for this cleanup/addition to the debuggability
of the makefiles!

/Magnus
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [10] Request for review and approval: JDK-8177770: Need more precise control on build system logging

Magnus Ihse Bursie
In reply to this post by David Holmes
On 2017-03-30 03:04, David Holmes wrote:
> Hi Alexey,
>
> Can't really review the details of this but wanted to ensure that this
> gets tested on all platforms before being pushed. Internally use JPRT
> and "-testset hotspot", but you should also reach out to the AIX-port
> folk to ensure it works as expected there too.

David,

What are your worries here? There are no platform-dependent changes,
apart from the actual logging script which might not work properly if
your local copy of time/flock is non-compliant. (We try to test for
this, at least for "time".) Unless you're running with LOG=profile, this
patch affects nothing.

If the profiling does not work properly on e.g. AIX, then some of the
AIX folks would need to supply an additional patch to update
shell-profiler.sh.

/Magnus

>
> Thanks,
> David
>
> On 30/03/2017 2:21 AM, alexey semenyuk wrote:
>> Please review and approve enhancement for JDK10.
>>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8177770
>> Webrev: http://cr.openjdk.java.net/~asemenyuk/8177770/webrev.00/
>>
>> Thanks,
>> Alexey
>>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [10] Request for review and approval: JDK-8177770: Need more precise control on build system logging

David Holmes
On 30/03/2017 5:10 PM, Magnus Ihse Bursie wrote:

> On 2017-03-30 03:04, David Holmes wrote:
>> Hi Alexey,
>>
>> Can't really review the details of this but wanted to ensure that this
>> gets tested on all platforms before being pushed. Internally use JPRT
>> and "-testset hotspot", but you should also reach out to the AIX-port
>> folk to ensure it works as expected there too.
>
> David,
>
> What are your worries here? There are no platform-dependent changes,
> apart from the actual logging script which might not work properly if
> your local copy of time/flock is non-compliant. (We try to test for
> this, at least for "time".) Unless you're running with LOG=profile, this
> patch affects nothing.

Just want to make sure it doesn't cause any unexpected failures on less
mainstream OS.

> If the profiling does not work properly on e.g. AIX, then some of the
> AIX folks would need to supply an additional patch to update
> shell-profiler.sh.

Sure. But it would be nice to not break things first. Some contributors
continually pull OpenJDK changes and run them through their own build
and test systems.

Thanks,
David

> /Magnus
>>
>> Thanks,
>> David
>>
>> On 30/03/2017 2:21 AM, alexey semenyuk wrote:
>>> Please review and approve enhancement for JDK10.
>>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8177770
>>> Webrev: http://cr.openjdk.java.net/~asemenyuk/8177770/webrev.00/
>>>
>>> Thanks,
>>> Alexey
>>>
>
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [10] Request for review and approval: JDK-8177770: Need more precise control on build system logging

Magnus Ihse Bursie
On 2017-03-30 09:13, David Holmes wrote:

> On 30/03/2017 5:10 PM, Magnus Ihse Bursie wrote:
>> On 2017-03-30 03:04, David Holmes wrote:
>>> Hi Alexey,
>>>
>>> Can't really review the details of this but wanted to ensure that this
>>> gets tested on all platforms before being pushed. Internally use JPRT
>>> and "-testset hotspot", but you should also reach out to the AIX-port
>>> folk to ensure it works as expected there too.
>>
>> David,
>>
>> What are your worries here? There are no platform-dependent changes,
>> apart from the actual logging script which might not work properly if
>> your local copy of time/flock is non-compliant. (We try to test for
>> this, at least for "time".) Unless you're running with LOG=profile, this
>> patch affects nothing.
>
> Just want to make sure it doesn't cause any unexpected failures on
> less mainstream OS.
>
>> If the profiling does not work properly on e.g. AIX, then some of the
>> AIX folks would need to supply an additional patch to update
>> shell-profiler.sh.
>
> Sure. But it would be nice to not break things first. Some
> contributors continually pull OpenJDK changes and run them through
> their own build and test systems.

Actually, I seriously doubt many people outside me and Erik have used
the LOG=trace (now LOG=profile) option. It is highly intrusive, and not
something that can be enabled on normal build systems.

/Magnus

>
> Thanks,
> David
>
>> /Magnus
>>>
>>> Thanks,
>>> David
>>>
>>> On 30/03/2017 2:21 AM, alexey semenyuk wrote:
>>>> Please review and approve enhancement for JDK10.
>>>>
>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8177770
>>>> Webrev: http://cr.openjdk.java.net/~asemenyuk/8177770/webrev.00/
>>>>
>>>> Thanks,
>>>> Alexey
>>>>
>>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [10] Request for review and approval: JDK-8177770: Need more precise control on build system logging

alexey semenyuk
In reply to this post by David Holmes
David,

I did quite a lot JPRT builds wit various values of LOG Make variable to
verify the changes are not breaking windows, linux, solaris and OSX builds.

Thanks,
Alexey

On 30.03.2017 10:13, David Holmes wrote:

> On 30/03/2017 5:10 PM, Magnus Ihse Bursie wrote:
>> On 2017-03-30 03:04, David Holmes wrote:
>>> Hi Alexey,
>>>
>>> Can't really review the details of this but wanted to ensure that this
>>> gets tested on all platforms before being pushed. Internally use JPRT
>>> and "-testset hotspot", but you should also reach out to the AIX-port
>>> folk to ensure it works as expected there too.
>>
>> David,
>>
>> What are your worries here? There are no platform-dependent changes,
>> apart from the actual logging script which might not work properly if
>> your local copy of time/flock is non-compliant. (We try to test for
>> this, at least for "time".) Unless you're running with LOG=profile, this
>> patch affects nothing.
>
> Just want to make sure it doesn't cause any unexpected failures on
> less mainstream OS.
>
>> If the profiling does not work properly on e.g. AIX, then some of the
>> AIX folks would need to supply an additional patch to update
>> shell-profiler.sh.
>
> Sure. But it would be nice to not break things first. Some
> contributors continually pull OpenJDK changes and run them through
> their own build and test systems.
>
> Thanks,
> David
>
>> /Magnus
>>>
>>> Thanks,
>>> David
>>>
>>> On 30/03/2017 2:21 AM, alexey semenyuk wrote:
>>>> Please review and approve enhancement for JDK10.
>>>>
>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8177770
>>>> Webrev: http://cr.openjdk.java.net/~asemenyuk/8177770/webrev.00/
>>>>
>>>> Thanks,
>>>> Alexey
>>>>
>>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [10] Request for review and approval: JDK-8177770: Need more precise control on build system logging

David Holmes
On 30/03/2017 8:00 PM, alexey semenyuk wrote:
> David,
>
> I did quite a lot JPRT builds wit various values of LOG Make variable to
> verify the changes are not breaking windows, linux, solaris and OSX builds.

Ok.

Thanks,
David

> Thanks,
> Alexey
>
> On 30.03.2017 10:13, David Holmes wrote:
>> On 30/03/2017 5:10 PM, Magnus Ihse Bursie wrote:
>>> On 2017-03-30 03:04, David Holmes wrote:
>>>> Hi Alexey,
>>>>
>>>> Can't really review the details of this but wanted to ensure that this
>>>> gets tested on all platforms before being pushed. Internally use JPRT
>>>> and "-testset hotspot", but you should also reach out to the AIX-port
>>>> folk to ensure it works as expected there too.
>>>
>>> David,
>>>
>>> What are your worries here? There are no platform-dependent changes,
>>> apart from the actual logging script which might not work properly if
>>> your local copy of time/flock is non-compliant. (We try to test for
>>> this, at least for "time".) Unless you're running with LOG=profile, this
>>> patch affects nothing.
>>
>> Just want to make sure it doesn't cause any unexpected failures on
>> less mainstream OS.
>>
>>> If the profiling does not work properly on e.g. AIX, then some of the
>>> AIX folks would need to supply an additional patch to update
>>> shell-profiler.sh.
>>
>> Sure. But it would be nice to not break things first. Some
>> contributors continually pull OpenJDK changes and run them through
>> their own build and test systems.
>>
>> Thanks,
>> David
>>
>>> /Magnus
>>>>
>>>> Thanks,
>>>> David
>>>>
>>>> On 30/03/2017 2:21 AM, alexey semenyuk wrote:
>>>>> Please review and approve enhancement for JDK10.
>>>>>
>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8177770
>>>>> Webrev: http://cr.openjdk.java.net/~asemenyuk/8177770/webrev.00/
>>>>>
>>>>> Thanks,
>>>>> Alexey
>>>>>
>>>
>
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [10] Request for review and approval: JDK-8177770: Need more precise control on build system logging

Magnus Ihse Bursie
In reply to this post by alexey semenyuk
I'll sponsor you and push this for you to jdk10.

/Magnus

On 2017-03-29 18:21, alexey semenyuk wrote:
> Please review and approve enhancement for JDK10.
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8177770
> Webrev: http://cr.openjdk.java.net/~asemenyuk/8177770/webrev.00/
>
> Thanks,
> Alexey
>

Loading...