On Feb 26, 2013, at 6:09 PM, Christian Thalinger <christian.thalin...@oracle.com> wrote:
> > 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. Btw. I've updated the webrev. I think we can also remove the -server switch from the run command line because we point the VM to the libjvm anyway. How about this change? -- Chris > > -- 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 >
