JDK-8215544: SA: Modify ClhsdbLauncher to add sudo privileges to enable MacOS tests on Mach5

[Jini George]

Thank you very much, Paul for the review. Yes, this is only needed for OSX.

Thanks,
Jini.

On 1/12/2019 5:41 AM, Hohensee, Paul wrote:
> A more generic approach that doesn’t check for a specific platform in ClhsdbLauncher.java would be nice, e.g., add a checkForPrivileges() method to Platform, but if OSX is the only platform that needs the check, I'm ok with this patch.
> 
> Thanks,
> 
> Paul
> 
> On 1/10/19, 8:16 PM, "serviceability-dev on behalf of Jini George" <serviceability-dev-bounces at openjdk.java.net on behalf of jini.george at oracle.com> wrote:
> 
>      Thanks again, JC! :-)
>      
>      Folks, Could I please get one more review on this ?
>      
>      -Jini.
>      
>      On 1/11/2019 9:38 AM, JC Beyler wrote:
>      > Hi Jini,
>      >
>      > Looks good to me now :) Thanks for handling the comments :)
>      > Jc
>      >
>      >
>      > On Tue, Jan 8, 2019 at 8:11 PM Jini George <jini.george at oracle.com
>      > <mailto:jini.george at oracle.com>> wrote:
>      >
>      >     Thank you very much for the review, JC. My comments inline:
>      >
>      >     On 1/8/2019 12:22 AM, JC Beyler wrote:
>      >      >
>      >      >
>      >     http://cr.openjdk.java.net/~jgeorge/8215544/webrev.03/test/hotspot/jtreg/serviceability/sa/ClhsdbLauncher.java.udiff.html
>      >      >    +                // by appending 'sudo' to it. Check now if sudo
>      >      > passes in this
>      >      >    +                // environment with a simple 'echo' command.
>      >      >       -> Really shouldn't provide implementation details about
>      >     what is
>      >      > being done by that method; if we change that, this comment will
>      >     rot :-)
>      >      > canAddPrivileges is explicit enough in my mind
>      >
>      >     Done. I have removed the comment.
>      >
>      >      >
>      >      >
>      >     http://cr.openjdk.java.net/~jgeorge/8215544/webrev.03/test/lib/jdk/test/lib/SA/SATestUtils.java.html
>      >
>      >      >
>      >      >      -> You put some System.out.println; are they intended to
>      >     still be
>      >      > there or were they for debug?
>      >
>      >     I guess you meant this one:
>      >     System.out.println("Adding 'sudo -E' to the command.");
>      >
>      >     This one is meant to be there.
>      >
>      >      >
>      >      >
>      >      >    77         for (String val: cmdStringList) {
>      >      >    78             outStringList.add(val);
>      >      >    79         }
>      >      >
>      >      >
>      >      > Couldn't we use addAll ?
>      >
>      >     Done.
>      >     The modified webrev is at:
>      >
>      >     http://cr.openjdk.java.net/~jgeorge/8215544/webrev.04/index.html
>      >
>      >     Thanks,
>      >     Jini.
>      >
>      >
>      >
>      >
>      >
>      > --
>      >
>      > Thanks,
>      > Jc
>      
>