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

[Staffan Larsen]

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/os_bsd.cpp            # Just spelling fix
> 
> Lastly, I wanted to reply to John Coomes comments earlier about the
> test_gamma 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 LFLAGS_LAUNCHER is
>> probably MacOS X specific. I think:
>> 
>> ifeq ($(OS_VENDOR), Darwin)
>> else
>> endif
>> 
>> will work in launcher.make also.
>> 
>> make/bsd/makefiles/vm.make
>> No comments.
>> 
>> src/os/bsd/vm/os_bsd.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/java_md.c
>> No comments.
>> 
>> Dan
>> 
>> 
> <test_gamma.before><test_gamma.after>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/hotspot-dev/attachments/20120120/fe0a3f7e/attachment.html