RFR (S): 7125793: MAC: test_gamma should always work (original) (raw)

Staffan Larsen staffan.larsen at oracle.com
Fri Jan 20 11:17:35 PST 2012

Sorry if I am late to the game, but I was just wishing for this functionality - and there was the web rev!

I've looked at this version of the patch: http://cr.openjdk.java.net/~jmelvin/7125793/webrev.03/

It looks good. Only two small comments:

java_md.c:GetJVMPath - I don't think the code inside "#ifndef GAMMA" needs to be changed. The code is always compiled with GAMMA defined. The "#ifndef GAMMA" is just there to facilitate comparison with the original code in the JDK.

os_bsd.cpp:2580 - (nit) incorrect indentation

I have also verified that the "hotspot" launcher works after this change.

/Staffan

On 10 jan 2012, at 22:05, James Melvin wrote:

Hi Dan,

Final webrev to reflect your comments... http://cr.openjdk.java.net/~jmelvin/7125793/webrev.02 Minor changes this round: make/bsd/makefiles/buildtree.make # Fail gracefully on Apple BOOTDIR make/bsd/makefiles/launcher.make # Link with framework only on Mac src/os/bsd/vm/osbsd.cpp # Just spelling fix Lastly, I wanted to reply to John Coomes comments earlier about the testgamma script simplification. Although I also value economy of expression, in this case I think the use of more advanced shell constructs increases the time for fresh eyes to decipher. Given performance and such is not an issue, I'd prefer to keep the simpler version I'm proposing with this change on Mac OS X, to make it easier on future maintenance. This should be a model for the other platforms when we reconcile. I've attached the before and after copies should there be further comments on the simplified short script. Thanks, Jim

On 1/9/12 6:37 PM, Daniel D. Daugherty wrote: On 1/7/12 9:38 AM, James Melvin wrote: WEBREV: http://cr.openjdk.java.net/~jmelvin/7125793/webrev.01

make/bsd/Makefile No comments. make/bsd/makefiles/buildtree.make No comments. make/bsd/makefiles/defs.make Thanks for fixing this one for BSD platforms. make/bsd/makefiles/launcher.make line 60: typo: 'inadvertenly' -> 'inadvertently' Sorry I missed this in my first review, but the addition of '-framework CoreFoundation' to LFLAGSLAUNCHER is probably MacOS X specific. I think: ifeq ($(OSVENDOR), Darwin) else endif will work in launcher.make also. make/bsd/makefiles/vm.make No comments. src/os/bsd/vm/osbsd.cpp line 2544: typo: 'overriden' -> 'overridden' line 2588: typo: 'overriden' -> 'overridden' Looks like old code line 2576 depended on the 'hotspot' symlink to refer to either 'client' or 'server' or whatever JVM you wanted to run. I'm fairly certain that the 'hotspot' symlink was retired; I'm just not sure when. src/os/posix/launcher/javamd.c No comments. Dan

<testgamma.before><testgamma.after>

More information about the macosx-port-dev mailing list