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
