Review request for OPENJDK6-35: backport of JDK-6650759 to openjdk6

Wed Jun 25 15:49:53 UTC 2014

Hi Andrew,

 From my previous experience I can say that it's common practice
to indicate in commit messages who prepared and integrated
these particular bits and who reviewed them. Here is a couple
of examples where original fixes and backports are nearly identical,
but their commit messages refer to different persons:

Original fix: http://hg.openjdk.java.net/jdk8u/jdk8u-dev/jdk/rev/3502753a9d66
Backport: http://hg.openjdk.java.net/jdk7u/jdk7u-dev/jdk/rev/589c21e1aa30
Original fix: http://hg.openjdk.java.net/jdk8u/jdk8u-dev/jdk/rev/610da7dcd1be
Backport: http://hg.openjdk.java.net/jdk7u/jdk7u-dev/jdk/rev/26ba36d4400c

We treated this data rather in terms of responsibility - who is
responsible for preparing/verification/approving this particular
backport. And I agree with you that we should have used the original
OpenJDK bug ID in the commit messages, to be in line with another
'rule' in OpenJDK - to use the 'main' bug ID in comments for all
backports of the 'main' fix.

Surely, we are open for your suggestions on comments format - let's
find the version that makes everybody happy. :)

Regarding 6638712 and 6650759 - the backports were prepared from
scratch, taking
http://hg.openjdk.java.net/jdk7/jdk7/langtools/rev/22872b24d38c
http://hg.openjdk.java.net/jdk7/jdk7/langtools/rev/dda7e13f09fb
as the base. I discovered the fact that they are ported to IcedTea
during the investigation whether the compilation problem we faced
reproduces with other OpenJDK-based products.

Thanks,
Nikolay

On 25.06.2014 17:58, Andrew Hughes wrote:
>
>
> ----- Original Message -----
>>> As this is a backport, there's no reason to allocate an OPENJDK6 bug
>> to it.
>> Will openjdk6 jcheck pass my changes with the 7-digit openjdk bug id? I
>> wasn't aware.
>
> It allows either. :)
>
>>
>>   >The original information could have been used, perhaps with the
>> additional reviewer.
>>
>> I am fine with any format. If you can point me to a web/wiki page with
>> proper guidelines - I would be happy to follow those as well.
>>
>>   >Now it looks like Azul wrote this patch, which I don't believe is the
>> case.
>>
>> Unlike the previous change (VS2010 support) here I did not update
>> copyrights on files exactly because I do not see us as true authors for
>> this particular patch. So it was certainly not my intention to make this
>> patch look like it was authored at Azul. However I did want to give the
>> guys credit for doing proper testing of the produced bits as part of the
>> backport effort.
>>
>
> I've tended to just retain the original information e.g.:
>
> changeset:   893:7aa071f95dac
> user:        prr
> date:        Wed Apr 30 13:10:39 2008 -0700
> files:       make/sun/font/FILES_c.gmk make/sun/font/Makefile src/share/classes/sun/font/FileFontStrike.java src/share/classes/sun/font/FontManager.java src/share/classes/sun/font/TrueTypeFont.java src/windows/classes/sun/awt/Win32GraphicsEnvironment.java src/windows/native/sun/font/lcdglyph.c test/java/awt/Graphics2D/DrawString/ScaledLCDTextMetrics.java
> description:
> 6656651: Windows Look and Feel LCD glyph images have some differences from native applications.
> Reviewed-by: igor, tdv
>
> but, as you say, that doesn't give credit for the backporting work. On the other hand, if I'd
> gone to the other extreme, over 90% of the OpenJDK 6 changesets would be credited to me which
> would look very odd!
>
> The only guidelines I'm aware of are:
>
> http://openjdk.java.net/guide/producingChangeset.html
>
> which don't cover this situation. Maybe it's worth raising on the OpenJDK discussion list?
>
> Did you backport these changes from scratch? I was a little confused, because
> the reference to the IcedTea backport patches made me think you might have used those.
>
>> We will respond to the other mail regardimg Deepak's finding in a
>> separate email.
>>
>> Thanks,
>>
>> Ivan
>>
>>> *From:* Andrew Hughes <mailto:gnu.andrew at redhat.com>
>>> *Sent:* ‎Tuesday‎, ‎June‎ ‎24‎, ‎2014 ‎22‎:‎30
>>> *To:* Ivan Krylov <mailto:ivan at azulsystems.com>
>>> *Cc:* jdk6-dev at openjdk.java.net <mailto:jdk6-dev at openjdk.java.net>
>>>
>>>
>>>
>>> ----- Original Message -----
>>>> Andrew, thank you.
>>>> Just pushed the changes.
>>>> BTW, icedtea patches 6650759-missing_inference.patch and
>>>> 6638712-wildcard_types.patch are now redundant.
>>>>
>>>
>>> In future, if you're backporting changes, can you please keep the original
>>> author and summary information?
>>>
>>> From:
>>>
>>> user:        mcimadamore
>>> 6638712: Inference with wildcard types causes selection of
>>> inapplicable method
>>> Summary: Added global sanity check in order to make sure that return
>>> type inference does not violate bounds constraints
>>> Reviewed-by: jjg
>>>
>>> to:
>>>
>>> user:        ikrylov
>>> OPENJDK6-34: OpenJDK6-b31 backport of JDK-6638712 to openjdk6
>>> Summary: Original bug synopsis-Inference of formal type parameter
>>> (unused in formal parameters) is not performed
>>> Reviewed-by: aph
>>> Contributed-by: nikgor <nikolay at azulsystems.com>
>>>
>>> As this is a backport, there's no reason to allocate an OPENJDK6 bug
>>> to it.
>>> The original information could have been used, perhaps with the
>>> additional reviewer.
>>> Now it looks like Azul wrote this patch, which I don't believe is the
>>> case.
>>>
>>>> Also, the following bugs may be closed OPENJDK6-32,33,34,35. I would
>>>> close those myself but I do not have the required permissions.
>>>
>>> Done.
>>>
>>>>
>>>> Thanks,
>>>> Ivan
>>>>
>>>> On 23/06/2014 17:09, Andrew Haley wrote:
>>>>> On 06/23/2014 02:05 PM, Ivan Krylov wrote:
>>>>>> The main motivation for this fix was exactly to fix building
>>> JBoss EAP
>>>>>> certification bundle (for 6.2.0).
>>>>>> This fix exists in IcedTea (6650759-missing_inference.patch) but for
>>>>>> whatever reason was never promoted to openjdk6.
>>>>> Ah, OK.  That's fine, then.
>>>>>
>>>>> Thanks,
>>>>> Andrew.
>>>>>
>>>>>
>>>>
>>>>
>>>
>>> --
>>> Andrew :)
>>>
>>> Free Java Software Engineer
>>> Red Hat, Inc. (http://www.redhat.com)
>>>
>>> PGP Key: 248BDC07 (https://keys.indymedia.org/)
>>> Fingerprint = EC5A 1F5E C0AD 1D15 8F1F  8F91 3B96 A578 248B DC07
>>>
>>>
>>
>>
>