Code Review Request for MacOS X build change (7117748)

[Mike Swingler]

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.