Regression in getImplementationVersion from Jigsaw

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

Regression in getImplementationVersion from Jigsaw

Michael D
Hi All,

Hope I'm in the right place.
The implementation of Jigsaw caused a regression in that
Package.getImplementationVersion is unusable if no other version
things are defined in the manifest.
This appears to be an unintentional regression caused by a missing null check.
I have prepared a patch and unit test for review here:
https://patch-diff.githubusercontent.com/raw/md-5/OpenJDK/pull/3.patch
Note that only testImplVersion fails, but I have included tests for
all the version elements for completeness.

NB: Not an OpenJDK dev so can't create webrev or bug report.

Thanks
Michael
Reply | Threaded
Open this post in threaded view
|

Re: Regression in getImplementationVersion from Jigsaw

Mandy Chung
Hi Michael,

It is indeed a bug and I file:
    https://bugs.openjdk.java.net/browse/JDK-8190987

The patch looks good and I will sponsor this patch for you.

Thanks for the regression test.  It'd be good to wrap the long lines and
I will clean that up before I push.

thanks
Mandy

On 11/8/17 5:07 PM, Michael D wrote:

> Hi All,
>
> Hope I'm in the right place.
> The implementation of Jigsaw caused a regression in that
> Package.getImplementationVersion is unusable if no other version
> things are defined in the manifest.
> This appears to be an unintentional regression caused by a missing null check.
> I have prepared a patch and unit test for review here:
> https://patch-diff.githubusercontent.com/raw/md-5/OpenJDK/pull/3.patch
> Note that only testImplVersion fails, but I have included tests for
> all the version elements for completeness.
>
> NB: Not an OpenJDK dev so can't create webrev or bug report.
>
> Thanks
> Michael

Reply | Threaded
Open this post in threaded view
|

Re: Regression in getImplementationVersion from Jigsaw

Michael D
Hi Mandy,

No problem. Wasn't sure where the limit was on line lengths, so I will
watch and learn :)

Thanks
Michael

On 9 November 2017 at 13:22, mandy chung <[hidden email]> wrote:

> Hi Michael,
>
> It is indeed a bug and I file:
>    https://bugs.openjdk.java.net/browse/JDK-8190987
>
> The patch looks good and I will sponsor this patch for you.
>
> Thanks for the regression test.  It'd be good to wrap the long lines and I
> will clean that up before I push.
>
> thanks
> Mandy
>
> On 11/8/17 5:07 PM, Michael D wrote:
>>
>> Hi All,
>>
>> Hope I'm in the right place.
>> The implementation of Jigsaw caused a regression in that
>> Package.getImplementationVersion is unusable if no other version
>> things are defined in the manifest.
>> This appears to be an unintentional regression caused by a missing null
>> check.
>> I have prepared a patch and unit test for review here:
>> https://patch-diff.githubusercontent.com/raw/md-5/OpenJDK/pull/3.patch
>> Note that only testImplVersion fails, but I have included tests for
>> all the version elements for completeness.
>>
>> NB: Not an OpenJDK dev so can't create webrev or bug report.
>>
>> Thanks
>> Michael
>
>
Reply | Threaded
Open this post in threaded view
|

Re: Regression in getImplementationVersion from Jigsaw

Mandy Chung
In reply to this post by Mandy Chung
FYI.
http://cr.openjdk.java.net/~mchung/jdk10/webrevs/8190987/webrev.00/index.html

While the line width is not consistent across all source files, we try
to keep 80-column width as the coding convention.

Mandy

On 11/8/17 6:25 PM, Michael D wrote:

> Hi Mandy,
>
> No problem. Wasn't sure where the limit was on line lengths, so I will
> watch and learn :)
>
> Thanks
> Michael
>
> On 9 November 2017 at 13:22, mandy chung <[hidden email]> wrote:
>> Hi Michael,
>>
>> It is indeed a bug and I file:
>>     https://bugs.openjdk.java.net/browse/JDK-8190987
>>
>> The patch looks good and I will sponsor this patch for you.
>>
>> Thanks for the regression test.  It'd be good to wrap the long lines and I
>> will clean that up before I push.
>>
>> thanks
>> Mandy
>>
>> On 11/8/17 5:07 PM, Michael D wrote:
>>> Hi All,
>>>
>>> Hope I'm in the right place.
>>> The implementation of Jigsaw caused a regression in that
>>> Package.getImplementationVersion is unusable if no other version
>>> things are defined in the manifest.
>>> This appears to be an unintentional regression caused by a missing null
>>> check.
>>> I have prepared a patch and unit test for review here:
>>> https://patch-diff.githubusercontent.com/raw/md-5/OpenJDK/pull/3.patch
>>> Note that only testImplVersion fails, but I have included tests for
>>> all the version elements for completeness.
>>>
>>> NB: Not an OpenJDK dev so can't create webrev or bug report.
>>>
>>> Thanks
>>> Michael
>>

Reply | Threaded
Open this post in threaded view
|

Re: Regression in getImplementationVersion from Jigsaw

Michael D
Noted,

Thanks again
Michael

On Thu, 9 Nov 2017 at 14:08, mandy chung <[hidden email]> wrote:

> FYI.
> http://cr.openjdk.java.net/~mchung/jdk10/webrevs/8190987/webrev.00/index.html
>
> While the line width is not consistent across all source files, we try to
> keep 80-column width as the coding convention.
>
>
> Mandy
>
>
> On 11/8/17 6:25 PM, Michael D wrote:
>
> Hi Mandy,
>
> No problem. Wasn't sure where the limit was on line lengths, so I will
> watch and learn :)
>
> Thanks
> Michael
>
> On 9 November 2017 at 13:22, mandy chung <[hidden email]> <[hidden email]> wrote:
>
> Hi Michael,
>
> It is indeed a bug and I file:
>    https://bugs.openjdk.java.net/browse/JDK-8190987
>
> The patch looks good and I will sponsor this patch for you.
>
> Thanks for the regression test.  It'd be good to wrap the long lines and I
> will clean that up before I push.
>
> thanks
> Mandy
>
> On 11/8/17 5:07 PM, Michael D wrote:
>
> Hi All,
>
> Hope I'm in the right place.
> The implementation of Jigsaw caused a regression in that
> Package.getImplementationVersion is unusable if no other version
> things are defined in the manifest.
> This appears to be an unintentional regression caused by a missing null
> check.
> I have prepared a patch and unit test for review here:https://patch-diff.githubusercontent.com/raw/md-5/OpenJDK/pull/3.patch
> Note that only testImplVersion fails, but I have included tests for
> all the version elements for completeness.
>
> NB: Not an OpenJDK dev so can't create webrev or bug report.
>
> Thanks
> Michael
>
>