OK. Then I'm fine with it. -kto
On Sep 15, 2012, at 4:03 AM, David Holmes wrote: > On 15/09/2012 8:44 AM, Kelly O'Hair wrote: >> >> It should get integrated through the hotspot-rt forest. Let me know if you >> need me to do that. >> >> http://cr.openjdk.java.net/~ihse/make-test-in-build-an-option/webrev.02/make/bsd/makefiles/buildtree.make.sdiff.html >> http://cr.openjdk.java.net/~ihse/make-test-in-build-an-option/webrev.02/make/linux/makefiles/buildtree.make.sdiff.html >> http://cr.openjdk.java.net/~ihse/make-test-in-build-an-option/webrev.02/make/solaris/makefiles/buildtree.make.sdiff.html >> >> Has some cross compiling changes removed? > > Yes that is handled in the make/defs.make file now: > > + ifeq ($(TEST_IN_BUILD),) > + # By default, run Queens test after building unless cross-compiling > + ifneq ($(CROSS_COMPILE_ARCH),) > + TEST_IN_BUILD = false > + else > + TEST_IN_BUILD = true > + endif > + endif > > David > ----- > >> Otherwise looks fine. >> >> I agree that the duplication is ugly, but I also agree with you that these >> hotspot Makefiles are riddles with duplication >> at a high level >> >> -kto >> >> >> >> On Sep 14, 2012, at 6:39 AM, Magnus Ihse Bursie wrote: >> >>> I'd like to make a new try at integrating this fix. :-) >>> >>> I posted two a webrev back in May. After some feedback, I posted a second >>> one at >>> http://cr.openjdk.java.net/~ihse/make-test-in-build-an-option/webrev.01. >>> This second webrev was reviewed and accepted by David Holmes, Dmitry >>> Samersoff, Coleen Phillimore and Vladimir Kozlov. However, Kelly O'Hair >>> expressed concern that it would not work on Windows due to changes in >>> make/defs.make being problematic for Windows nmake. Also, in private >>> conversation with me, Kelly found build problems on MacOSX. >>> >>> I didn't have time at the moment to address those issues, so I dropped it >>> for the time being. >>> >>> However, I now checked more carefully. I'm not using any syntax in >>> defs.make that is not already there, so I shouldn't introduce something >>> Windows can't handle. >>> >>> The patch did have a problem though -- the TEST_IN_BUILD argument was not >>> properly propagated. So I have made a new patch (third time's a charm!), >>> and the webrev is here: >>> http://cr.openjdk.java.net/~ihse/make-test-in-build-an-option/webrev.02/ >>> >>> The only difference to the previous one is that I add >>> TEST_IN_BUILD=$(TEST_IN_BUILD) to the BUILDTREE_VARS. (But it needs to be >>> done three times, once for each duplicated platform file...) >>> >>> I am currently running a series of tests on all platforms, both using the >>> old build system and the new. Not all tests have finished running, but it's >>> looking good so far and I believe there should be no more problems. (I'll >>> let you know otherwise! :)) >>> >>> It feels like what should be a simple fix has grown somewhat out of >>> proportion. I'm hoping I can get some final reviews on this, and some >>> advice as to wether this fix should be integrated through the hotspot or >>> the build forest. (On one hand, it's hotspot only, but on the other hand, >>> it's just affecting build.) >>> >>> /Magnus >>> >>> >>> On 2012-05-14 15:05, Magnus Ihse Bursie wrote: >>>> As part of the new build system created in the build-infra project, we >>>> want to make it a configurable option wether or not to run the Queens test >>>> as part of the build. >>>> >>>> Here is a patch that introduces a new make variable, TEST_IN_BUILD, which >>>> controls wether to run the Queens test (test_gamma.sh) or not. If the >>>> variable is not explicitely set, it will default to true, mening that the >>>> default behaviour will be as before, that is, to run the Queens test. >>>> However, if you (or configure) explicitely set it to false, the Queens >>>> test will be skipped. >>>> >>>> http://cr.openjdk.java.net/~ihse/make-test-in-build-an-option/webrev.00 >>>> >>>> /Magnus >>>> >>> >>> >>
