Fix for 6888888 breaks the build

[Jonathan Gibbons]

Andrew John Hughes wrote:
> 2009/10/15 Tim Bell <Tim.Bell at sun.com>:
>   
>> Andrew John Hughes wrote:
>>     
>>> The fix for bug 6888888:
>>>
>>> http://hg.openjdk.java.net/jdk7/build/jdk/rev/777714bd992a
>>>
>>> breaks the build as it tries to use javah from ALT_JDK_IMPORT_DIR (via
>>> JAVA_TOOLS_DIR) which is not set on normal builds.
>>> The README tells the user to set ALT_BOOTDIR and changing
>>> JAVA_TOOLS_DIR to be set to BOOTDIR makes the build work again:
>>>       
>> Sorry about that - my mistake.
>>
>> As Andrew pointed out on IRC:
>>
>>   you can work around the bug by setting ALT_JDK_IMPORT_DIR to the same as ALT_BOOTDIR
>>
>> Jon Gibbons has a fix to 6889255 out for review, which will remove the 6888888
>> workaround:
>>   http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6889255
>>   http://cr.openjdk.java.net/~jjg/6889255-classreader.1
>>
>>     
>>> http://cr.openjdk.java.net/~andrew/javah/webrev.01/
>>>
>>> Ok to push?
>>>       
>> Unfortunately it is too late to fix in b74, and hopefully 6889255 will be
>> fixed in b75.  If not, I will come back to your patch.
>>
>> Tim
>>
>>     
>
>
> I think there's still an issue here that makes this patch worth
> pushing.   The 6888888 fix didn't cause the bug, but merely made it
> visible to a lot more people.  So 6889255 will only hide it again.
> The build uses JAVA_TOOLS_DIR for javac, javah and javadoc:
>
>   # If no explicit tools, use boot tools (add VM flags in this case)
>   JAVAC_CMD     = $(JAVA_TOOLS_DIR)/javac $(JAVAC_JVM_FLAGS) \
>                   $(JAVACFLAGS)
>   JAVAH_CMD     = $(JAVA_TOOLS_DIR)/javah \
>                   $(JAVAHFLAGS)
>   JAVADOC_CMD   = $(JAVA_TOOLS_DIR)/javadoc $(JAVA_TOOLS_FLAGS:%=-J%)
>
>
> but only when LANGTOOLS_DIST is not defined.  Normally, langtools is
> built first so LANGTOOLS_DIST is defined.  What your fix for 6888888
> did was cause the setting for JAVAH to get used in all cases and, as
> JAVA_TOOLS_DIR defaults to ALT_JDK_IMPORT_PATH rather than BOOTDIR
> this fails in many cases as the user sets ALT_BOOTDIR not
> ALT_JDK_IMPORT_PATH.
>
> In jdk_generic_profile, it seems ALT_JDK_IMPORT_PATH is set to
> ${jdk_instances}/${importjdk} if it exists.  On Solaris,
> ${jdk_instances} is set to /usr/jdk/instances which probably explains
> why Sun engineers building on Solaris may not see this bug either.
> ${jdk_instances} is set to /opt/java on GNU/Linux, whereas distros
> tend to have standardised on /usr/lib/jvm.  It's simply "C:" on
> Windows.  As mentioned on IRC, release engineering are setting
> ALT_JDK_IMPORT_PATH explicitly so also would never see this bug.
>
> As we presumably want the tools from the bootstrap JDK, and not from
> 'the import JDK to be used to get hotspot VM if not built', I suggest
> we switch to BOOTDIR by default by applying this patch.
>   

This probably explains problems I've reported informally to Kelly a 
while back, but never got around to formally investigating.     FWIW, 
this fix does not interfere with the fix for 6889255 coming up, and this 
fix seems like a good idea to me.

-- Jon


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/build-dev/attachments/20091015/05e56f10/attachment.htm>