Re: Need reviewer
Kelly O'Hair wrote: Mark Reinhold wrote: Okay, I can live with this. Thanks. (At the risk of stating the obvious: Please be sure to use hg mv when you rename these files, otherwise the historical changeset trail will be broken.) Will do. -kto Sorry, I wasn't paying attention to this thread. Renaming these sources to -template is okay with me (although I kinda liked the -X convention). -Alan.
Fix for 6888888 breaks the build
The fix for bug 688: http://hg.openjdk.java.net/jdk7/build/jdk/rev/14bd992a 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: http://cr.openjdk.java.net/~andrew/javah/webrev.01/ Ok to push? -- Andrew :-) Free Java Software Engineer Red Hat, Inc. (http://www.redhat.com) Support Free Java! Contribute to GNU Classpath and the OpenJDK http://www.gnu.org/software/classpath http://openjdk.java.net PGP Key: 94EFD9D8 (http://subkeys.pgp.net) Fingerprint: F8EF F1EA 401E 2E60 15FA 7927 142C 2591 94EF D9D8
hg: jdk7/build: 6891677: java/build integrate zero assembler JDK changes
Changeset: 608937d41381 Author:gbenson Date: 2009-10-15 13:26 +0100 URL: http://hg.openjdk.java.net/jdk7/build/rev/608937d41381 6891677: java/build integrate zero assembler JDK changes Summary: Build changes for the Zero assembler port Reviewed-by: ohair, tbell ! make/hotspot-rules.gmk
hg: jdk7/build/corba: 6891677: java/build integrate zero assembler JDK changes
Changeset: 34a68fa0680b Author:gbenson Date: 2009-10-15 13:28 +0100 URL: http://hg.openjdk.java.net/jdk7/build/corba/rev/34a68fa0680b 6891677: java/build integrate zero assembler JDK changes Summary: Build changes for the Zero assembler port Reviewed-by: ohair, tbell ! make/common/Defs-linux.gmk ! make/common/shared/Compiler-gcc.gmk
hg: jdk7/build/jdk: 6891677: java/build integrate zero assembler JDK changes
Changeset: e2de121c27c4 Author:gbenson Date: 2009-10-15 13:27 +0100 URL: http://hg.openjdk.java.net/jdk7/build/jdk/rev/e2de121c27c4 6891677: java/build integrate zero assembler JDK changes Summary: Build changes for the Zero assembler port Reviewed-by: ohair, tbell ! make/common/Defs-linux.gmk ! make/common/Program.gmk ! make/java/instrument/Makefile ! make/java/jli/Makefile ! make/java/redist/Makefile ! make/javax/sound/SoundDefs.gmk ! make/jdk_generic_profile.sh ! src/share/native/com/sun/media/sound/SoundDefs.h + src/solaris/bin/ergo_zero.c + src/solaris/bin/zero/jvm.cfg
Re: Review request: Zero buildsystem changes
2009/10/15 Tim Bell : > Gary Benson wrote: > >> As an update to my previous message, the HotSpot part of this patch >> was tested and pushed to hotspot-comp last night, so the remaining >> code for review is this: >> >> http://cr.openjdk.java.net/~gbenson/zero-11-build/ >> >> I'm going on vacation on Friday 16, returning Monday 26, so could >> anyone replying off-list please Cc gnu_and...@member.fsf.org, who >> is going to handle this in my absence. We'd love to see this in >> in time for Milestone 5, and we'd hate to miss the boat because >> an email was sitting in my inbox unread! > > (Adding Andrew as requested). > > These changes look OK to Kelly and myself. Also, my test run on our > internal build farm called JPRT (a full control build of everything after > adding your patch) worked: > >> JPRT: [sfbay] job notification - success with job >> 2009-10-14-203220.tbell.zero-11-build > > How would you like to proceed? With the hotspot work, Tom Rodriguez (AKA > 'never') > did the push as an agent for you via JPRT because that is a hotspot > requirement. > > I am willing to do that as well for the JDK changes, or you could create > the changeset and push yourself. We are not quite as strict about requiring > a JPRT grid build before pushing on the JDK side. I wish we were, but so > far I have not won that contest :-) > > > I filed bug ID 6891677 "java/build integrate zero assembler JDK changes" for > this, but it may not be visible externally for a day or two. > > Let me know - I'd be glad to help get this changeset in. > > Tim > > Pushed on Gary's behalf: http://hg.openjdk.java.net/jdk7/build/rev/608937d41381 http://hg.openjdk.java.net/jdk7/build/corba/rev/34a68fa0680b http://hg.openjdk.java.net/jdk7/build/jdk/rev/e2de121c27c4 Thanks, -- Andrew :-) Free Java Software Engineer Red Hat, Inc. (http://www.redhat.com) Support Free Java! Contribute to GNU Classpath and the OpenJDK http://www.gnu.org/software/classpath http://openjdk.java.net PGP Key: 94EFD9D8 (http://subkeys.pgp.net) Fingerprint: F8EF F1EA 401E 2E60 15FA 7927 142C 2591 94EF D9D8
Re: Review request: Zero buildsystem changes
Andrew John Hughes wrote: > 2009/10/15 Tim Bell : > > Gary Benson wrote: > > > As an update to my previous message, the HotSpot part of this > > > patch was tested and pushed to hotspot-comp last night, so the > > > remaining code for review is this: > > > > > > http://cr.openjdk.java.net/~gbenson/zero-11-build/ > > > > > > I'm going on vacation on Friday 16, returning Monday 26, so > > > could anyone replying off-list please Cc > > > gnu_and...@member.fsf.org, who is going to handle this in my > > > absence. We'd love to see this in in time for Milestone 5, and > > > we'd hate to miss the boat because an email was sitting in my > > > inbox unread! > > > > (Adding Andrew as requested). > > > > These changes look OK to Kelly and myself. Also, my test run on > > our internal build farm called JPRT (a full control build of > > everything after adding your patch) worked: > > > > > JPRT: [sfbay] job notification - success with job > > > 2009-10-14-203220.tbell.zero-11-build > > > > How would you like to proceed? With the hotspot work, Tom > > Rodriguez (AKA 'never') did the push as an agent for you via JPRT > > because that is a hotspot requirement. > > > > I am willing to do that as well for the JDK changes, or you could > > create the changeset and push yourself. We are not quite as > > strict about requiring a JPRT grid build before pushing on the JDK > > side. I wish we were, but so far I have not won that contest :-) > > > > I filed bug ID 6891677 "java/build integrate zero assembler JDK changes" for > > this, but it may not be visible externally for a day or two. > > > > Let me know - I'd be glad to help get this changeset in. > > Pushed on Gary's behalf: > > http://hg.openjdk.java.net/jdk7/build/rev/608937d41381 > http://hg.openjdk.java.net/jdk7/build/corba/rev/34a68fa0680b > http://hg.openjdk.java.net/jdk7/build/jdk/rev/e2de121c27c4 Tim, Andrew, many thanks :) Cheers, Gary -- http://gbenson.net/
Re: Fix for 6888888 breaks the build
Andrew John Hughes wrote: > The fix for bug 688: > > http://hg.openjdk.java.net/jdk7/build/jdk/rev/14bd992a > > 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 688 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
Re: Fix for 6888888 breaks the build
2009/10/15 Tim Bell :
> Andrew John Hughes wrote:
>> The fix for bug 688:
>>
>> http://hg.openjdk.java.net/jdk7/build/jdk/rev/14bd992a
>>
>> 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 688
> 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 688 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 688
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.
--
Andrew :-)
Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)
Support Free Java!
Contribute to GNU Classpath and the OpenJDK
http://www.gnu.org/software/classpath
http://openjdk.java.net
PGP Key: 94EFD9D8 (http://subkeys.pgp.net)
Fingerprint: F8EF F1EA 401E 2E60 15FA 7927 142C 2591 94EF D9D8
Re: Fix for 6888888 breaks the build
Andrew John Hughes wrote:
2009/10/15 Tim Bell :
Andrew John Hughes wrote:
The fix for bug 688:
http://hg.openjdk.java.net/jdk7/build/jdk/rev/14bd992a
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 688
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 688 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 688
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
Re: Fix for 6888888 breaks the build
> Andrew John Hughes wrote:
(snip!)
>> I think there's still an issue here that makes this patch worth
>> pushing. The 688 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 688
>> 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.
>>
Jonathan Gibbons wrote:
> 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.
OK - I filed a new bug report:
6892021 "Build tools from ALT_JDK_IMPORT_PATH versus BOOTDIR"
with a pointer to this email thread.
Kelly - do you have an opinion on this?
Thanks-
Tim
