[7u6] Review request for 7131021: [macosx] Consider using system properties to pass arguments from the launcher to AWT/SplashScreen

[Kumar Srinivasan]

Hi Anthony,

Looks good!, and a big thanks for doing this.

Kumar

----- anthony.petrov at oracle.com wrote:

> From: anthony.petrov at oracle.com
> To: kumar.x.srinivasan at oracle.com, swingler at apple.com
> Cc: macosx-port-dev at openjdk.java.net
> Sent: Wednesday, April 11, 2012 4:54:38 AM GMT -08:00 US/Canada Pacific
> Subject: Re: [7u6] Review request for 7131021: [macosx] Consider using system properties to pass arguments from the
> launcher to AWT/SplashScreen
>
> Hi Kumar and Mike,
> 
> Thanks for the review!
> 
> Initially I wanted to invoke the sub-tests from Java, too, but the 
> Runtime functionality won't let me redirect input/output/error streams
> 
> from a child process, so I ended up with a shell test.
> 
> Given that launcher tests already employ the ProcessBuilder, and thus
> 
> deal with IO streams redirection, I like the idea of using it for this
> 
> test as well. The changes suggested by Kumar look great. Thanks!
> 
> I've also incorporated feedback from Mike and added a line to the 
> comments that explains the necessity for the _<pid> part.
> 
> Here's the updated webrev:
> 
> http://cr.openjdk.java.net/~anthony/7u6-2-keepEnvVars-7131021.1/
> 
> --
> best regards,
> Anthony
> 
> On 4/11/2012 7:16 AM, Kumar Srinivasan wrote:
> > Hi Anthony,
> > 
> > I personally don't want to see shell based test, I spent
> > several hours cleaning up the shell tests in the launcher area
> > for jdk8, there are few left, I will get to them sooner
> > or later. I am assuming this will be forward ported to jdk8.
> > 
> > Having said that, I have done the following, which simplifies
> > a great deal.
> > 
> > 1. Removed the shell script
> > 2. Piggy back the test control onto TestSpecialArgs.java
> >    which was introduced for MacOSX, anyway.
> > 3. Java invoking java exposed an unique problem where
> >    we have multiple env variables
> > ex:
> > JAVA_MAIN_CLASS_ABC=com.sun.javatest.regtest.Main
> > JAVA_MAIN_CLASS_XYZ=EnvironmentVariables
> > 
> >    To fix the above, TestHelper needed some adjustments.
> > 
> > 
> > It was simpler to show you the changes in a webrev. :)
> > Full webrev is here:
> > http://cr.openjdk.java.net/~ksrini/7131021/webrev/
> > 
> > The incremental changes wrt. your changes are here:
> >
> http://cr.openjdk.java.net/~ksrini/7131021/webrev/webrev.delta/index.html
> > 
> > I have tested the above changes on Mac and Solaris.
> > 
> > Thanks
> > Kumar
> > 
> > ----- anthony.petrov at oracle.com wrote:
> > 
> >> From: anthony.petrov at oracle.com
> >> To: macosx-port-dev at openjdk.java.net,
> kumar.x.srinivasan at oracle.COM
> >> Sent: Tuesday, April 10, 2012 10:23:19 AM GMT -08:00 US/Canada
> Pacific
> >> Subject: [7u6] Review request for 7131021: [macosx] Consider using
> system properties to pass arguments from the
> >> launcher to AWT/SplashScreen
> >>
> >> Hello,
> >>
> >> Please review a fix for
> http://bugs.sun.com/view_bug.do?bug_id=7131021
> >> at:
> >>
> >> http://cr.openjdk.java.net/~anthony/7u6-2-keepEnvVars-7131021.0/
> >>
> >> The comments added to java_md_macosx.c have been approved by CCC 
> >> already. This is mainly a review for the newly added test that
> checks
> >>
> >> for the environment variables that can be set by the launcher on
> the
> >> Mac.
> >>
> >> A short background to avoid any confusion regarding the synopsis
> of
> >> the 
> >> bug. We've discussed the issue thoroughly and come to a conclusion
> >> that 
> >> we want to preserve the existing behavior, i.e. use the environment
> 
> >> variables to pass some information from the launcher to AWT. With
> this
> >>
> >> fix we add comments to the source code to make sure maintainers of
> the
> >>
> >> code are aware of compatibility issues related to the variables,
> >> should 
> >> they need to modify this code. Also, we're adding an automatic 
> >> regression test that verifies that the launcher correctly sets
> these 
> >> variables.
> >>
> >> --
> >> best regards,
> >> Anthony