On Feb 25, 2013, at 11:24 PM, David Holmes <david.hol...@oracle.com> wrote:
> On 26/02/2013 4:42 AM, Christian Thalinger wrote: >> On Feb 24, 2013, at 2:54 PM, David Holmes <david.hol...@oracle.com> wrote: >>> On 23/02/2013 1:55 PM, Christian Thalinger wrote: >>>> I talked to a lot of people about this today. What we really want is to >>>> not run tests when we build. Mikael and I were looking into how we could >>>> do that without gamma and there is a way: >>>> >>>> http://cr.openjdk.java.net/~twisti/8006965/ >>>> >>>> This would be the first of three fixes: >>>> >>>> Fix 1) The patch above removes test_gamma and uses some weirdness in the >>>> VM (-Dsun.java.launcher=gamma) to run it with an existing JDK; add >>>> test_{product,fastdebug,debug} targets >>> >>> This logic is not suitable: >>> >>> 541 # Testing the built JVM >>> 542 ifeq ($(JAVA_HOME),) >>> 543 RUN_JVM=JAVA_HOME=$(JDK_IMPORT_PATH) $(JDK_IMPORT_PATH)/bin/java >>> -d$(ARCH_DATA_MODEL) -server -XXaltjvm=$(MISC_DIR)/$(VM_FLAVOR) >>> -Dsun.java.launcher=gamma >>> 544 else >>> 545 RUN_JVM=$(JAVA_HOME)/bin/java -d$(ARCH_DATA_MODEL) -server >>> -XXaltjvm=$(MISC_DIR)/$(VM_FLAVOR) -Dsun.java.launcher=gamma >>> 546 endif >>> >>> I have JAVA_HOME set in my environment for use by other tools/scripts and >>> it points at JDK7. The existing logic does not use my environments >>> JAVA_HOME setting so neither should the revised logic! >> >> That's not entirely correct. test_gamma uses your JAVA_HOME setting: > > This is so confusing. Our makefiles are an abomination! I couldn't agree more. > > So this all started because the makefile has: > > JAVA_HOME=$(ABS_BOOTDIR) > > which was flagged as wrong because gamma would run in the boot JDK. But now > it seems the make variable JAVA_HOME is irrelevant anyway because the > test_gamma script will use the JAVA_HOME environment variable. > > So how did the boot JDK come back into this??? > >> cthaling@sc14a501:/export/twisti/build/graal/build/linux_amd64_compiler2/product$ >> export JAVA_HOME=/java/re/jdk/8/latest/binaries/linux-x64 >> cthaling@sc14a501:/export/twisti/build/graal/build/linux_amd64_compiler2/product$ >> ./test_gamma >> Using java runtime at: /java/re/jdk/8/latest/binaries/linux-x64/jre >> <snip> >> cthaling@sc14a501:/export/twisti/build/graal/build/linux_amd64_compiler2/product$ >> export JAVA_HOME=/foo >> cthaling@sc14a501:/export/twisti/build/graal/build/linux_amd64_compiler2/product$ >> ./test_gamma >> JAVA_HOME must point to a 64-bit OpenJDK. >> >> And here comes this little snippet into play: >> >> -MAKE_ARGS += JAVA_HOME=$(ABS_BOOTDIR) >> +MAKE_ARGS += BOOTDIR=$(ABS_BOOTDIR) >> >> Only setting JAVA_HOME to ABS_BOOTDIR make test_gamma work even if you have >> a JAVA_HOME set. > > I still don't get this. What has BOOTDIR got to do with JAVA_HOME? Where is > this BOOTDIR value being used? There is no use of it in > http://cr.openjdk.java.net/~twisti/8006965/8006965.patch and I don't see it > pre-existing ?? I talked to John Coomes about that yesterday and we can remove that line. ABS_BOOTDIR is only used by Windows. -- Chris > > Thanks, > David > >> I have added this logic so that users can control what JDK is used when >> running the test. In fact they should use ALT_JDK_IMPORT_PATH if they want >> to control that. >> >>> >>> I also don't see why the above sets JAVA_HOME at #543 - what will read that >>> environment variable? >> >> It's the odd logic in os::jvm_path guarded by >> Arguments::created_by_gamma_launcher(). A clean-up of that logic would be >> part of Fix 3. >> >>> >>> I still have concerns over what JDK_IMPORT_PATH will point to for different >>> JDK builders. >> >> It's either JDK_IMPORT_PATH or JDK_IMAGE_DIR. Since most people don't want >> to export their libjvm into a JDK we have to use JDK_IMPORT_PATH. We could >> add some logic that checks if JDK_IMAGE_DIR exists and use that one. >> >> -- Chris >> >>> >>> And this addition still makes no sense to me: >>> >>> MAKE_ARGS += BOOTDIR=$(ABS_BOOTDIR) >>> >>> Why do you need to add BOOTDIR to the MAKE_ARGS? >>> >>> David >>> >>> >>>> Fix 2) Remove gamma and all the ugly code that comes with it (copies of >>>> the jdk launcher in hotspot and other pieces); make the hotspot script >>>> work like the test targets in Fix 1 >>>> >>>> Fix 3) Remove the -Dsun.java.launcher=gamma and possibly replace the >>>> existing -Dsun.boot.library.path weirdness by explicit command line >>>> options like -Xbootlibrarypath:{/p,/a} >>>> >>>> -- Chris >>>> >>>> On Feb 22, 2013, at 3:21 PM, Christian Thalinger >>>> <christian.thalin...@oracle.com> wrote: >>>> >>>>> >>>>> On Feb 22, 2013, at 12:58 AM, Staffan Larsen <staffan.lar...@oracle.com> >>>>> wrote: >>>>> >>>>>> I'm not sure what the correct solution is, but when you do find out, the >>>>>> jdkpath.sh target should also be updated. >>>>> >>>>> How many are actually using the hotspot script? Would people be very >>>>> sentimental if we would remove the gamma launcher altogether? >>>>> >>>>> Taking to people here it seems that most are copying their libjvm into a >>>>> JDK and use java anyway. >>>>> >>>>> -- Chris >>>>> >>>>>> >>>>>> Thanks, >>>>>> /Staffan >>>>>> >>>>>> On 22 feb 2013, at 03:40, Christian Thalinger >>>>>> <christian.thalin...@oracle.com> wrote: >>>>>> >>>>>>> http://cr.openjdk.java.net/~twisti/8006965 >>>>>>> >>>>>>> 8006965: test_gamma should run with import JDK >>>>>>> Reviewed-by: >>>>>>> >>>>>>> Right now test_gamma runs with the boot JDK which is JDK n-1 (where >>>>>>> JDK n is the version we are actually compiling for). This setup is >>>>>>> unsupported and thus should not be done during HotSpot builds. >>>>>>> >>>>>>> The fix is to always use JDK_IMPORT_PATH instead of JAVA_HOME when >>>>>>> running test_gamma. >>>>>>> >>>>>>> make/bsd/makefiles/buildtree.make >>>>>>> make/defs.make >>>>>>> make/linux/makefiles/buildtree.make >>>>>>> make/solaris/makefiles/buildtree.make >>>>>>> >>>>>> >>>>> >>>> >>
