[7u4-osx] Please review: 7124089: launcher refactoring v2.0

[Joe Darcy]

Launcher changes looks good,

-Joe

On 01/21/2012 10:05 AM, Kumar Srinivasan wrote:
> Hi Kelly et. al.,
>
>  I have beautified/fixed the Makefiles addressing Kellys' comments below:
>
> 1. Indented the Makefiles correctly.
> 2. Annotated with more  trailing comments to the if/else/endif clauses
> 3. Removed the offending \ escapes
> 4. Removed the * from Release.gmk, it turns out the files being copied
>    were not quite right (missing files), fixed it such that all the 
> appropriate files
>    are copied.
> 5. Added comments for the MacOSX specific cflags.
>
> The incremental webrev is here:
> http://cr.openjdk.java.net/~ksrini/7124089/webrev.2/webrev.delta/index.html 
>
>
> The full webrev is here:
>  http://cr.openjdk.java.net/~ksrini/7124089/webrev.2/index.html
>
> Thanks
> Kumar
>
>> On the Makefiles....
>>
>> Please refrain from using any wildcards (e.g. * ) in the make rules. 
>> Better to be explicit, or if necessary
>> use something like FILES=$(wildcard $(SOMEDIR)/*) and a cp $(FILES) 
>> $(SOMEPLACE)
>> so that we can at least see in the Makefile log what it really copied.
>>
>> Please indent the Makefile if/else/endif statements.
>>
>> Thank you for the trailing comments on the endif's. ;^)
>>
>> Please try to avoid escaped quotes on the compile lines, use this  
>> -DX='"abc"' rather than this -DX=\"abc\"
>> escaped quotes are very problematic on Windows and I know this isn't 
>> Windows, but it tempts windows
>> people to use it, it will not work in all situations. Where '"abc"' 
>> does.
>>
>> Please add a comment on what the -Os compiler option means, and also 
>> the -x objective-c, I could guess
>> but would be better to document it in the makefile.
>>
>> -kto
>>
>> On Jan 20, 2012, at 8:24 AM, Kumar Srinivasan wrote:
>>
>>> Hi All,
>>>
>>> Based on all the comments from Anthony, Joe and David,
>>> here is the modified version:
>>>
>>> Highlights:
>>> 1. re-factored code in solaris directory  to be shared with macosx,
>>>     reducing duplication across the *nixes.
>>>
>>> 2. adjusted the makefilesto allow the above
>>>
>>> 2. eliminated all conditionals from the shared  java.c
>>>
>>> 3. added a new launcher regression test for the macosx specific -X 
>>> options
>>>
>>> For those who have already reviewed the 0th version, here is an
>>> incremental webrev to make it easier reviewing the differences.
>>> http://cr.openjdk.java.net/~ksrini/7124089/webrev.1/webrev.delta/index.html 
>>>
>>>
>>> Here is the complete webrev:
>>> http://cr.openjdk.java.net/~ksrini/7124089/webrev.1/index.html
>>>
>>> Thanks
>>> Kumar
>>>
>>>
>