On Mar 13, 2013, at 4:44 PM, Christian Thalinger <christian.thalin...@oracle.com> wrote:
> > 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? > > Ha! I can remember. It's used by jmvti.make (implicitly). The logic in > rules.make is: > > ifdef ALT_BOOTDIR > > COMPILE.JAVAC = $(ALT_BOOTDIR)/bin/javac > > else > > ifdef BOOTDIR > > COMPILE.JAVAC = $(BOOTDIR)/bin/javac > > else > > ifdef JAVA_HOME > else > > endif > endif > endif > > BOOTDIR is not defined in rules.make and JAVA_HOME is picked up (which is set > to ABS_BOOTDIR). Back to my favorite review these days :-) I put the BOOTDIR setting back because we need it. Any reviewers who approve or veto pushing this change? -- Chris > > This all sucks and needs to be replaced by something completely different. > Soon. > > -- Chris > >>> 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 >>>>>>>>> >>>>>>>> >>>>>>> >>>>>> >>>> >> >
