RFR(S):8251374:jmap -dump should not accept invalid options(Internet mail)
Daniel D. Daugherty
daniel.daugherty at oracle.com
Sat Aug 15 13:57:00 UTC 2020
First, Paul sent his request on the wrong email thread and I didn't
notice until just now that we should have been using this one:
Subject: RFR(L): 8215624: add parallel heap inspection support for
jmap histo(G1)(Internet mail)
Fortunately, the correction was applied to the proper changeset.
The process that Stefan K. proposed and that I executed was this:
$ hg export 5036ca733469 > 8215624.patch
<edit 8215624.patch and replace author phh with author lzang>
# Switch the repo to the parent changeset of 5036ca733469:
$ hg update 7b7be8c2b336
# Import the fixed patch as a sibling changeset:
$ hg import 8215624.patch
# Make sure it looks correct:
$ hg log -r tip
changeset: 60553:b1afb7c82d59
parent: 60550:7b7be8c2b336
user: lzang
date: Thu Aug 13 11:31:37 2020 -0700
summary: 8215624: Add parallel heap iteration for jmap –histo
# Switch the repo back to the main line:
$ hg update
# Merge the two branches:
$ hg merge
# Commit the "Merge":
$ hg commit
<with "Merge" as the comment>
# Push the repair:
$ hg push
The result of this repair is almost identical to the same patch being
backported to JDK-(N-1) after being pushed first to JDK-N. When JDK-(N-1)
is sync'ed into JDK-N, we'll have two (almost identical) changesets in
JDK-N for the same bug ID.
In the usual backport scenario, the only difference is the changeset ID.
In this case, the only differences are the changeset ID and the author.
So no [BACKOUT] was involved at all and the "Merge" changeset is exactly
that: a merge.
Paul added a link to the corrected changeset two days ago. I added a
comment with a handcrafted "notification" comment this morning.
Dan
On 8/15/20 1:07 AM, David Holmes wrote:
> So am I right in thinking that what you did was forge a Merge
> changeset that actually did a backout, and then recreated the
> changeset correctly and pushed under the same bug number?
>
> I don't think that is a process we want to endorse. A "Merge"
> changeset should be exactly that.
>
> Also the bug needs updating with a link to the second changeset.
>
> David
> -----
>
> On 14/08/2020 5:52 am, Daniel D. Daugherty wrote:
>> s/like a change/like a charm/
>>
>> Typing too fast today...
>>
>> Dan
>>
>>
>> On 8/13/20 3:51 PM, Daniel D. Daugherty wrote:
>>> Stefan K's idea worked like a change. A corrected changeset has
>>> been created, merged and pushed.
>>>
>>> Dan
>>>
>>>
>>> On 8/13/20 3:34 PM, Daniel D. Daugherty wrote:
>>>> Paul,
>>>>
>>>> Hold up on trying to fix this.
>>>>
>>>> I'm discussing another idea with Stefan K.
>>>>
>>>> Dan
>>>>
>>>>
>>>> On 8/13/20 3:01 PM, Daniel D. Daugherty wrote:
>>>>> That's something that's very hard to do. It would involve black
>>>>> listing
>>>>> the existing changeset and repushing a new changeset. Black listing a
>>>>> changeset is very, very rarely done and in the past Ops has
>>>>> declined to
>>>>> do that for something like an authorship error.
>>>>>
>>>>> Two options:
>>>>>
>>>>> 1) Manually remember that this changeset should be credited to Lin
>>>>> as author.
>>>>> 2a) [BACKOUT] the changeset using a new bug ID.
>>>>> 2b) [REDO] the changeset with corrected author information with a
>>>>> new bug ID.
>>>>>
>>>>> Dan
>>>>>
>>>>> On 8/13/20 2:36 PM, Hohensee, Paul wrote:
>>>>>> I mistakenly committed and pushed Lin's patch with myself as
>>>>>> author. Would someone with repo access please change the author
>>>>>> to 'lzang'? Or tell me how to do it myself?
>>>>>>
>>>>>> https://hg.openjdk.java.net/jdk/jdk/rev/5036ca733469
>>>>>>
>>>>>> Thanks,
>>>>>> Paul
>>>>>>
>>>>>> On 8/13/20, 9:48 AM, "serviceability-dev on behalf of Hohensee,
>>>>>> Paul" <serviceability-dev-retn at openjdk.java.net on behalf of
>>>>>> hohensee at amazon.com> wrote:
>>>>>>
>>>>>> Will do.
>>>>>>
>>>>>> On 8/13/20, 7:08 AM, "linzang(臧琳)" <linzang at tencent.com>
>>>>>> wrote:
>>>>>>
>>>>>> Thanks Paul！
>>>>>> May I ask your help to push it?
>>>>>>
>>>>>> BRs,
>>>>>> Lin
>>>>>>
>>>>>> > On Aug 13, 2020, at 10:06 PM, Hohensee, Paul
>>>>>> <hohensee at amazon.com> wrote:
>>>>>> >
>>>>>> > +1, except that the indentation for the final 'else'
>>>>>> clause needs to be 4 spaces instead of 3. :)
>>>>>> >
>>>>>> > Thanks,
>>>>>> > Paul
>>>>>> >
>>>>>> > On 8/12/20, 6:21 PM, "serguei.spitsyn at oracle.com"
>>>>>> <serguei.spitsyn at oracle.com> wrote:
>>>>>> >
>>>>>> > Hi Lin.
>>>>>> >
>>>>>> > Thank you for the update.
>>>>>> > It looks good.
>>>>>> >
>>>>>> > Thanks,
>>>>>> > Serguei
>>>>>> >
>>>>>> >
>>>>>> >> On 8/12/20 17:08, linzang(臧琳) wrote:
>>>>>> >> Hi Paul and Serguei,
>>>>>> >> Thanks for your comments, here is the updated
>>>>>> patch: http://cr.openjdk.java.net/~lzang/8251374/webrev02/
>>>>>> >>
>>>>>> >> BRs,
>>>>>> >> Lin
>>>>>> >>
>>>>>> >> On 2020/8/13, 12:55 AM, "serguei.spitsyn at oracle.com"
>>>>>> <serguei.spitsyn at oracle.com> wrote:
>>>>>> >>
>>>>>> >> Hi Lin,
>>>>>> >>
>>>>>> >> It looks good.
>>>>>> >> Just one comment.
>>>>>> >>
>>>>>> >> + System.err.println("Fail: invalid option:
>>>>>> '" + subopt +"'");
>>>>>> >> + System.exit(1);
>>>>>> >>
>>>>>> >> Exit needs to be replaced wit usage for consistency.
>>>>>> >>
>>>>>> >> Thanks,
>>>>>> >> Serguei
>>>>>> >>
>>>>>> >>
>>>>>> >>> On 8/10/20 19:57, linzang(臧琳) wrote:
>>>>>> >>> Here is the webrev:
>>>>>> http://cr.openjdk.java.net/~lzang/8251374/webrev01/
>>>>>> >>>
>>>>>> >>> BRs,
>>>>>> >>> Lin
>>>>>> >>>
>>>>>> >>>> On 2020/8/11, 10:52 AM, "linzang(臧琳)"
>>>>>> <linzang at tencent.com> wrote:
>>>>>> >>>
>>>>>> >>> Hi All,
>>>>>> >>> May I ask your help to review this tiny
>>>>>> patch? It fix an issue that jmap -dump could wrongly accept
>>>>>> invalid optioins.
>>>>>> >>> Bugs:
>>>>>> https://bugs.openjdk.java.net/browse/JDK-8251374
>>>>>> >>> Patch: (Can not connect to webrev ftp
>>>>>> currently, will try it later, following are all code changes)
>>>>>> >>>
>>>>>> >>> ################################
>>>>>> >>> ---
>>>>>> old/src/jdk.jcmd/share/classes/sun/tools/jmap/JMap.java
>>>>>> 2020-08-11 10:42:32.044567791 +0800
>>>>>> >>> +++
>>>>>> new/src/jdk.jcmd/share/classes/sun/tools/jmap/JMap.java
>>>>>> 2020-08-11 10:42:31.876568681 +0800
>>>>>> >>> @@ -207,6 +207,11 @@
>>>>>> >>> liveopt = "-live";
>>>>>> >>> } else if
>>>>>> (subopt.startsWith("file=")) {
>>>>>> >>> filename = parseFileName(subopt);
>>>>>> >>> + } else if
>>>>>> (subopt.equals("format=b")) {
>>>>>> >>> + // ignore format (not needed at
>>>>>> this time)
>>>>>> >>> + } else {
>>>>>> >>> + System.err.println("Fail: invalid option: '" +
>>>>>> subopt +"'");
>>>>>> >>> + System.exit(1);
>>>>>> >>> }
>>>>>> >>> }
>>>>>> >>> ################################
>>>>>> >>>
>>>>>> >>> Thanks,
>>>>>> >>> Lin
>>>>>> >>>
>>>>>> >>>
>>>>>> >>
>>>>>> >>
>>>>>> >>
>>>>>> >
>>>>>> >
>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
More information about the serviceability-dev
mailing list