Code Review Request for MacOS X build change (7117748)

[Mike Swingler]

Yeah, the webrev looks fine to me.

Mike Swingler
Apple Inc.

On Dec 4, 2011, at 12:31 PM, Daniel D. Daugherty wrote:

> Mike,
> 
> Are you OK if I proceed with this changeset as is and deal with
> any additional gamma logic tweaks as part of making sure that
> gamma works?
> 
> Dan
> 
> 
> On 12/4/11 12:05 PM, Mike Swingler wrote:
>> On Dec 4, 2011, at 10:35 AM, Daniel D. Daugherty wrote:
>> 
>>> Hey Mike,
>>> 
>>> Thanks for the quick review!
>>> 
>>> On 12/4/11 9:43 AM, Mike Swingler wrote:
>>>> On Dec 3, 2011, at 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.
>>>> My concern is that you are skipping the gamma checks in the case when OpenJDK is being built with OpenJDK on the Mac.
>>> My understanding is that gamma doesn't currently build on Mac OS X
>>> in either the macosx-port forest or in the 7u-osx forest. Jim Melvin
>>> has been working to resolve that issue and when that work is done,
>>> then the ALWAYS_PASS_TEST_GAMMA stuff can go away.
>>> 
>>>> The gamma tests (and the SA_CLASSPATH) logic only needs to be adjusted when you are building OpenJDK with Apple's Java SE 6.
>>> The SA_CLASSPATH stuff only checks for Apple's Java SE 6 layout when
>>> it can't find tools.jar. Please see my answer to Vladimir for when
>>> Apple's Java SE 6 is used as the boot JDK.
>> Yeah, I read that after I replied. :-P
>> 
>>> As for gamma, are you saying that the gamma build is not broken when
>>> the MacOS X port is built with OpenJDK7 as the boot JDK? That's not
>>> my understanding, but I haven't tested it myself (yet).
>> I thought it works, but only when bootstrapped under JDK7...but I could be mistaken.
>> 
>>>> I like the general direction of these fixes, but could you conditionalize the logic so that these hacky patches only get set when the current bootstrap Java is 1.6?
>>> Hacky? :-) And I thought it was hacky to have to pass those variables
>>> on the build command line... :-) More seriously, the goal of this
>>> changeset is to get those variables off the command line so that
>>> MacOS X JPRT jobs can be submitted just like the other platforms...
>> Oh, of course...I don't think we should have anything more on the command line than "make". Personally, I wish the parallelization options could be auto-determined based on cores and total ram size ratio, and you should pass a parameter when you don't want them, or want to override them.
>> 
>>> For the SA_CLASSPATH stuff, I think the logic is conditional enough:
>>> 
>>>    - look for tools.jar
>>>    - if not found and building MacOS X, then look for
>>>      Apple's Java SE 6 layout (classes.jar)
>>>    - complain and fail if JDI classes cannot be found
>>> 
>>> For gamma, all that logic will get deleted when we start being able to
>>> build gamma. If gamma builds now when OpenJDK7 is used as the boot JDK,
>>> then we'll rework the logic to be conditional when Apple's Java SE 6 is
>>> used as the boot JDK.
>>> 
>>> Please let me know if this addresses your concerns.
>> I was under the impression that gamma actually passes when JDK7 is bootstrapped with JDK7, but I haven't tested that recently.
>> 
>> Cheers,
>> Mike Swingler
>> Apple Inc.
>>