Code Review Request for MacOS X build change (7117748)

[Vladimir Kozlov]

Dan,

I understand that module code was there before this fix but I am still concern that if lib/modules is present the 
makefile will continue execution even if SA_CLASSPATH does not exits. It looks like current code works for us because we 
don't have lib/modules in our build environment. Someone should explain why we need this modules code. Can we remove it?

I am fine with the rest changes.

Thanks,
Vladimir

On 12/4/11 10:22 AM, Daniel D. Daugherty wrote:
> Vladimir,
>
> Thanks for the quick review!
>
>
> On 12/3/11 11:58 PM, Vladimir Kozlov wrote:
>> Sorry for my ignorance (and I can't access bugster now)
>
> And the bug hasn't appeared on bug.sun.com yet... sigh...
>
>
>> but why we should care about building current Hotspot with Apple's JDK? Is is because Macs in JPRT have only Apple's JDK?
>
> The default release in JPRT is 'jdk7' which means that the import
> JDK is 'jdk7'. However, the boot JDK for a 'jdk7' release job is
> 'jdk6u18'. For MacOS X, we use Apple's bits for the 'jdk6u18'
> boot JDK. For a 'jdk8' release job, we use 'jdk7' bits as the
> boot JDK. For MacOS X, we use Oracle's 'jdk7' bits...
>
> This means that my fix has to work with an Apple JDK or an Oracle
> JDK as the boot JDK.
>
>
>> What is _JUNK_ line for?
>
> Just a dummy variable so that the "INFO" mesg can be output. I tend
> to output INFO lines whenever an "ALT_*" variable is used.
>
>
>> Why there is no separate check for -f $(SA_CLASSPATH)? Current check also requires presence of lib/modules directory.
>
> For these lines:
>
> 88 $(QUIETLY) if [ ! -f "$(SA_CLASSPATH)" -a ! -d $(MODULELIB_PATH) ] ; then \
> 89 echo "Cannot find JDI classes. Use 1.6.0 or later version of JDK."; \
>
> all I did was add quotes around $(SA_CLASSPATH) on line 88
> and change the output message on line 89. Since SA_CLASSPATH
> can now be empty, I needed to make sure that the '-f' check
> didn't blow up due to the empty variable and the error mesg
> was confusing with the empty variable also.
>
> As for the lib/modules directory, 1) I didn't add that logic and
> 2) the logic doesn't require the lib/modules directory. The logic
> issues the error message and fails if the file named by
> SA_CLASSPATH does not exist and if the lib/modules directory
> does not exist. Whoever put that logic in the Makefile is assuming
> that if the lib/modules directory exists, then the JDI classes
> will be found (somehow).
>
> Please let me know if I've resolved your concerns.
>
> Dan
>
>
>>
>> thanks,
>> Vladimir
>>
>> On 12/3/11 8:13 PM, Daniel D. Daugherty wrote:
>>> Greetings,
>>>
>>> I have a fix that allows HSX-23 to be built on MacOS X via JPRT
>>> without specifying SA_APPLE_BOOT_JAVA or ALWAYS_PASS_TEST_GAMMA
>>> on the command line. I'm targeting this fix at RT_Baseline for
>>> the HSX-23-B08 snapshot.
>>>
>>> Here is the webrev URL:
>>>
>>> http://cr.openjdk.java.net/~dcubed/7117748-webrev/0/
>>>
>>> I tested this fix with the default JPRT boot JDK (JDK6 from Apple)
>>> and with JDK7 boot JDK (JDK7 bits from Oracle).
>>>
>>> Thanks, in advance, for any reviews.
>>>
>>> Dan
>>>
>>> P.S.
>>> This fix does _not_ get "gamma" working on MacOS X.
>>> That work is being done separately.