Code Review Request for MacOS X build change (7117748)

[Daniel D. Daugherty]

On 12/5/11 2:33 PM, Mandy Chung wrote:
> On 12/5/11 8:57 AM, Daniel D. Daugherty wrote:
>>
>> The logic before Mandy's change says:
>>
>> - if the $(SA_CLASSPATH) file doesn't exist, then complain and exit
>>
>> Mandy's changeset updated that logic to:
>>
>> - if the $(SA_CLASSPATH) file doesn't exist AND if the lib/modules
>>   directory does not exist, then complain and exit
>>
>> Mandy also added a TODO note about needing to add a check to see
>> if the SA module is installed.
>>
>> So what does all this mean?
>>
>> - the modules logic that Mandy added is partial
>> - there is a TODO note about what else needs to be added
>> - if a proper boot JDK is provided, then the modules
>>   logic that Mandy added is never executed
>> - if an improper boot JDK is provided AND the lib/modules
>>   directory exists, then the error message and exit will
>>   be skipped, but the build will fail later because the
>>   JDI classes won't be found
>>
>
> I think Dan already got what he needs and Vladmir's approval to the 
> review.
>
> FWIW.  Some background to the SA build logic and modules image to 
> explain the change I made:
>
> The check for SA_CLASSPATH exists in the boot JDK is to detect if the 
> classpath was setup properly to compile the SA JDI connectors that 
> references com.sun.jdi.* API.   The build will fail before it compiles 
> any sa-jdi classes.   In the modular world, tools.jar, rt.jar, and 
> other jar files no longer exist.  Instead, com.sun.jdi will be 
> installed in the modular JDK as a module.   For a skip boot cycle 
> build, the JDK will be built with itself.  In other words, for jigsaw, 
> it will build itself with a modular JDK.  The makefile needs to detect 
> if the boot JDK is a modular JDK or legacy JDK (i.e. existing JDK 
> releases) and then checks for tools.jar or the module for JDI.  A 
> modular JDK will have a new "lib/modules" directory in which the jdk 
> modules are installed.  That's why the "lib/modules" check was added 
> as the partial fix.  A complete fix for this modules logic is to check 
> if the module for JDI exists.  In any case, this check can be removed 
> if it causes any confusion.
>
> Mandy

Mandy,

Thanks for filling in the history. I think I'm going with the fix for
7117748 as it is since I have reviews from Vladimir and Mike S from
Apple.

Dan