PING: RFR: 8209790: SA tools not providing option to connect to debug server

[Chris Plummer]

Sorry, I meant "jhsdb debugd", not "clhsdb debugd".

Chris

On 7/8/19 12:09 AM, Chris Plummer wrote:
> Hi Yasumasa,
>
> I just noticed one more thing:
>
>   65             System.out.println("    --connect <[id@]host>   To 
> connect to a remote debug server (sadebugd).");
>
> Earlier on I incorrectly said that jstack and other tools had a 
> references to "sadebugd" in the help output. It is actually 
> "jsadebugd". But that was the name of the command line tool, which is 
> now gone. Instead now we have "clhsdb debugd ...", so maybe it should 
> just mention "debugd".
>
> thanks,
>
> Chris
>
> On 7/7/19 10:24 PM, Chris Plummer wrote:
>> Hi Yasumasa,
>>
>> I think it should be "[<id>@]<host>". Angle brackets are used to 
>> indicate non-literal values (ones to be replaced by something the 
>> user chooses), but the '@' is a literal value, so should not be 
>> enclosed in the angle brackets.
>>
>> Also one minor thing. You ended up duplicating the //:
>>
>>   60         // // --connect <[id@]host>
>>
>> thanks,
>>
>> Chris
>>
>> On 7/6/19 5:55 PM, Yasumasa Suenaga wrote:
>>> Hi Chris,
>>>
>>> Thank you for your advise.
>>> I uploaded new webrev. How about this?
>>>
>>>   http://cr.openjdk.java.net/~ysuenaga/JDK-8209790/webrev.04/
>>>
>>> I replaced "server" to "host" as you said.
>>> Other comments from you also have been fixed in it.
>>>
>>> For example, `jhsdb jstack --help` shows as below:
>>>
>>> ```
>>> $ jhsdb jstack --help
>>>     --locks                 To print java.util.concurrent locks.
>>>     --mixed                 To print both Java and native frames 
>>> (mixed mode).
>>>     --pid <pid>             To attach to and operate on the given 
>>> live process.
>>>     --core <corefile>       To operate on the given core file.
>>>     --exe <executable for corefile>
>>>     --connect <[id@]host>   To connect to a remote debug server 
>>> (sadebugd).
>>>
>>>     The --core and --exe options must be set together to give the core
>>>     file, and associated executable, to operate on. They can use
>>>     absolute or relative paths.
>>>     The --pid option can be set to operate on a live process.
>>>     The --connect option can be set to connect to a debug server 
>>> (sadebugd).
>>>     <--core --exe> and <--pid> and <--connect> are exclusive.
>>>
>>>     Examples: jhsdb jstack --pid 1234
>>>           or  jhsdb jstack --core ./core.1234 --exe ./myexe
>>>           or  jhsdb jstack --connect debugserver
>>>           or  jhsdb jstack --connect id at debugserver
>>> ```
>>>
>>> If you are ok, I will reopen CSR (JDK-8224979).
>>>
>>>
>>> Yasumasa
>>>
>>>
>>> On 2019/07/06 6:26, Chris Plummer wrote:
>>>> On 7/4/19 3:53 PM, Yasumasa Suenaga wrote:
>>>>> Hi Chris,
>>>>>
>>>>> On 2019/07/05 4:24, Chris Plummer wrote:
>>>>>> Hi Yasumasa,
>>>>>>
>>>>>> On 7/4/19 5:30 AM, Yasumasa Suenaga wrote:
>>>>>>> Hi Chris,
>>>>>>>
>>>>>>> Thank you for your review.
>>>>>>>
>>>>>>> On 2019/07/04 8:07, Chris Plummer wrote:
>>>>>>>> Hi Yasumasa,
>>>>>>>>
>>>>>>>> Overall these changes look good, but I think there is a bit of 
>>>>>>>> cleanup still needed.
>>>>>>>>
>>>>>>>> You've changed the indentation of the help to always have a few 
>>>>>>>> spaces before the /t. If you are going to do this, then perhaps 
>>>>>>>> you don't need the /t anymore.
>>>>>>>
>>>>>>> Ok, I will replace '\t' to whitespace.
>>>>>>>
>>>>>>>
>>>>>>>> There are 3 checks for "remote != null". Shouldn't they be 
>>>>>>>> "remote != NO_REMOTE"?
>>>>>>>
>>>>>>> I will fix them.
>>>>>>>
>>>>>>>
>>>>>>>> The old jstack help output is a bit more informative than your 
>>>>>>>> output:
>>>>>>>>
>>>>>>>>      jstack [-m] [-l] [server_id@]<remote server IP or hostname>
>>>>>>>>          (to connect to a remote debug server)
>>>>>>>>
>>>>>>>> vs
>>>>>>>>
>>>>>>>>      --connect <[id@]server> To operate on the remote debug 
>>>>>>>> server.
>>>>>>>>
>>>>>>>> More specifically, you replaced <remote server IP or hostname> 
>>>>>>>> with "server". I think the former is a bit more self explanatory.
>>>>>>>
>>>>>>> Exactly, but it is too long to show on the console.
>>>>>>> So I used short word: server.
>>>>>>>
>>>>>>> If we can ignore long line, or can add newline (\n) on help 
>>>>>>> message,
>>>>>>> I can change it. What do you think?
>>>>>> I don't see why not. Isn't there already help output that relies 
>>>>>> on newlines?
>>>>>
>>>>> As you can see on the CSR (JDK-8224979), format of jhsdb help 
>>>>> messages is:
>>>>>
>>>>>   <option name> <blank (includes tab)> <description>
>>>>>
>>>>> Thus I think long option / description is not comfortable.
>>>>> If we need to describe more, I think we should write it as below:
>>>>>
>>>>>   --connect <[server-id@]remote server IP or hostname>
>>>>>       To connect to a remote debug server.
>>>> Hi Yasumasa,
>>>>
>>>> We already have multiline examples like:
>>>>
>>>> |--pid <pid> To attach to and operate on the given live process.
>>>>
>>>> Is your concerned that the long option name will extend into the 
>>>> columns where the description normally is? If so, you can start the 
>>>> description on a new line as you suggest, or maybe come up with a 
>>>> different name for the option that isn't quite as long. My main 
>>>> objection to just saying "server" is it in no way conveys we are 
>>>> talking about an IP host. Even just saying <host> would be better. 
>>>> It is also more correct since the host isn't necessarily what 
>>>> someone would consider to be a server (it could just be someone's 
>>>> desktop). <server-id> on the other hand is the correct term because 
>>>> we are talking about the id of the debug server running on a 
>>>> particular host.
>>>>
>>>> Chris
>>>> |
>>>>
>>
>>
>
>