Code Review Request for MacOS X build change (7117748)

[Daniel D. Daugherty]

Thanks Mike!

Dan


On 12/4/11 1:38 PM, Mike Swingler wrote:
> 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.
>>>